User generic in cert approve controller (#309)

Signed-off-by: Jian Qiu <jqiu@redhat.com>

pkg/hub/csr/controller.go

@@ -2,51 +2,68 @@ package csr
 
 import (
 	"context"
-
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
 	certificatesv1 "k8s.io/api/certificates/v1"
+	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	certificatesinformers "k8s.io/client-go/informers/certificates/v1"
 	"k8s.io/client-go/kubernetes"
-	certificateslisters "k8s.io/client-go/listers/certificates/v1"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
 	"open-cluster-management.io/registration/pkg/helpers"
 )
 
+type CSR interface {
+	*certificatesv1.CertificateSigningRequest | *certificatesv1beta1.CertificateSigningRequest
+}
+
+type CSRLister[T CSR] interface {
+	Get(name string) (T, error)
+}
+
+type CSRApprover[T CSR] interface {
+	approve(ctx context.Context, csr T) approveCSRFunc
+	isInTerminalState(csr T) bool
+}
+
 // csrApprovingController auto approve the renewal CertificateSigningRequests for an accepted spoke cluster on the hub.
-type csrApprovingController struct {
-	csrLister   certificateslisters.CertificateSigningRequestLister
+type csrApprovingController[T CSR] struct {
+	lister      CSRLister[T]
+	approver    CSRApprover[T]
 	reconcilers []Reconciler
 }
 
 // NewCSRApprovingController creates a new csr approving controller
-func NewCSRApprovingController(
-	csrInformer certificatesinformers.CertificateSigningRequestInformer,
+func NewCSRApprovingController[T CSR](
+	csrInformer cache.SharedIndexInformer,
+	lister CSRLister[T],
+	approver CSRApprover[T],
 	reconcilers []Reconciler,
 	recorder events.Recorder) factory.Controller {
-	c := &csrApprovingController{
-		csrLister:   csrInformer.Lister(),
+	c := &csrApprovingController[T]{
+		lister:      lister,
+		approver:    approver,
 		reconcilers: reconcilers,
 	}
+
 	return factory.New().
 		WithInformersQueueKeyFunc(func(obj runtime.Object) string {
 			accessor, _ := meta.Accessor(obj)
 			return accessor.GetName()
-		}, csrInformer.Informer()).
+		}, csrInformer).
 		WithSync(c.sync).
 		ToController("CSRApprovingController", recorder)
 }
 
-func (c *csrApprovingController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
+func (c *csrApprovingController[T]) sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	csrName := syncCtx.QueueKey()
 	klog.V(4).Infof("Reconciling CertificateSigningRequests %q", csrName)
 
-	csr, err := c.csrLister.Get(csrName)
+	csr, err := c.lister.Get(csrName)
 	if errors.IsNotFound(err) {
 		return nil
 	}
@@ -54,15 +71,13 @@ func (c *csrApprovingController) sync(ctx context.Context, syncCtx factory.SyncC
 		return err
 	}
 
-	csr = csr.DeepCopy()
-	// Current csr is in terminal state, do nothing.
-	if helpers.IsCSRInTerminalState(&csr.Status) {
+	if c.approver.isInTerminalState(csr) {
 		return nil
 	}
 
 	csrInfo := newCSRInfo(csr)
 	for _, r := range c.reconcilers {
-		state, err := r.Reconcile(ctx, csrInfo, approveCSRV1Func(ctx, csr))
+		state, err := r.Reconcile(ctx, csrInfo, c.approver.approve(ctx, csr))
 		if err != nil {
 			return err
 		}
@@ -74,16 +89,57 @@ func (c *csrApprovingController) sync(ctx context.Context, syncCtx factory.SyncC
 	return nil
 }
 
-func approveCSRV1Func(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) approveCSRFunc {
+// CSRV1Approver implement CSRApprover interface
+type CSRV1Approver struct {
+	kubeClient kubernetes.Interface
+}
+
+func NewCSRV1Approver(client kubernetes.Interface) *CSRV1Approver {
+	return &CSRV1Approver{kubeClient: client}
+}
+
+func (c *CSRV1Approver) isInTerminalState(csr *certificatesv1.CertificateSigningRequest) bool {
+	return helpers.IsCSRInTerminalState(&csr.Status)
+}
+
+func (c *CSRV1Approver) approve(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) approveCSRFunc {
 	return func(kubeClient kubernetes.Interface) error {
+		csrCopy := csr.DeepCopy()
 		// Auto approve the spoke cluster csr
-		csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{
+		csrCopy.Status.Conditions = append(csr.Status.Conditions, certificatesv1.CertificateSigningRequestCondition{
 			Type:    certificatesv1.CertificateApproved,
 			Status:  corev1.ConditionTrue,
 			Reason:  "AutoApprovedByHubCSRApprovingController",
 			Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.",
 		})
-		_, err := kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csr.Name, csr, metav1.UpdateOptions{})
+		_, err := kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrCopy.Name, csrCopy, metav1.UpdateOptions{})
+		return err
+	}
+}
+
+type CSRV1beta1Approver struct {
+	kubeClient kubernetes.Interface
+}
+
+func NewCSRV1beta1Approver(client kubernetes.Interface) *CSRV1beta1Approver {
+	return &CSRV1beta1Approver{kubeClient: client}
+}
+
+func (c *CSRV1beta1Approver) isInTerminalState(csr *certificatesv1beta1.CertificateSigningRequest) bool {
+	return helpers.Isv1beta1CSRInTerminalState(&csr.Status)
+}
+
+func (c *CSRV1beta1Approver) approve(ctx context.Context, csr *certificatesv1beta1.CertificateSigningRequest) approveCSRFunc {
+	return func(kubeClient kubernetes.Interface) error {
+		csrCopy := csr.DeepCopy()
+		// Auto approve the spoke cluster csr
+		csrCopy.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{
+			Type:    certificatesv1beta1.CertificateApproved,
+			Status:  corev1.ConditionTrue,
+			Reason:  "AutoApprovedByHubCSRApprovingController",
+			Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.",
+		})
+		_, err := kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, csrCopy, metav1.UpdateOptions{})
 		return err
 	}
 }

pkg/hub/csr/controller_beta.go

@@ -1,89 +0,0 @@
-package csr
-
-import (
-	"context"
-
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/klog/v2"
-	"open-cluster-management.io/registration/pkg/helpers"
-
-	"github.com/openshift/library-go/pkg/controller/factory"
-	"github.com/openshift/library-go/pkg/operator/events"
-	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
-	corev1 "k8s.io/api/core/v1"
-	certificatesv1beta1informers "k8s.io/client-go/informers/certificates/v1beta1"
-	certificatesv1beta1lister "k8s.io/client-go/listers/certificates/v1beta1"
-)
-
-// v1beta1CSRApprovingController auto approve the renewal CertificateSigningRequests for an accepted spoke cluster on the hub.
-type v1beta1CSRApprovingController struct {
-	csrLister   certificatesv1beta1lister.CertificateSigningRequestLister
-	reconcilers []Reconciler
-}
-
-func NewV1beta1CSRApprovingController(
-	v1beta1CSRInformer certificatesv1beta1informers.CertificateSigningRequestInformer,
-	reconcilers []Reconciler,
-	recorder events.Recorder) factory.Controller {
-
-	c := &v1beta1CSRApprovingController{
-		csrLister:   v1beta1CSRInformer.Lister(),
-		reconcilers: reconcilers,
-	}
-
-	return factory.New().WithInformersQueueKeyFunc(func(obj runtime.Object) string {
-		accessor, _ := meta.Accessor(obj)
-		return accessor.GetName()
-	}, v1beta1CSRInformer.Informer()).
-		WithSync(c.sync).
-		ToController("V1Beta1CSRApprovingController", recorder)
-}
-
-func (c *v1beta1CSRApprovingController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
-	csrName := syncCtx.QueueKey()
-	klog.V(4).Infof("Reconciling CertificateSigningRequests %q", csrName)
-	csr, err := c.csrLister.Get(csrName)
-	if errors.IsNotFound(err) {
-		return nil
-	}
-	if err != nil {
-		return err
-	}
-
-	csr = csr.DeepCopy()
-	// Current csr is in terminal state, do nothing.
-	if helpers.Isv1beta1CSRInTerminalState(&csr.Status) {
-		return nil
-	}
-
-	csrInfo := newCSRInfo(csr)
-	for _, r := range c.reconcilers {
-		state, err := r.Reconcile(ctx, csrInfo, approveCSRV1beta1Func(ctx, csr))
-		if err != nil {
-			return err
-		}
-		if state == reconcileStop {
-			break
-		}
-	}
-
-	return nil
-}
-
-func approveCSRV1beta1Func(ctx context.Context, csr *certificatesv1beta1.CertificateSigningRequest) approveCSRFunc {
-	return func(kubeClient kubernetes.Interface) error {
-		// Auto approve the spoke cluster csr
-		csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{
-			Type:    certificatesv1beta1.CertificateApproved,
-			Status:  corev1.ConditionTrue,
-			Reason:  "AutoApprovedByHubCSRApprovingController",
-			Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.",
-		})
-		_, err := kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, csr, metav1.UpdateOptions{})
-		return err
-	}
-}

pkg/hub/csr/controller_beta_test.go

@@ -143,9 +143,10 @@ func Test_v1beta1CSRApprovingController_sync(t *testing.T) {
 				}
 			}
 
