Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add permissions for acm #188

Merged
merged 4 commits into from
Jan 27, 2022
Merged

Add permissions for acm #188

merged 4 commits into from
Jan 27, 2022

Conversation

sebastien-rosset
Copy link
Contributor

These two PRs in community.aws add support for adding/removing tags to certificates, and requesting/renewing certificates.

  1. Add support for requesting public and private ACM certificate ansible-collections/community.aws#869
  2. Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests ansible-collections/community.aws#870

I know acm permissions need to be added, but I'm unclear where. I see the same acm permissions are repeated multiple times. For example the acm:ImportCertificate permission is added under

  1. AllowResourceRestrictedActionsWhichIncurNoFees
  2. AllowRegionalUnrestrictedResourceActionsWhichIncurNoFees

@sebastien-rosset
Copy link
Contributor Author

In the aws-terminator, how do I determine whether to add acm permissions to AllowResourceRestrictedActionsWhichIncurNoFees or AllowRegionalUnrestrictedResourceActionsWhichIncurNoFees? I see both Sids currently have acm permissions.

I have determined the following permissions are required for the aws_acm module:
AWS ACTIONS: ['acm:AddTagsToCertificate', 'acm:DeleteCertificate', 'acm:DescribeCertificate', 'acm:GetCertificate', 'acm:ImportCertificate', 'acm:ListCertificates', 'acm:ListTagsForCertificate', 'acm:RemoveTagsFromCertificate']

@jillr
Copy link
Collaborator

jillr commented Jan 20, 2022

UnrestrictedResource actions are the ones where no resource restriction is applied/needed. Those Sids have a key like:

    Resource: "*"

ResourceRestricted actions will have a specific resource they can be applied to like this:

    Resource:
      - 'arn:aws:acm:{{ aws_region }}:{{ aws_account_id }}:certificate/*'

Policy actions shouldn't be duplicated in multiple sections, that sounds like an oversight for that action. Looks like ImportCertificate is supposed to be resource restricted https://docs.aws.amazon.com/service-authorization/latest/reference/list_awscertificatemanager.html, thanks for catching it! Resource restrictions should be used if the action supports it.

@sebastien-rosset
Copy link
Contributor Author

UnrestrictedResource actions are the ones where no resource restriction is applied/needed. Those Sids have a key like:

    Resource: "*"

ResourceRestricted actions will have a specific resource they can be applied to like this:

    Resource:
      - 'arn:aws:acm:{{ aws_region }}:{{ aws_account_id }}:certificate/*'

Policy actions shouldn't be duplicated in multiple sections, that sounds like an oversight for that action. Looks like ImportCertificate is supposed to be resource restricted https://docs.aws.amazon.com/service-authorization/latest/reference/list_awscertificatemanager.html, thanks for catching it! Resource restrictions should be used if the action supports it.

ok. I have removed the duplicate entry for acm:ImportCertificate. Is this what you had in mind?

@sebastien-rosset
Copy link
Contributor Author

@jillr , is there anything else I need to do?

Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This policy looks good, thanks @sebastien-rosset (apologies for the delay, I typically only work on this repo about once a week).

These IAM changes will be deployed to our CI account shortly.

@jillr jillr merged commit 5bbf63a into mattclay:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants