From 4d6aee629513e543028dfc08b58fb4c75d65ccb1 Mon Sep 17 00:00:00 2001 From: Florian Stadler Date: Thu, 30 Jan 2025 12:30:55 +0100 Subject: [PATCH 1/2] Fix disabling auto mode --- internal/service/eks/cluster.go | 101 ++++++++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 5 deletions(-) diff --git a/internal/service/eks/cluster.go b/internal/service/eks/cluster.go index c0f792399f6a..fe486547cde3 100644 --- a/internal/service/eks/cluster.go +++ b/internal/service/eks/cluster.go @@ -635,7 +635,7 @@ func resourceClusterRead(ctx context.Context, d *schema.ResourceData, meta inter if cluster.OutpostConfig != nil { d.Set("cluster_id", cluster.Id) } - if err := d.Set("compute_config", flattenComputeConfigResponse(cluster.ComputeConfig)); err != nil { + if err := d.Set("compute_config", cleanComputeConfig(cluster, d)); err != nil { return sdkdiag.AppendErrorf(diags, "setting compute_config: %s", err) } d.Set(names.AttrCreatedAt, cluster.CreatedAt.Format(time.RFC3339)) @@ -662,7 +662,7 @@ func resourceClusterRead(ctx context.Context, d *schema.ResourceData, meta inter } d.Set(names.AttrRoleARN, cluster.RoleArn) d.Set(names.AttrStatus, cluster.Status) - if err := d.Set("storage_config", flattenStorageConfigResponse(cluster.StorageConfig)); err != nil { + if err := d.Set("storage_config", cleanStorageConfig(cluster, d)); err != nil { return sdkdiag.AppendErrorf(diags, "setting storage_config: %s", err) } if err := d.Set("upgrade_policy", flattenUpgradePolicy(cluster.UpgradePolicy)); err != nil { @@ -730,9 +730,41 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int } if d.HasChanges("compute_config", "kubernetes_network_config", "storage_config") { - computeConfig := expandComputeConfigRequest(d.Get("compute_config").([]interface{})) - kubernetesNetworkConfig := expandKubernetesNetworkConfigRequest(d.Get("kubernetes_network_config").([]interface{})) - storageConfig := expandStorageConfigRequest(d.Get("storage_config").([]interface{})) + oldComputeConfig, computeConfig := getComputeConfigChange(d) + oldKubernetesNetworkConfig, kubernetesNetworkConfig := getKubernetesNetworkConfigChange(d) + oldStorageConfig, storageConfig := getStorageConfigChange(d) + + oldAutoModeEnabled := autoModeEnabled(oldComputeConfig, oldKubernetesNetworkConfig, oldStorageConfig) + newAutoModeEnabled := autoModeEnabled(computeConfig, kubernetesNetworkConfig, storageConfig) + + // If we are disabling auto mode, we might need to populate auto mode config options that are nil. + // Setting some explicitly to false and others to nil will cause InputValidationErrors + if oldAutoModeEnabled && !newAutoModeEnabled { + if computeConfig == nil { + computeConfig = &types.ComputeConfigRequest{ + Enabled: aws.Bool(false), + } + } + + if kubernetesNetworkConfig == nil { + kubernetesNetworkConfig = &types.KubernetesNetworkConfigRequest{} + } + if kubernetesNetworkConfig.ElasticLoadBalancing == nil { + kubernetesNetworkConfig.ElasticLoadBalancing = &types.ElasticLoadBalancing{ + Enabled: aws.Bool(false), + } + } + + if storageConfig == nil { + storageConfig = &types.StorageConfigRequest{} + } + + if storageConfig.BlockStorage == nil { + storageConfig.BlockStorage = &types.BlockStorage{ + Enabled: aws.Bool(false), + } + } + } input := &eks.UpdateClusterConfigInput{ Name: aws.String(d.Id()), @@ -1790,3 +1822,62 @@ func validateAutoModeCustomizeDiff(_ context.Context, d *schema.ResourceDiff, _ return nil } + +func getComputeConfigChange(d *schema.ResourceData) (old, new *types.ComputeConfigRequest) { + oldRaw, newRaw := d.GetChange("compute_config") + old = expandComputeConfigRequest(oldRaw.([]interface{})) + new = expandComputeConfigRequest(newRaw.([]interface{})) + return old, new +} + +func getKubernetesNetworkConfigChange(d *schema.ResourceData) (old, new *types.KubernetesNetworkConfigRequest) { + oldRaw, newRaw := d.GetChange("kubernetes_network_config") + old = expandKubernetesNetworkConfigRequest(oldRaw.([]interface{})) + new = expandKubernetesNetworkConfigRequest(newRaw.([]interface{})) + return old, new +} + +func getStorageConfigChange(d *schema.ResourceData) (old, new *types.StorageConfigRequest) { + oldRaw, newRaw := d.GetChange("storage_config") + old = expandStorageConfigRequest(oldRaw.([]interface{})) + new = expandStorageConfigRequest(newRaw.([]interface{})) + return old, new +} + +func autoModeEnabled(computeConfig *types.ComputeConfigRequest, networkConfig *types.KubernetesNetworkConfigRequest, storageConfig *types.StorageConfigRequest) bool { + return (computeConfig != nil && computeConfig.Enabled != nil && aws.ToBool(computeConfig.Enabled)) && + (networkConfig != nil && networkConfig.ElasticLoadBalancing != nil && networkConfig.ElasticLoadBalancing.Enabled != nil && aws.ToBool(networkConfig.ElasticLoadBalancing.Enabled)) && + (storageConfig != nil && storageConfig.BlockStorage != nil && storageConfig.BlockStorage.Enabled != nil && aws.ToBool(storageConfig.BlockStorage.Enabled)) +} + +func cleanComputeConfig(cluster *types.Cluster, d *schema.ResourceData) []map[string]interface{} { + computeConfig := flattenComputeConfigResponse(cluster.ComputeConfig) + var currentComputeConfig *types.ComputeConfigRequest + if v, ok := d.GetOk("compute_config"); ok { + currentComputeConfig = expandComputeConfigRequest(v.([]interface{})) + } + + // If the compute_config was removed after having auto mode enabled before, the API will now return enabled=false. + // Zero out the compute config in order to align the state with the config. + if currentComputeConfig == nil && cluster.ComputeConfig != nil && cluster.ComputeConfig.Enabled != nil && !*cluster.ComputeConfig.Enabled && len(cluster.ComputeConfig.NodePools) == 0 && cluster.ComputeConfig.NodeRoleArn == nil { + computeConfig = nil + } + + return computeConfig +} + +func cleanStorageConfig(cluster *types.Cluster, d *schema.ResourceData) []interface{} { + storageConfig := flattenStorageConfigResponse(cluster.StorageConfig) + var currentStorageConfig *types.StorageConfigRequest + if v, ok := d.GetOk("storage_config"); ok { + currentStorageConfig = expandStorageConfigRequest(v.([]interface{})) + } + + // If the storage_config was removed after having auto mode enabled before, the API will now return enabled=false. + // Zero out the storage config in order to align the state with the config. + if currentStorageConfig == nil && cluster.StorageConfig != nil && cluster.StorageConfig.BlockStorage != nil && !*cluster.StorageConfig.BlockStorage.Enabled { + storageConfig = nil + } + + return storageConfig +} From d762b9d8abfa9fd7cae5a6a61299da80d9006301 Mon Sep 17 00:00:00 2001 From: Florian Stadler Date: Thu, 30 Jan 2025 12:31:04 +0100 Subject: [PATCH 2/2] Add test --- internal/service/eks/cluster.go | 2 +- internal/service/eks/cluster_test.go | 70 ++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/internal/service/eks/cluster.go b/internal/service/eks/cluster.go index fe486547cde3..6cec2a53168b 100644 --- a/internal/service/eks/cluster.go +++ b/internal/service/eks/cluster.go @@ -738,7 +738,7 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, meta int newAutoModeEnabled := autoModeEnabled(computeConfig, kubernetesNetworkConfig, storageConfig) // If we are disabling auto mode, we might need to populate auto mode config options that are nil. - // Setting some explicitly to false and others to nil will cause InputValidationErrors + // Setting some explicitly to false and others to nil will cause InvalidParameterException if oldAutoModeEnabled && !newAutoModeEnabled { if computeConfig == nil { computeConfig = &types.ComputeConfigRequest{ diff --git a/internal/service/eks/cluster_test.go b/internal/service/eks/cluster_test.go index 7be69b93ef3e..6e1cac9c946a 100644 --- a/internal/service/eks/cluster_test.go +++ b/internal/service/eks/cluster_test.go @@ -292,7 +292,7 @@ func TestAccEKSCluster_BootstrapSelfManagedAddons_migrate(t *testing.T) { func TestAccEKSCluster_ComputeConfig_OnCreate(t *testing.T) { ctx := acctest.Context(t) - var cluster1, cluster2, cluster3 types.Cluster + var cluster1, cluster2, cluster3, cluster4 types.Cluster rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_eks_cluster.test" @@ -338,10 +338,12 @@ func TestAccEKSCluster_ComputeConfig_OnCreate(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"bootstrap_self_managed_addons"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + // omitting compute_config and storage_config after having auto mode enabled before will not cleanly import as + // the API will now return enabled=false for both whereas a clean cluster with auto mode disabled will return null. + ImportStateVerifyIgnore: []string{"bootstrap_self_managed_addons", "compute_config", "storage_config"}, }, { Config: testAccClusterConfig_computeConfig(rName, true, "aws_iam_role.node.arn"), @@ -361,6 +363,27 @@ func TestAccEKSCluster_ComputeConfig_OnCreate(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"bootstrap_self_managed_addons"}, }, + { + Config: testAccClusterConfig_computeConfig_omitted(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckClusterExists(ctx, resourceName, &cluster4), + testAccCheckClusterNotRecreated(&cluster3, &cluster4), + resource.TestCheckResourceAttr(resourceName, "compute_config.#", "0"), + resource.TestCheckResourceAttr(resourceName, "storage_config.#", "0"), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.0.elastic_load_balancing.#", "1"), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.0.elastic_load_balancing.0.enabled", acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.0.ip_family", "ipv4"), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.0.service_ipv4_cidr", "172.20.0.0/16"), + resource.TestCheckResourceAttr(resourceName, "kubernetes_network_config.0.service_ipv6_cidr", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"bootstrap_self_managed_addons", "compute_config", "storage_config"}, + }, }, }) } @@ -1648,6 +1671,43 @@ resource "aws_eks_cluster" "test" { `, rName, enabled, role)) } +func testAccClusterConfig_computeConfig_omitted(rName string) string { + return acctest.ConfigCompose(testAccClusterConfig_computeConfigBase(rName), fmt.Sprintf(` +resource "aws_eks_cluster" "test" { + name = %[1]q + role_arn = aws_iam_role.cluster.arn + + access_config { + # Either "API" or "API_AND_CONFIG_MAP" is required with compute config + authentication_mode = "API" + bootstrap_cluster_creator_admin_permissions = true + } + + bootstrap_self_managed_addons = false + + # Cannot omit kubernetes_network_config because it is a computed/optional attribute, those default + # to the previous value if not set (see hashicorp/terraform-plugin-sdk#1101). + kubernetes_network_config { + elastic_load_balancing { + enabled = false + } + } + + vpc_config { + subnet_ids = aws_subnet.test[*].id + } + + depends_on = [ + aws_iam_role_policy_attachment.cluster_AmazonEKSClusterPolicy, + aws_iam_role_policy_attachment.cluster_AmazonEKSComputePolicy, + aws_iam_role_policy_attachment.cluster_AmazonEKSBlockStoragePolicy, + aws_iam_role_policy_attachment.cluster_AmazonEKSLoadBalancingPolicy, + aws_iam_role_policy_attachment.cluster_AmazonEKSNetworkingPolicy, + ] +} +`, rName)) +} + func testAccClusterConfig_computeConfig_onUpdateSetup(rName string) string { return acctest.ConfigCompose(testAccClusterConfig_computeConfigBase(rName), fmt.Sprintf(` resource "aws_eks_cluster" "test" {