Add support for legacyAbac to google_container_cluster
#261
Conversation
google/resource_container_cluster.go
Outdated
| Schema: map[string]*schema.Schema{ | ||
| "enabled": { | ||
| Type: schema.TypeBool, | ||
| Required: true, |
There was a problem hiding this comment.
I suspect we want a default value here
google/resource_container_cluster.go
Outdated
| @@ -409,6 +424,12 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er | |||
| cluster.Description = v.(string) | |||
| } | |||
|
|
|||
| if _, ok := d.GetOk("legacy_abac"); ok { | |||
There was a problem hiding this comment.
if the user specifies false, this block won't get triggered, right? So we'll never send false, if I recall correctly
There was a problem hiding this comment.
In addition, LegacyAbac.Enabled is a boolean that's marshaled with omitempty, so even if we do go in here, we won't actually send false unless this field is added to the LegacyAbac.ForceSendFields slice.
There was a problem hiding this comment.
Good point. Fixed.
google/resource_container_cluster.go
Outdated
| @@ -149,6 +149,21 @@ func resourceContainerCluster() *schema.Resource { | |||
| Elem: &schema.Schema{Type: schema.TypeString}, | |||
| }, | |||
|
|
|||
| "legacy_abac": { | |||
There was a problem hiding this comment.
Are there any other parameters here? Should we simplify this for the users into a simple toggle?
enable_legacy_abac ?
There was a problem hiding this comment.
Yeah, I was leaving the option open in case any others get added, but realistically I don't think they will so I went ahead and made the change.
| @@ -291,6 +317,7 @@ func testAccCheckContainerCluster(n string) resource.TestCheckFunc { | |||
| {"description", cluster.Description}, | |||
| {"endpoint", cluster.Endpoint}, | |||
| {"instance_group_urls", igUrls}, | |||
| {"legacy_abac.0.enabled", strconv.FormatBool(cluster.LegacyAbac.Enabled)}, | |||
There was a problem hiding this comment.
I don't believe this is accurately testing this as the code stands. We can't send false, so we never actually change the value. The test doesn't seem to verify that the value is now false.
There was a problem hiding this comment.
Yup, you're right. Fixed.
* revendor container api * Add support for legacyAbac to `google_container_cluster` * change to single enabled field
* revendor container api * Add support for legacyAbac to `google_container_cluster` * change to single enabled field
<!-- This change is generated by MagicModules. --> /cc @Chupaka
|
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
terraform-google-modules/terraform-google-kubernetes-engine#261 Bumbed minimal provider version to 2.18
Fixes #219.