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(modules)!: enable cloudbuildv2 repository support on tf_cloudbuild_builder and tf_cloudbuild_workspace #299

Merged
merged 52 commits into from
Aug 15, 2024

Conversation

caetano-colin
Copy link
Member

  • The additions were designed in a way to have backwards compatibility with CSR on terraform-example-foundation

tf_cloudbuild_builder module:

  • Use source_to_build argument for cloudbuildv2 repository, in this case the user shall not specify dockerfile_repo_uri (it is used for CSR) and specify the variable dockerfile_repo_id with the cloudbuildv2 id.

tf_cloudbuild_workspace module:

  • Added new type under tf_repo_type called CLOUDBUILD_V2_REPOSITORY that shall be used when specifying a cloudbuildv2 repository on cloudbuildv2_repository_id variable

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit daniel-cit requested a review from a team as a code owner August 7, 2024 12:49
@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit daniel-cit requested a review from apeabody August 9, 2024 04:22
@daniel-cit
Copy link
Contributor

@apeabody Could you PTAL again?

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! This is currently marked as a breaking change, but I wonder if that could be avoided if we keep more of the sub-module variable defaults?

modules/tf_cloudbuild_builder/README.md Outdated Show resolved Hide resolved
modules/tf_cloudbuild_builder/variables.tf Show resolved Hide resolved
modules/tf_cloudbuild_builder/variables.tf Show resolved Hide resolved
modules/tf_cloudbuild_workspace/variables.tf Outdated Show resolved Hide resolved
modules/tf_cloudbuild_workspace/variables.tf Outdated Show resolved Hide resolved
@daniel-cit
Copy link
Contributor

/gcbrun

@daniel-cit
Copy link
Contributor

/gcbrun

@@ -70,7 +70,7 @@ variable "gar_repo_name" {
variable "gar_repo_location" {
description = "Name of the location for the Google Artifact Repository."
type = string
default = "us"
default = "us-central1"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's probably better to remove the default entirely, rather than change the default value. That way during an module upgrade an error is surfaced if using the default, rather than the value unexpectedly changing.

@@ -94,12 +94,19 @@ variable "trigger_name" {
variable "trigger_location" {
description = "Location of the Cloud Build trigger building the Terraform builder. If using private pools should be the same location as the pool."
type = string
default = "global"
default = "us-central1"
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@@ -28,7 +28,7 @@ variable "location" {
variable "trigger_location" {
description = "Location of for Cloud Build triggers created in the workspace. If using private pools should be the same location as the pool."
type = string
default = "global"
default = "us-central1"
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@daniel-cit
Copy link
Contributor

/gcbrun

@apeabody apeabody self-assigned this Aug 15, 2024
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!

@apeabody apeabody merged commit 62f5f7d into terraform-google-modules:master Aug 15, 2024
4 checks passed
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.

3 participants