Code review

Signed-off-by: irbekrm <irbekrm@gmail.com>
2023-04-25 12:12:00 +01:00 · 2023-04-25 12:12:00 +01:00 · 300fe72ff0
commit 300fe72ff0
parent 0d1d66d900
8 changed files with 24 additions and 25 deletions
--- a/cmd/controller/app/controller.go
+++ b/cmd/controller/app/controller.go
@ -211,7 +211,7 @@ func Run(opts *options.ControllerOptions, stopCh <-chan struct{}) error {
 	log.V(logf.DebugLevel).Info("starting shared informer factories")
 	ctx.SharedInformerFactory.Start(rootCtx.Done())
 	ctx.KubeSharedInformerFactory.Start(rootCtx.Done())
-	ctx.MetadataInformerFactory.Start(rootCtx.Done())
+	ctx.HTTP01ResourceMetadataInformersFactory.Start(rootCtx.Done())

 	if utilfeature.DefaultFeatureGate.Enabled(feature.ExperimentalGatewayAPISupport) {
 		ctx.GWShared.Start(rootCtx.Done())
--- a/pkg/controller/acmechallenges/controller.go
+++ b/pkg/controller/acmechallenges/controller.go
@ -94,8 +94,8 @@ func (c *controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin
 	secretInformer := ctx.KubeSharedInformerFactory.Secrets()
 	// we register these informers here so the HTTP01 solver has a synced
 	// cache when managing pod/service/ingress resources
-	podInformer := ctx.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods"))
-	serviceInformer := ctx.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services"))
+	podInformer := ctx.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods"))
+	serviceInformer := ctx.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services"))
 	ingressInformer := ctx.KubeSharedInformerFactory.Ingresses()

 	// build a list of InformerSynced functions that will be returned by the Register method.
--- a/pkg/controller/context.go
+++ b/pkg/controller/context.go
@ -105,9 +105,9 @@ type Context struct {
 	// instances for cert-manager.io types
 	SharedInformerFactory informers.SharedInformerFactory

-	// MetadataInformerFactory can be used to start partial metadata
-	// informers
-	MetadataInformerFactory metadatainformer.SharedInformerFactory
+	// HTTP01ResourceMetadataInformersFactory is a metadata only informers
+	// factory with a http-01 resource label filter selector
+	HTTP01ResourceMetadataInformersFactory metadatainformer.SharedInformerFactory

 	// GWShared can be used to obtain SharedIndexInformer instances for
 	// gateway.networking.k8s.io types
@ -289,7 +289,7 @@ func NewContextFactory(ctx context.Context, opts ContextOptions) (*ContextFactor
 		panic(fmt.Errorf("internal error: failed to build label selector to filter HTTP-01 challenge resources: %w", err))
 	}
 	isHTTP01ChallengeResourceLabelSelector := labels.NewSelector().Add(*r)
-	metadataInformerFactory := metadatainformer.NewFilteredSharedInformerFactory(clients.metadataOnlyClient, resyncPeriod, opts.Namespace, func(listOptions *metav1.ListOptions) {
+	http01ResourceMetadataInformerFactory := metadatainformer.NewFilteredSharedInformerFactory(clients.metadataOnlyClient, resyncPeriod, opts.Namespace, func(listOptions *metav1.ListOptions) {
 		// metadataInformersFactory is at the moment only used for pods
 		// and services for http-01 challenge which can be identified by
 		// the same label keys, so it is okay to set the label selector
@ -305,14 +305,14 @@ func NewContextFactory(ctx context.Context, opts ContextOptions) (*ContextFactor
 		baseRestConfig: restConfig,
 		log:            logf.FromContext(ctx),
 		ctx: &Context{
-			RootContext:               ctx,
-			StopCh:                    ctx.Done(),
-			KubeSharedInformerFactory: kubeSharedInformerFactory,
-			SharedInformerFactory:     sharedInformerFactory,
-			GWShared:                  gwSharedInformerFactory,
-			GatewaySolverEnabled:      clients.gatewayAvailable,
-			MetadataInformerFactory:   metadataInformerFactory,
-			ContextOptions:            opts,
+			RootContext:                            ctx,
+			StopCh:                                 ctx.Done(),
+			KubeSharedInformerFactory:              kubeSharedInformerFactory,
+			SharedInformerFactory:                  sharedInformerFactory,
+			GWShared:                               gwSharedInformerFactory,
+			GatewaySolverEnabled:                   clients.gatewayAvailable,
+			HTTP01ResourceMetadataInformersFactory: http01ResourceMetadataInformerFactory,
+			ContextOptions:                         opts,
 		},
 	}, nil
 }
--- a/pkg/controller/test/context_builder.go
+++ b/pkg/controller/test/context_builder.go
@ -151,7 +151,7 @@ func (b *Builder) Init() {
 	b.KubeSharedInformerFactory = internalinformers.NewBaseKubeInformerFactory(b.Client, informerResyncPeriod, "")
 	b.SharedInformerFactory = informers.NewSharedInformerFactory(b.CMClient, informerResyncPeriod)
 	b.GWShared = gwinformers.NewSharedInformerFactory(b.GWClient, informerResyncPeriod)
-	b.MetadataInformerFactory = metadatainformer.NewFilteredSharedInformerFactory(b.MetadataClient, informerResyncPeriod, "", func(listOptions *metav1.ListOptions) {})
+	b.HTTP01ResourceMetadataInformersFactory = metadatainformer.NewFilteredSharedInformerFactory(b.MetadataClient, informerResyncPeriod, "", func(listOptions *metav1.ListOptions) {})
 	b.stopCh = make(chan struct{})
 	b.Metrics = metrics.New(logs.Log, clock.RealClock{})

@ -322,7 +322,7 @@ func (b *Builder) Start() {
 	b.KubeSharedInformerFactory.Start(b.stopCh)
 	b.SharedInformerFactory.Start(b.stopCh)
 	b.GWShared.Start(b.stopCh)
-	b.MetadataInformerFactory.Start(b.stopCh)
+	b.HTTP01ResourceMetadataInformersFactory.Start(b.stopCh)

 	// wait for caches to sync
 	b.Sync()
@ -338,7 +338,7 @@ func (b *Builder) Sync() {
 	if err := mustAllSync(b.GWShared.WaitForCacheSync(b.stopCh)); err != nil {
 		panic("Error waiting for GWShared to sync: " + err.Error())
 	}
-	if err := mustAllSyncGVR(b.MetadataInformerFactory.WaitForCacheSync(b.stopCh)); err != nil {
+	if err := mustAllSyncGVR(b.HTTP01ResourceMetadataInformersFactory.WaitForCacheSync(b.stopCh)); err != nil {
 		panic("Error waiting for MetadataInformerFactory to sync:" + err.Error())
 	}
 	if b.additionalSyncFuncs != nil {
--- a/pkg/issuer/acme/http/http.go
+++ b/pkg/issuer/acme/http/http.go
@ -74,8 +74,8 @@ type reachabilityTest func(ctx context.Context, url *url.URL, key string, dnsSer
 func NewSolver(ctx *controller.Context) (*Solver, error) {
 	return &Solver{
 		Context:          ctx,
-		podLister:        ctx.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
-		serviceLister:    ctx.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
+		podLister:        ctx.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
+		serviceLister:    ctx.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
 		ingressLister:    ctx.KubeSharedInformerFactory.Ingresses().Lister(),
 		httpRouteLister:  ctx.GWShared.Gateway().V1beta1().HTTPRoutes().Lister(),
 		testReachability: testReachability,
--- a/pkg/issuer/acme/http/pod_test.go
+++ b/pkg/issuer/acme/http/pod_test.go
@ -163,7 +163,7 @@ func TestEnsurePod(t *testing.T) {
 			scenario.builder.InitWithRESTConfig()
 			s := &Solver{
 				Context:   scenario.builder.Context,
-				podLister: scenario.builder.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
+				podLister: scenario.builder.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
 			}
 			s.Context.ACMEOptions = controller.ACMEOptions{
 				HTTP01SolverResourceRequestCPU:    cpuRequest,
@ -254,7 +254,7 @@ func TestGetPodsForChallenge(t *testing.T) {
 			scenario.builder.InitWithRESTConfig()
 			s := &Solver{
 				Context:   scenario.builder.Context,
-				podLister: scenario.builder.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
+				podLister: scenario.builder.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("pods")).Lister(),
 			}
 			defer scenario.builder.Stop()
 			scenario.builder.Start()
--- a/pkg/issuer/acme/http/service.go
+++ b/pkg/issuer/acme/http/service.go
@ -81,7 +81,6 @@ func (s *Solver) getServicesForChallenge(ctx context.Context, ch *cmacme.Challen

 	var relevantServices []*metav1.PartialObjectMetadata
 	for _, service := range serviceList {
-		// TODO: can we use a metadata specific lister instead?
 		s, ok := service.(*metav1.PartialObjectMetadata)
 		if !ok {
 			return nil, fmt.Errorf("internal error: cannot cast Service PartialObjectMetadata")
--- a/pkg/issuer/acme/http/service_test.go
+++ b/pkg/issuer/acme/http/service_test.go
@ -145,7 +145,7 @@ func TestEnsureService(t *testing.T) {
 			scenario.builder.InitWithRESTConfig()
 			s := &Solver{
 				Context:       scenario.builder.Context,
-				serviceLister: scenario.builder.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
+				serviceLister: scenario.builder.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
 			}
 			scenario.builder.Start()
 			defer scenario.builder.Stop()
@ -229,7 +229,7 @@ func TestGetServicesForChallenge(t *testing.T) {
 			scenario.builder.InitWithRESTConfig()
 			s := &Solver{
 				Context:       scenario.builder.Context,
-				serviceLister: scenario.builder.MetadataInformerFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
+				serviceLister: scenario.builder.HTTP01ResourceMetadataInformersFactory.ForResource(corev1.SchemeGroupVersion.WithResource("services")).Lister(),
 			}
 			scenario.builder.Start()
 			defer scenario.builder.Stop()