google_container_cluster Disable the kubelet read-only port

[oscar-b]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment. If the issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If the issue is assigned to a user, that user is claiming responsibility for the issue. If the issue is assigned to "hashibot", a community member has claimed the issue already.

Description

Port 10255 should be disabled, which can be done using --no-enable-insecure-kubelet-readonly-port

https://cloud.google.com/kubernetes-engine/docs/how-to/disable-kubelet-readonly-port

New or Affected Resource(s)

• google_container_cluster

Potential Terraform Configuration

# Propose what you think the configuration to take advantage of this feature should look like.
# We may not use it verbatim, but it's helpful in understanding your intent.

References

• #0000

b/292264600

[ramondeklein]

Adding no_enable_insecure_kubelet_readonly_port to google_container_cluster is just the basic functionality. Google also added the insecureKubeletReadonlyPortEnabled flag in the kubelet_config block for both clusters and node-pools (source). I guess this needs to be added as well.

[gabegorelick]

Google Cloud is sunsetting the insecure kubelet port. They've begun emailing customers about migrating off of it. So I expect a lot of renewed interest in the ability to disable it via Terraform.

[wyardley]

With the announcement going out to customers (https://cloud.google.com/kubernetes-engine/docs/how-to/disable-kubelet-readonly-port#migrate-apps) this probably becomes more urgent / critical.

[wyardley]

I started a draft; the node_pool updates look slightly easier than the container_cluster ones (adding NodeKubeletConfig defaults) are; if anyone's got some thoughts / ideas, feel free to comment. I will hopefully have some more time to dig into it a bit more in a week or two.

[wyardley]

ps - I'm guessing implementing this as a non default or optional option will be easier to get released vs. a technically breaking one, even if the breaking way is better from a security standpoint?

[shinji62]

Impacted as well, received the notification ... But can't disable using TF.

[enricojonas]

Same here - doing this through TF would be the preferred way for us.

[felipesbs]

We agree, using Terraform for this would be ideal.

[kustodian]

I also received an email. This is the first thing I found when I didn't see it implemented in TF. Hopefully, it will be ready soon. There is a lot of progress in GoogleCloudPlatform/magic-modules#11272 :)

[otherguy]

I've done this manually now but would like the change reflected in Terraform.

[hoskeri]

Thanks for the PR! Just one comment about an additional field. everything else LGTM.

node_kubelet_config field is also available in node_pool_auto_config.

Could you update that in a similar manner as node_config_defaults as well?

https://github.com/wyardley/magic-modules/blob/46909c20c7a2dc63b5c6ea80be1b276d36f34dd2/mmv1/third_party/terraform/services/container/go/resource_container_cluster.go.tmpl#L1509

What's the behavior when the value goes from unset -> set outside of terraform?

[wyardley]

@hoskeri good call, I can take a look at updating that as well. However, there are still some kinks in the PR in progress.

What's the behavior when the value goes from unset -> set outside of terraform?

I think as they've asked me to implement it (see the bit about using an enum instead of a bool), if it goes from unset to set (to true), if the user doesn't have it set in the config, they will probably see drift (i.e., if the setting goes from unset (false) to true at the API level, the user will see permadrift unless they add "TRUE" .

However, if the API default changes, I think it will be a noop if the user doesn't have it set.

[wyardley]

node_kubelet_config field is also available in node_pool_auto_config.

I see it in the library / API docs: https://pkg.go.dev/google.golang.org/api/container/v1#NodeKubeletConfig

However, from what I can see, node_kubelet_config isn't implemented at all (with or without this setting) in the provider as of now.

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#nested_node_pool_auto_config

My initial thought is that it probably is better to implement this without that at first, and for someone to add node_kublet_config support separately, just in the sense that it's already a very large set of changes with a lot of moving parts.

[hoskeri]

Thanks, a follow up is fine. Thanks!

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.