From 5530e6e3e0b817dbd6ad79addd997199c2bbb125 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Wed, 12 Jul 2023 09:17:03 -0600 Subject: [PATCH] *: use correct clients in the secretannotator We were (again) accidentally watching all ConfigMaps in the cluster. This patch should fix the errant code by using the correct client as well as by removing the RBAC to do so in the first place. Signed-off-by: Steve Kuznetsov --- manifests/01-cluster-role.yaml | 1 - manifests/01-role.yaml | 10 +++- .../secretannotator/aws/reconciler.go | 19 +++---- .../secretannotator/aws/reconciler_test.go | 54 +++++++++++++------ .../secretannotator/azure/reconciler.go | 25 ++++----- .../secretannotator/azure/reconciler_test.go | 10 ++-- .../secretannotator/gcp/reconciler.go | 19 +++---- .../secretannotator/gcp/reconciler_test.go | 43 ++++++++++----- .../secretannotator/openstack/reconciler.go | 27 +++++----- .../openstack/reconciler_test.go | 20 ++++--- .../secretannotator_controller.go | 12 ++--- pkg/operator/secretannotator/utils/utils.go | 7 +-- .../secretannotator/vsphere/reconciler.go | 24 +++++---- .../vsphere/reconciler_test.go | 21 +++++--- 14 files changed, 182 insertions(+), 110 deletions(-) diff --git a/manifests/01-cluster-role.yaml b/manifests/01-cluster-role.yaml index 0588bb32d1..0d6a8cb117 100644 --- a/manifests/01-cluster-role.yaml +++ b/manifests/01-cluster-role.yaml @@ -25,7 +25,6 @@ rules: - "" resources: - secrets - - configmaps - events verbs: - get diff --git a/manifests/01-role.yaml b/manifests/01-role.yaml index 0bf2ac952a..e998762b07 100644 --- a/manifests/01-role.yaml +++ b/manifests/01-role.yaml @@ -11,12 +11,20 @@ rules: - "" resources: - secrets - - configmaps - events - serviceaccounts - services verbs: - "*" +- apiGroups: + - "" + resources: + - configmaps + resourceNames: + - cloud-credential-operator-config + - cloud-credential-operator-leader + verbs: + - "*" - apiGroups: - apps resources: diff --git a/pkg/operator/secretannotator/aws/reconciler.go b/pkg/operator/secretannotator/aws/reconciler.go index d34d67ee5d..5de5a07db4 100644 --- a/pkg/operator/secretannotator/aws/reconciler.go +++ b/pkg/operator/secretannotator/aws/reconciler.go @@ -38,10 +38,10 @@ const ( AwsSecretAccessKeyName = "aws_secret_access_key" ) -func NewReconciler(mgr manager.Manager) reconcile.Reconciler { - c := mgr.GetClient() +func NewReconciler(c client.Client, mgr manager.Manager) reconcile.Reconciler { r := &ReconcileCloudCredSecret{ Client: c, + RootCredClient: mgr.GetClient(), Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), AWSClientBuilder: awsutils.ClientBuilder, } @@ -56,9 +56,9 @@ func cloudCredSecretObjectCheck(secret metav1.Object) bool { return secret.GetNamespace() == constants.CloudCredSecretNamespace && secret.GetName() == constants.AWSCloudCredSecretName } -func Add(mgr manager.Manager, r reconcile.Reconciler) error { +func Add(mgr, rootCredMgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller - c, err := controller.New(constants.SecretAnnotatorControllerName, mgr, controller.Options{Reconciler: r}) + c, err := controller.New(constants.SecretAnnotatorControllerName, rootCredMgr, controller.Options{Reconciler: r}) if err != nil { return err } @@ -75,12 +75,12 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { return cloudCredSecretObjectCheck(e.Object) }, } - err = c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) + err = c.Watch(source.Kind(rootCredMgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) if err != nil { return err } - err = secretutils.WatchCCOConfig(c, types.NamespacedName{ + err = secretutils.WatchCCOConfig(mgr.GetCache(), c, types.NamespacedName{ Namespace: constants.CloudCredSecretNamespace, Name: constants.AWSCloudCredSecretName, }, mgr) @@ -94,7 +94,8 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { var _ reconcile.Reconciler = &ReconcileCloudCredSecret{} type ReconcileCloudCredSecret struct { - client.Client + Client client.Client + RootCredClient client.Client Logger log.FieldLogger AWSClientBuilder func(accessKeyID, secretAccessKey []byte, c client.Client) (ccaws.Client, error) } @@ -133,7 +134,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc } secret := &corev1.Secret{} - err = r.Get(context.Background(), request.NamespacedName, secret) + err = r.RootCredClient.Get(context.Background(), request.NamespacedName, secret) if err != nil { if errors.IsNotFound(err) { r.Logger.Info("parent credential secret does not exist") @@ -232,5 +233,5 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret secretAnnotations[constants.AnnotationKey] = value secret.SetAnnotations(secretAnnotations) - return r.Update(context.Background(), secret) + return r.RootCredClient.Update(context.Background(), secret) } diff --git a/pkg/operator/secretannotator/aws/reconciler_test.go b/pkg/operator/secretannotator/aws/reconciler_test.go index b5e8b487f7..f333e92a50 100644 --- a/pkg/operator/secretannotator/aws/reconciler_test.go +++ b/pkg/operator/secretannotator/aws/reconciler_test.go @@ -89,6 +89,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { tests := []struct { name string existing []runtime.Object + existingRootCred []runtime.Object expectErr bool mockAWSClient func(mockCtrl *gomock.Controller) *mockaws.MockClient validateAnnotationValue string @@ -96,9 +97,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "cred minter mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -111,24 +114,30 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "operator disabled via configmap", existing: []runtime.Object{ - testSecret(), testOperatorConfigMap("true"), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, }, { name: "operator disabled", existing: []runtime.Object{ - testSecret(), testOperatorConfig(operatorv1.CloudCredentialsModeManual), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, }, { name: "detect root user creds", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetRootUser(mockAWSClient) @@ -140,9 +149,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "cred passthrough mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -158,9 +169,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "cred passthrough mode wrong region permission", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -176,9 +189,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "useless creds", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -202,6 +217,9 @@ func TestSecretAnnotatorReconcile(t *testing.T) { name: "secret missing key", expectErr: true, existing: []runtime.Object{ + testOperatorConfig(""), + }, + existingRootCred: []runtime.Object{ &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: testSecretName, @@ -212,28 +230,32 @@ func TestSecretAnnotatorReconcile(t *testing.T) { "not_aws_secret_access_key": []byte(testAWSSecretAccessKey), }, }, - testOperatorConfig(""), }, }, { name: "annotation matches forced mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(operatorv1.CloudCredentialsModeMint), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, validateAnnotationValue: constants.MintAnnotation, }, { name: "unknown mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig("notARealMode"), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, expectErr: true, }, { - name: "error on missing config CR", - existing: []runtime.Object{ + name: "error on missing config CR", + existing: []runtime.Object{}, + existingRootCred: []runtime.Object{ testSecret(), }, expectErr: true, @@ -263,6 +285,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { existing := append(test.existing, infra) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(test.existingRootCred...).Build() fakeAWSClient := mockaws.NewMockClient(mockCtrl) if test.mockAWSClient != nil { @@ -270,8 +293,9 @@ func TestSecretAnnotatorReconcile(t *testing.T) { } rcc := &annaws.ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testController"), AWSClientBuilder: func(accessKeyID, secretAccessKey []byte, c client.Client) (ccaws.Client, error) { return fakeAWSClient, nil }, @@ -289,7 +313,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { } if test.validateAnnotationValue != "" { - validateSecretAnnotation(fakeClient, t, test.validateAnnotationValue) + validateSecretAnnotation(fakeRootCredClient, t, test.validateAnnotationValue) } }) } diff --git a/pkg/operator/secretannotator/azure/reconciler.go b/pkg/operator/secretannotator/azure/reconciler.go index 2700301a1b..258ae2ab9d 100644 --- a/pkg/operator/secretannotator/azure/reconciler.go +++ b/pkg/operator/secretannotator/azure/reconciler.go @@ -33,15 +33,16 @@ import ( var _ reconcile.Reconciler = &ReconcileCloudCredSecret{} type ReconcileCloudCredSecret struct { - client.Client - Logger log.FieldLogger + Client client.Client + RootCredClient client.Client + Logger log.FieldLogger } -func NewReconciler(mgr manager.Manager) reconcile.Reconciler { - c := mgr.GetClient() +func NewReconciler(c client.Client, mgr manager.Manager) reconcile.Reconciler { r := &ReconcileCloudCredSecret{ - Client: c, - Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), + Client: c, + RootCredClient: mgr.GetClient(), + Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), } s := status.NewSecretStatusHandler(c) @@ -50,9 +51,9 @@ func NewReconciler(mgr manager.Manager) reconcile.Reconciler { return r } -func Add(mgr manager.Manager, r reconcile.Reconciler) error { +func Add(mgr, rootCredMgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller - c, err := controller.New(constants.SecretAnnotatorControllerName, mgr, controller.Options{Reconciler: r}) + c, err := controller.New(constants.SecretAnnotatorControllerName, rootCredMgr, controller.Options{Reconciler: r}) if err != nil { return err } @@ -69,12 +70,12 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { return cloudCredSecretObjectCheck(e.Object) }, } - err = c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) + err = c.Watch(source.Kind(rootCredMgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) if err != nil { return err } - secretutils.WatchCCOConfig(c, types.NamespacedName{ + secretutils.WatchCCOConfig(mgr.GetCache(), c, types.NamespacedName{ Namespace: constants.CloudCredSecretNamespace, Name: constants.AzureCloudCredSecretName, }, mgr) @@ -115,7 +116,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc // From here down we must be in Passthrough mode secret := &corev1.Secret{} - err = r.Get(context.Background(), request.NamespacedName, secret) + err = r.RootCredClient.Get(context.Background(), request.NamespacedName, secret) if err != nil { r.Logger.Debugf("secret not found: %v", err) return reconcile.Result{}, err @@ -151,5 +152,5 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret secretAnnotations[constants.AnnotationKey] = value secret.SetAnnotations(secretAnnotations) - return r.Update(context.Background(), secret) + return r.RootCredClient.Update(context.Background(), secret) } diff --git a/pkg/operator/secretannotator/azure/reconciler_test.go b/pkg/operator/secretannotator/azure/reconciler_test.go index 813eb3323b..b3b75f293d 100644 --- a/pkg/operator/secretannotator/azure/reconciler_test.go +++ b/pkg/operator/secretannotator/azure/reconciler_test.go @@ -67,11 +67,13 @@ func TestAzureSecretAnnotatorReconcile(t *testing.T) { t.Run(test.name, func(t *testing.T) { credsSecret := getInputSecret() - fakeClient := fake.NewClientBuilder().WithObjects(credsSecret, test.operatorConfig).Build() + fakeClient := fake.NewClientBuilder().WithObjects(test.operatorConfig).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(credsSecret).Build() rcc := &ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testSecretAnnotatorController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testSecretAnnotatorController"), } _, err := rcc.Reconcile(context.TODO(), reconcile.Request{ @@ -84,7 +86,7 @@ func TestAzureSecretAnnotatorReconcile(t *testing.T) { require.NoError(t, err, "unexpected error in secret annotator controller") secret := &corev1.Secret{} - err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: TestSecretName, Namespace: TestNamespace}, secret) + err = fakeRootCredClient.Get(context.TODO(), client.ObjectKey{Name: TestSecretName, Namespace: TestNamespace}, secret) require.NoError(t, err, "error fetching object from fake client") assert.Equal(t, string(test.expectedAnnotation), string(secret.ObjectMeta.Annotations[constants.AnnotationKey])) diff --git a/pkg/operator/secretannotator/gcp/reconciler.go b/pkg/operator/secretannotator/gcp/reconciler.go index 5160c8dcc0..c8bbfdad54 100644 --- a/pkg/operator/secretannotator/gcp/reconciler.go +++ b/pkg/operator/secretannotator/gcp/reconciler.go @@ -40,10 +40,10 @@ const ( GCPAuthJSONKey = "service_account.json" ) -func NewReconciler(mgr manager.Manager, projectName string) reconcile.Reconciler { - c := mgr.GetClient() +func NewReconciler(c client.Client, mgr manager.Manager, projectName string) reconcile.Reconciler { r := &ReconcileCloudCredSecret{ Client: c, + RootCredClient: mgr.GetClient(), Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), GCPClientBuilder: ccgcp.NewClientFromJSON, ProjectName: projectName, @@ -59,9 +59,9 @@ func cloudCredSecretObjectCheck(secret metav1.Object) bool { return secret.GetNamespace() == constants.CloudCredSecretNamespace && secret.GetName() == constants.GCPCloudCredSecretName } -func Add(mgr manager.Manager, r reconcile.Reconciler) error { +func Add(mgr, rootCredMgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller - c, err := controller.New(constants.SecretAnnotatorControllerName, mgr, controller.Options{Reconciler: r}) + c, err := controller.New(constants.SecretAnnotatorControllerName, rootCredMgr, controller.Options{Reconciler: r}) if err != nil { return err } @@ -78,12 +78,12 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { return cloudCredSecretObjectCheck(e.Object) }, } - err = c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) + err = c.Watch(source.Kind(rootCredMgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) if err != nil { return err } - err = secretutils.WatchCCOConfig(c, types.NamespacedName{ + err = secretutils.WatchCCOConfig(mgr.GetCache(), c, types.NamespacedName{ Namespace: constants.CloudCredSecretNamespace, Name: constants.GCPCloudCredSecretName, }, mgr) @@ -97,7 +97,8 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { var _ reconcile.Reconciler = &ReconcileCloudCredSecret{} type ReconcileCloudCredSecret struct { - client.Client + Client client.Client + RootCredClient client.Client ProjectName string Logger log.FieldLogger GCPClientBuilder func(projectName string, authJSON []byte) (ccgcp.Client, error) @@ -137,7 +138,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc } secret := &corev1.Secret{} - err = r.Get(context.Background(), request.NamespacedName, secret) + err = r.RootCredClient.Get(context.Background(), request.NamespacedName, secret) if err != nil { r.Logger.Debugf("secret not found: %v", err) return reconcile.Result{}, err @@ -217,5 +218,5 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret secretAnnotations[constants.AnnotationKey] = value secret.SetAnnotations(secretAnnotations) - return r.Update(context.TODO(), secret) + return r.RootCredClient.Update(context.TODO(), secret) } diff --git a/pkg/operator/secretannotator/gcp/reconciler_test.go b/pkg/operator/secretannotator/gcp/reconciler_test.go index b4ba31a765..9b594ca457 100644 --- a/pkg/operator/secretannotator/gcp/reconciler_test.go +++ b/pkg/operator/secretannotator/gcp/reconciler_test.go @@ -41,7 +41,7 @@ import ( ccgcp "github.com/openshift/cloud-credential-operator/pkg/gcp" mockgcp "github.com/openshift/cloud-credential-operator/pkg/gcp/mock" - cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/api/cloudresourcemanager/v1" iamadminpb "google.golang.org/genproto/googleapis/iam/admin/v1" "github.com/openshift/cloud-credential-operator/pkg/operator/constants" @@ -80,6 +80,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { tests := []struct { name string existing []runtime.Object + existingRootCred []runtime.Object expectErr bool mockGCPClient func(mockCtrl *gomock.Controller) *mockgcp.MockClient validateAnnotationValue string @@ -87,9 +88,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "cred minter mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -104,10 +107,12 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "operator disabled via configmap", existing: []runtime.Object{ - testSecret(), testOperatorConfigMap("true"), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) return mockGCPClient @@ -116,16 +121,20 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "operator disabled", existing: []runtime.Object{ - testSecret(), testOperatorConfig(operatorv1.CloudCredentialsModeManual), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, }, { name: "cred passthrough mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -141,9 +150,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "useless creds", existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -165,6 +176,9 @@ func TestSecretAnnotatorReconcile(t *testing.T) { { name: "secret missing key", existing: []runtime.Object{ + testOperatorConfig(""), + }, + existingRootCred: []runtime.Object{ &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: testSecretName, @@ -174,24 +188,27 @@ func TestSecretAnnotatorReconcile(t *testing.T) { "not-the-right-key": []byte(testGCPAuthJSON), }, }, - testOperatorConfig(""), }, validateAnnotationValue: constants.InsufficientAnnotation, }, { name: "annotation matches forced mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig(operatorv1.CloudCredentialsModeMint), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, validateAnnotationValue: constants.MintAnnotation, }, { name: "unknown mode", existing: []runtime.Object{ - testSecret(), testOperatorConfig("notARealMode"), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, expectErr: true, }, } @@ -214,6 +231,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { existing := append(test.existing, infra) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(test.existingRootCred...).Build() fakeGCPClient := mockgcp.NewMockClient(mockCtrl) if test.mockGCPClient != nil { @@ -221,8 +239,9 @@ func TestSecretAnnotatorReconcile(t *testing.T) { } rcc := &anngcp.ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testController"), GCPClientBuilder: func(projectName string, authJSON []byte) (ccgcp.Client, error) { return fakeGCPClient, nil }, @@ -238,7 +257,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { if test.expectErr { assert.Error(t, err, "expected error to be returned") } else if test.validateAnnotationValue != "" { - validateSecretAnnotation(fakeClient, t, test.validateAnnotationValue) + validateSecretAnnotation(fakeRootCredClient, t, test.validateAnnotationValue) } }) } diff --git a/pkg/operator/secretannotator/openstack/reconciler.go b/pkg/operator/secretannotator/openstack/reconciler.go index 2956d61fd6..27bc42ff12 100644 --- a/pkg/operator/secretannotator/openstack/reconciler.go +++ b/pkg/operator/secretannotator/openstack/reconciler.go @@ -48,11 +48,11 @@ import ( "github.com/openshift/cloud-credential-operator/pkg/operator/utils" ) -func NewReconciler(mgr manager.Manager) reconcile.Reconciler { - c := mgr.GetClient() +func NewReconciler(c client.Client, mgr manager.Manager) reconcile.Reconciler { r := &ReconcileCloudCredSecret{ - Client: c, - Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), + Client: c, + RootCredClient: mgr.GetClient(), + Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), } s := status.NewSecretStatusHandler(c) @@ -65,9 +65,9 @@ func cloudCredSecretObjectCheck(secret metav1.Object) bool { return secret.GetNamespace() == constants.CloudCredSecretNamespace && secret.GetName() == constants.OpenStackCloudCredsSecretName } -func Add(mgr manager.Manager, r reconcile.Reconciler) error { +func Add(mgr, rootCredMgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller - c, err := controller.New(constants.SecretAnnotatorControllerName, mgr, controller.Options{Reconciler: r}) + c, err := controller.New(constants.SecretAnnotatorControllerName, rootCredMgr, controller.Options{Reconciler: r}) if err != nil { return err } @@ -84,12 +84,12 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { return cloudCredSecretObjectCheck(e.Object) }, } - err = c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) + err = c.Watch(source.Kind(rootCredMgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) if err != nil { return err } - err = secretutils.WatchCCOConfig(c, types.NamespacedName{ + err = secretutils.WatchCCOConfig(mgr.GetCache(), c, types.NamespacedName{ Namespace: constants.CloudCredSecretNamespace, Name: constants.OpenStackCloudCredsSecretName, }, mgr) @@ -103,8 +103,9 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { var _ reconcile.Reconciler = &ReconcileCloudCredSecret{} type ReconcileCloudCredSecret struct { - client.Client - Logger log.FieldLogger + Client client.Client + RootCredClient client.Client + Logger log.FieldLogger } // Reconcile will typically annotate the cloud cred secret to indicate the capabilities of the cloud credentials: @@ -149,7 +150,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc } secret := &corev1.Secret{} - err = r.Get(context.Background(), request.NamespacedName, secret) + err = r.RootCredClient.Get(context.Background(), request.NamespacedName, secret) if err != nil { r.Logger.Debugf("secret not found: %v", err) return reconcile.Result{}, err @@ -169,7 +170,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc if cloudsUpdated { openstack.SetRootCloudCredentialsSecretData(secret, clouds) - err := r.Update(context.TODO(), secret) + err := r.RootCredClient.Update(context.TODO(), secret) if err != nil { r.Logger.WithError(err).Error("error writing updated root secret") } @@ -270,5 +271,5 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret secretAnnotations[constants.AnnotationKey] = value secret.SetAnnotations(secretAnnotations) - return r.Update(context.TODO(), secret) + return r.RootCredClient.Update(context.TODO(), secret) } diff --git a/pkg/operator/secretannotator/openstack/reconciler_test.go b/pkg/operator/secretannotator/openstack/reconciler_test.go index e625ed426d..e738918037 100644 --- a/pkg/operator/secretannotator/openstack/reconciler_test.go +++ b/pkg/operator/secretannotator/openstack/reconciler_test.go @@ -179,12 +179,14 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { secret := testSecret(fmt.Sprintf(cloudsWithCACert, correctCACertFile)) - existing := append(tc.existing, infra, secret, testOperatorConfig(tc.mode)) + existing := append(tc.existing, infra, testOperatorConfig(tc.mode)) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret).Build() r := &ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testController"), } _, err := r.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{ Name: "openstack-credentials", @@ -198,7 +200,7 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { } reconciledSecret := &corev1.Secret{} - err = fakeClient.Get(context.TODO(), client.ObjectKey{ + err = fakeRootCredClient.Get(context.TODO(), client.ObjectKey{ Namespace: secret.Namespace, Name: secret.Name, }, reconciledSecret) @@ -267,12 +269,14 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { secret := testSecret(tc.cloudsYAML) - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(infra, passthrough, secret).Build() + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(infra, passthrough).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret).Build() t.Logf("clouds.yaml: %s", tc.cloudsYAML) r := &ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testController"), } _, reconcileErr := r.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{ @@ -281,7 +285,7 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { }}) reconciledSecret := &corev1.Secret{} - err := fakeClient.Get(context.TODO(), client.ObjectKey{ + err := fakeRootCredClient.Get(context.TODO(), client.ObjectKey{ Namespace: secret.Namespace, Name: secret.Name, }, reconciledSecret) diff --git a/pkg/operator/secretannotator/secretannotator_controller.go b/pkg/operator/secretannotator/secretannotator_controller.go index dff44841e3..e045e76b94 100644 --- a/pkg/operator/secretannotator/secretannotator_controller.go +++ b/pkg/operator/secretannotator/secretannotator_controller.go @@ -40,19 +40,19 @@ func Add(mgr, rootCredentialManager manager.Manager, kubeconfig string) error { switch platformType { case configv1.AzurePlatformType: - return azure.Add(rootCredentialManager, azure.NewReconciler(rootCredentialManager)) + return azure.Add(mgr, rootCredentialManager, azure.NewReconciler(mgr.GetClient(), rootCredentialManager)) case configv1.AWSPlatformType: - return aws.Add(rootCredentialManager, aws.NewReconciler(rootCredentialManager)) + return aws.Add(mgr, rootCredentialManager, aws.NewReconciler(mgr.GetClient(), rootCredentialManager)) case configv1.GCPPlatformType: if infraStatus.PlatformStatus == nil || infraStatus.PlatformStatus.GCP == nil { log.Fatalf("Missing GCP configuration in infrastructure platform status") } - return gcp.Add(rootCredentialManager, gcp.NewReconciler(rootCredentialManager, infraStatus.PlatformStatus.GCP.ProjectID)) + return gcp.Add(mgr, rootCredentialManager, gcp.NewReconciler(mgr.GetClient(), rootCredentialManager, infraStatus.PlatformStatus.GCP.ProjectID)) case configv1.VSpherePlatformType: - return vsphere.Add(rootCredentialManager, vsphere.NewReconciler(rootCredentialManager)) + return vsphere.Add(mgr, rootCredentialManager, vsphere.NewReconciler(mgr.GetClient(), rootCredentialManager)) case configv1.OpenStackPlatformType: - return openstack.Add(rootCredentialManager, openstack.NewReconciler(rootCredentialManager)) + return openstack.Add(mgr, rootCredentialManager, openstack.NewReconciler(mgr.GetClient(), rootCredentialManager)) default: // returning the AWS implementation for default to avoid changing any behavior - return aws.Add(rootCredentialManager, aws.NewReconciler(rootCredentialManager)) + return aws.Add(mgr, rootCredentialManager, aws.NewReconciler(mgr.GetClient(), rootCredentialManager)) } } diff --git a/pkg/operator/secretannotator/utils/utils.go b/pkg/operator/secretannotator/utils/utils.go index 16b3ff702c..442e3e64f5 100644 --- a/pkg/operator/secretannotator/utils/utils.go +++ b/pkg/operator/secretannotator/utils/utils.go @@ -2,8 +2,10 @@ package utils import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/client" @@ -23,8 +25,7 @@ var cloudSecretKeyPassThru types.NamespacedName // WatchCCOConfig will add a watch to the provided controller for the operator // config resource which will schedule the provided secret for reconciliation. -func WatchCCOConfig(c controller.Controller, cloudSecretKey types.NamespacedName, mgr manager.Manager) error { - operatorCache := mgr.GetCache() +func WatchCCOConfig(cache cache.Cache, c controller.Controller, cloudSecretKey types.NamespacedName, mgr manager.Manager) error { cloudSecretKeyPassThru = cloudSecretKey configPredicate := predicate.Funcs{ @@ -39,7 +40,7 @@ func WatchCCOConfig(c controller.Controller, cloudSecretKey types.NamespacedName }, } - err := c.Watch(source.Kind(operatorCache, &operatorv1.CloudCredential{}), + err := c.Watch(source.Kind(cache, &operatorv1.CloudCredential{}), handler.EnqueueRequestsFromMapFunc(cloudCredSecretRequest), configPredicate, ) diff --git a/pkg/operator/secretannotator/vsphere/reconciler.go b/pkg/operator/secretannotator/vsphere/reconciler.go index 851118e36c..74d9e33146 100644 --- a/pkg/operator/secretannotator/vsphere/reconciler.go +++ b/pkg/operator/secretannotator/vsphere/reconciler.go @@ -52,22 +52,24 @@ var _ reconcile.Reconciler = &ReconcileCloudCredSecret{} // ReconcileCloudCredSecret is for reconciling the cloud cred secret for the purpose of annotating it. type ReconcileCloudCredSecret struct { - client.Client - Logger log.FieldLogger + Client client.Client + RootCredClient client.Client + Logger log.FieldLogger } // NewReconciler will return a reconciler for handling vSphere cloud cred secrets. -func NewReconciler(mgr manager.Manager) reconcile.Reconciler { +func NewReconciler(c client.Client, mgr manager.Manager) reconcile.Reconciler { return &ReconcileCloudCredSecret{ - Client: mgr.GetClient(), - Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), + Client: c, + RootCredClient: mgr.GetClient(), + Logger: log.WithField("controller", constants.SecretAnnotatorControllerName), } } // Add sets up a controller for watching the cloud cred secret. -func Add(mgr manager.Manager, r reconcile.Reconciler) error { +func Add(mgr, rootCredMgr manager.Manager, r reconcile.Reconciler) error { // Create a new controller - c, err := controller.New(constants.SecretAnnotatorControllerName, mgr, controller.Options{Reconciler: r}) + c, err := controller.New(constants.SecretAnnotatorControllerName, rootCredMgr, controller.Options{Reconciler: r}) if err != nil { return err } @@ -84,12 +86,12 @@ func Add(mgr manager.Manager, r reconcile.Reconciler) error { return cloudCredSecretObjectCheck(e.Object) }, } - err = c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) + err = c.Watch(source.Kind(rootCredMgr.GetCache(), &corev1.Secret{}), &handler.EnqueueRequestForObject{}, p) if err != nil { return err } - err = secretutils.WatchCCOConfig(c, types.NamespacedName{ + err = secretutils.WatchCCOConfig(mgr.GetCache(), c, types.NamespacedName{ Namespace: constants.CloudCredSecretNamespace, Name: constants.VSphereCloudCredSecretName, }, mgr) @@ -134,7 +136,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc } secret := &corev1.Secret{} - err = r.Get(context.Background(), request.NamespacedName, secret) + err = r.RootCredClient.Get(context.Background(), request.NamespacedName, secret) if err != nil { r.Logger.Debugf("secret not found: %v", err) return reconcile.Result{}, err @@ -179,5 +181,5 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret secretAnnotations[constants.AnnotationKey] = value secret.SetAnnotations(secretAnnotations) - return r.Update(context.Background(), secret) + return r.RootCredClient.Update(context.Background(), secret) } diff --git a/pkg/operator/secretannotator/vsphere/reconciler_test.go b/pkg/operator/secretannotator/vsphere/reconciler_test.go index 1bb09291ba..cd0d052488 100644 --- a/pkg/operator/secretannotator/vsphere/reconciler_test.go +++ b/pkg/operator/secretannotator/vsphere/reconciler_test.go @@ -58,32 +58,39 @@ func TestSecretAnnotatorReconcile(t *testing.T) { tests := []struct { name string existing []runtime.Object + existingRootCred []runtime.Object expectErr bool validateAnnotationValue string }{ { name: "operator disabled", existing: []runtime.Object{ - testSecret(), testOperatorConfigMap("true"), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, }, { name: "operator enabled", existing: []runtime.Object{ - testSecret(), testOperatorConfigMap("false"), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, }, { name: "annotate passthrough mode", // right now only passthrough mode is supported so any secret works existing: []runtime.Object{ - testSecret(), testOperatorConfig(""), }, + existingRootCred: []runtime.Object{ + testSecret(), + }, validateAnnotationValue: constants.PassthroughAnnotation, }, { @@ -110,10 +117,12 @@ func TestSecretAnnotatorReconcile(t *testing.T) { existing := append(test.existing, infra) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(test.existingRootCred...).Build() rcc := &ReconcileCloudCredSecret{ - Client: fakeClient, - Logger: log.WithField("controller", "testController"), + Client: fakeClient, + RootCredClient: fakeRootCredClient, + Logger: log.WithField("controller", "testController"), } _, err := rcc.Reconcile(context.TODO(), reconcile.Request{ @@ -128,7 +137,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { } if test.validateAnnotationValue != "" { - validateSecretAnnotation(fakeClient, t, test.validateAnnotationValue) + validateSecretAnnotation(fakeRootCredClient, t, test.validateAnnotationValue) } }) }