Fix path check on secret role read

[lmickh]

This is to address #78

The code currently forces a backend/roles/name format when it should be able to take a nested path as the backend such as backend/path/roles/name. This PR changes the check to ensure it is at least as large as it should be and that roles is the second to last of the slice.

[martinssipenko]

This one would be very good to get released.

[martinssipenko]

@lmickh looking at your change, it appears to me that it's only a partial fix.

If you look further down the code lines 105 and 106, you can see that pathPieces[2] is used for name, but in case of nested path the name would be the last part of pathPieces, not the one with index 2. I think it should be something like this:

d.Set("backend", strings.Join(pathPieces[:len(pathPieces)-2], "/"))
d.Set("name", pathPieces[len(pathPieces)-1])

Used this snippet to test it out https://play.golang.org/p/9tuwCuC5BSM

[paultyng]

It would be good to add some testing around that alternative path if possible as well in this PR.

[lmickh]

@martinssipenko nice catch. Not sure how I even missed that.

Thanks for the feedback. I'm still in the process of sorting out testing. Haven't done any testing in go before much less one of these providers.

[martinssipenko]

Just use nested path over here:

https://github.com/lmickh/terraform-provider-vault/blob/213f441d4a5ac60e219e586d2df4d7c3f48e3f3f/vault/resource_aws_secret_backend_role_test.go#L19

[lmickh]

Made the change. Let me know if anything else needs to be done or if that isn't what you had in mind.

[paultyng]

While this works I think you should just copy/paste it to TestAccAWSSecretBackendRole_nested or something so that we are also still testing the original path as well.

[paultyng]

This looks good to me, but I'll let @paddycarver take a quick pass before merging.

[martinssipenko]

Guys, I've noticed that tests for this resource provider are broken, and it looks that no-one has actually tried to run them :)

I've created a Pull Request to @lmickh to fix tests, and hopefully when he merges it the fixed tests will land into this PR. lmickh#1

Before fixing tests:

$make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_basic -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_basic
--- FAIL: TestAccAWSSecretBackendRole_basic (0.50s)
    testing.go:434: Step 0 error: Check failed: Check 1/6 error: Not found: vault_aws_secret_backend_role.test_policy_inline
FAIL
FAIL    github.com/terraform-providers/terraform-provider-vault/vault   0.522s
make: *** [testacc] Error 1

$ make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_import'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_import -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_import
--- FAIL: TestAccAWSSecretBackendRole_import (0.49s)
    testing.go:434: Step 0 error: Check failed: Check 1/6 error: Not found: vault_aws_secret_backend_role.test_policy_inline
FAIL
FAIL    github.com/terraform-providers/terraform-provider-vault/vault   0.510s
make: *** [testacc] Error 1

$ make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_nested'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_nested -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_nested
--- FAIL: TestAccAWSSecretBackendRole_nested (0.50s)
    testing.go:434: Step 0 error: Check failed: Check 1/6 error: Not found: vault_aws_secret_backend_role.test_policy_inline
FAIL
FAIL    github.com/terraform-providers/terraform-provider-vault/vault   0.523s
make: *** [testacc] Error 1

After fixing tests:

$ make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_basic -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_basic
--- PASS: TestAccAWSSecretBackendRole_basic (1.64s)
PASS
ok      github.com/terraform-providers/terraform-provider-vault/vault   1.663s

$ make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_import'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_import -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_import
--- PASS: TestAccAWSSecretBackendRole_import (1.06s)
PASS
ok      github.com/terraform-providers/terraform-provider-vault/vault   1.088s

$ make testacc TESTARGS='-run=TestAccAWSSecretBackendRole_nested'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSSecretBackendRole_nested -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
=== RUN   TestAccAWSSecretBackendRole_nested
--- PASS: TestAccAWSSecretBackendRole_nested (1.60s)
PASS
ok      github.com/terraform-providers/terraform-provider-vault/vault   1.624s

[martinssipenko]

@lmickh can you update your branch so it's not out of date with the base?

[lmickh]

@martinssipenko I rebased it on master. Let me know if you need anything else.

[martinssipenko]

This is great, hoping this gets a release soon.

[paddycarver]

This makes sense to me! Thanks for fixing it, and sorry for the oversight.