diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index e5bf553b..b94cea4b 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,13 +1,13 @@ ack_generate_info: - build_date: "2026-02-19T00:10:17Z" - build_hash: af61bc1478baf08196fb6e2c81590518a754e45e + build_date: "2026-02-24T04:42:15Z" + build_hash: b0103744b8b2c48e9f8938ff6c2bf3b95ad95406 go_version: go1.25.6 - version: v0.57.0-9-gaf61bc1 + version: v0.57.0-14-gb010374 api_directory_checksum: df9ec56e987fdc93ed2e7f55c55cf0bc3372b109 api_version: v1alpha1 aws_sdk_go_version: v1.40.1 generator_config_info: - file_checksum: a0b97e79bcd3136d63cafd2a8b98e4b6b5a52c15 + file_checksum: 01b155972bea16ddf80b42849a78ca34f00f52eb original_file_name: generator.yaml last_modification: reason: API generation diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index 342cdad6..1f6c33b0 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -179,6 +179,9 @@ resources: BootstrapSelfManagedAddons: compare: is_ignored: true + Logging: + compare: + is_ignored: true exceptions: errors: 404: @@ -197,7 +200,7 @@ resources: - ValidationError hooks: delta_pre_compare: - code: customPreCompare(a, b) + code: customPreCompare(delta, a, b) sdk_create_post_set_output: template_path: hooks/cluster/sdk_create_post_set_output.go.tpl sdk_read_one_post_set_output: diff --git a/generator.yaml b/generator.yaml index 342cdad6..1f6c33b0 100644 --- a/generator.yaml +++ b/generator.yaml @@ -179,6 +179,9 @@ resources: BootstrapSelfManagedAddons: compare: is_ignored: true + Logging: + compare: + is_ignored: true exceptions: errors: 404: @@ -197,7 +200,7 @@ resources: - ValidationError hooks: delta_pre_compare: - code: customPreCompare(a, b) + code: customPreCompare(delta, a, b) sdk_create_post_set_output: template_path: hooks/cluster/sdk_create_post_set_output.go.tpl sdk_read_one_post_set_output: diff --git a/pkg/resource/cluster/delta.go b/pkg/resource/cluster/delta.go index ce160f9a..28e7cd6e 100644 --- a/pkg/resource/cluster/delta.go +++ b/pkg/resource/cluster/delta.go @@ -41,7 +41,7 @@ func newResourceDelta( delta.Add("", a, b) return delta } - customPreCompare(a, b) + customPreCompare(delta, a, b) if ackcompare.HasNilDifference(a.ko.Spec.AccessConfig, b.ko.Spec.AccessConfig) { delta.Add("Spec.AccessConfig", a.ko.Spec.AccessConfig, b.ko.Spec.AccessConfig) @@ -122,17 +122,6 @@ func newResourceDelta( } } } - if ackcompare.HasNilDifference(a.ko.Spec.Logging, b.ko.Spec.Logging) { - delta.Add("Spec.Logging", a.ko.Spec.Logging, b.ko.Spec.Logging) - } else if a.ko.Spec.Logging != nil && b.ko.Spec.Logging != nil { - if len(a.ko.Spec.Logging.ClusterLogging) != len(b.ko.Spec.Logging.ClusterLogging) { - delta.Add("Spec.Logging.ClusterLogging", a.ko.Spec.Logging.ClusterLogging, b.ko.Spec.Logging.ClusterLogging) - } else if len(a.ko.Spec.Logging.ClusterLogging) > 0 { - if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.Logging.ClusterLogging, b.ko.Spec.Logging.ClusterLogging) { - delta.Add("Spec.Logging.ClusterLogging", a.ko.Spec.Logging.ClusterLogging, b.ko.Spec.Logging.ClusterLogging) - } - } - } if ackcompare.HasNilDifference(a.ko.Spec.Name, b.ko.Spec.Name) { delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name) } else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil { diff --git a/pkg/resource/cluster/hook.go b/pkg/resource/cluster/hook.go index 100f1f69..37c17da6 100644 --- a/pkg/resource/cluster/hook.go +++ b/pkg/resource/cluster/hook.go @@ -37,10 +37,6 @@ import ( "github.com/aws-controllers-k8s/eks-controller/pkg/util" ) -const ( - LoggingNoChangesError = "No changes needed for the logging config provided" -) - // Taken from the list of cluster statuses on the boto3 documentation // https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/eks.html#EKS.Client.describe_cluster const ( @@ -185,12 +181,38 @@ func (rm *resourceManager) clusterInUse(ctx context.Context, r *resource) (bool, } func customPreCompare( + delta *ackcompare.Delta, a *resource, b *resource, ) { if a.ko.Spec.UpgradePolicy == nil && b.ko.Spec.UpgradePolicy != nil { a.ko.Spec.UpgradePolicy = b.ko.Spec.UpgradePolicy.DeepCopy() } + // Custom logging comparison. The EKS API always returns both enabled and + // disabled log type entries, but users may only specify enabled ones. A + // missing enabled list means everything is disabled. We compare the + // effective set of enabled log types to avoid false positives. + desiredEnabled := enabledLogTypes(a) + latestEnabled := enabledLogTypes(b) + if !ackcompare.SliceStringPEqual(desiredEnabled, latestEnabled) { + delta.Add("Spec.Logging", a.ko.Spec.Logging, b.ko.Spec.Logging) + } +} + +// enabledLogTypes returns the list of log type strings that are enabled in the +// resource's logging config. A nil logging config or missing entries means +// nothing is enabled. +func enabledLogTypes(r *resource) []*string { + var enabled []*string + if r.ko.Spec.Logging == nil { + return enabled + } + for _, ls := range r.ko.Spec.Logging.ClusterLogging { + if ls.Enabled != nil && *ls.Enabled { + enabled = append(enabled, ls.Types...) + } + } + return enabled } func (rm *resourceManager) customUpdate( @@ -249,15 +271,12 @@ func (rm *resourceManager) customUpdate( if err := rm.updateConfigLogging(ctx, desired); err != nil { awsErr, ok := extractAWSError(err) - // The API responds with an error if there were no changes applied - if !ok || awsErr.Message != LoggingNoChangesError { - return nil, err - } - // Check to see if we've raced an async update call and need to requeue if ok && awsErr.Code == "ResourceInUseException" { return nil, requeueAfterAsyncUpdate() } + + return nil, err } return returnClusterUpdating(updatedRes) } @@ -472,6 +491,15 @@ func (rm *resourceManager) updateVersion( return nil } +// allEKSLogTypes is the complete set of EKS control plane log types. +var allEKSLogTypes = []svcsdktypes.LogType{ + svcsdktypes.LogTypeApi, + svcsdktypes.LogTypeAudit, + svcsdktypes.LogTypeAuthenticator, + svcsdktypes.LogTypeControllerManager, + svcsdktypes.LogTypeScheduler, +} + func (rm *resourceManager) updateConfigLogging( ctx context.Context, r *resource, @@ -481,7 +509,7 @@ func (rm *resourceManager) updateConfigLogging( defer exit(err) input := &svcsdk.UpdateClusterConfigInput{ Name: r.ko.Spec.Name, - Logging: rm.newLogging(r), + Logging: newFullLogging(r), } _, err = rm.sdkapi.UpdateClusterConfig(ctx, input) @@ -493,6 +521,61 @@ func (rm *resourceManager) updateConfigLogging( return nil } +// newFullLogging builds a complete EKS logging config with all log types +// explicitly set to enabled or disabled. The EKS API expects the full config; +// sending only enabled types causes "No changes needed" errors when the +// effective state hasn't changed. +// +// All user-specified enabled types are passed through to the API (including +// unknown ones, so that EKS can validate and reject them). The disabled set +// is built from the known EKS log types minus whatever the user enabled. +func newFullLogging(r *resource) *svcsdktypes.Logging { + // Collect the set of log types the user wants enabled. + enabled := make(map[svcsdktypes.LogType]bool) + var enabledTypes []svcsdktypes.LogType + if r.ko.Spec.Logging != nil { + for _, ls := range r.ko.Spec.Logging.ClusterLogging { + if ls.Enabled != nil && *ls.Enabled { + for _, t := range ls.Types { + if t != nil { + lt := svcsdktypes.LogType(*t) + if !enabled[lt] { + enabled[lt] = true + enabledTypes = append(enabledTypes, lt) + } + } + } + } + } + } + + // Disabled types: known EKS log types that the user did not enable. + var disabledTypes []svcsdktypes.LogType + for _, lt := range allEKSLogTypes { + if !enabled[lt] { + disabledTypes = append(disabledTypes, lt) + } + } + + var clusterLogging []svcsdktypes.LogSetup + if len(enabledTypes) > 0 { + clusterLogging = append(clusterLogging, svcsdktypes.LogSetup{ + Enabled: aws.Bool(true), + Types: enabledTypes, + }) + } + if len(disabledTypes) > 0 { + clusterLogging = append(clusterLogging, svcsdktypes.LogSetup{ + Enabled: aws.Bool(false), + Types: disabledTypes, + }) + } + + return &svcsdktypes.Logging{ + ClusterLogging: clusterLogging, + } +} + func newAccessConfig(r *resource) *svcsdktypes.UpdateAccessConfigRequest { cfg := &svcsdktypes.UpdateAccessConfigRequest{} if r.ko.Spec.AccessConfig != nil { diff --git a/pkg/resource/cluster/sdk.go b/pkg/resource/cluster/sdk.go index e39dde3b..2ae88c52 100644 --- a/pkg/resource/cluster/sdk.go +++ b/pkg/resource/cluster/sdk.go @@ -1183,37 +1183,6 @@ func (rm *resourceManager) terminalAWSError(err error) bool { } } -// newLogging returns a Logging object -// with each the field set by the resource's corresponding spec field. -func (rm *resourceManager) newLogging( - r *resource, -) *svcsdktypes.Logging { - res := &svcsdktypes.Logging{} - - if r.ko.Spec.Logging.ClusterLogging != nil { - resf0 := []svcsdktypes.LogSetup{} - for _, resf0iter := range r.ko.Spec.Logging.ClusterLogging { - resf0elem := &svcsdktypes.LogSetup{} - if resf0iter.Enabled != nil { - resf0elem.Enabled = resf0iter.Enabled - } - if resf0iter.Types != nil { - resf0elemf1 := []svcsdktypes.LogType{} - for _, resf0elemf1iter := range resf0iter.Types { - var resf0elemf1elem string - resf0elemf1elem = string(*resf0elemf1iter) - resf0elemf1 = append(resf0elemf1, svcsdktypes.LogType(resf0elemf1elem)) - } - resf0elem.Types = resf0elemf1 - } - resf0 = append(resf0, *resf0elem) - } - res.ClusterLogging = resf0 - } - - return res -} - // newVpcConfigRequest returns a VpcConfigRequest object // with each the field set by the resource's corresponding spec field. func (rm *resourceManager) newVpcConfigRequest( diff --git a/templates/hooks/cluster/sdk_file_end.go.tpl b/templates/hooks/cluster/sdk_file_end.go.tpl index 486d9070..94013abd 100644 --- a/templates/hooks/cluster/sdk_file_end.go.tpl +++ b/templates/hooks/cluster/sdk_file_end.go.tpl @@ -3,7 +3,7 @@ {{/* Find the structure field within the operation */}} {{- range $fieldName, $field := $CRD.SpecFields -}} -{{- if (or (eq $field.Path "Logging") (eq $field.Path "ResourcesVPCConfig")) }} +{{- if (eq $field.Path "ResourcesVPCConfig") }} {{- $shapeName := $field.ShapeRef.ShapeName }} diff --git a/test/e2e/tests/test_cluster.py b/test/e2e/tests/test_cluster.py index cb713027..83380d52 100644 --- a/test/e2e/tests/test_cluster.py +++ b/test/e2e/tests/test_cluster.py @@ -233,7 +233,10 @@ def test_create_update_delete_cluster(self, eks_client, simple_cluster): aws_res = eks_client.describe_cluster(name=cluster_name) assert sorted(aws_res["cluster"]["resourcesVpcConfig"]["subnetIds"]) == sorted(subnets_ids) - # Update the logging fields + # Update the logging fields specifying only enabled types. The + # controller must handle partial specs correctly — the EKS API + # returns both enabled and disabled entries, but users should not + # be required to list disabled types explicitly. updates = { "spec": { "logging": { @@ -242,10 +245,6 @@ def test_create_update_delete_cluster(self, eks_client, simple_cluster): "enabled": True, "types": ["api"] }, - { - "enabled": False, - "types": ["audit", "authenticator", "controllerManager", "scheduler"] - }, ] }, } @@ -257,10 +256,11 @@ def test_create_update_delete_cluster(self, eks_client, simple_cluster): wait_for_cluster_active(eks_client, cluster_name) aws_res = eks_client.describe_cluster(name=cluster_name) - assert len(aws_res["cluster"]["logging"]["clusterLogging"]) > 0 - logging = aws_res["cluster"]["logging"]["clusterLogging"][0] - assert logging["enabled"] == True - assert logging["types"] == ["api"] + enabled_types = [] + for log_setup in aws_res["cluster"]["logging"]["clusterLogging"]: + if log_setup.get("enabled"): + enabled_types.extend(log_setup["types"]) + assert enabled_types == ["api"] # Update the AccessConfig field updates = {