Merge pull request openshift#2052 from marcolan018/ocm-7987

OCM-7985 | fix: add attach policy command for rosa upgrade roles
itsmitul9 · May 20, 2024 · 482fb7c · 482fb7c
2 parents 7e94db3 + 3dfa121
commit 482fb7c
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 22 deletions.
diff --git a/cmd/upgrade/accountroles/cmd.go b/cmd/upgrade/accountroles/cmd.go
@@ -335,13 +335,13 @@ func buildCommands(prefix string, partition string, accountID string, isUpgradeN
 			accRoleName := common.GetRoleName(prefix, role.Name)
 			policyARN := aws.GetPolicyARN(partition, accountID, accRoleName, policyPath)
 			_, err := awsClient.IsPolicyExists(policyARN)
-			hasPolicy := err == nil
+			policyExists := err == nil
 			policyName := aws.GetPolicyName(accRoleName)
 			upgradeAccountPolicyCommands := awscbRoles.ManualCommandsForUpgradeAccountRolePolicy(
 				awscbRoles.ManualCommandsForUpgradeAccountRolePolicyInput{
 					DefaultPolicyVersion: defaultPolicyVersion,
 					RoleName:             accRoleName,
-					HasPolicy:            hasPolicy,
+					PolicyExists:         policyExists,
 					Prefix:               prefix,
 					File:                 file,
 					PolicyName:           policyName,

diff --git a/cmd/upgrade/roles/cmd.go b/cmd/upgrade/roles/cmd.go
@@ -529,11 +529,6 @@ func handleAccountRolePolicyARN(
 		return generatedPolicyARN, nil
 	}
 
-	if len(attachedPoliciesDetail) == 1 {
-		policyDetail := attachedPoliciesDetail[0]
-		return policyDetail.PolicyArn, nil
-	}
-
 	policyArn, err := awsClient.GetAccountRoleDefaultPolicy(roleName, prefix)
 	if err != nil {
 		return "", err
@@ -667,13 +662,22 @@ func buildAccountRoleCommandsFromCluster(
 				return "", err
 			}
 			_, err = awsClient.IsPolicyExists(policyARN)
-			hasPolicy := err == nil
+			policyExists := err == nil
+			policyAttached := false
+			if policyExists {
+				for _, policy := range rolePolicyDetails[accRoleName] {
+					if policy.PolicyArn == policyARN {
+						policyAttached = true
+					}
+				}
+			}
 			policyName := aws.GetPolicyName(accRoleName)
 			upgradeAccountPolicyCommands := awscbRoles.ManualCommandsForUpgradeAccountRolePolicy(
 				awscbRoles.ManualCommandsForUpgradeAccountRolePolicyInput{
 					DefaultPolicyVersion: defaultPolicyVersion,
 					RoleName:             accRoleName,
-					HasPolicy:            hasPolicy,
+					PolicyExists:         policyExists,
+					PolicyAttached:       policyAttached,
 					Prefix:               prefix,
 					File:                 file,
 					PolicyName:           policyName,
@@ -918,15 +922,24 @@ func buildOperatorRoleCommandsFromCluster(
 			operator.Name(),
 		)
 		_, err = awsClient.IsPolicyExists(policyARN)
-		hasPolicy := err == nil
+		policyExists := err == nil
+		policyAttached := false
+		if policyExists && operatorRoleName != "" {
+			for _, policy := range rolePolicyDetails[operatorRoleName] {
+				if policy.PolicyArn == policyARN {
+					policyAttached = true
+				}
+			}
+		}
 
 		isSharedVpc := cluster.AWS().PrivateHostedZoneRoleARN() != ""
 		fileName := aws.GetOperatorPolicyKey(credrequest, cluster.Hypershift().Enabled(), isSharedVpc)
 		fileName = aws.GetFormattedFileName(fileName)
 
 		upgradePoliciesCommands := awscbRoles.ManualCommandsForUpgradeOperatorRolePolicy(
 			awscbRoles.ManualCommandsForUpgradeOperatorRolePolicyInput{
-				HasPolicy:                hasPolicy,
+				PolicyExists:             policyExists,
+				PolicyAttached:           policyAttached,
 				OperatorRolePolicyPrefix: operatorRolePolicyPrefix,
 				Operator:                 operator,
 				CredRequest:              credrequest,
@@ -968,11 +981,6 @@ func handleOperatorRolePolicyARN(
 		return generatedPolicyARN, nil
 	}
 
-	if len(attachedPoliciesDetails) == 1 {
-		policyDetail := attachedPoliciesDetails[0]
-		return policyDetail.PolicyArn, nil
-	}
-
 	policyArn, err := awsClient.GetOperatorRoleDefaultPolicy(operatorRoleName)
 	if err != nil {
 		return "", err

diff --git a/pkg/aws/commandbuilder/helper/roles/roles.go b/pkg/aws/commandbuilder/helper/roles/roles.go
@@ -51,7 +51,8 @@ func ManualCommandsForMissingOperatorRole(input ManualCommandsForMissingOperator
 }
 
 type ManualCommandsForUpgradeOperatorRolePolicyInput struct {
-	HasPolicy                bool
+	PolicyExists             bool
+	PolicyAttached           bool
 	OperatorRolePolicyPrefix string
 	Operator                 *cmv1.STSOperator
 	CredRequest              string
@@ -65,7 +66,12 @@ type ManualCommandsForUpgradeOperatorRolePolicyInput struct {
 
 func ManualCommandsForUpgradeOperatorRolePolicy(input ManualCommandsForUpgradeOperatorRolePolicyInput) []string {
 	commands := make([]string, 0)
-	if !input.HasPolicy {
+	attachRolePolicy := awscb.NewIAMCommandBuilder().
+		SetCommand(awscb.AttachRolePolicy).
+		AddParam(awscb.RoleName, input.OperatorRoleName).
+		AddParam(awscb.PolicyArn, input.PolicyARN).
+		Build()
+	if !input.PolicyExists {
 		iamTags := map[string]string{
 			common.OpenShiftVersion: input.DefaultPolicyVersion,
 			tags.RolePrefix:         input.OperatorRolePolicyPrefix,
@@ -81,6 +87,9 @@ func ManualCommandsForUpgradeOperatorRolePolicy(input ManualCommandsForUpgradeOp
 			AddParam(awscb.Path, input.OperatorPolicyPath).
 			Build()
 		commands = append(commands, createPolicy)
+		if input.OperatorRoleName != "" {
+			commands = append(commands, attachRolePolicy)
+		}
 	} else {
 		policyTags := map[string]string{
 			common.OpenShiftVersion: input.DefaultPolicyVersion,
@@ -98,6 +107,9 @@ func ManualCommandsForUpgradeOperatorRolePolicy(input ManualCommandsForUpgradeOp
 			AddTags(policyTags).
 			AddParam(awscb.PolicyArn, input.PolicyARN).
 			Build()
+		if !input.PolicyAttached && input.OperatorRoleName != "" {
+			commands = append(commands, attachRolePolicy)
+		}
 		commands = append(commands, createPolicyVersion, tagPolicy)
 	}
 	return commands
@@ -106,7 +118,8 @@ func ManualCommandsForUpgradeOperatorRolePolicy(input ManualCommandsForUpgradeOp
 type ManualCommandsForUpgradeAccountRolePolicyInput struct {
 	DefaultPolicyVersion string
 	RoleName             string
-	HasPolicy            bool
+	PolicyExists         bool
+	PolicyAttached       bool
 	Prefix               string
 	File                 string
 	PolicyName           string
@@ -131,7 +144,7 @@ func ManualCommandsForUpgradeAccountRolePolicy(input ManualCommandsForUpgradeAcc
 		AddParam(awscb.RoleName, input.RoleName).
 		AddParam(awscb.PolicyArn, input.PolicyARN).
 		Build()
-	if !input.HasPolicy {
+	if !input.PolicyExists {
 		iamTags := map[string]string{
 			common.OpenShiftVersion: input.DefaultPolicyVersion,
 			tags.RolePrefix:         input.Prefix,
@@ -159,6 +172,9 @@ func ManualCommandsForUpgradeAccountRolePolicy(input ManualCommandsForUpgradeAcc
 			AddTags(iamRoleTags).
 			AddParam(awscb.PolicyArn, input.PolicyARN).
 			Build()
+		if !input.PolicyAttached {
+			commands = append(commands, attachRolePolicy)
+		}
 		commands = append(commands, createPolicyVersion, tagPolicies, tagRole)
 	}
 	return commands

diff --git a/pkg/aws/sts.go b/pkg/aws/sts.go
@@ -190,13 +190,13 @@ func BuildOperatorRoleCommands(prefix string, partition string, accountID string
 			operator.Name(),
 		)
 		_, err := awsClient.IsPolicyExists(policyARN)
-		hasPolicy := err == nil
+		policyExists := err == nil
 		isSharedVpc := cluster.AWS().PrivateHostedZoneRoleARN() != ""
 		fileName := GetOperatorPolicyKey(credrequest, cluster.Hypershift().Enabled(), isSharedVpc)
 		fileName = GetFormattedFileName(fileName)
 		upgradePoliciesCommands := awscbRoles.ManualCommandsForUpgradeOperatorRolePolicy(
 			awscbRoles.ManualCommandsForUpgradeOperatorRolePolicyInput{
-				HasPolicy:                hasPolicy,
+				PolicyExists:             policyExists,
 				OperatorRolePolicyPrefix: prefix,
 				Operator:                 operator,
 				CredRequest:              credrequest,