Merge pull request #1573 from thomasmckay/4571-cherry-pick-4644

OCM-4644 | fix: Create cluster - filter classic ROSA account roles

cmd/create/cluster/cmd.go

@@ -1214,7 +1214,12 @@ func run(cmd *cobra.Command, _ []string) {
 		role := aws.AccountRoles[aws.InstallerAccountRole]
 
 		// Find all installer roles in the current account using AWS resource tags
-		roleARNs, err := awsClient.FindRoleARNs(aws.InstallerAccountRole, minor)
+		var roleARNs []string
+		if isHostedCP {
+			roleARNs, err = awsClient.FindRoleARNs(aws.InstallerAccountRole, minor)
+		} else {
+			roleARNs, err = awsClient.FindRoleARNsClassic(aws.InstallerAccountRole, minor)
+		}
 		if err != nil {
 			r.Reporter.Errorf("Failed to find %s role: %s", role.Name, err)
 			os.Exit(1)
@@ -1281,7 +1286,11 @@ func run(cmd *cobra.Command, _ []string) {
 					// Not needed for Hypershift clusters
 					continue
 				}
-				roleARNs, err := awsClient.FindRoleARNs(roleType, minor)
+				if isHostedCP {
+					roleARNs, err = awsClient.FindRoleARNs(roleType, minor)
+				} else {
+					roleARNs, err = awsClient.FindRoleARNsClassic(roleType, minor)
+				}
 				if err != nil {
 					r.Reporter.Errorf("Failed to find %s role: %s", role.Name, err)
 					os.Exit(1)

pkg/aws/client.go

@@ -120,6 +120,7 @@ type Client interface {
 	DeleteOpenIDConnectProvider(providerURL string) error
 	HasOpenIDConnectProvider(issuerURL string, accountID string) (bool, error)
 	FindRoleARNs(roleType string, version string) ([]string, error)
+	FindRoleARNsClassic(roleType string, version string) ([]string, error)
 	FindPolicyARN(operator Operator, version string) (string, error)
 	ListUserRoles() ([]Role, error)
 	ListOCMRoles() ([]Role, error)
@@ -185,8 +186,7 @@ type Client interface {
 	PutPublicReadObjectInS3Bucket(bucketName string, body io.ReadSeeker, key string) error
 	CreateSecretInSecretsManager(name string, secret string) (string, error)
 	DeleteSecretInSecretsManager(secretArn string) error
-	ValidateAccountRoleVersionCompatibility(
-		roleName string, roleType string, minVersion string) (bool, error)
+	ValidateAccountRoleVersionCompatibility(roleName string, roleType string, minVersion string) (bool, error)
 	GetDefaultPolicyDocument(policyArn string) (string, error)
 	GetAccountRoleByArn(roleArn string) (*Role, error)
 	GetSecurityGroupIds(vpcId string) ([]*ec2.SecurityGroup, error)

pkg/aws/policies.go

@@ -486,18 +486,71 @@ func (c *awsClient) FindRoleARNs(roleType string, version string) ([]string, err
 	return roleARNs, nil
 }
 
+func (c *awsClient) FindRoleARNsClassic(roleType string, version string) ([]string, error) {
+	roleARNs := []string{}
+	roles, err := c.ListRoles()
+	if err != nil {
+		return roleARNs, err
+	}
+	for _, role := range roles {
+		if !strings.Contains(aws.StringValue(role.RoleName), AccountRoles[roleType].Name) {
+			continue
+		}
+		isValid, err := c.validateAccountRoleVersionCompatibilityClassic(*role.RoleName, roleType, version)
+		if err != nil {
+			return roleARNs, err
+		}
+		if !isValid {
+			continue
+		}
+		roleARNs = append(roleARNs, aws.StringValue(role.Arn))
+	}
+	return roleARNs, nil
+}
+
 // FIXME: refactor similar calls to use this instead
-func (c *awsClient) ValidateAccountRoleVersionCompatibility(
-	roleName string, roleType string, minVersion string) (bool, error) {
+func (c *awsClient) ValidateAccountRoleVersionCompatibility(roleName string, roleType string,
+	minVersion string) (bool, error) {
 	listRoleTagsOutput, err := c.iamClient.ListRoleTags(&iam.ListRoleTagsInput{
 		RoleName: aws.String(roleName),
 	})
 	if err != nil {
 		return false, err
 	}
+
+	return isAccountRoleVersionCompatible(listRoleTagsOutput.Tags, roleType, minVersion)
+}
+
+func (c *awsClient) validateAccountRoleVersionCompatibilityClassic(roleName string, roleType string,
+	minVersion string) (bool, error) {
+	listRoleTagsOutput, err := c.iamClient.ListRoleTags(&iam.ListRoleTagsInput{
+		RoleName: aws.String(roleName),
+	})
+	if err != nil {
+		return false, err
+	}
+
+	isCompatible, err := isAccountRoleVersionCompatible(listRoleTagsOutput.Tags, roleType, minVersion)
+	if err != nil {
+		return false, err
+	}
+	if !isCompatible {
+		return false, nil
+	}
+
+	// Account roles with HCP policies are not compatible with classic clusters
+	if common.IamResourceHasTag(listRoleTagsOutput.Tags, tags.HypershiftPolicies, tags.True) {
+		return false, nil
+	}
+
+	return true, nil
+}
+
+func isAccountRoleVersionCompatible(tagsList []*iam.Tag, roleType string,
+	minVersion string) (bool, error) {
 	skip := false
 	isTagged := false
-	for _, tag := range listRoleTagsOutput.Tags {
+	for _, tag := range tagsList {
 		tagValue := aws.StringValue(tag.Value)
 		switch aws.StringValue(tag.Key) {
 		case tags.RoleType:
@@ -509,7 +562,7 @@ func (c *awsClient) ValidateAccountRoleVersionCompatibility(
 		case common.OpenShiftVersion:
 			isTagged = true
 
-			if common.IamResourceHasTag(listRoleTagsOutput.Tags, common.ManagedPolicies, tags.True) {
+			if common.IamResourceHasTag(tagsList, common.ManagedPolicies, tags.True) {
 				// Managed policies will be up-to-date no need to check version tags
 				break
 			}
@@ -536,6 +589,7 @@ func (c *awsClient) ValidateAccountRoleVersionCompatibility(
 	if !isTagged || skip {
 		return false, nil
 	}
+
 	return true, nil
 }
 

pkg/aws/policies_test.go

@@ -0,0 +1,61 @@
+package aws
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/iam"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Is Account Role Version Compatible", func() {
+	When("Role isn't an account role", func() {
+		It("Should return not compatible", func() {
+			isCompatible, err := isAccountRoleVersionCompatible([]*iam.Tag{}, InstallerAccountRole, "4.14")
+			Expect(err).To(BeNil())
+			Expect(isCompatible).To(Equal(false))
+		})
+	})
+	When("Role OCP version isn't compatible", func() {
+		It("Should return not compatible", func() {
+			tagsList := []*iam.Tag{
+				{
+					Key:   aws.String("rosa_openshift_version"),
+					Value: aws.String("4.13"),
+				},
+			}
+			isCompatible, err := isAccountRoleVersionCompatible(tagsList, InstallerAccountRole, "4.14")
+			Expect(err).To(BeNil())
+			Expect(isCompatible).To(Equal(false))
+		})
+	})
+	When("Role version is compatible", func() {
+		It("Should return compatible", func() {
+			tagsList := []*iam.Tag{
+				{
+					Key:   aws.String("rosa_openshift_version"),
+					Value: aws.String("4.14"),
+				},
+			}
+			isCompatible, err := isAccountRoleVersionCompatible(tagsList, InstallerAccountRole, "4.14")
+			Expect(err).To(BeNil())
+			Expect(isCompatible).To(Equal(true))
+		})
+	})
+	When("Role has managed policies, ignores openshift version", func() {
+		It("Should return compatible", func() {
+			tagsList := []*iam.Tag{
+				{
+					Key:   aws.String("rosa_openshift_version"),
+					Value: aws.String("4.12"),
+				},
+				{
+					Key:   aws.String("rosa_managed_policies"),
+					Value: aws.String("true"),
+				},
+			}
+			isCompatible, err := isAccountRoleVersionCompatible(tagsList, InstallerAccountRole, "4.14")
+			Expect(err).To(BeNil())
+			Expect(isCompatible).To(Equal(true))
+		})
+	})
+})