OCM-10017 | fix: re-enable adding EC2 policy to worker role

openshift · Oct 21, 2024 · 9aa356e · 9aa356e
1 parent 7955300
commit 9aa356e
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 4 deletions.
diff --git a/cmd/create/accountroles/creators.go b/cmd/create/accountroles/creators.go
@@ -326,6 +326,12 @@ func (hcp *hcpManagedPoliciesCreator) createRoles(r *rosa.Runtime, input *accoun
 		for _, policyKey := range policyKeys {
 			policyARN, err := aws.GetManagedPolicyARN(input.policies, policyKey)
 			if err != nil {
+				// EC2 policy is only available to orgs for zero-egress feature toggle enabled
+				if policyKey == aws.WorkerEC2RegistryKey {
+					r.Reporter.Infof("Skip attaching policy '%s' to role '%s' (zero egress feature toggle is not enabled)",
+						policyKey, accRoleName)
+					continue
+				}
 				return err
 			}
 
@@ -352,6 +358,12 @@ func (hcp *hcpManagedPoliciesCreator) printCommands(r *rosa.Runtime, input *acco
 		for _, policyKey := range policyKeys {
 			policyARN, err := aws.GetManagedPolicyARN(input.policies, policyKey)
 			if err != nil {
+				// EC2 policy is only available to orgs for zero-egress feature toggle enabled
+				if policyKey == aws.WorkerEC2RegistryKey {
+					r.Reporter.Infof("Skip attaching policy '%s' to role '%s' (zero egress feature toggle is not enabled)",
+						policyKey, accRoleName)
+					continue
+				}
 				return err
 			}
 

diff --git a/cmd/create/accountroles/creators_test.go b/cmd/create/accountroles/creators_test.go
@@ -18,7 +18,7 @@ var _ = Describe("Accountroles", Ordered, func() {
 			mockCtrl := gomock.NewController(GinkgoT())
 			mockClient := mock.NewMockClient(mockCtrl)
 			mockClient.EXPECT().EnsureRole(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
-				gomock.Any(), gomock.Any(), gomock.Any()).Return("role-123", nil)
+				gomock.Any(), gomock.Any(), gomock.Any()).Return("role-123", nil).AnyTimes()
 
 			r := rosa.NewRuntime()
 			r.AWSClient = mockClient
@@ -29,10 +29,34 @@ var _ = Describe("Accountroles", Ordered, func() {
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("failed to find policy ARN for"))
 		})
-		It("createRole succeeds", func() {
+		It("createRole succeeds without ec2 policy", func() {
+			mockCtrl := gomock.NewController(GinkgoT())
+			mockClient := mock.NewMockClient(mockCtrl)
+			mockClient.EXPECT().AttachRolePolicy(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(3)
+			mockClient.EXPECT().EnsureRole(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
+				gomock.Any(), gomock.Any(), gomock.Any()).Return("role-123", nil).AnyTimes()
+
+			r := rosa.NewRuntime()
+			r.AWSClient = mockClient
+			r.Creator = &mock.Creator{ARN: "arn-123"}
+			installerPolicy, _ := (&cmv1.AWSSTSPolicyBuilder{}).ARN("arn::installer").Build()
+			workerPolicy, _ := (&cmv1.AWSSTSPolicyBuilder{}).ARN("arn::worker").Build()
+			supportPolicy, _ := (&cmv1.AWSSTSPolicyBuilder{}).ARN("arn::support").Build()
+
+			policies := map[string]*cmv1.AWSSTSPolicy{
+				"sts_hcp_installer_permission_policy":       installerPolicy,
+				"sts_hcp_instance_worker_permission_policy": workerPolicy,
+				"sts_hcp_support_permission_policy":         supportPolicy,
+			}
+
+			accountRolesCreationInput := buildRolesCreationInput("test", "", "account-123", "stage", policies, "", "")
+			err := (&hcpManagedPoliciesCreator{}).createRoles(r, accountRolesCreationInput)
+			Expect(err).ToNot(HaveOccurred())
+		})
+		It("createRole succeeds with ec2 policy", func() {
 			mockCtrl := gomock.NewController(GinkgoT())
 			mockClient := mock.NewMockClient(mockCtrl)
-			mockClient.EXPECT().AttachRolePolicy(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
+			mockClient.EXPECT().AttachRolePolicy(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(4)
 			mockClient.EXPECT().EnsureRole(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
 				gomock.Any(), gomock.Any(), gomock.Any()).Return("role-123", nil).AnyTimes()
 

diff --git a/pkg/aws/helpers.go b/pkg/aws/helpers.go
@@ -961,6 +961,9 @@ func GetAccountRolePolicyKeys(roleType string) []string {
 // GetAccountRolePolicyKeys returns the policy key for fetching the managed policy ARN
 func GetHcpAccountRolePolicyKeys(roleType string) []string {
 	policyKeys := []string{fmt.Sprintf("sts_hcp_%s_permission_policy", roleType)}
+	if roleType == HCPWorkerRole {
+		policyKeys = append(policyKeys, WorkerEC2RegistryKey)
+	}
 
 	return policyKeys
 }

diff --git a/pkg/aws/helpers_test.go b/pkg/aws/helpers_test.go
@@ -251,8 +251,9 @@ var _ = Describe("GetHcpAccountRolePolicyKeys", func() {
 	When("role_type contains instance_worker", func() {
 		It("should return correct policy keys", func() {
 			policyKeys := GetHcpAccountRolePolicyKeys("instance_worker")
-			Expect(len(policyKeys)).To(Equal(1))
+			Expect(len(policyKeys)).To(Equal(2))
 			Expect(policyKeys[0]).To(Equal("sts_hcp_instance_worker_permission_policy"))
+			Expect(policyKeys[1]).To(Equal("sts_hcp_ec2_registry_permission_policy"))
 		})
 	})
 	When("role_type contains installer", func() {