From 53691de809b5adff78cc6588d9c819a8922c1319 Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Tue, 23 Jul 2024 12:53:33 -0700 Subject: [PATCH] shared utility function to check if a cluster is workload identity (#3683) * shared utility function to check if a cluster is workload identity * adding license statement * apply suggestions from code review * apply suggestions from code review * remove TODO comment * apply code review suggestions --------- Co-authored-by: Sanjana Lawande --- pkg/cluster/delete.go | 3 +- pkg/cluster/deploybaseresources.go | 3 +- pkg/frontend/openshiftcluster_putorpatch.go | 4 +- pkg/operator/deploy/deploy.go | 4 +- pkg/util/wimi/iswimi.go | 14 ++++++ pkg/util/wimi/iswimi_test.go | 49 +++++++++++++++++++++ 6 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 pkg/util/wimi/iswimi.go create mode 100644 pkg/util/wimi/iswimi_test.go diff --git a/pkg/cluster/delete.go b/pkg/cluster/delete.go index 16a37073a59..2492087b855 100644 --- a/pkg/cluster/delete.go +++ b/pkg/cluster/delete.go @@ -30,6 +30,7 @@ import ( "github.com/Azure/ARO-RP/pkg/util/oidcbuilder" "github.com/Azure/ARO-RP/pkg/util/rbac" "github.com/Azure/ARO-RP/pkg/util/stringutils" + utilwimi "github.com/Azure/ARO-RP/pkg/util/wimi" ) // deleteNic deletes the network interface resource by first fetching the resource using the interface @@ -423,7 +424,7 @@ func (m *manager) Delete(ctx context.Context) error { return err } - if m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile == nil && m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile != nil { + if utilwimi.IsWimi(m.doc.OpenShiftCluster) { m.log.Printf("deleting OIDC configuration") blobContainerURL := oidcbuilder.GenerateBlobContainerURL(m.env) azBlobClient, err := m.rpBlob.GetAZBlobClient(blobContainerURL, &azblob.ClientOptions{}) diff --git a/pkg/cluster/deploybaseresources.go b/pkg/cluster/deploybaseresources.go index 5e89e68e79f..af862e86e16 100644 --- a/pkg/cluster/deploybaseresources.go +++ b/pkg/cluster/deploybaseresources.go @@ -28,6 +28,7 @@ import ( "github.com/Azure/ARO-RP/pkg/util/oidcbuilder" "github.com/Azure/ARO-RP/pkg/util/pointerutils" "github.com/Azure/ARO-RP/pkg/util/stringutils" + utilwimi "github.com/Azure/ARO-RP/pkg/util/wimi" ) var nsgNotReadyErrorRegex = regexp.MustCompile("Resource.*networkSecurityGroups.*referenced by resource.*not found") @@ -39,7 +40,7 @@ func (m *manager) createDNS(ctx context.Context) error { } func (m *manager) createOIDC(ctx context.Context) error { - if m.doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil || m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile == nil { + if !utilwimi.IsWimi(m.doc.OpenShiftCluster) { return nil } diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index a507f641984..34d4bc44593 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -23,6 +23,7 @@ import ( "github.com/Azure/ARO-RP/pkg/frontend/middleware" "github.com/Azure/ARO-RP/pkg/operator" "github.com/Azure/ARO-RP/pkg/util/version" + utilwimi "github.com/Azure/ARO-RP/pkg/util/wimi" ) var errMissingIdentityParameter error = fmt.Errorf("identity parameter not provided but required for workload identity cluster") @@ -140,8 +141,7 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus. if isCreate { // Persist identity URL and tenant ID only for managed/workload identity cluster create // We don't support updating cluster managed identity after cluster creation - // TODO - use a common function to check if the cluster is a managed/workload identity cluster - if !(doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil || doc.OpenShiftCluster.Identity == nil) { + if utilwimi.IsWimi(doc.OpenShiftCluster) { if err := validateIdentityUrl(doc.OpenShiftCluster, putOrPatchClusterParameters.identityURL); err != nil { return nil, err } diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index 99f6898edc1..d2ebba558f3 100644 --- a/pkg/operator/deploy/deploy.go +++ b/pkg/operator/deploy/deploy.go @@ -45,6 +45,7 @@ import ( "github.com/Azure/ARO-RP/pkg/util/pullsecret" "github.com/Azure/ARO-RP/pkg/util/ready" "github.com/Azure/ARO-RP/pkg/util/restconfig" + utilwimi "github.com/Azure/ARO-RP/pkg/util/wimi" ) //go:embed staticresources @@ -468,8 +469,7 @@ func (o *operator) RenewMDSDCertificate(ctx context.Context) error { } func (o *operator) EnsureUpgradeAnnotation(ctx context.Context) error { - if o.oc.Properties.PlatformWorkloadIdentityProfile == nil || - o.oc.Properties.ServicePrincipalProfile != nil { + if !utilwimi.IsWimi(o.oc) { return nil } diff --git a/pkg/util/wimi/iswimi.go b/pkg/util/wimi/iswimi.go new file mode 100644 index 00000000000..ab0adfa1ff5 --- /dev/null +++ b/pkg/util/wimi/iswimi.go @@ -0,0 +1,14 @@ +package wimi + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import "github.com/Azure/ARO-RP/pkg/api" + +// IsWimi checks whether a cluster is a Workload Identity cluster or Service Principal cluster +func IsWimi(oc *api.OpenShiftCluster) bool { + if oc.Properties.PlatformWorkloadIdentityProfile != nil && oc.Properties.ServicePrincipalProfile == nil { + return true + } + return false +} diff --git a/pkg/util/wimi/iswimi_test.go b/pkg/util/wimi/iswimi_test.go new file mode 100644 index 00000000000..8b416e1d311 --- /dev/null +++ b/pkg/util/wimi/iswimi_test.go @@ -0,0 +1,49 @@ +package wimi + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "fmt" + "testing" + + "github.com/Azure/ARO-RP/pkg/api" +) + +func TestIswimi(t *testing.T) { + tests := []*struct { + name string + oc api.OpenShiftCluster + want bool + }{ + { + name: "Cluster is Workload Identity", + oc: api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{}, + ServicePrincipalProfile: nil, + }, + }, + want: true, + }, + { + name: "Cluster is Service Principal", + oc: api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + PlatformWorkloadIdentityProfile: nil, + ServicePrincipalProfile: &api.ServicePrincipalProfile{}, + }, + }, + want: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := IsWimi(&test.oc) + if got != test.want { + t.Error(fmt.Errorf("got != want: %v != %v", got, test.want)) + } + }) + } +}