-
Notifications
You must be signed in to change notification settings - Fork 148
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!: Update Dockerfile to install terraform-validator from gcloud #156
feat!: Update Dockerfile to install terraform-validator from gcloud #156
Conversation
@bharathkkb could you be the reviewer? |
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.
Thanks for the PR @iyabchen
A few places we also need to update
ARG TERRAFORM_VALIDATOR_RELEASE=v0.6.0 |
ENV ENV_TERRAFORM_VALIDATOR_RELEASE=$TERRAFORM_VALIDATOR_RELEASE |
_TERRAFORM_VALIDATOR_RELEASE: 'v0.6.0' |
gcloud ${local.impersonate_service_account} builds submit ${path.module}/cloudbuild_builder/ --project ${module.cloudbuild_project.project_id} --config=${path.module}/cloudbuild_builder/cloudbuild.yaml --substitutions=_TERRAFORM_VERSION=${var.terraform_version},_TERRAFORM_VERSION_SHA256SUM=${var.terraform_version_sha256sum},_TERRAFORM_VALIDATOR_RELEASE=${var.terraform_validator_release},_REGION=${google_artifact_registry_repository.tf-image-repo.location},_REPOSITORY=${local.gar_name} |
terraform-google-bootstrap/modules/cloudbuild/variables.tf
Lines 146 to 155 in e1d3952
variable "terraform_validator_release" { | |
description = "Default terraform-validator release." | |
type = string | |
default = "v0.6.0" | |
validation { | |
condition = try(tonumber(trimprefix(replace(var.terraform_validator_release, ".", ""), "v")), 0) >= 60 | |
error_message = "Terraform-validator release must be >= v0.6.0." | |
} | |
} |
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.
Can we also add an upgrade guide to inform users of this switch and linking to gcloud vet docs so they can make appropriate changes in their CI.
https://github.com/terraform-google-modules/terraform-google-bootstrap/tree/master/docs
docs/upgrading_to_v5.0.md
Outdated
@@ -2,9 +2,9 @@ | |||
|
|||
The v5.0 release of *bootstrap* is a backwards incompatible release. | |||
|
|||
## Terraform Validator < `v0.6.0` no longer supported |
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.
This file should be kept as is since its for an old release. Next release will be v6.0
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.
Reverted
/builder/google-cloud-sdk/bin/gcloud -q components install alpha beta && \ | ||
/builder/google-cloud-sdk/bin/gcloud components update && \ | ||
/builder/google-cloud-sdk/bin/gcloud components install terraform-tools && \ |
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.
Are these automatically installed?
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.
It comes with the base image.
@@ -12,7 +12,8 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
|
|||
FROM gcr.io/cloud-builders/gcloud-slim | |||
ARG GCLOUD_VERSION=slim | |||
FROM google/cloud-sdk:${GCLOUD_VERSION} |
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.
We should allow overriding this via TF like how we do for TERRAFORM_VERSION
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.
Added variables into the tf modules
@iyabchen |
#153