-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: Make GKE components ADC compatible #2440
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
fix: Make GKE components ADC compatible #2440
Conversation
…riable project to project_id
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 @Daisyprakash
/gcbrun |
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 @Daisyprakash!
Looks like a few additional changes are required to align the tests with these changes (project
> project_id
:
terraform_validate ./test/fixtures/gke_autopilot_cluster
╷
│ Error: Missing required argument
│
│ on ../../../examples/gke_autopilot_cluster/main.tf line 35, in module "gke":
│ 35: module "gke" {
│
│ The argument "project_id" is required, but no definition was found.
╵
╷
│ Error: Unsupported argument
│
│ on ../../../examples/gke_autopilot_cluster/main.tf line 39, in module "gke":
│ 39: project = var.project_id
│
│ An argument named "project" is not expected here.
╵
terraform_validate ./test/fixtures/gke_standard_cluster
╷
│ Error: Unsupported argument
│
│ on ../../../examples/gke_standard_cluster/main.tf line 40, in module "gke":
│ 40: project = var.project_id
│
│ An argument named "project" is not expected here.
╵
terraform_validate ./examples/gke_autopilot_cluster
╷
│ Error: Missing required argument
│
│ on main.tf line 35, in module "gke":
│ 35: module "gke" {
│
│ The argument "project_id" is required, but no definition was found.
╵
╷
│ Error: Unsupported argument
│
│ on main.tf line 39, in module "gke":
│ 39: project = var.project_id
│
│ An argument named "project" is not expected here.
╵
terraform_validate ./examples/gke_standard_cluster
╷
│ Error: Missing required argument
│
│ on main.tf line 86, in module "node_pool":
│ 86: module "node_pool" {
│
│ The argument "project_id" is required, but no definition was found.
╵
╷
│ Error: Unsupported argument
│
│ on main.tf line 90, in module "node_pool":
│ 90: project = var.project_id
│
│ An argument named "project" is not expected here.
/gcbrun |
/gcbrun |
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.
LGTM.
@Daisyprakash Please validate if the regex introduced are RE2 compliant as needed by ADC service. @pawan1210 Could you review the regex once!
name: cluster_telemetry | ||
title: Cluster Telemetry | ||
properties: | ||
type: |
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.
is type
field of the cluster_telemetry
?
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.
Yes, type is the field of cluster_telemetry
Doc for ref- https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#type-1
security_group: | ||
name: security_group | ||
title: Security Group | ||
regexValidation: ^gke-security-groups@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$ |
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.
@Daisyprakash , please test this validation once in autopush UI.
No description provided.