OCM-11295 | fix: adjust check for policy tags

openshift · Sep 20, 2024 · 8509eb3 · 8509eb3
1 parent 356a60b
commit 8509eb3
Show file tree

Hide file tree

Showing 2 changed files with 126 additions and 10 deletions.
diff --git a/pkg/aws/policies.go b/pkg/aws/policies.go
@@ -1320,7 +1320,7 @@ func (c *awsClient) GetAttachedPolicyWithTags(role *string,
  }
 
  for _, policy := range attachedPoliciesOutput.AttachedPolicies {
- hasTags, err := isPolicyHasTags(c.iamClient, policy.PolicyArn, tagFilter)
+ hasTags, err := doesPolicyHaveTags(c.iamClient, policy.PolicyArn, tagFilter)
  if err != nil {
  return policies, excludedPolicies, err
  }
@@ -2111,7 +2111,7 @@ func (c *awsClient) listRoleAttachedPolicies(roleName string) ([]iamtypes.Attach
 }
 
 // check whether the policy contains specified tags
-func isPolicyHasTags(c client.IamApiClient, poilcyArn *string, tagFilter map[string]string) (bool, error) {
+func doesPolicyHaveTags(c client.IamApiClient, poilcyArn *string, tagFilter map[string]string) (bool, error) {
  if len(tagFilter) != 0 {
  tags, err := c.ListPolicyTags(context.Background(),
  &iam.ListPolicyTagsInput{
@@ -2121,19 +2121,18 @@ func isPolicyHasTags(c client.IamApiClient, poilcyArn *string, tagFilter map[str
  if err != nil {
  return false, err
  }
- fitTagSize := 0
+ foundTagsCounter := 0
  for _, tag := range tags.Tags {
  value, ok := tagFilter[aws.ToString(tag.Key)]
- if ok && value != aws.ToString(tag.Value) {
- return false, nil
+ if ok && value == aws.ToString(tag.Value) {
+ foundTagsCounter++
  }
- fitTagSize++
  }
- if fitTagSize < len(tagFilter) {
- return false, nil
+ if foundTagsCounter == len(tagFilter) {
+ return true, nil
  }
  }
- return true, nil
+ return false, nil
 }
 
 func getAttachedPolicies(c client.IamApiClient, role string,
@@ -2148,7 +2147,7 @@ func getAttachedPolicies(c client.IamApiClient, role string,
  return policyArr, excludedPolicyArr, err
  }
  for _, policy := range policiesOutput.AttachedPolicies {
- hasTags, err := isPolicyHasTags(c, policy.PolicyArn, tagFilter)
+ hasTags, err := doesPolicyHaveTags(c, policy.PolicyArn, tagFilter)
  if err != nil {
  return policyArr, excludedPolicyArr, err
  }

diff --git a/pkg/aws/policies_test.go b/pkg/aws/policies_test.go
@@ -691,3 +691,120 @@ var _ = Describe("CheckIfROSAOperatorRole", func() {
  })
  })
 })
+
+var _ = Describe("isPolicyHasTags", func() {
+ var (
+ mockIamAPI *mocks.MockIamApiClient
+ mockCtrl *gomock.Controller
+ )
+ testRoleArn := "fake-role-arn"
+ testePolicyArn := "fake-policy-arn"
+ BeforeEach(func() {
+ mockCtrl = gomock.NewController(GinkgoT())
+ mockIamAPI = mocks.NewMockIamApiClient(mockCtrl)
+ })
+ It("Should have a red-hat-managed policy identified", func() {
+ mockIamAPI.EXPECT().ListRoleTags(gomock.Any(), gomock.Any()).Return(&iam.ListRoleTagsOutput{
+ Tags: []iamtypes.Tag{
+ {
+ Key: aws.String(tags.OperatorName),
+ Value: aws.String("ebs-cloud-credentials"),
+ },
+ {
+ Key: aws.String(tags.OperatorNamespace),
+ Value: aws.String("openshift-cluster-csi-drivers"),
+ },
+ {
+ Key: aws.String(tags.RedHatManaged),
+ Value: aws.String("true"),
+ },
+ },
+ IsTruncated: false,
+ }, nil)
+ filter, err := getOperatorRolePolicyTags(mockIamAPI, testRoleArn)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(filter).To(HaveLen(3))
+ mockIamAPI.EXPECT().ListPolicyTags(gomock.Any(), gomock.Any()).Return(&iam.ListPolicyTagsOutput{
+ Tags: []iamtypes.Tag{
+ {
+ Key: aws.String(tags.OperatorName),
+ Value: aws.String("ebs-cloud-credentials"),
+ },
+ {
+ Key: aws.String(tags.OperatorNamespace),
+ Value: aws.String("openshift-cluster-csi-drivers"),
+ },
+ {
+ Key: aws.String(tags.RedHatManaged),
+ Value: aws.String("true"),
+ },
+ {
+ Key: aws.String("t1"),
+ Value: aws.String("v1"),
+ },
+ {
+ Key: aws.String("t2"),
+ Value: aws.String("v2"),
+ },
+ {
+ Key: aws.String("t3"),
+ Value: aws.String("v3"),
+ },
+ {
+ Key: aws.String("t4"),
+ Value: aws.String("v4"),
+ },
+ },
+ IsTruncated: false,
+ }, nil)
+ result, err := doesPolicyHaveTags(mockIamAPI, &testePolicyArn, filter)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(result).To(BeTrue())
+ })
+ It("Should not have a red-hat-managed policy identified", func() {
+ mockIamAPI.EXPECT().ListRoleTags(gomock.Any(), gomock.Any()).Return(&iam.ListRoleTagsOutput{
+ Tags: []iamtypes.Tag{
+ {
+ Key: aws.String(tags.OperatorName),
+ Value: aws.String("ebs-cloud-credentials"),
+ },
+ {
+ Key: aws.String(tags.OperatorNamespace),
+ Value: aws.String("openshift-cluster-csi-drivers"),
+ },
+ {
+ Key: aws.String(tags.RedHatManaged),
+ Value: aws.String("true"),
+ },
+ },
+ IsTruncated: false,
+ }, nil)
+ filter, err := getOperatorRolePolicyTags(mockIamAPI, testRoleArn)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(filter).To(HaveLen(3))
+ mockIamAPI.EXPECT().ListPolicyTags(gomock.Any(), gomock.Any()).Return(&iam.ListPolicyTagsOutput{
+ Tags: []iamtypes.Tag{
+ {
+ Key: aws.String("t1"),
+ Value: aws.String("v1"),
+ },
+ {
+ Key: aws.String("t2"),
+ Value: aws.String("v2"),
+ },
+ {
+ Key: aws.String("t3"),
+ Value: aws.String("v3"),
+ },
+ {
+ Key: aws.String("t4"),
+ Value: aws.String("v4"),
+ },
+ },
+ IsTruncated: false,
+ }, nil)
+ result, err := doesPolicyHaveTags(mockIamAPI, &testePolicyArn, filter)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(result).To(BeFalse())
+ })
+})