hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.36k stars 1.75k forks source link

`google_container_cluster` Disable the kubelet read-only port #15208

Closed oscar-b closed 3 months ago

oscar-b commented 1 year ago

Community Note

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)

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

b/292264600

ramondeklein commented 1 year ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

enricojonas commented 4 months ago

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

felipesbs commented 4 months ago

We agree, using Terraform for this would be ideal.

kustodian commented 4 months ago

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 https://github.com/GoogleCloudPlatform/magic-modules/pull/11272 :)

otherguy commented 3 months ago

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

hoskeri commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

Thanks, a follow up is fine. Thanks!

github-actions[bot] commented 2 months ago

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.