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

removed conflicts between inline policy and policy_arns, updated tests #344

Merged
merged 5 commits into from
Apr 3, 2019
Merged

removed conflicts between inline policy and policy_arns, updated tests #344

merged 5 commits into from
Apr 3, 2019

Conversation

mliang2
Copy link
Contributor

@mliang2 mliang2 commented Mar 7, 2019

It's ok to have policy_document and policy_arns defined at the same time for the vault_aws_secret_backend_role provider.

CLI equalivent:

vault write aws/roles/foo credential_type=iam_user [email protected] policy_arns=arn:aws:iam::123456789012:policy/policy1,arn:aws:iam::123456789012:policy/policy2

@ghost ghost added the size/XS label Mar 7, 2019
@ghost ghost added size/S and removed size/XS labels Mar 7, 2019
@mliang2
Copy link
Contributor Author

mliang2 commented Mar 25, 2019

Are there any concerns on getting this merged? Would really like to get this done.
Thx!

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Apr 1, 2019
@tyrannosaurus-becks
Copy link
Contributor

tyrannosaurus-becks commented Apr 1, 2019

Hi @mliang2 ! I see you're right that they can be provided together, thanks for catching this.

When I run the tests against the current branch of Vault master, I get this:

=== RUN   TestAccAWSSecretBackendRole_basic
--- FAIL: TestAccAWSSecretBackendRole_basic (0.00s)
    testing.go:538: Step 0 error: Error loading configuration: Error parsing /tmp/tf-test741671546/main.tf: At 24:21: error parsing list, expected comma or list end, got: IDENT
=== RUN   TestAccAWSSecretBackendRole_import
--- FAIL: TestAccAWSSecretBackendRole_import (0.00s)
    testing.go:538: Step 0 error: Error loading configuration: Error parsing /tmp/tf-test998350737/main.tf: At 24:21: error parsing list, expected comma or list end, got: IDENT
=== RUN   TestAccAWSSecretBackendRole_nested
--- FAIL: TestAccAWSSecretBackendRole_nested (0.00s)
    testing.go:538: Step 0 error: Error loading configuration: Error parsing /tmp/tf-test467045820/main.tf: At 24:21: error parsing list, expected comma or list end, got: IDENT
FAIL

Can you post the version of Vault you're testing against, as well as the test output you're seeing for these?

@mliang2
Copy link
Contributor Author

mliang2 commented Apr 2, 2019

My fork was out of date w/ master. Once I synced w/ master, make testacc passed with Vault 1.0.0 and 1.1.0

@mliang2
Copy link
Contributor Author

mliang2 commented Apr 2, 2019

Turns out I was not testing w/ my AWS key set, so those tests were skipped; I'll look into how I can fix this

@mliang2
Copy link
Contributor Author

mliang2 commented Apr 2, 2019

@tyrannosaurus-becks This errors you're seeing should fixed. It was caused by me putting the wrong order of the sprintf items. (putting ARNs variables into the policy_document field instead of policy_arns. Tested w/ Vault 1.0.0 and 1.1.0 and here's what I got w/ regards to the failed test you were seeing:

=== RUN   TestAccAWSSecretBackendRole_basic
--- PASS: TestAccAWSSecretBackendRole_basic (0.15s)
=== RUN   TestAccAWSSecretBackendRole_import
--- PASS: TestAccAWSSecretBackendRole_import (0.10s)
=== RUN   TestAccAWSSecretBackendRole_nested
--- PASS: TestAccAWSSecretBackendRole_nested (0.16s)

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

@mliang2 fantastic! The tests are working for me now too. Thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit 5fb10b9 into hashicorp:master Apr 3, 2019
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
removed conflicts between inline policy and policy_arns, updated tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants