-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Add Grafana service account #38101
Add Grafana service account #38101
Conversation
Community NoteVoting for Prioritization
For Submitters
|
Thank you for your contribution! 🚀 Please note that typically Go dependency changes are handled in this repository by dependabot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the Additional details:
|
Got a few questions for reviewers myself |
PreCheck: func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.Grafana) }, | ||
ErrorCheck: acctest.ErrorCheck(t, grafana.ServiceID), | ||
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, | ||
CheckDestroy: testAccCheckWorkspaceServiceAccountDestroy(ctx), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CheckDestroy
throws an error that looks like a race condition and I don't know how to fix it.
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
make: Building provider...
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.4 test ./internal/service/grafana/... -v -count 1 -parallel 20 -run='TestAccWorkspaceServiceAccount' -timeout 360m
=== RUN TestAccWorkspaceServiceAccount_basic
=== PAUSE TestAccWorkspaceServiceAccount_basic
=== RUN TestAccWorkspaceServiceAccount_disappears
=== PAUSE TestAccWorkspaceServiceAccount_disappears
=== CONT TestAccWorkspaceServiceAccount_basic
=== CONT TestAccWorkspaceServiceAccount_disappears
--- PASS: TestAccWorkspaceServiceAccount_disappears (339.76s)
=== NAME TestAccWorkspaceServiceAccount_basic
workspace_service_account_test.go:29: Error running post-test destroy, there may be dangling resources: operation error grafana: ListWorkspaceServiceAccounts, https response error StatusCode: 404, RequestID: 108fce02-80f4-49d1-8541-4d78d559a454, ResourceNotFoundException: Workspace with id=g-ca7016d798 of account=339743103717 does not exist.
--- FAIL: TestAccWorkspaceServiceAccount_basic (369.06s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/grafana 374.937s
FAIL
make: *** [testacc] Error 1
However, the same tests outside with a normal main.tf
and apply/destroy do not generate any error. Any insights on how to move forward?
I think the workspace gets destroyed before the service token is deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used acctest.CheckDestroyNoop
on CheckDestroy
. Happy to revisit if there's another path
3rd question - should the implementation of service account token (which share a lot with this PR) continue with the same PR or be separate and branched out of this one? |
any insights on this @justinretzolk @ewbankkit? |
As service account by itself is not really useful, I merged locally my branch on service account token. Happy to drop this commit if it needs to be submitted separately commit 6b70e6e59b7dc0bd1ac652ce3038413f65c11e8b Author: Rodrigue Koffi <[email protected]> Date: Thu Jun 27 21:55:00 2024 +0200 Add service account token doc commit bd3fad4e84eb75815516cd658dc155ece76f5791 Author: Rodrigue Koffi <[email protected]> Date: Thu Jun 27 16:05:28 2024 +0200 Optimize tests paths commit 560148e983736a2e193442d24782e5c755be2fca Author: Rodrigue Koffi <[email protected]> Date: Thu Jun 27 12:41:41 2024 +0200 Add acceptance testing commit 39090f6e22252ff979405715a6e0f094f383933b Author: Rodrigue Koffi <[email protected]> Date: Thu Jun 27 11:32:00 2024 +0200 Add service account token resource
|
# Conflicts: # go.mod # go.sum # internal/conns/awsclient_gen.go # internal/service/grafana/exports_test.go # internal/service/grafana/generate.go # internal/service/grafana/service_endpoint_resolver_gen.go # internal/service/grafana/service_endpoints_gen_test.go # internal/service/grafana/service_package_gen.go # names/data/names_data.hcl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccGrafanaWorkspaceServiceAccount_basic' PKG=grafana ACCTEST_PARALLELISM=2
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.4 test ./internal/service/grafana/... -v -count 1 -parallel 2 -run=TestAccGrafanaWorkspaceServiceAccount_basic -timeout 360m
=== RUN TestAccGrafanaWorkspaceServiceAccount_basic
=== PAUSE TestAccGrafanaWorkspaceServiceAccount_basic
=== CONT TestAccGrafanaWorkspaceServiceAccount_basic
--- PASS: TestAccGrafanaWorkspaceServiceAccount_basic (380.37s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/grafana 385.341s
% make testacc TESTARGS='-run=TestAccGrafanaWorkspaceServiceAccount_disappears\|TestAccGrafanaWorkspaceServiceAccountToken_' PKG=grafana ACCTEST_PARALLELISM=2
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.4 test ./internal/service/grafana/... -v -count 1 -parallel 2 -run=TestAccGrafanaWorkspaceServiceAccount_disappears\|TestAccGrafanaWorkspaceServiceAccountToken_ -timeout 360m
=== RUN TestAccGrafanaWorkspaceServiceAccount_disappears
=== PAUSE TestAccGrafanaWorkspaceServiceAccount_disappears
=== RUN TestAccGrafanaWorkspaceServiceAccountToken_basic
=== PAUSE TestAccGrafanaWorkspaceServiceAccountToken_basic
=== RUN TestAccGrafanaWorkspaceServiceAccountToken_disappears
=== PAUSE TestAccGrafanaWorkspaceServiceAccountToken_disappears
=== CONT TestAccGrafanaWorkspaceServiceAccount_disappears
=== CONT TestAccGrafanaWorkspaceServiceAccountToken_disappears
--- PASS: TestAccGrafanaWorkspaceServiceAccountToken_disappears (352.25s)
=== CONT TestAccGrafanaWorkspaceServiceAccountToken_basic
--- PASS: TestAccGrafanaWorkspaceServiceAccount_disappears (378.66s)
--- PASS: TestAccGrafanaWorkspaceServiceAccountToken_basic (333.86s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/grafana 691.105s
@bonclay7 Thanks for the contribution 🎉 👏. |
This functionality has been released in v5.59.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! |
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. |
Description
Adds support for Grafana Service Account with Amazon Managed Grafana
Relations
Closes #37645
References
Output from Acceptance Testing