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

fix!: allow dns endpoints only #2217

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

TheKangaroo
Copy link
Contributor

fixes #2216

@TheKangaroo TheKangaroo requested review from ericyz and a team as code owners December 16, 2024 09:12
@apeabody
Copy link
Collaborator

/gcbrun

Copy link
Collaborator

@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 @TheKangaroo!

autogen/main/cluster.tf.tmpl Outdated Show resolved Hide resolved
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody apeabody self-assigned this Dec 17, 2024
@apeabody apeabody changed the title fix: allow dns endpoints only fix!: allow dns endpoints only Dec 18, 2024
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

Hi @TheKangaroo - We could likely resolve this error with for_each = {% if private_cluster %}var.enable_private_endpoint || {% endif %} var.master_authorized_networks != null ? [true] : [], however my impression from #2216 is your use case is specifically private_endpoint = true with no master_authorized_networks_config block? If so, it appears that combination might not be possible?

TestSimpleAutopilotPrivate 2024-12-18T00:49:44Z command.go:185: module.example.module.gke.google_container_cluster.primary: Creating...
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: 
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: Error: googleapi: Error 400: 
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: 	- invalid value for "cluster.master_authorized_networks_config": "cluster.master_authorized_networks_config.enable_master_authorized_networks" should be enabled if private endpoint is enabled
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: 	- master authorized networks must be enabled if private endpoint is enabled.
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: Details:
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: [
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185:   {
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185:     "@type": "type.googleapis.com/google.rpc.RequestInfo",
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185:     "requestId": "0x31e37f9a8468168d"
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185:   }
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: ]
TestSimpleAutopilotPrivate 2024-12-18T00:49:47Z command.go:185: , badRequest

@TheKangaroo
Copy link
Contributor Author

@apeabody You are right that that's exactly what I am trying to achieve: a private endpoint cluster (with a DNS endpoint) and no master_authorized_networks_config. I can plan/apply my cluster with this feature branch, so it seems to be fine with the beta-private-cluster-update-variant with:

enable_private_endpoint       = true
deploy_using_private_endpoint = true

to enable the DNS endpoint. To be honest, I don't fully understand which combinations of variables are possible and supported by the modules and the API. The only things I can imagine are:

  • This works differently on autopilot, and I need to exclude autopilot in the config.
  • Omitting master_authorized_networks_config is only allowed if the DNS endpoint is enabled.

@apeabody
Copy link
Collaborator

apeabody commented Dec 19, 2024

I can plan/apply my cluster with this feature branch, so it seems to be fine with the beta-private-cluster-update-variant with:

enable_private_endpoint       = true
deploy_using_private_endpoint = true

to enable the DNS endpoint.

Thanks @TheKangaroo - I suspect this resulted in the IP Endpoints enabled? Unfortunately, I suspect we need hashicorp/terraform-provider-google#20369 to "cleanly" implement your goal. This is because currently setting enable_private_endpoint = true results in IPEndpointsConfig. EnablePublicEndpoint = false (but with hardcoded IPEndpointsConfig.Enabled = true).

More research here: #2216 (comment)

@TheKangaroo
Copy link
Contributor Author

Yes, this resulted in IP endpoints being enabled, but with my PR, without master_authorized_networks_config configured, I can disable IP endpoints in the UI, and Terraform won't complain.

I read through your explanation in #2216, and I'll continue working on this PR when the provider supports disabling IPEndpointsConfig.

@apeabody apeabody added the blocked Blocked by some other work label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked by some other work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use the new DNS endpoint exclusively.
2 participants