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

Add support for Cloud Run v2 jobs #1954

Merged
merged 10 commits into from
Feb 18, 2024
Merged

Add support for Cloud Run v2 jobs #1954

merged 10 commits into from
Feb 18, 2024

Conversation

wiktorn
Copy link
Collaborator

@wiktorn wiktorn commented Dec 28, 2023

Global changes

  • (tests) add input variable project_number for examples and examples_e2e tests to allow assigning IAM permissions to Service Accounts in fixtures
  • (tests) fix not outputting the path within inventory, when object is not found in inventory
  • (tests) fix create_e2e_sandbox.sh - now it properly finds root of the repo

Secret Manager

  • added version_versions output, to allow specifying versions in other modules. versions is sensitive and it makes it unsuitable for for_each values

New test fixtures

  • pubsub.tf - creating one topic
  • secret-credential.tf - creating Secret Manager credential secret
  • shared-vpc.tf - creating two projects (host and service), and vpc in host project
  • vpc-connector.tf - creating VPC Access Connector instance

Cloud Run v2 module changes

  • create a separate file for service creation (service.tf) and job (job.tf) - for easy comparison
  • add E2E tests where possibile (only eventarc triggers skipped)
  • remove default value for input variable region
  • fix subnet range VPC Access Connector example
  • add creation of service account for audit logs call (trigger requires service account)
  • use provided trigger service account email in local.trigger_sa_email, so explicitly provided SA is passed to trigger
  • set default value for vpc_connector_create.throughput.max, to match what is set by GCP API, as provider uses wrong default of 300 which results in perma-diff
  • create inventory files for all examples
  • fix non-empty plan after apply when providing volumes with empty/null cloud_sql_instances

Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass
  • Check status of b/318055956

@wiktorn wiktorn requested a review from juliodiez December 28, 2023 16:26
@wiktorn wiktorn force-pushed the wiktorn-cloudrun-jobs branch from 8fbf94e to 030f02e Compare December 28, 2023 16:31
@wiktorn wiktorn marked this pull request as draft December 29, 2023 10:52
@wiktorn
Copy link
Collaborator Author

wiktorn commented Dec 29, 2023

I'll add more E2E tests, as we have fixtures.

@github-actions github-actions bot added the on:tools New or changed tool label Dec 29, 2023
@wiktorn wiktorn marked this pull request as ready for review December 29, 2023 14:48
@wiktorn wiktorn requested a review from juliocc December 29, 2023 14:48
@wiktorn
Copy link
Collaborator Author

wiktorn commented Dec 29, 2023

E2E Tests run

Copy link
Collaborator

@juliodiez juliodiez left a comment

Choose a reason for hiding this comment

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

Good job! (no pun intended :)
I have reviewed the CR module so far, probably others are more skilled to review the rest.

modules/cloud-run-v2/variables.tf Show resolved Hide resolved
modules/cloud-run-v2/vpcconnector.tf Show resolved Hide resolved
modules/cloud-run-v2/service.tf Outdated Show resolved Hide resolved
modules/cloud-run-v2/service.tf Outdated Show resolved Hide resolved
modules/cloud-run-v2/vpcconnector.tf Show resolved Hide resolved
modules/cloud-run-v2/job.tf Outdated Show resolved Hide resolved
modules/cloud-run-v2/service.tf Outdated Show resolved Hide resolved
modules/cloud-run-v2/README.md Outdated Show resolved Hide resolved
modules/cloud-run-v2/README.md Show resolved Hide resolved
wiktorn and others added 9 commits February 13, 2024 19:56
* create a separate file for service creation (service.tf) and job
  (job.tf) - for easy comparison
* add E2E tests where possibile
* remove default value for input variable `region`
* fix subnet range VPC Access Connector example
* add creation of service account for audit logs call (trigger requires
  service account)
* use provided trigger service account email in
  `local.trigger_sa_email`, so explicitly provided SA is passed to
  trigger
* set default value for vpc_connector_create.throughput.max, to match
  what is set by GCP API, as provider uses wrong default of 300 which
  results in perma-diff
* create inventory fiels for all examples
Global changes
* (tests) add input variable `project_number`, to allow assigning IAM permissions to Service Accounts in fixtures
* (tests) fix not outputting the path, when object is not found in inventory
* (tests) fix `create_e2e_sandbox.sh` - now it properly finds root of the repo

Secret Manager
* added `version_versions` output, to allow specifying versions in other modules. `versions` is sensitive and it makes it unsuitable for `for_each` values

New test fixtures
* `pubsub.tf` - creating one topic
* `secret-credential.tf` - creating Secret Manager `credential` secret
* `shared-vpc.tf` - creating two projects (host and service), and vpc in host project
* `vpc-connector.tf` - creating VPC Access Connector instance

Cloud Run v2 module changes
* fix non-empty plan after apply when providing `volumes` with empty/null `cloud_sql_instances`
* update inventory files
* add additional E2E tests using fixtures
@wiktorn wiktorn force-pushed the wiktorn-cloudrun-jobs branch from 27b196d to 5d0e7e4 Compare February 13, 2024 19:59
@ludoo
Copy link
Collaborator

ludoo commented Feb 18, 2024

@wiktorn can we merge this? I don't expect clarification on the last item to come anytime soon. :)

@wiktorn
Copy link
Collaborator Author

wiktorn commented Feb 18, 2024

@wiktorn can we merge this? I don't expect clarification on the last item to come anytime soon. :)

Happy to merge it, just need a review :-)

Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

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

great job :)

@wiktorn wiktorn merged commit bee3072 into master Feb 18, 2024
13 checks passed
@wiktorn wiktorn deleted the wiktorn-cloudrun-jobs branch February 18, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on:modules on:tools New or changed tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants