Add oidc token support

[salrashid123]

PR adds oidc token support for this provider.
see #5669

This pr is now complete and does has working testcases;

[salrashid123]

I updated the go.mod but still see conflict for some reason.

Anyway, i've reverified its functionality. if the testcases are sufficient (which i actually just copied from the existing data_source_google_service_account_access_token_test, can we proceed w/ review?

[danawillow]

For the merge conflict, I'd recommend just rebasing or merging on top of master. Are the go.mod changes 100% necessary? If so, they need to be done in a separate PR here and also in the beta provider, because the other changes will be upstreamed to Magic Modules (see https://github.com/terraform-providers/terraform-provider-google/blob/master/.github/CONTRIBUTING.md#autogenerated-resources). It's fine to do the review all at once here though and split it up later if that's easier.

The logic generally looks sound, though I had some style and documentation suggestions.

[salrashid123]

ok, i made a number of changes:

• removed the static access_token thing; ther'es no way that woudl've worked since the library terraform uses underlying credentials into which you can add in a token like that. I suppose if a static access_token is set directly, i could somehow use config.AccessToken but i'll have to revert that later in code. It'd be confusing and a very error prone.

• re: conflicts. it looks like the primary go.mod conflict is with golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e which i think comes through with "google.golang.org/api/idtoken"

• re: docs. I refactored the whole doc set and removed a lot of the stuff. I marked the comments you had as resolved since those sections were deleted. PTAL

• re: upstream/magic modules. Lets finish the review here...i'd likely mess up the PR if i did complicated stuff like that!

[danawillow]

In order for resolve the merge conflicts that GitHub is showing, you'll still want to merge or rebase on top of master.

[salrashid123]

Not sure why the build failed on that documentation page (its not included in this PR)...maybe rerun travis again?

[tbpg]

I left a few (untested) suggestions for how to use types instead of reflection to identify static token sources.

Here is a slightly modified example with a simplified GetCredential call: https://play.golang.org/p/RLCXjYiZzUq.

[salrashid123]

@tbpg Thanks for the suggestions!

@danawillow i'm not sure if the last website/google.erb change that appeared would help with the travis error or not. should i rebase and retry?

[salrashid123]

yeah, i don't understand the docs error there...the make website-test exits correctly for me

$ make website-test

Found no broken links.

FINISHED --2020-06-29 14:41:45--
Total wall clock time: 12s
Downloaded: 323 files, 30M in 0.06s (515 MB/s)
tf-website-google-temp
make[1]: Leaving directory '/path/to/src/github.com/hashicorp/terraform-website'

$ echo $?
0

[danawillow]

Oh, the website test failure is unrelated. Don't worry about it.

[salrashid123]

ok, i added in actual tests..but here's the catch:

the basic tests seem to work fine

export  GOOGLE_REGION=us-central1
export  GOOGLE_CLOUD_KEYFILE_JSON=/path/to/svc_account.json
export  GOOGLE_ZONE=us-central1-a
export  GOOGLE_PROJECT=someproject
export  GOOGLE_SERVICE_ACCOUNT=svcaccount1@someproject.iam.gserviceaccount.com


$ make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleServiceAccountIdToken_basic'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccDataSourceGoogleServiceAccountIdToken_basic -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceGoogleServiceAccountIdToken_basic
=== PAUSE TestAccDataSourceGoogleServiceAccountIdToken_basic
=== CONT  TestAccDataSourceGoogleServiceAccountIdToken_basic
--- PASS: TestAccDataSourceGoogleServiceAccountIdToken_basic (2.34s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	2.521s

---

but the one with impersonation needs a delay due to iam propagation delay. Any suggestion on how to do that ...i suppose a null resource+sleep ~60s somehow?

at the moment, the first run will do the IAM assocation but fail since the permissions to impersonate hasn't propagated

$ make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleServiceAccountIdToken_impersonation'
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccDataSourceGoogleServiceAccountIdToken_impersonation -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceGoogleServiceAccountIdToken_impersonation
=== PAUSE TestAccDataSourceGoogleServiceAccountIdToken_impersonation
=== CONT  TestAccDataSourceGoogleServiceAccountIdToken_impersonation
2020/06/29 23:40:19 [INFO] Authenticating using configured Google JSON 'credentials'...
    TestAccDataSourceGoogleServiceAccountIdToken_impersonation: testing.go:674: Step 0 error: errors during refresh:
        
        Error: googleapi: Error 403: The caller does not have permission, forbidden
--- FAIL: TestAccDataSourceGoogleServiceAccountIdToken_impersonation (2.42s)

which will fail but implictly apply an iam binding thats needed

$ gcloud iam service-accounts get-iam-policy  tf-bootstrap-service-account@project.iam.gserviceaccount.com
bindings:
- members:
  - serviceAccount:svcaccount1@project.iam.gserviceaccount.com
  - serviceAccount:tf-bootstrap-service-account@project.iam.gserviceaccount.com
  role: roles/iam.serviceAccountTokenCreator

but if you wait ~60s and rerun the tests, it works

$ make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleServiceAccountIdToken_impersonation'
--- PASS: TestAccDataSourceGoogleServiceAccountIdToken_impersonation (8.09s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	8.274s

[salrashid123]

@danawillow think its ready now;
i'm not sure how the acc tests are setup internally (i.,e if you use the same set of svc accounts on the same project), but in the event that these iam bindings are setup one and persist test to test, then the acc testsin this case would pass the impersonation one

[danawillow]

I don't totally understand your most recent question, but any environment variable will be the same for all tests, and we often run tests in parallel. Does that answer the question?

For the question around waiting, take a look at this test, which I think had the same problem: https://github.com/terraform-providers/terraform-provider-google/blob/master/google/provider_test.go#L477

[salrashid123]

thx, i'm what i meant by that is when i ran just the impersonation acc test locally, it would fail the first time but associate the IAM binding w/ the service accounts i specified in the env-var. If i ran the same test again, the test would pass since the binding was done a couple mins earlier.

I believe this step does that per test run:

https://github.com/terraform-providers/terraform-provider-google/blob/master/google/bootstrap_utils_test.go#L164

that bit is what allows the following existing test to work

• https://github.com/terraform-providers/terraform-provider-google/blob/master/google/data_source_google_service_account_access_token_test.go#L52

make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleServiceAccountAccessToken_basic'

i guess what i'm trying to say is if the acc tests you're running now allows the test above to run, the impersonation test i've got in this PR should also work (i.,e they both require the same iam settings). Could you rerun the acc tests?

make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleServiceAccountIdToken_impersonation'

[salrashid123]

@danawillow could you rerun the test and see if it passes? I'm not really doing anything different to bootstrap impersonated credentials than that existing test.

[danawillow]

Yup, it passes. I should have some time to do a final review later today or early tomorrow.

[danawillow]

This all looks good! I've noticed that your PR description still says this is a prototype. Am I good to merge this into the provider? If so, can you please update the description?

[salrashid123]

done; updated description;

thanks! :)

[danawillow]

I had to make a small change to the upstream one and I'm a bit nervous about merge conflicts, so I'm going to close this. The code will be merged in via GoogleCloudPlatform/magic-modules#3739.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!