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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Require specifying the Google Service Account to run as. #20

Merged
merged 1 commit into from
Feb 26, 2023

Conversation

mattmoor
Copy link
Member

馃巵 This changes the module to allow the caller to specify the google service account as which to run.

This allows the caller to have greater control over the service account name, and to authorize the service account to take actions prior to invoking the module to spin up the Cloud Run service.

/kind feature

main.tf Outdated
# Service accounts can be 30 characters long, so truncate var.name to 23 chars.
account_id = "${substr(var.name, 0, 23)}-prober"
# Service accounts can be 30 characters long, so truncate var.name to 26 chars.
account_id = "${substr(var.name, 0, 26)}-prb"
Copy link
Member Author

Choose a reason for hiding this comment

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

The count change will trigger a delete/create, so this change is intentional in order to avoid a name collision with the tombstoned service account, since I don't believe TF is smart enough to simple bring back the tombstoned GSA.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I am tempted to just mail this required moving forward since the most simple case of feeding a GSA .email in here leads to:

Error: Invalid count argument

  on .terraform/.../main.tf line 25, in resource "google_service_account" "prober":
  25:   count = var.service-account != "" ? 0 : 1

The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the
count depends on.

}

// Build the prober into an image we can run on Cloud Run.
resource "ko_image" "image" {
repo = local.repo
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an improvement in newer releases of tf-ko :)

@mattmoor mattmoor force-pushed the allow-specifying-gsa branch 3 times, most recently from 153ba95 to fdcca9a Compare February 25, 2023 21:16
@mattmoor mattmoor enabled auto-merge (squash) February 25, 2023 21:17
Comment on lines +19 to +22
- name: Dump README
if: failure()
run: |
cat README.md
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes it easy to [ab]use CI to generate what the README should look like.

@mattmoor mattmoor force-pushed the allow-specifying-gsa branch 2 times, most recently from e107ff2 to 6215d43 Compare February 25, 2023 22:01
@mattmoor mattmoor changed the title Feature: Allow specifying the Google Service Account to run as. Feature: Require specifying the Google Service Account to run as. Feb 25, 2023
馃巵 This changes the module to require the caller to specify the google service account as which to run.

This allows the caller to have greater control over the service account name, and to authorize the service
account to take actions prior to invoking the module to spin up the Cloud Run service.

/kind feature
@mattmoor
Copy link
Member Author

Going to merge this, since I believe it satisfies our needs, but will hold off on cutting a release until sometime next week.

@mattmoor mattmoor merged commit 3fb53bb into chainguard-dev:main Feb 26, 2023
@mattmoor mattmoor deleted the allow-specifying-gsa branch February 26, 2023 19:04
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