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: dns endpoint for external traffic #2244

Merged

Conversation

mmorejon
Copy link
Contributor

Fix #2231

This is not an enhancement; it is a bug.

@mmorejon mmorejon requested review from apeabody, ericyz and a team as code owners January 15, 2025 15:09
@mmorejon
Copy link
Contributor Author

mmorejon commented Jan 15, 2025

@apeabody , could you check this minor fix? 🙏

@apeabody apeabody changed the title Fix dns endpoint for external traffic Fix: dns endpoint for external traffic Jan 15, 2025
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody apeabody changed the title Fix: dns endpoint for external traffic Fix!: dns endpoint for external traffic Jan 15, 2025
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 @mmorejon!

I've triggered the tests to gauge the impact of always creating this block for now.

@apeabody apeabody self-assigned this Jan 15, 2025
@mmorejon mmorejon requested a review from apeabody January 20, 2025 21:20
@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 @mmorejon!

LGTM - Pending all green test result

@apeabody apeabody changed the title Fix!: dns endpoint for external traffic Fix: dns endpoint for external traffic Jan 21, 2025
@apeabody apeabody enabled auto-merge (squash) January 21, 2025 17:11
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

Hi @mmorejon - It appears the TestSaferClusterIapBastion test is consistently failing. I haven't looked into it yet, so it's certainly possible the test/example needs to be updated, rather than your per-say your change.

Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:25Z command.go:100: Running command gcloud with args [beta compute ssh safer-cluster-iap-bastion-bastion --tunnel-through-iap --verbosity=error --project ci-gke-16636728-df61 --zone us-central1-a -q --command=curl -H "Authorization: Bearer $(gcloud auth print-access-token)" -H "Content-Type: application/json" -sS https://gke-8adb62e58fde4356b90939f5e09bf0b0a0df-520208420566.us-central1.gke.goog/version -k]
Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:29Z command.go:185: control_plane_endpoints_config.dns_endpoint_config.allow_external_traffic is disabled, permission denied

@mmorejon
Copy link
Contributor Author

Hi @mmorejon - It appears the TestSaferClusterIapBastion test is consistently failing. I haven't looked into it yet, so it's certainly possible the test/example needs to be updated, rather than your per-say your change.

Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:25Z command.go:100: Running command gcloud with args [beta compute ssh safer-cluster-iap-bastion-bastion --tunnel-through-iap --verbosity=error --project ci-gke-16636728-df61 --zone us-central1-a -q --command=curl -H "Authorization: Bearer $(gcloud auth print-access-token)" -H "Content-Type: application/json" -sS https://gke-8adb62e58fde4356b90939f5e09bf0b0a0df-520208420566.us-central1.gke.goog/version -k]
Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:29Z command.go:185: control_plane_endpoints_config.dns_endpoint_config.allow_external_traffic is disabled, permission denied

Thanks @apeabody for let me know! After a quick search I found an error reference in Google docs, maybe it can help to fix the error. [link]

@apeabody
Copy link
Collaborator

Hi @mmorejon - It appears the TestSaferClusterIapBastion test is consistently failing. I haven't looked into it yet, so it's certainly possible the test/example needs to be updated, rather than your per-say your change.

Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:25Z command.go:100: Running command gcloud with args [beta compute ssh safer-cluster-iap-bastion-bastion --tunnel-through-iap --verbosity=error --project ci-gke-16636728-df61 --zone us-central1-a -q --command=curl -H "Authorization: Bearer $(gcloud auth print-access-token)" -H "Content-Type: application/json" -sS https://gke-8adb62e58fde4356b90939f5e09bf0b0a0df-520208420566.us-central1.gke.goog/version -k]
Step #67 - "verify safer-cluster-iap-bastion-local": TestSaferClusterIapBastion 2025-01-23T01:19:29Z command.go:185: control_plane_endpoints_config.dns_endpoint_config.allow_external_traffic is disabled, permission denied

Thanks @apeabody for let me know! After a quick search I found an error reference in Google docs, maybe it can help to fix the error. [link]

Sure - My initial suspicion is this needs to be changed to true: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/examples/safer_cluster_iap_bastion/cluster.tf#L28

@apeabody apeabody disabled auto-merge January 23, 2025 22:49
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

Thanks @mmorejon!

@apeabody apeabody merged commit 4726ab2 into terraform-google-modules:main Jan 24, 2025
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.

Add Input option to set control_plan_endpoints_config.dns_endpoint_config.allow_external_traffic = false
2 participants