-			ctrl := &v1beta1CSRApprovingController{
-				informerFactory.Certificates().V1beta1().CertificateSigningRequests().Lister(),
-				[]Reconciler{
+			ctrl := &csrApprovingController[*certificatesv1beta1.CertificateSigningRequest]{
+				lister:   informerFactory.Certificates().V1beta1().CertificateSigningRequests().Lister(),
+				approver: NewCSRV1beta1Approver(kubeClient),
+				reconcilers: []Reconciler{
 					&csrBootstrapReconciler{},
 					&csrRenewalReconciler{
 						kubeClient:    kubeClient,

pkg/hub/csr/controller_test.go

@@ -198,9 +198,10 @@ func TestSync(t *testing.T) {
 			}
 
 			recorder := eventstesting.NewTestingEventRecorder(t)
-			ctrl := &csrApprovingController{
-				informerFactory.Certificates().V1().CertificateSigningRequests().Lister(),
-				[]Reconciler{
+			ctrl := &csrApprovingController[*certificatesv1.CertificateSigningRequest]{
+				lister:   informerFactory.Certificates().V1().CertificateSigningRequests().Lister(),
+				approver: NewCSRV1Approver(kubeClient),
+				reconcilers: []Reconciler{
 					&csrBootstrapReconciler{
 						kubeClient:    kubeClient,
 						eventRecorder: recorder,

pkg/hub/manager.go

@@ -2,6 +2,8 @@ package hub
 
 import (
 	"context"
+	certv1 "k8s.io/api/certificates/v1"
+	certv1beta1 "k8s.io/api/certificates/v1beta1"
 	"time"
 
 	ocmfeature "open-cluster-management.io/api/feature"
@@ -124,8 +126,10 @@ func (m *HubManagerOptions) RunControllerManager(ctx context.Context, controller
 		}
 
 		if !v1CSRSupported && v1beta1CSRSupported {
-			csrController = csr.NewV1beta1CSRApprovingController(
-				kubeInfomers.Certificates().V1beta1().CertificateSigningRequests(),
+			csrController = csr.NewCSRApprovingController[*certv1beta1.CertificateSigningRequest](
+				kubeInfomers.Certificates().V1beta1().CertificateSigningRequests().Informer(),
+				kubeInfomers.Certificates().V1beta1().CertificateSigningRequests().Lister(),
+				csr.NewCSRV1beta1Approver(kubeClient),
 				csrReconciles,
 				controllerContext.EventRecorder,
 			)
@@ -133,8 +137,10 @@ func (m *HubManagerOptions) RunControllerManager(ctx context.Context, controller
 		}
 	}
 	if csrController == nil {
-		csrController = csr.NewCSRApprovingController(
-			kubeInfomers.Certificates().V1().CertificateSigningRequests(),
+		csrController = csr.NewCSRApprovingController[*certv1.CertificateSigningRequest](
+			kubeInfomers.Certificates().V1().CertificateSigningRequests().Informer(),
+			kubeInfomers.Certificates().V1().CertificateSigningRequests().Lister(),
+			csr.NewCSRV1Approver(kubeClient),
 			csrReconciles,
 			controllerContext.EventRecorder,
 		)