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

Introduce role_arns, max_sts_ttl and default_sts_ttl #329

Conversation

onorua
Copy link

@onorua onorua commented Feb 27, 2019

Catch up with Vault API for aws_secret_backend_role
Fix test with assumed_role without role_arns, which is not possible in real life.
Extend tests to make sure assumed_role is validated

@ghost ghost added the size/M label Feb 27, 2019
@onorua
Copy link
Author

onorua commented Feb 27, 2019

I did not touch TestDataSourcePolicyDocument but it is failing for some reason, do you believe it is some race?

Description: "ARN for an existing IAM policy the role should use.",
Deprecated: `Use "policy_arns".`,
},
"policy_document": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"policy_arns", "policy_arn", "policy"},
ConflictsWith: []string{"policy_arns", "policy_arn", "policy", "role_arns"},

Choose a reason for hiding this comment

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

Per the documentation, there is no conflict
https://www.vaultproject.io/api/secret/aws/index.html#policy_document
Says it will just act as a filter

}

if policy == "" && len(policyARNs) == 0 && len(roleARNs) == 0 {
return fmt.Errorf("either policy or policy_arn or role_arns must be set")

Choose a reason for hiding this comment

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

The error is misleading, you are checking policy, policyARN and roleARN but mentioning only 2 of them in the error

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the review, will address shortly

@patoarvizu
Copy link

Any update on this? Any way I can help?

@onorua
Copy link
Author

onorua commented Mar 22, 2019

@patoarvizu I've added a new tests, to confirm that the sts ttls are working, and it is constantly failing on read, while with the real vault - it works fine, can't figure out what is wrong :(
I'll push my current status, maybe you could help.

@onorua onorua changed the title Introduce role_arns, max_sts_ttl and default_sts_ttl [WIP] Introduce role_arns, max_sts_ttl and default_sts_ttl Mar 22, 2019
@onorua
Copy link
Author

onorua commented Mar 22, 2019

weird, checks passed here, but did not on my own environment :(
I've spent like a week trying to fix something which was not broken it seems.

Catch up with Vault API for aws_secret_backend_role
Fix test with assumed_role without role_arns, which
is not possible in real life.
Extend tests to make sure assumed_role is validated
@onorua onorua force-pushed the catch-up-api-with-vault-for-resource_aws_secret_backend_role branch from b5f320e to e1e13db Compare March 22, 2019 08:08
@onorua onorua changed the title [WIP] Introduce role_arns, max_sts_ttl and default_sts_ttl Introduce role_arns, max_sts_ttl and default_sts_ttl Mar 22, 2019
@onorua
Copy link
Author

onorua commented Mar 22, 2019

@ShimiTaNaka It passed the second build in a row, and it is working for me, I believe it is ready.

@patoarvizu
Copy link

Hi there! Just following up with the maintainers to see if it would be possible to push this through. Thanks!

@lattwood
Copy link

@tyrannosaurus-becks hey, this is another fix for the issue fixed here: #259

Right now we're using a forked version of the provider due to the semantic versioning violation, is there a timeline for reviewing these PRs?

@lattwood
Copy link

I think there's another PR that was closed as well.

@tyrannosaurus-becks
Copy link
Contributor

@lattwood should be able to dig into these next week!

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

@onorua thanks for working on this!

When I test against Vault's current master branch, I get:

GOROOT=/usr/local/go #gosetup
GOPATH=/home/tbex/go #gosetup
/usr/local/go/bin/go test -c -o /tmp/___resource_aws_secret_backend_role_test_go github.com/terraform-providers/terraform-provider-vault/vault #gosetup
/usr/local/go/bin/go tool test2json -t /tmp/___resource_aws_secret_backend_role_test_go -test.v -test.run "^TestAccAWSSecretBackendRole_basic|TestAccAWSSecretBackendRole_import|TestAccAWSSecretBackendRole_nested$" #gosetup
=== RUN   TestAccAWSSecretBackendRole_basic
--- PASS: TestAccAWSSecretBackendRole_basic (0.13s)
=== RUN   TestAccAWSSecretBackendRole_import
--- FAIL: TestAccAWSSecretBackendRole_import (0.07s)
    testing.go:434: Step 3 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) {
        }
        
        
        (map[string]string) (len=2) {
         (string) (len=15) "default_sts_ttl": (string) (len=4) "1800",
         (string) (len=11) "max_sts_ttl": (string) (len=4) "5400"
        }
        
=== RUN   TestAccAWSSecretBackendRole_nested
--- PASS: TestAccAWSSecretBackendRole_nested (0.11s)
FAIL

Process finished with exit code 1

What version of Vault are you testing with?

@tyrannosaurus-becks
Copy link
Contributor

I also still find a test failure on version 11.6 of Vault, FWIW.

@Lngramos
Copy link

Lngramos commented Apr 5, 2019

This is great @onorua! I can't wait to make use of this.

@cludden
Copy link

cludden commented Apr 14, 2019

Anything I can do to help nudge this along? I need this merged in to support terraform provisioning AWS backend roles with credential type assumed_role which doesn't seem to work correctly without the role_arn parameter

@bodhi
Copy link

bodhi commented Apr 15, 2019

@cludden

I need this merged in to support terraform provisioning AWS backend roles with credential type assumed_role which doesn't seem to work correctly without the role_arn parameter

A workaround that I've been able to use is:

  1. Don't set credential_type on the vault_aws_secret_backend_role

  2. Use policy_arn instead of role_arn. (and add policy_arn to ignore_changes, because Vault will move policy_arn to role_arn automatically)

  3. To generate credentials for the IAM role, use aws/sts/<role_name> instead of aws/creds/<role_name>.

@ccday
Copy link

ccday commented Apr 16, 2019

Also would love to know if there's anything that could be done to help this get merged so roles with the assumed_role credential type are usable.

@skj-dev
Copy link

skj-dev commented May 5, 2019

FWIW, I just bumped into this as well. I'm managing AWS and Vault with Terraform, and was wanting to read in an AWS role from a state file data source, and use that as a role_arn in the Vault vault_aws_secret_backend_role. As a work around, I'm creating the roles with Terraform on the AWS side, creating the Vault backend roles with the Vault CLI, and then importing those in the Vault Terraform config. It works, but is far from ideal.

@tyrannosaurus-becks
Copy link
Contributor

Closing due to inactivity. Please feel free to open it again if you have time to circle back and get it into order! Thank you!

@cludden
Copy link

cludden commented May 20, 2019

@bodhi thanks for sharing a workaround. At this point I'm willing to try it since it doesn't appear like official support will be added, however, it fails because credential_type is required. If I set credential type to assumed_role, it fails with cannot supply policy_arns when credential_type isn't iam_user.. do you have an example of your workaround that you can share?

@bodhi
Copy link

bodhi commented May 20, 2019

@cludden you might be using a newer version of the provider than me? This is the configuration that we use:

# This enables Vault clients to generate credentials for the service's
# AWS IAM role.
resource vault_aws_secret_backend_role service {
  backend = "aws"
  name = "${local.service_id}"

  # Terraform doesn't support `credential_type` yet.
  #credential_type = "assumed_role"

  # Terraform doesn't support `role_arns` yet, so use `policy_arn`
  # instead. This means this role needs to be used via `aws/sts/app`
  # instead of `aws/creds/app`.
  #role_arn = "${aws_iam_role.service.arn}"
  policy_arn = "${aws_iam_role.service.arn}"

  # Vault copies `policy_arn` to `policy_arns`, so this is always
  # blank when fetching current state from Vault.
  lifecycle {
    ignore_changes = ["policy_arn"]
  }
}

This worked with v1.5.0 of terraform-provider-vault.

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.

10 participants