r/aws_iam_user_login_profile - Make the PGP key in optional

[DrFaust92]

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 #4564

Release note for CHANGELOG:

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSUserLoginProfile_'
--- PASS: TestAccAWSUserLoginProfile_notAKey (34.68s)
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (36.00s)
--- PASS: TestAccAWSUserLoginProfile_disappears (51.75s)
--- PASS: TestAccAWSUserLoginProfile_no_pgp (56.59s)
--- PASS: TestAccAWSUserLoginProfile_keybase (56.59s)
--- PASS: TestAccAWSUserLoginProfile_PasswordLength (57.24s)
--- PASS: TestAccAWSUserLoginProfile_basic (65.99s)
...

Cant test this unfortunately as i don't have delete user permissions in my account :(

[ewbankkit]

Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSUserLoginProfile_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSUserLoginProfile_ -timeout 120m
=== RUN   TestAccAWSUserLoginProfile_basic
=== PAUSE TestAccAWSUserLoginProfile_basic
=== RUN   TestAccAWSUserLoginProfile_no_pgp
=== PAUSE TestAccAWSUserLoginProfile_no_pgp
=== RUN   TestAccAWSUserLoginProfile_keybase
=== PAUSE TestAccAWSUserLoginProfile_keybase
=== RUN   TestAccAWSUserLoginProfile_keybaseDoesntExist
=== PAUSE TestAccAWSUserLoginProfile_keybaseDoesntExist
=== RUN   TestAccAWSUserLoginProfile_notAKey
=== PAUSE TestAccAWSUserLoginProfile_notAKey
=== RUN   TestAccAWSUserLoginProfile_PasswordLength
=== PAUSE TestAccAWSUserLoginProfile_PasswordLength
=== CONT  TestAccAWSUserLoginProfile_basic
=== CONT  TestAccAWSUserLoginProfile_notAKey
=== CONT  TestAccAWSUserLoginProfile_keybaseDoesntExist
=== CONT  TestAccAWSUserLoginProfile_keybase
=== CONT  TestAccAWSUserLoginProfile_no_pgp
=== CONT  TestAccAWSUserLoginProfile_PasswordLength
--- FAIL: TestAccAWSUserLoginProfile_basic (16.23s)
    testing.go:654: Step 0 error: Check failed: Check 2/7 error: No password in state
--- FAIL: TestAccAWSUserLoginProfile_keybase (16.40s)
    testing.go:654: Step 0 error: errors during apply:
        
        Error: Error retrieving Public Key for keybase:terraformacctest
        : parse https://keybase.io/_/api/1.0/user/lookup.json?usernames=terraformacctest
        &fields=public_keys: net/url: invalid control character in URL
        
          on /tmp/tf-test249940899/main.tf line 36:
          (source code not available)
        
        
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (18.28s)
--- PASS: TestAccAWSUserLoginProfile_notAKey (18.57s)
--- FAIL: TestAccAWSUserLoginProfile_no_pgp (27.20s)
    testing.go:654: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) (len=2) {
         (string) (len=18) "encrypted_password": (string) "",
         (string) (len=15) "key_fingerprint": (string) ""
        }
        
        
        (map[string]string) {
        }
        
--- PASS: TestAccAWSUserLoginProfile_PasswordLength (27.41s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	27.455s
FAIL
GNUmakefile:26: recipe for target 'testacc' failed
make: *** [testacc] Error 1

Not sure why TestAccAWSUserLoginProfile_keybase is failing...

I think it's due to an extra \n character introduced by the <<EOF construct.

[tomelliff]

@DrFaust92 I appreciate this is still at a draft stage but it looks like it needs a bit more work to actually implement it usefully (eg still return an unencrypted password when pgp_key isn't set) and fix tests highlighted by @ewbankkit etc.

Are you still interested in finishing this off? I'd be tempted to do the small amount of work on top of yours to get this over the line if you're unable to commit anything more towards it as it might be useful for us.

[DrFaust92]

@tomelliff thanks for bringing this up, ill give it a go again (at least ill rebase and push). if i don't get this in the next few days feel free to take over

[DrFaust92]

Fixed all test issues:

--- PASS: TestAccAWSUserLoginProfile_notAKey (34.68s)
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (36.00s)
--- PASS: TestAccAWSUserLoginProfile_disappears (51.75s)
--- PASS: TestAccAWSUserLoginProfile_no_pgp (56.59s)
--- PASS: TestAccAWSUserLoginProfile_keybase (56.59s)
--- PASS: TestAccAWSUserLoginProfile_PasswordLength (57.24s)
--- PASS: TestAccAWSUserLoginProfile_basic (65.99s)

[DrFaust92]

maybe not the best way to handle this and a dedicated test config would be better

[jsmilani]

This seems to have stalled. According to https://www.terraform.io/docs/extend/best-practices/sensitive-state.html#don-39-t-encrypt-state it is necessary to deprecate PGP encrypted state. Can we keep this moving?

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[DrFaust92]

Rebased

--- PASS: TestAccIAMUserLoginProfile_nogpg (46.59s)
--- PASS: TestAccIAMUserLoginProfile_passwordLength (46.70s)
--- PASS: TestAccIAMUserLoginProfile_notAKey (47.34s)
--- PASS: TestAccIAMUserLoginProfile_keybaseDoesntExist (49.97s)
--- PASS: TestAccIAMUserLoginProfile_disappears (61.82s)
--- PASS: TestAccIAMUserLoginProfile_basic (63.68s)

keybase test fails and im unsure how to tackle it

    user_login_profile_test.go:110: Step 1/2 error: Error running apply: exit status 1
        Error: error retrieving GPG Key during IAM User Login Profile (tf-acc-test-4386915330115076656) creation: Error retrieving Public Key for keybase:terraformacctest
--- FAIL: TestAccIAMUserLoginProfile_keybase (48.69s)

[ewbankkit]

LGTM 🚀.

% make testacc TESTS=TestAccIAMUserLoginProfile_ PKG=iam
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMUserLoginProfile_'  -timeout 180m
=== RUN   TestAccIAMUserLoginProfile_basic
=== PAUSE TestAccIAMUserLoginProfile_basic
=== RUN   TestAccIAMUserLoginProfile_keybase
=== PAUSE TestAccIAMUserLoginProfile_keybase
=== RUN   TestAccIAMUserLoginProfile_keybaseDoesntExist
=== PAUSE TestAccIAMUserLoginProfile_keybaseDoesntExist
=== RUN   TestAccIAMUserLoginProfile_notAKey
=== PAUSE TestAccIAMUserLoginProfile_notAKey
=== RUN   TestAccIAMUserLoginProfile_passwordLength
=== PAUSE TestAccIAMUserLoginProfile_passwordLength
=== RUN   TestAccIAMUserLoginProfile_nogpg
=== PAUSE TestAccIAMUserLoginProfile_nogpg
=== RUN   TestAccIAMUserLoginProfile_disappears
=== PAUSE TestAccIAMUserLoginProfile_disappears
=== CONT  TestAccIAMUserLoginProfile_basic
=== CONT  TestAccIAMUserLoginProfile_passwordLength
=== CONT  TestAccIAMUserLoginProfile_notAKey
=== CONT  TestAccIAMUserLoginProfile_keybaseDoesntExist
=== CONT  TestAccIAMUserLoginProfile_nogpg
=== CONT  TestAccIAMUserLoginProfile_disappears
=== CONT  TestAccIAMUserLoginProfile_keybase
    user_login_profile_test.go:110: Step 1/2 error: Error running apply: exit status 1
        
        Error: error retrieving GPG Key during IAM User Login Profile (tf-acc-test-1042917404291852556) creation: Error retrieving Public Key for keybase:terraformacctest
        : parse "https://keybase.io/_/api/1.0/user/lookup.json?usernames=terraformacctest\n&fields=public_keys": net/url: invalid control character in URL
        
          with aws_iam_user_login_profile.test,
          on terraform_plugin_test.tf line 35, in resource "aws_iam_user_login_profile" "test":
          35: resource "aws_iam_user_login_profile" "test" {
        
--- PASS: TestAccIAMUserLoginProfile_nogpg (26.92s)
--- PASS: TestAccIAMUserLoginProfile_passwordLength (28.38s)
--- PASS: TestAccIAMUserLoginProfile_notAKey (33.37s)
--- PASS: TestAccIAMUserLoginProfile_keybaseDoesntExist (33.42s)
--- FAIL: TestAccIAMUserLoginProfile_keybase (33.58s)
--- PASS: TestAccIAMUserLoginProfile_disappears (39.09s)
--- PASS: TestAccIAMUserLoginProfile_basic (41.84s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/iam	45.927s
FAIL
make: *** [testacc] Error 1

Failure is unrelated to this change.

[ewbankkit]

@DrFaust92 Thanks for the contribution 🎉 👏.

[github-actions]

This functionality has been released in v4.1.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

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.