From 2c87af89d1466238bcd27eaa3494981615672faf Mon Sep 17 00:00:00 2001
From: Tanmay Satam <tsatam@redhat.com>
Date: Wed, 25 Sep 2024 16:54:18 -0400
Subject: [PATCH] Extract ExplicitIdentity access/handling in clustermsi to
 common function

---
 pkg/cluster/clustermsi.go      | 33 ++++++++++++++++++++++++---------
 pkg/cluster/clustermsi_test.go |  2 +-
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/pkg/cluster/clustermsi.go b/pkg/cluster/clustermsi.go
index 099fdeca0da..882e10f0dd8 100644
--- a/pkg/cluster/clustermsi.go
+++ b/pkg/cluster/clustermsi.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/msi-dataplane/pkg/dataplane"
+	"github.com/Azure/msi-dataplane/pkg/dataplane/swagger"
 	"github.com/Azure/msi-dataplane/pkg/store"
 
 	"github.com/Azure/ARO-RP/pkg/api"
@@ -66,12 +67,16 @@ func (m *manager) ensureClusterMsiCertificate(ctx context.Context) error {
 	if m.env.FeatureIsSet(env.FeatureUseMockMsiRp) {
 		expirationDate = now.AddDate(0, 0, mockMsiCertValidityDays)
 	} else {
-		if msiCredObj.CredentialsObject.ExplicitIdentities == nil || len(msiCredObj.CredentialsObject.ExplicitIdentities) == 0 || msiCredObj.CredentialsObject.ExplicitIdentities[0] == nil || msiCredObj.CredentialsObject.ExplicitIdentities[0].NotAfter == nil {
+		identity, err := getSingleExplicitIdentity(msiCredObj)
+		if err != nil {
+			return err
+		}
+		if identity.NotAfter == nil {
 			return errors.New("unable to pull NotAfter from the MSI CredentialsObject")
 		}
 
 		// The swagger API spec for the MI RP specifies that NotAfter will be "in the format 2017-03-01T14:11:00Z".
-		expirationDate, err = time.Parse(time.RFC3339, *msiCredObj.CredentialsObject.ExplicitIdentities[0].NotAfter)
+		expirationDate, err = time.Parse(time.RFC3339, *identity.NotAfter)
 		if err != nil {
 			return err
 		}
@@ -151,7 +156,7 @@ func (m *manager) clusterMsiSecretName() (string, error) {
 
 func (m *manager) clusterIdentityIDs(ctx context.Context) error {
 	if !m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
-		return fmt.Errorf("platformWorkloadIdentityIDs called for CSP cluster")
+		return fmt.Errorf("clusterIdentityIDs called for CSP cluster")
 	}
 
 	clusterMsiResourceId, err := m.doc.OpenShiftCluster.ClusterMsiResourceId()
@@ -170,12 +175,12 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error {
 		return err
 	}
 
-	if msiCredObj.CredentialsObject.ExplicitIdentities == nil ||
-		len(msiCredObj.CredentialsObject.ExplicitIdentities) == 0 ||
-		msiCredObj.CredentialsObject.ExplicitIdentities[0] == nil ||
-		msiCredObj.CredentialsObject.ExplicitIdentities[0].ClientID == nil ||
-		msiCredObj.CredentialsObject.ExplicitIdentities[0].ObjectID == nil {
-		return errClusterMsiNotPresentInResponse
+	identity, err := getSingleExplicitIdentity(msiCredObj)
+	if err != nil {
+		return err
+	}
+	if identity.ClientID == nil || identity.ObjectID == nil {
+		return fmt.Errorf("unable to pull clientID and objectID from the MSI CredentialsObject")
 	}
 
 	clientId := *msiCredObj.CredentialsObject.ExplicitIdentities[0].ClientID
@@ -193,3 +198,13 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error {
 
 	return err
 }
+
+func getSingleExplicitIdentity(msiCredObj *dataplane.UserAssignedIdentities) (*swagger.NestedCredentialsObject, error) {
+	if msiCredObj.CredentialsObject.ExplicitIdentities == nil ||
+		len(msiCredObj.CredentialsObject.ExplicitIdentities) == 0 ||
+		msiCredObj.CredentialsObject.ExplicitIdentities[0] == nil {
+		return nil, errClusterMsiNotPresentInResponse
+	}
+
+	return msiCredObj.CredentialsObject.ExplicitIdentities[0], nil
+}
diff --git a/pkg/cluster/clustermsi_test.go b/pkg/cluster/clustermsi_test.go
index 397bc40dc05..0e99d2b5f81 100644
--- a/pkg/cluster/clustermsi_test.go
+++ b/pkg/cluster/clustermsi_test.go
@@ -350,7 +350,7 @@ Response contained no body
 					},
 				},
 			},
-			wantErr: "platformWorkloadIdentityIDs called for CSP cluster",
+			wantErr: "clusterIdentityIDs called for CSP cluster",
 		},
 		{
 			name: "error - invalid resource ID (theoretically not possible, but still)",