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

feat: allow usage of other sources than keybase for gpg keys #23227

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hbollon
Copy link

@hbollon hbollon commented Feb 16, 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 #21249

Output from acceptance testing:

$ make testacc TESTS=TestAccIAMUserLoginProfile_keybase PKG=iam

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMUserLoginProfile_keybase'  -timeout 180m
=== RUN   TestAccIAMUserLoginProfile_keybase
=== PAUSE TestAccIAMUserLoginProfile_keybase
=== RUN   TestAccIAMUserLoginProfile_keybaseDoesntExist
=== PAUSE TestAccIAMUserLoginProfile_keybaseDoesntExist
=== CONT  TestAccIAMUserLoginProfile_keybase
=== CONT  TestAccIAMUserLoginProfile_keybaseDoesntExist
=== CONT  TestAccIAMUserLoginProfile_keybase
    acctest.go:195: at least one environment variable of [AWS_PROFILE AWS_ACCESS_KEY_ID AWS_CONTAINER_CREDENTIALS_FULL_URI] must be set. Usage: credentials for running acceptance testing
--- FAIL: TestAccIAMUserLoginProfile_keybase (0.00s)
=== CONT  TestAccIAMUserLoginProfile_keybaseDoesntExist
    user_login_profile_test.go:144: Step 1/1, expected an error with pattern, no match on: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
        The argument "region" is required, but was not set.
--- FAIL: TestAccIAMUserLoginProfile_keybaseDoesntExist (0.89s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/iam	0.937s
FAIL
make: *** [GNUmakefile:36: testacc] Error 1

Acceptance test (TestAccIAMUserLoginProfile_keybase) also failing on main branch.

Note: I created unit tests for the package pgpkeys but I used my Github/Gitlab profile which have linked GPG keys, maybe we should use others hashicorp's accounts.
Furthermore, I think we should create (or update existing) new acceptance tests like TestAccIAMUserLoginProfile_keybase but with external GPG key on Github or Gitlab for exemple.

@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. service/iam Issues and PRs that pertain to the iam service. service/lightsail Issues and PRs that pertain to the lightsail service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. needs-triage Waiting for first response or review from a maintainer. size/L Managed by automation to categorize the size of a PR. labels Feb 16, 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 @hbollon 👋

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! 😃

@justinretzolk justinretzolk added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 16, 2022
@yann-soubeyrand
Copy link

Hello, @justinretzolk is there something we can do to help moving forward with this PR?

@justinretzolk
Copy link
Member

Hey y'all 👋 Thank you for the work here, and for checking in on this! On a quick look, the only thing that appears to be missing is a changelog entry. Please note that I'm the community manager for the provider, so once one of the engineers on the team has a chance to look at it, there may be additional changes that they catch.

Unfortunately, I'm not able to provide an estimate on when this will be merged due to the potential of shifting priorities. We prioritize work by count of ":+1:" reactions, as well as a few other things. A larger prioritization document is in the works, but in the meantime additional information may be found in the FAQ.

@ohookins
Copy link

@hbollon could you add the required changelog entry? This would be a super useful feature to have.

@ohookins
Copy link

@justinretzolk anything else needed?

@github-actions github-actions bot added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 19, 2022
@justinretzolk
Copy link
Member

Hey @ohookins 👋 Thank you for checking in on this. As far as I can tell, things appear to be in good shape now. We'll just need to wait for this to be prioritized for review/merge (see my April 6th comment around that).

@ohookins
Copy link

@justinretzolk any likelihood of it being merged? Admittedly now there are conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service. service/lightsail Issues and PRs that pertain to the lightsail service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_iam_user_login_profile with PGP using other source than just keybase
4 participants