From fec517c2fc0305934e2526fe2c96f1fc72780bb8 Mon Sep 17 00:00:00 2001 From: Alex Chvatal Date: Fri, 9 Aug 2024 14:09:49 -0400 Subject: [PATCH] test the operator identity secret generation code properly --- pkg/cluster/arooperator_test.go | 50 +---------------------- pkg/operator/deploy/deploy.go | 55 ++++++++++++++----------- pkg/operator/deploy/deploy_test.go | 64 ++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 72 deletions(-) diff --git a/pkg/cluster/arooperator_test.go b/pkg/cluster/arooperator_test.go index dc4de63a0be..2d57c8c6bea 100644 --- a/pkg/cluster/arooperator_test.go +++ b/pkg/cluster/arooperator_test.go @@ -141,60 +141,12 @@ func TestInstallAROOperator(t *testing.T) { wantErr string }{ { - name: "install success with service principal", + name: "install success", doc: &api.OpenShiftClusterDocument{ Key: strings.ToLower(key), OpenShiftCluster: &api.OpenShiftCluster{ ID: key, Properties: api.OpenShiftClusterProperties{ - ServicePrincipalProfile: &api.ServicePrincipalProfile{}, - IngressProfiles: []api.IngressProfile{ - { - Visibility: api.VisibilityPublic, - Name: "default", - }, - }, - ProvisioningState: api.ProvisioningStateAdminUpdating, - ClusterProfile: api.ClusterProfile{ - Version: "4.8.18", - }, - NetworkProfile: api.NetworkProfile{ - APIServerPrivateEndpointIP: "1.2.3.4", - }, - }, - }, - }, - mocks: func(dep *mock_deploy.MockOperator) { - dep.EXPECT(). - Install(gomock.Any()). - Return(nil) - }, - }, - { - name: "install success with identity", - doc: &api.OpenShiftClusterDocument{ - Key: strings.ToLower(key), - OpenShiftCluster: &api.OpenShiftCluster{ - ID: key, - Identity: &api.Identity{ - UserAssignedIdentities: api.UserAssignedIdentities{ - "fakeIdentity": api.ClusterUserAssignedIdentity{ - ClientID: "fake", - PrincipalID: "alsoFake", - }, - }, - }, - Properties: api.OpenShiftClusterProperties{ - PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{ - PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{ - { - OperatorName: "openshift-azure-operator", - ObjectID: "00000000-0000-0000-0000-000000000000", - ClientID: "11111111-1111-1111-1111-111111111111", - ResourceID: "/subscriptions/22222222-2222-2222-2222-222222222222/resourceGroups/something/providers/Microsoft.ManagedIdentity/userAssignedIdentities/identity-name", - }, - }, - }, IngressProfiles: []api.IngressProfile{ { Visibility: api.VisibilityPublic, diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index 2d7fce7ced3..6c16d372ad3 100644 --- a/pkg/operator/deploy/deploy.go +++ b/pkg/operator/deploy/deploy.go @@ -199,31 +199,12 @@ func (o *operator) resources(ctx context.Context) ([]kruntime.Object, error) { // then dynamic resources if o.oc.UsesWorkloadIdentity() { - var operatorIdentity *api.PlatformWorkloadIdentity // use a pointer to make it easy to check if we found an identity below - for _, i := range o.oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities { - if i.OperatorName == pkgoperator.OperatorIdentityName { - operatorIdentity = &i - break - } - } - - if operatorIdentity == nil { - return nil, errors.New(fmt.Sprintf("operator identity %s not found", pkgoperator.OperatorIdentityName)) + operatorIdentitySecret, err := o.generateOperatorIdentitySecret() + if err != nil { + return nil, err } - results = append(results, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: pkgoperator.Namespace, - Name: pkgoperator.OperatorIdentitySecretName, - }, - StringData: map[string]string{ - "azure_client_id": operatorIdentity.ClientID, - "azure_tenant_id": o.subscriptiondoc.Subscription.Properties.TenantID, - "azure_region": o.oc.Location, - "azure_subscription_id": o.subscriptiondoc.ID, - "azure_federated_token_file": pkgoperator.OperatorTokenFile, - }, - }) + results = append(results, operatorIdentitySecret) } key, cert := o.env.ClusterGenevaLoggingSecret() @@ -259,6 +240,34 @@ func (o *operator) resources(ctx context.Context) ([]kruntime.Object, error) { ), nil } +func (o *operator) generateOperatorIdentitySecret() (*corev1.Secret, error) { + var operatorIdentity *api.PlatformWorkloadIdentity // use a pointer to make it easy to check if we found an identity below + for _, i := range o.oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities { + if i.OperatorName == pkgoperator.OperatorIdentityName { + operatorIdentity = &i + break + } + } + + if operatorIdentity == nil { + return nil, fmt.Errorf("operator identity %s not found", pkgoperator.OperatorIdentityName) + } + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkgoperator.Namespace, + Name: pkgoperator.OperatorIdentitySecretName, + }, + StringData: map[string]string{ + "azure_client_id": operatorIdentity.ClientID, + "azure_tenant_id": o.subscriptiondoc.Subscription.Properties.TenantID, + "azure_region": o.oc.Location, + "azure_subscription_id": o.subscriptiondoc.ID, + "azure_federated_token_file": pkgoperator.OperatorTokenFile, + }, + }, nil +} + func (o *operator) clusterObject() (*arov1alpha1.Cluster, error) { vnetID, _, err := apisubnet.Split(o.oc.Properties.MasterProfile.SubnetID) if err != nil { diff --git a/pkg/operator/deploy/deploy_test.go b/pkg/operator/deploy/deploy_test.go index 44f5e257a63..5960c1292a0 100644 --- a/pkg/operator/deploy/deploy_test.go +++ b/pkg/operator/deploy/deploy_test.go @@ -19,6 +19,7 @@ import ( ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/Azure/ARO-RP/pkg/api" + pkgoperator "github.com/Azure/ARO-RP/pkg/operator" "github.com/Azure/ARO-RP/pkg/util/cmp" mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env" utilerror "github.com/Azure/ARO-RP/test/util/error" @@ -582,3 +583,66 @@ func TestTestEnsureUpgradeAnnotation(t *testing.T) { }) } } + +func TestGenerateOperatorIdentitySecret(t *testing.T) { + tests := []struct { + Name string + Operator *operator + ExpectedSecret *corev1.Secret + ExpectError bool + }{ + { + Name: "valid cluster operator", + Operator: &operator{ + oc: &api.OpenShiftCluster{ + Location: "eastus1", + Properties: api.OpenShiftClusterProperties{ + PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{ + PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{ + { + OperatorName: pkgoperator.OperatorIdentityName, + ClientID: "11111111-1111-1111-1111-111111111111", + }, + }, + }, + }, + }, + subscriptiondoc: &api.SubscriptionDocument{ + ID: "00000000-0000-0000-0000-000000000000", + Subscription: &api.Subscription{ + Properties: &api.SubscriptionProperties{ + TenantID: "22222222-2222-2222-2222-222222222222", + }, + }, + }, + }, + ExpectedSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkgoperator.OperatorIdentitySecretName, + Namespace: pkgoperator.Namespace, + }, + // StringData is only converted to Data in live kubernetes + StringData: map[string]string{ + "azure_client_id": "11111111-1111-1111-1111-111111111111", + "azure_tenant_id": "22222222-2222-2222-2222-222222222222", + "azure_region": "eastus1", + "azure_subscription_id": "00000000-0000-0000-0000-000000000000", + "azure_federated_token_file": pkgoperator.OperatorTokenFile, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + actualSecret, err := test.Operator.generateOperatorIdentitySecret() + if test.ExpectError == (err == nil) { + t.Errorf("generateOperatorIdentitySecret() %s: ExpectError: %t, actual error: %s\n", test.Name, test.ExpectError, err) + } + + if !reflect.DeepEqual(actualSecret, test.ExpectedSecret) { + t.Errorf("generateOperatorIdentitySecret() %s:\nexpected:\n%+v\n\ngot:\n%+v\n", test.Name, test.ExpectedSecret, actualSecret) + } + }) + } +}