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 <[email protected]>
Azure · Jul 23, 2024 · 53691de · 53691de
1 parent bfb554c
commit 53691de
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))
+			}
+		})
+	}
+}