Skip to content

Commit

Permalink
OCM-13175 | fix: optional work role check for AmazonEC2ContainerRegis…
Browse files Browse the repository at this point in the history
…tryReadOnly optional
  • Loading branch information
philipwu08 committed Dec 20, 2024
1 parent 8396ed1 commit 2133993
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
12 changes: 7 additions & 5 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -2175,13 +2175,15 @@ func (c *awsClient) GetOperatorRoleDefaultPolicy(roleName string) (string, error

func (c *awsClient) validateManagedPolicy(policies map[string]*cmv1.AWSSTSPolicy, policyKey string,
roleName string) error {
// EC2 policy is now returned from CS for all orgs. It is optional since it's only required
// to create zero egress clusters
if policyKey == WorkerEC2RegistryKey {
c.logger.Infof("Ignored check for policy key '%s' (only required for zero egress enabled clusters)", policyKey)
return nil
}

managedPolicyARN, err := GetManagedPolicyARN(policies, policyKey)
if err != nil {
// EC2 policy is only available to orgs for zero-egress feature toggle enabled
if policyKey == WorkerEC2RegistryKey {
c.logger.Infof("Ignored check for policy key '%s' (zero egress feature toggle is not enabled)", policyKey)
return nil
}
return err
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/aws/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ var _ = Describe("validateManagedPolicy", func() {
DescribeTable("validate ECR policy", func(
policies map[string]*cmv1.AWSSTSPolicy, policyKey, roleName, expectedErr string,
) {
mockIamAPI.EXPECT().ListAttachedRolePolicies(gomock.Any(), gomock.Any()).Return(nil, nil).Times(0)
err := client.validateManagedPolicy(policies, policyKey, roleName)
if expectedErr == "" {
Expect(err).To(BeNil())
Expand All @@ -856,6 +857,11 @@ var _ = Describe("validateManagedPolicy", func() {
Entry("succeeds if ECR policy does not exist", map[string]*cmv1.AWSSTSPolicy{
"sts_hcp_instance_worker_permission_policy": workerPolicy},
"sts_hcp_ec2_registry_permission_policy", "worker", ""),
Entry("succeeds if ECR policy exist but skips check if policy is attached",
map[string]*cmv1.AWSSTSPolicy{
"sts_hcp_instance_worker_permission_policy": workerPolicy,
"sts_hcp_ec2_registry_permission_policy": ec2ContainerPolicy},
"sts_hcp_ec2_registry_permission_policy", "worker", ""),
Entry("fails to find worker policy", map[string]*cmv1.AWSSTSPolicy{
"sts_hcp_ec2_registry_permission_policy": ec2ContainerPolicy},
"sts_hcp_instance_worker_permission_policy", "worker",
Expand Down

0 comments on commit 2133993

Please sign in to comment.