Disable Kubernetes Dashboard by default #3011
Conversation
|
cc @ndmckinley |
google/resource_container_cluster.go
Outdated
| @@ -1597,6 +1597,12 @@ func flattenClusterAddonsConfig(c *containerBeta.AddonsConfig) []map[string]inte | |||
| "disabled": c.KubernetesDashboard.Disabled, | |||
| }, | |||
| } | |||
| } else { | |||
There was a problem hiding this comment.
Unfortunately this is backwards - flatteners go from api objects to terraform objects, but expanders go from terraform objects to api objects. You'll need to make a similar change in the expander. Any reason you didn't choose to set the default in the schema object instead?
|
That would be interesting! I'll run that through the generation system and run the tests on it. |
|
Ah, right - that doesn't compile, k8s doesn't have the |
Kubernetes Dashboard has been deprecated and should not be enabled by default.
|
@ndmckinley Sorry, I didn't intend for this change to be reviewed yet. I did notice that k8s didn't have An alternate is to set |
|
Hi, That might work! I think that setting it to false by default has a chance of working depending on how the underlying api calls wind up working. Are you comfortable implementing and testing that? I'm confident k8s won't accept that PR - their client code is autogenerated. |
s/false/true/ but I think you know what I meant. :)
I will give it a shot. |
vbhavsar
force-pushed
the
vishal/disable-k8s-dashboard-by-default
branch
from
212f310 to
25b7729
Compare
|
Hi @vbhavsar, sorry for the delay! I'll be taking over this review while Nathan is out. Have you tested this change? Just looking at the code, I think it'll only work successfully if the user specifies an empty |
There was a problem hiding this comment.
The code side of this isn't actually going to have any immediate effect due to an existing bug where putting an empty block in here crashes Terraform, but it doesn't hurt, and the docs update is good and should be included (since not providing the block at all does disable it by default on the api side).
|
Hi @danawillow. I should have posted a comment here when I tried and failed to implement your suggestion. I'm happy to know that the documentation is useful at least. :)
Let me know if there is anything I need to do with this pull request. |
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Kubernetes Dashboard has been deprecated and should not be enabled by default.
Fixes #2547.