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.

Sign up for GitHub

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!: Adds support for labels #96

Merged
merged 10 commits into from
Jun 27, 2022
Merged

feat!: Adds support for labels #96

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bdf4dbf
Adds support for labels
May 20, 2022
09637a2
Merge branch 'master' into label-support
bharathkkb Jun 16, 2022
5505339
Add provider_meta
Jun 21, 2022
a18c53c
forwarding rule does not need beta provider
Jun 22, 2022
3a79610
Use provider version 4.26
Jun 22, 2022
2a60503
Upgrade GCP/lb/google to 3.1
Jun 22, 2022
2e239a3
bump provider versions
Jun 22, 2022
f66baf4
bump test setup providers
Jun 22, 2022
387ea26
Update instance_template module to allow provider v4
Jun 23, 2022
af5db6e
Update project factory module to allow provider v4
Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ data "google_compute_subnetwork" "network" {
}

resource "google_compute_forwarding_rule" "default" {
provider = google-beta
Copy link
Member

Choose a reason for hiding this comment

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

I was doing some more digging and it seemed to me like the regular provider may actually support this but it is not documented. Did you try this with the regular provider before using beta?
https://github.com/hashicorp/terraform-provider-google/blob/922b67a420efa3bfabde02dd097ef1e1d68d7316/google/resource_compute_forwarding_rule.go#L289

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested and confirmed that forwarding rules support labels without the beta provider, so I've removed the provider attribute from that resource. These docs may be slightly out of date: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_forwarding_rule#labels

The health checks do still require the beta provider, so I'll keep it in versions.tf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are currently using provider version >= 3.53, < 5.0, which does not contain label support outside of beta. Should we bump our version (to >= 4.26, < 5.0), or leave the beta provider on the forwarding rule resource for now?

Copy link
Member

Choose a reason for hiding this comment

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

@willroberts Lets bump the version to 4.26

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @willroberts - It looks like you will need to (at least) also bump

terraform-google-lb-internal/examples/simple/main.tf

Line 19 in 7bdf7ab

version = "~> 2.3"
to 3.1 to support the TPG4 requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still looking at this; the instance_template dependency also needed to be updated, and I'm tracking down some additional sources of provider < 4.0.0 constraints.

The terraform_validate steps in make docker_test_lint appear to be installing provider 3.62.0 and beta provider 3.90.1.

Copy link
Contributor Author

@willroberts willroberts Jun 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Got this working locally. /usr/local/bin/terraform_validate in the container uses terraform init -backend=false. Adding -upgrade to the arguments resolved the dependencies successfully.

Looks like it's also passing in CI now.

project = var.project
name = var.name
region = var.region
Expand All @@ -41,6 +42,7 @@ resource "google_compute_forwarding_rule" "default" {
ports = var.ports
all_ports = var.all_ports
service_label = var.service_label
labels = var.labels
}

resource "google_compute_region_backend_service" "default" {
Expand Down
6 changes: 6 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,9 @@ variable "firewall_enable_logging" {
default = false
type = bool
}

variable "labels" {
description = "The labels to attach to resources created by this module."
default = {}
type = map(string)
}
9 changes: 8 additions & 1 deletion versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,22 @@
terraform {
required_version = ">= 0.13"
required_providers {

google = {
source = "hashicorp/google"
version = ">= 3.53, < 5.0"
}

google-beta = {
source = "hashicorp/google-beta"
version = ">= 3.53, < 5.0"
}
}

provider_meta "google" {
willroberts marked this conversation as resolved.
Show resolved Hide resolved
module_name = "blueprints/terraform/terraform-google-lb-internal/v4.6.0"
}

provider_meta "google-beta" {
module_name = "blueprints/terraform/terraform-google-lb-internal/v4.6.0"
}
}