From de44fa32649f58ee9039c174be980c45c64de2ba Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Thu, 3 Sep 2026 15:06:01 +1000 Subject: [PATCH] fix: cover the full ELBv2 attribute key set (#990) ModifyLoadBalancerAttributes and ModifyTargetGroupAttributes reject any key absent from the per-type default maps with ValidationError, and the rejection fails the whole call rather than the single key. Terraform and the AWS Load Balancer Controller send the full attribute set on every apply, so one missing key breaks the resource outright. Adds the 17 keys the ELBv2 API reference documents but the default maps lacked: ALB: health_check_logs.s3.{enabled,bucket,prefix}, ipv6.deny_all_igw_traffic NLB: secondary_ips.auto_assigned.per_subnet Target group: load_balancing.algorithm.anomaly_mitigation, stickiness.app_cookie.{cookie_name,duration_seconds}, lambda.multi_value_headers.enabled, target_group_health.* (4), deregistration_delay.connection_termination.enabled, preserve_client_ip.enabled, target_health_state.unhealthy.* (2), target_failover.{on_deregistration,on_unhealthy} health_check_logs.s3.* is what surfaced this: it is ALB-only, and an aws_lb apply against the sandbox failed on it. The target group map stays a single flat union rather than splitting by protocol, so it remains a permissive superset. Values are store-and-echo only; nothing in the data plane reads them. --- spinifex/handlers/elbv2/service_impl_test.go | 63 ++++++++++++++++--- spinifex/handlers/elbv2/types.go | 58 +++++++++++------ .../components/elbv2/attributes-editor.tsx | 15 +++++ 3 files changed, 109 insertions(+), 27 deletions(-) diff --git a/spinifex/handlers/elbv2/service_impl_test.go b/spinifex/handlers/elbv2/service_impl_test.go index 768b30de6..0c13e01d0 100644 --- a/spinifex/handlers/elbv2/service_impl_test.go +++ b/spinifex/handlers/elbv2/service_impl_test.go @@ -2661,6 +2661,45 @@ func TestUpdateStoredConfig_MissingTargetGroup(t *testing.T) { // --- Attribute tests --- +// TestDefaultTargetGroupAttributes_CoversTerraformKeys guards the full +// TargetGroupAttribute key set from the ELBv2 API reference. A missing key +// fails the whole ModifyTargetGroupAttributes call with ValidationError. +func TestDefaultTargetGroupAttributes_CoversTerraformKeys(t *testing.T) { + t.Parallel() + attrs := DefaultTargetGroupAttributes() + + expected := map[string]string{ + "deregistration_delay.timeout_seconds": "300", + "stickiness.enabled": "false", + "stickiness.type": "lb_cookie", + "stickiness.lb_cookie.duration_seconds": "86400", + "stickiness.app_cookie.cookie_name": "", + "stickiness.app_cookie.duration_seconds": "86400", + "load_balancing.cross_zone.enabled": "use_load_balancer_configuration", + "load_balancing.algorithm.type": "round_robin", + "load_balancing.algorithm.anomaly_mitigation": "off", + "slow_start.duration_seconds": "0", + "lambda.multi_value_headers.enabled": "false", + "target_group_health.dns_failover.minimum_healthy_targets.count": "1", + "target_group_health.dns_failover.minimum_healthy_targets.percentage": "off", + "target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1", + "target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off", + "proxy_protocol_v2.enabled": "false", + "deregistration_delay.connection_termination.enabled": "false", + "preserve_client_ip.enabled": "false", + "target_health_state.unhealthy.connection_termination.enabled": "true", + "target_health_state.unhealthy.draining_interval_seconds": "0", + "target_failover.on_deregistration": "no_rebalance", + "target_failover.on_unhealthy": "no_rebalance", + } + + for k, v := range expected { + got, ok := attrs[k] + assert.True(t, ok, "target group default attributes missing key %q — terraform will hit ValidationError", k) + assert.Equal(t, v, got, "target group default value mismatch for key %s", k) + } +} + func TestDescribeTargetGroupAttributes_Defaults(t *testing.T) { t.Parallel() svc := setupTestService(t) @@ -2746,6 +2785,10 @@ func TestDefaultLoadBalancerAttributes_ALBCoversTerraformKeys(t *testing.T) { "connection_logs.s3.enabled": "false", "connection_logs.s3.bucket": "", "connection_logs.s3.prefix": "", + "health_check_logs.s3.enabled": "false", + "health_check_logs.s3.bucket": "", + "health_check_logs.s3.prefix": "", + "ipv6.deny_all_igw_traffic": "false", "idle_timeout.timeout_seconds": "60", "client_keep_alive.seconds": "3600", "routing.http.desync_mitigation_mode": "defensive", @@ -2773,14 +2816,15 @@ func TestDefaultLoadBalancerAttributes_NLBCoversExpectedKeys(t *testing.T) { attrs := DefaultLoadBalancerAttributes(LoadBalancerTypeNetwork) expected := map[string]string{ - "deletion_protection.enabled": "false", - "load_balancing.cross_zone.enabled": "false", - "access_logs.s3.enabled": "false", - "access_logs.s3.bucket": "", - "access_logs.s3.prefix": "", - "dns_record.client_routing_policy": "any_availability_zone", - "ipv6.deny_all_igw_traffic": "false", - "zonal_shift.config.enabled": "false", + "deletion_protection.enabled": "false", + "load_balancing.cross_zone.enabled": "false", + "access_logs.s3.enabled": "false", + "access_logs.s3.bucket": "", + "access_logs.s3.prefix": "", + "dns_record.client_routing_policy": "any_availability_zone", + "ipv6.deny_all_igw_traffic": "false", + "zonal_shift.config.enabled": "false", + "secondary_ips.auto_assigned.per_subnet": "0", } for k, v := range expected { @@ -2812,6 +2856,9 @@ func TestModifyLoadBalancerAttributes_AcceptsConnectionLogsKey(t *testing.T) { LoadBalancerArn: arn, Attributes: []*elbv2.LoadBalancerAttribute{ {Key: aws.String("connection_logs.s3.enabled"), Value: aws.String("false")}, + {Key: aws.String("health_check_logs.s3.enabled"), Value: aws.String("false")}, + {Key: aws.String("health_check_logs.s3.bucket"), Value: aws.String("")}, + {Key: aws.String("health_check_logs.s3.prefix"), Value: aws.String("")}, {Key: aws.String("routing.http.desync_mitigation_mode"), Value: aws.String("defensive")}, {Key: aws.String("waf.fail_open.enabled"), Value: aws.String("false")}, {Key: aws.String("zonal_shift.config.enabled"), Value: aws.String("false")}, diff --git a/spinifex/handlers/elbv2/types.go b/spinifex/handlers/elbv2/types.go index ab69d740e..e9dd7dd85 100644 --- a/spinifex/handlers/elbv2/types.go +++ b/spinifex/handlers/elbv2/types.go @@ -366,6 +366,10 @@ var ( "connection_logs.s3.enabled": "false", "connection_logs.s3.bucket": "", "connection_logs.s3.prefix": "", + "health_check_logs.s3.enabled": "false", + "health_check_logs.s3.bucket": "", + "health_check_logs.s3.prefix": "", + "ipv6.deny_all_igw_traffic": "false", "idle_timeout.timeout_seconds": "60", "client_keep_alive.seconds": "3600", "routing.http.desync_mitigation_mode": "defensive", @@ -380,27 +384,43 @@ var ( } nlbAttributeDefaults = map[string]string{ - "deletion_protection.enabled": "false", - "load_balancing.cross_zone.enabled": "false", - "access_logs.s3.enabled": "false", - "access_logs.s3.bucket": "", - "access_logs.s3.prefix": "", - "dns_record.client_routing_policy": "any_availability_zone", - "ipv6.deny_all_igw_traffic": "false", - "zonal_shift.config.enabled": "false", + "deletion_protection.enabled": "false", + "load_balancing.cross_zone.enabled": "false", + "access_logs.s3.enabled": "false", + "access_logs.s3.bucket": "", + "access_logs.s3.prefix": "", + "dns_record.client_routing_policy": "any_availability_zone", + "ipv6.deny_all_igw_traffic": "false", + "zonal_shift.config.enabled": "false", + "secondary_ips.auto_assigned.per_subnet": "0", } targetGroupAttributeDefaults = map[string]string{ - "deregistration_delay.timeout_seconds": "300", - "stickiness.enabled": "false", - "stickiness.type": "lb_cookie", - "stickiness.lb_cookie.duration_seconds": "86400", - "load_balancing.cross_zone.enabled": "use_load_balancer_configuration", - "load_balancing.algorithm.type": "round_robin", - "slow_start.duration_seconds": "0", - // NLB target-group default. The AWS Load Balancer Controller always - // writes this on NLB target groups (e.g. ingress-nginx); without it as a - // known key the modify is rejected and LBC loops on the Service forever. - "proxy_protocol_v2.enabled": "false", + "deregistration_delay.timeout_seconds": "300", + "stickiness.enabled": "false", + "stickiness.type": "lb_cookie", + "stickiness.lb_cookie.duration_seconds": "86400", + "load_balancing.cross_zone.enabled": "use_load_balancer_configuration", + "load_balancing.algorithm.type": "round_robin", + "slow_start.duration_seconds": "0", + "load_balancing.algorithm.anomaly_mitigation": "off", + "stickiness.app_cookie.cookie_name": "", + "stickiness.app_cookie.duration_seconds": "86400", + "lambda.multi_value_headers.enabled": "false", + "target_group_health.dns_failover.minimum_healthy_targets.count": "1", + "target_group_health.dns_failover.minimum_healthy_targets.percentage": "off", + "target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1", + "target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off", + // NLB target-group defaults. The AWS Load Balancer Controller always + // writes these on NLB target groups (e.g. ingress-nginx); without them as + // known keys the modify is rejected and LBC loops on the Service forever. + "proxy_protocol_v2.enabled": "false", + "deregistration_delay.connection_termination.enabled": "false", + "preserve_client_ip.enabled": "false", + "target_health_state.unhealthy.connection_termination.enabled": "true", + "target_health_state.unhealthy.draining_interval_seconds": "0", + // GWLB-only; AWS requires both to hold the same value. + "target_failover.on_deregistration": "no_rebalance", + "target_failover.on_unhealthy": "no_rebalance", } ) diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/attributes-editor.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/attributes-editor.tsx index 65cc9f362..37eab6d26 100644 --- a/spinifex/services/spinifexui/frontend/src/components/elbv2/attributes-editor.tsx +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/attributes-editor.tsx @@ -345,6 +345,21 @@ export const albAttributeSpecs: AttributeSpec[] = [ label: "Connection logs: S3 prefix", type: "text", }, + { + key: "health_check_logs.s3.enabled", + label: "Health check logs: enabled", + type: "bool", + }, + { + key: "health_check_logs.s3.bucket", + label: "Health check logs: S3 bucket", + type: "text", + }, + { + key: "health_check_logs.s3.prefix", + label: "Health check logs: S3 prefix", + type: "text", + }, ] // Spec: DefaultTargetGroupAttributes. Keys must match