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

feat: workload-identity: Allow passing Google Service Account display_name and description #1834

Merged
merged 10 commits into from
Jan 16, 2024

Conversation

kosta
Copy link
Contributor

@kosta kosta commented Jan 3, 2024

Straightforward change to allow passing display_name and description to the Service Account created by workload identity (as input variables gcp_sa_display_name and gcp_sa_description).

If these are not given (they default to null), the behaviour is the same as before.

Use case: we want to implement some conventions around service account display_name and description for all service accounts to follow (e.g. document how they were created). Without this change, this is only possible by moving the google service account terraform resource outside of the workload-identity module, which is cumbersome state juggling for existing service accounts.

I am not exactly sure what kind of tests are needed here, please advise.

@kosta kosta requested review from ericyz and a team as code owners January 3, 2024 19:55
Copy link

google-cla bot commented Jan 3, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kosta kosta changed the title workload-identity: Allow passing Google Service Account display_name and description feat: workload-identity: Allow passing Google Service Account display_name and description Jan 3, 2024
@apeabody
Copy link
Contributor

apeabody commented Jan 3, 2024

/gcbrun

@kosta
Copy link
Contributor Author

kosta commented Jan 5, 2024

I had to fix a copy/paste mistake in the variable validation test. Could you /gcbrun again? :)

@apeabody
Copy link
Contributor

apeabody commented Jan 8, 2024

/gcbrun

@apeabody
Copy link
Contributor

apeabody commented Jan 8, 2024

Hmm, the CI error doesn't immediately appear to be related to this change:

Step #72 - "verify sandbox-enabled-local":         	            	Diff:
Step #72 - "verify sandbox-enabled-local":         	            	--- Expected
Step #72 - "verify sandbox-enabled-local":         	            	+++ Actual
Step #72 - "verify sandbox-enabled-local":         	            	@@ -3,5 +3,3 @@
Step #72 - "verify sandbox-enabled-local":         	            	     "dnsCacheConfig": {},
Step #72 - "verify sandbox-enabled-local":         	            	-    "gcePersistentDiskCsiDriverConfig": {
Step #72 - "verify sandbox-enabled-local":         	            	-      "enabled": true
Step #72 - "verify sandbox-enabled-local":         	            	-    },
Step #72 - "verify sandbox-enabled-local":         	            	+    "gcePersistentDiskCsiDriverConfig": {},
Step #72 - "verify sandbox-enabled-local":         	            	     "gcpFilestoreCsiDriverConfig": {},
Step #72 - "verify sandbox-enabled-local":         	Test:       	TestSandboxEnabled

Might be related to hashicorp/terraform-provider-google#16794 as this was run using the TPG 5.11.0 driver.

@kosta
Copy link
Contributor Author

kosta commented Jan 9, 2024

Is there anything I can do about the test failure? All I could think of was rebasing on current main branch but there are no new commits to rebase to.

@apeabody
Copy link
Contributor

apeabody commented Jan 9, 2024

Is there anything I can do about the test failure? All I could think of was rebasing on current main branch but there are no new commits to rebase to.

I just merged in a change to fix the CI, re-running the tests.

@apeabody
Copy link
Contributor

apeabody commented Jan 9, 2024

/gcbrun

@apeabody
Copy link
Contributor

apeabody commented Jan 9, 2024

/gcbrun

@kosta
Copy link
Contributor Author

kosta commented Jan 15, 2024

I merged master once more, maybe another /gcbrun will be green?

@apeabody
Copy link
Contributor

/gcbrun

@apeabody
Copy link
Contributor

/gcbrun

@apeabody
Copy link
Contributor

/gcbrun

@apeabody
Copy link
Contributor

CI is intermittently hitting a quota:

googleapi: Error 403: Quota exceeded for quota metric 'Read requests' and limit 'Read requests per minute per region' of service 'compute.googleapis.com' for consumer

@apeabody
Copy link
Contributor

/gcbrun

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @kosta!

@apeabody apeabody merged commit b387621 into terraform-google-modules:master Jan 16, 2024
4 checks passed
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants