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

#13994: Attempt to allow usage of service principals for KMS grants. #25360

Conversation

RonaldTechnative
Copy link
Contributor

@RonaldTechnative RonaldTechnative commented Jun 15, 2022

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #13994

Output from acceptance testing:

ronald@Ronalds-MacBook-Pro terraform-provider-aws % make testacc TESTS=TestAccKMSGrant_service PKG=kms
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/kms/... -v -count 1 -parallel 20 -run='TestAccKMSGrant_service'  -timeout 180m
=== RUN   TestAccKMSGrant_service
=== PAUSE TestAccKMSGrant_service
=== CONT  TestAccKMSGrant_service
--- PASS: TestAccKMSGrant_service (27.00s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/kms        29.064s
ronald@Ronalds-MacBook-Pro terraform-provider-aws % make testacc TESTS=TestAccKMSGrant_basic PKG=kms  
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/kms/... -v -count 1 -parallel 20 -run='TestAccKMSGrant_basic'  -timeout 180m
=== RUN   TestAccKMSGrant_basic
=== PAUSE TestAccKMSGrant_basic
=== CONT  TestAccKMSGrant_basic
--- PASS: TestAccKMSGrant_basic (43.55s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/kms        45.616s
ronald@Ronalds-MacBook-Pro terraform-provider-aws % go test github.com/hashicorp/terraform-provider-aws/internal/verify
ok      github.com/hashicorp/terraform-provider-aws/internal/verify     0.360s
ronald@Ronalds-MacBook-Pro terraform-provider-aws % go test github.com/hashicorp/terraform-provider-aws/internal/service/kms

ok      github.com/hashicorp/terraform-provider-aws/internal/service/kms        2.053s
...

@github-actions
Copy link

Hey @RonaldTechnative 👋 Thank you very much for your contribution! At times, our maintainers need to make direct edits to pull requests in order to help get it ready to be merged. Your current settings do not allow maintainers to make such edits. To help facilitate this, update your pull request to allow such edits as described in GitHub's Allowing changes to a pull request branch created from a fork documentation. (If you're using a fork owned by an organization, your organization may not allow you to change this setting. If that is the case, let us know.)

@github-actions github-actions bot added size/M Managed by automation to categorize the size of a PR. service/kms Issues and PRs that pertain to the kms service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. verify Pertains to the verify package (i.e., provider-level validating, diff suppression, etc.) needs-triage Waiting for first response or review from a maintainer. labels Jun 15, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @RonaldTechnative 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@RonaldTechnative
Copy link
Contributor Author

Hey @RonaldTechnative 👋 Thank you very much for your contribution! At times, our maintainers need to make direct edits to pull requests in order to help get it ready to be merged. Your current settings do not allow maintainers to make such edits. To help facilitate this, update your pull request to allow such edits as described in GitHub's Allowing changes to a pull request branch created from a fork documentation. (If you're using a fork owned by an organization, your organization may not allow you to change this setting. If that is the case, let us know.)

It appears that I'm unable to grant access to allow anyone here to push. I think we can work around by giving people write access to the forked repository which I'm ok with.

This is my first contribution so please let me know if I missed anything.

@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jun 16, 2022
@im-dim
Copy link

im-dim commented Sep 30, 2022

Hello all,
When would it be released?
We are on v4.19.0 and can't grant access to encryption keys to services like "backup.us-east-1.amazonaws.com"...

@RonaldTechnative
Copy link
Contributor Author

Hello all, When would it be released? We are on v4.19.0 and can't grant access to encryption keys to services like "backup.us-east-1.amazonaws.com"...

It appears that Hashicorp has a backlog on these issues and is prioritizing based on thumbs op on the main issue. Can you do that? I'm asking our customers to do the same 😄 .

@mad-it
Copy link

mad-it commented Mar 8, 2023

@ewbankkit sorry for pinging you directly, but it seems like you are the latest active contributor.
Is there any way this fix can be reviewed/merged? Its a small validation fix.

@github-actions github-actions bot added the service/organizations Issues and PRs that pertain to the organizations service. label Mar 8, 2023
@github-actions github-actions bot removed the service/organizations Issues and PRs that pertain to the organizations service. label Mar 8, 2023
@mad-it
Copy link

mad-it commented Mar 10, 2023

@RonaldTechnative there is a small fix that needs to be made in reading the resource on this line: https://github.com/hashicorp/terraform-provider-aws/pull/25360/files#diff-d07eb791766a340751ea762e950c2b06b06d77a6dcb7112209a365745c765e7dR330

The read operation also needs to use the new ValidARNOrServicePrincipal.

@github-actions github-actions bot added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Mar 10, 2023
@nam054 nam054 self-assigned this Jul 10, 2023
@breathingdust breathingdust added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Jul 21, 2023
@nam054
Copy link
Contributor

nam054 commented Jul 21, 2023

Hey @RonaldTechnative 👋 Given the permissions difficulties, I've opened a separate pull request that covers this and a couple of other minor changes. With that in mind, we'll close this one out in favor of that one. Thank you for taking the time to contribute; we really appreciate it!

@nam054 nam054 closed this Jul 21, 2023
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. service/kms Issues and PRs that pertain to the kms service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. verify Pertains to the verify package (i.e., provider-level validating, diff suppression, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_kms_grant doesn't accept service principals in grantee_principal fiend
6 participants