From 7b9eb9cc4e5ee2a456f2be5e472cda0dc1bf7f05 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Wed, 3 Jun 2026 21:49:25 +1000 Subject: [PATCH] feat(elbv2): tag/SG/IP-type setters, ip targets, redirect actions, delete-cascade hardening (#282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(elbv2): make resource deletes idempotent on absent DeleteLoadBalancer, DeleteTargetGroup, DeleteListener and DeleteRule returned a NotFound error when the target was already gone. Real AWS ELBv2 deletes are idempotent (success on absent), so terraform-provider-aws fails a destroy step on retry. Return an empty success output instead, mirroring the KV store which already tolerates absent keys. Adds TestRLC1_ELBv2DeleteIdempotentOnAbsent enforcing the contract across the ELBv2 delete surface; converts the three _NotFound delete tests to assert idempotent success. Refs mulga-siv-172 (idempotency clause; cascade-composition clause stays open). * feat: ELBv2 AddTags/RemoveTags, SetSecurityGroups, SetIpAddressType - O(1) GetListenerByArn via ARN-parse short-ID lookup + ARN equality check - persist tags on listeners/rules; seed at create time - AddTags/RemoveTags across LB, target group, listener, rule resources - SetSecurityGroups (ALB only) reattaches SGs to each ENI via live port update - SetIpAddressType (ipv4-only, idempotent) - wire all four ops through gateway, NATS client, and daemon handlers - frontend: editable Tags tab on LB/TG detail, Security groups tab on LB detail * feat(elbv2): redirect listener/rule actions - add redirect action type; parse/store/read-back RedirectActionConfig - validate status code + render-safe fields (strip AWS #{...} placeholders) - accept redirect and fixed-response on listener rules (not just defaults) - HAProxy: http-request redirect scheme (HTTP->HTTPS fast path) or rebuilt location with placeholder->fetch translation; synthetic per-rule backends * refactor: dedupe ELBv2 attribute handlers and harden store lookups Collapse the four near-identical Modify/Describe attribute handlers (target group and load balancer mirror pairs) onto generic helpers in both the service impl and the gateway, and table-drive the mirror-pair tests over a TG/LB kind. - service_impl_attributes.go: generic attrResource/modifyResourceAttributes/ describeResourceAttributes plus sdkAttrsIn/sdkAttrsOut; folds in nil-input guards, explicit empty-accountID rejection, fresh-string output, and a no-op skip-persist for unchanged Modify calls. - gateway/elbv2/attributes.go: generic attributeOp + shared validateAttrArn, replacing four per-endpoint files. - types.go: default attribute sets are now package-level maps cloned on access instead of rebuilt per Describe; document the access_logs empty-string defaults. - store.go: drop the stale 'linear scan' docstring and log ARN mismatches on GetLoadBalancerByArn/GetTargetGroupByArn instead of silently returning nil. Adds cross-account isolation, wrong-account no-mutation, and empty-string-clear coverage. * feat: support ip target type in ELBv2 RegisterTargets RegisterTargets resolved every target ID through an instance ENI lookup, so ip-type target groups silently no-op'd with an empty PrivateIP and never received traffic. - types.go: add TargetTypeInstance/TargetTypeIP constants. - CreateTargetGroup: default to instance; reject target types other than instance/ip with InvalidParameterValue. - RegisterTargets: resolve PrivateIP per target type via resolveRegisteredTargetIP — ip targets use the supplied IP directly (validated with net.ParseIP), instance targets keep the ENI lookup and reject IP-shaped IDs. * fix: cascade rule deletion in ELBv2 DeleteLoadBalancer DeleteLoadBalancer deleted listeners via store.DeleteListener directly, which bypasses the rule cascade in the DeleteListener handler. Orphan rules survived LB teardown and pinned their target groups as ResourceInUse, leaving the TG permanently undeletable and breaking tofu destroy on multinode. Extract deleteListenerCascade (rules then listener record) and call it from both DeleteListener and the DeleteLoadBalancer cleanup loop. Regression: TestDeleteLoadBalancer_CascadesRuleDeletion. * fix: harden ELBv2 delete lifecycle against orphaned cascades - DeleteLoadBalancer: fail-fast on listener-list/cascade error and run the rule cascade before backing-artifact teardown, so a transient failure leaves the record intact for retry instead of orphaning rules. - DeleteTargetGroup: in-use scan skips listeners/rules whose owning load balancer no longer exists, so a partial-teardown orphan can't pin a target group as ResourceInUse permanently. - DeleteRule: propagate the listener lookup error instead of discarding it, so a failed HAProxy reload is no longer reported as success. - Add lifecycle invariants for no-orphan completeness, no store-bypass cascade, and target-group deletability after LB teardown. - Drop plan/bead references from comments. * test: add NATS e2e round-trips for new ELBv2 ops Cover the client→NATS→handler marshalling path that unit tests skip: tags lifecycle (AddTags/DescribeTags/RemoveTags), target-group and load-balancer attribute round-trips, redirect-action default listener, ip target-type registration, and SetSecurityGroups/SetIpAddressType. * fix(elbv2): correct misleading InvalidConfigurationRequest message The error code is shared by five unrelated conditions (dualstack rejection in SetIpAddressType, cross-listener rule moves, unsupported rule action types, and the two genuine NLB security-group cases), but its canonical message claimed security groups were unsupported on network load balancers. Callers hitting it from SetIpAddressType or the rule paths got a message about security groups and NLBs that did not match their request. Neutralize to a generic, condition-agnostic message. The AWS error code is unchanged. * style(ui): oxfmt security-groups-editor test --- spinifex/awserrors/awserrors.go | 2 +- spinifex/daemon/daemon.go | 4 + spinifex/daemon/daemon_handlers_elbv2.go | 16 + spinifex/gateway/elbv2.go | 12 + spinifex/gateway/elbv2/AddTags.go | 29 + .../elbv2/DescribeLoadBalancerAttributes.go | 37 - .../elbv2/DescribeTargetGroupAttributes.go | 37 - .../elbv2/ModifyLoadBalancerAttributes.go | 40 - .../elbv2/ModifyTargetGroupAttributes.go | 40 - spinifex/gateway/elbv2/RemoveTags.go | 29 + spinifex/gateway/elbv2/SetIpAddressType.go | 31 + spinifex/gateway/elbv2/SetSecurityGroups.go | 31 + spinifex/gateway/elbv2/attributes.go | 106 ++ spinifex/gateway/elbv2_test.go | 4 + spinifex/handlers/elbv2/haproxy.go | 217 ++- spinifex/handlers/elbv2/haproxy_test.go | 153 ++ .../elbv2/lifecycle_invariants_test.go | 187 +++ spinifex/handlers/elbv2/service.go | 4 + spinifex/handlers/elbv2/service_impl.go | 491 +++---- .../handlers/elbv2/service_impl_attributes.go | 265 ++++ .../elbv2/service_impl_health_test.go | 2 +- .../handlers/elbv2/service_impl_lb_network.go | 109 ++ .../elbv2/service_impl_lb_network_test.go | 136 ++ spinifex/handlers/elbv2/service_impl_rules.go | 72 +- .../elbv2/service_impl_rules_extra_test.go | 27 +- .../handlers/elbv2/service_impl_rules_test.go | 105 +- spinifex/handlers/elbv2/service_impl_tags.go | 185 +++ .../handlers/elbv2/service_impl_tags_test.go | 200 +++ spinifex/handlers/elbv2/service_impl_test.go | 1263 +++++++++-------- .../handlers/elbv2/service_impl_vpc_test.go | 2 +- spinifex/handlers/elbv2/service_nats.go | 16 + .../handlers/elbv2/service_nats_e2e_test.go | 203 +++ spinifex/handlers/elbv2/store.go | 42 +- spinifex/handlers/elbv2/store_test.go | 10 + spinifex/handlers/elbv2/types.go | 135 +- .../elbv2/security-groups-editor.test.tsx | 114 ++ .../elbv2/security-groups-editor.tsx | 127 ++ .../src/components/elbv2/tags-editor.test.tsx | 128 ++ .../src/components/elbv2/tags-editor.tsx | 170 +++ .../frontend/src/mutations/elbv2.ts | 73 + .../-components/load-balancer-detail-page.tsx | 67 +- .../describe-load-balancers/$id.test.tsx | 6 + .../-components/target-group-detail-page.tsx | 35 +- 43 files changed, 3799 insertions(+), 1163 deletions(-) create mode 100644 spinifex/gateway/elbv2/AddTags.go delete mode 100644 spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go delete mode 100644 spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go delete mode 100644 spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go delete mode 100644 spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go create mode 100644 spinifex/gateway/elbv2/RemoveTags.go create mode 100644 spinifex/gateway/elbv2/SetIpAddressType.go create mode 100644 spinifex/gateway/elbv2/SetSecurityGroups.go create mode 100644 spinifex/gateway/elbv2/attributes.go create mode 100644 spinifex/handlers/elbv2/lifecycle_invariants_test.go create mode 100644 spinifex/handlers/elbv2/service_impl_attributes.go create mode 100644 spinifex/handlers/elbv2/service_impl_lb_network.go create mode 100644 spinifex/handlers/elbv2/service_impl_lb_network_test.go create mode 100644 spinifex/handlers/elbv2/service_impl_tags.go create mode 100644 spinifex/handlers/elbv2/service_impl_tags_test.go create mode 100644 spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.test.tsx create mode 100644 spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.tsx create mode 100644 spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.test.tsx create mode 100644 spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.tsx diff --git a/spinifex/awserrors/awserrors.go b/spinifex/awserrors/awserrors.go index 68e9853d0..9d982e7f8 100644 --- a/spinifex/awserrors/awserrors.go +++ b/spinifex/awserrors/awserrors.go @@ -937,7 +937,7 @@ var ErrorLookup = map[string]ErrorMessage{ ErrorELBv2InvalidScheme: {HTTPCode: 400, Message: "The specified scheme is not valid. Specify 'internet-facing' or 'internal'."}, ErrorELBv2SubnetNotFound: {HTTPCode: 400, Message: "The specified subnet does not exist."}, ErrorELBv2AvailabilityZoneNotSupported: {HTTPCode: 400, Message: "The specified Availability Zone is not supported."}, - ErrorELBv2InvalidConfigurationRequest: {HTTPCode: 400, Message: "Security groups are not supported for load balancers with type 'network'."}, + ErrorELBv2InvalidConfigurationRequest: {HTTPCode: 400, Message: "The requested configuration change is not valid."}, ErrorELBv2RuleNotFound: {HTTPCode: 400, Message: "One or more rules not found."}, ErrorELBv2PriorityInUse: {HTTPCode: 400, Message: "The specified priority is in use."}, ErrorELBv2TooManyRules: {HTTPCode: 400, Message: "You've reached the limit on the number of rules per load balancer."}, diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 5ffe47b71..6fd535d65 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -840,12 +840,16 @@ func (d *Daemon) subscribeAll() error { natsSub{"elbv2.DescribeRules", d.handleELBv2DescribeRules, "spinifex-workers"}, natsSub{"elbv2.SetRulePriorities", d.handleELBv2SetRulePriorities, "spinifex-workers"}, natsSub{"elbv2.DescribeTags", d.handleELBv2DescribeTags, "spinifex-workers"}, + natsSub{"elbv2.AddTags", d.handleELBv2AddTags, "spinifex-workers"}, + natsSub{"elbv2.RemoveTags", d.handleELBv2RemoveTags, "spinifex-workers"}, natsSub{"elbv2.LBAgentHeartbeat", d.handleELBv2LBAgentHeartbeat, "spinifex-workers"}, natsSub{"elbv2.GetLBConfig", d.handleELBv2GetLBConfig, "spinifex-workers"}, natsSub{"elbv2.ModifyTargetGroupAttributes", d.handleELBv2ModifyTargetGroupAttributes, "spinifex-workers"}, natsSub{"elbv2.DescribeTargetGroupAttributes", d.handleELBv2DescribeTargetGroupAttributes, "spinifex-workers"}, natsSub{"elbv2.ModifyLoadBalancerAttributes", d.handleELBv2ModifyLoadBalancerAttributes, "spinifex-workers"}, natsSub{"elbv2.DescribeLoadBalancerAttributes", d.handleELBv2DescribeLoadBalancerAttributes, "spinifex-workers"}, + natsSub{"elbv2.SetSecurityGroups", d.handleELBv2SetSecurityGroups, "spinifex-workers"}, + natsSub{"elbv2.SetIpAddressType", d.handleELBv2SetIpAddressType, "spinifex-workers"}, ) } diff --git a/spinifex/daemon/daemon_handlers_elbv2.go b/spinifex/daemon/daemon_handlers_elbv2.go index df70be1df..17deb4a8c 100644 --- a/spinifex/daemon/daemon_handlers_elbv2.go +++ b/spinifex/daemon/daemon_handlers_elbv2.go @@ -82,6 +82,14 @@ func (d *Daemon) handleELBv2DescribeTags(msg *nats.Msg) { handleNATSRequest(msg, d.elbv2Service.DescribeTags) } +func (d *Daemon) handleELBv2AddTags(msg *nats.Msg) { + handleNATSRequest(msg, d.elbv2Service.AddTags) +} + +func (d *Daemon) handleELBv2RemoveTags(msg *nats.Msg) { + handleNATSRequest(msg, d.elbv2Service.RemoveTags) +} + func (d *Daemon) handleELBv2LBAgentHeartbeat(msg *nats.Msg) { handleNATSRequest(msg, d.elbv2Service.LBAgentHeartbeat) } @@ -105,3 +113,11 @@ func (d *Daemon) handleELBv2ModifyLoadBalancerAttributes(msg *nats.Msg) { func (d *Daemon) handleELBv2DescribeLoadBalancerAttributes(msg *nats.Msg) { handleNATSRequest(msg, d.elbv2Service.DescribeLoadBalancerAttributes) } + +func (d *Daemon) handleELBv2SetSecurityGroups(msg *nats.Msg) { + handleNATSRequest(msg, d.elbv2Service.SetSecurityGroups) +} + +func (d *Daemon) handleELBv2SetIpAddressType(msg *nats.Msg) { + handleNATSRequest(msg, d.elbv2Service.SetIpAddressType) +} diff --git a/spinifex/gateway/elbv2.go b/spinifex/gateway/elbv2.go index 4bdf0a3e7..e4d3fc4c3 100644 --- a/spinifex/gateway/elbv2.go +++ b/spinifex/gateway/elbv2.go @@ -99,6 +99,12 @@ var elbv2Actions = map[string]ELBv2Handler{ "DescribeTags": elbv2Handler(func(input *elbv2.DescribeTagsInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.DescribeTags(input, gw.NATSConn, accountID) }), + "AddTags": elbv2Handler(func(input *elbv2.AddTagsInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_elbv2.AddTags(input, gw.NATSConn, accountID) + }), + "RemoveTags": elbv2Handler(func(input *elbv2.RemoveTagsInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_elbv2.RemoveTags(input, gw.NATSConn, accountID) + }), "LBAgentHeartbeat": elbv2Handler(func(input *handlers_elbv2.LBAgentHeartbeatInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.LBAgentHeartbeat(input, gw.NATSConn, accountID) }), @@ -120,6 +126,12 @@ var elbv2Actions = map[string]ELBv2Handler{ "DescribeLoadBalancerAttributes": elbv2Handler(func(input *elbv2.DescribeLoadBalancerAttributesInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.DescribeLoadBalancerAttributes(input, gw.NATSConn, accountID) }), + "SetSecurityGroups": elbv2Handler(func(input *elbv2.SetSecurityGroupsInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_elbv2.SetSecurityGroups(input, gw.NATSConn, accountID) + }), + "SetIpAddressType": elbv2Handler(func(input *elbv2.SetIpAddressTypeInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_elbv2.SetIpAddressType(input, gw.NATSConn, accountID) + }), "DescribeListenerAttributes": elbv2Handler(func(input *gateway_elbv2.DescribeListenerAttributesInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.DescribeListenerAttributes(input, accountID) }), diff --git a/spinifex/gateway/elbv2/AddTags.go b/spinifex/gateway/elbv2/AddTags.go new file mode 100644 index 000000000..c58b24d73 --- /dev/null +++ b/spinifex/gateway/elbv2/AddTags.go @@ -0,0 +1,29 @@ +package gateway_elbv2 + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" +) + +// AddTags handles the ELBv2 AddTags API call. It adds or overwrites the given +// tags on each ELBv2 resource (load balancer, target group, listener, listener +// rule) named in ResourceArns. +func AddTags(input *elbv2.AddTagsInput, natsConn *nats.Conn, accountID string) (elbv2.AddTagsOutput, error) { + var output elbv2.AddTagsOutput + + if input == nil || len(input.ResourceArns) == 0 || len(input.Tags) == 0 { + return output, errors.New(awserrors.ErrorMissingParameter) + } + + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + result, err := svc.AddTags(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go b/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go deleted file mode 100644 index 0ed9a930f..000000000 --- a/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go +++ /dev/null @@ -1,37 +0,0 @@ -package gateway_elbv2 - -import ( - "errors" - - "github.com/aws/aws-sdk-go/service/elbv2" - "github.com/mulgadc/spinifex/spinifex/awserrors" - handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" - "github.com/nats-io/nats.go" -) - -func ValidateDescribeLoadBalancerAttributesInput(input *elbv2.DescribeLoadBalancerAttributesInput) error { - if input == nil { - return errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return errors.New(awserrors.ErrorMissingParameter) - } - return nil -} - -// DescribeLoadBalancerAttributes handles the ELBv2 DescribeLoadBalancerAttributes API call. -func DescribeLoadBalancerAttributes(input *elbv2.DescribeLoadBalancerAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.DescribeLoadBalancerAttributesOutput, error) { - var output elbv2.DescribeLoadBalancerAttributesOutput - - if err := ValidateDescribeLoadBalancerAttributesInput(input); err != nil { - return output, err - } - - svc := handlers_elbv2.NewNATSELBv2Service(natsConn) - result, err := svc.DescribeLoadBalancerAttributes(input, accountID) - if err != nil { - return output, err - } - - return *result, nil -} diff --git a/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go b/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go deleted file mode 100644 index 2402be09d..000000000 --- a/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go +++ /dev/null @@ -1,37 +0,0 @@ -package gateway_elbv2 - -import ( - "errors" - - "github.com/aws/aws-sdk-go/service/elbv2" - "github.com/mulgadc/spinifex/spinifex/awserrors" - handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" - "github.com/nats-io/nats.go" -) - -func ValidateDescribeTargetGroupAttributesInput(input *elbv2.DescribeTargetGroupAttributesInput) error { - if input == nil { - return errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return errors.New(awserrors.ErrorMissingParameter) - } - return nil -} - -// DescribeTargetGroupAttributes handles the ELBv2 DescribeTargetGroupAttributes API call. -func DescribeTargetGroupAttributes(input *elbv2.DescribeTargetGroupAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.DescribeTargetGroupAttributesOutput, error) { - var output elbv2.DescribeTargetGroupAttributesOutput - - if err := ValidateDescribeTargetGroupAttributesInput(input); err != nil { - return output, err - } - - svc := handlers_elbv2.NewNATSELBv2Service(natsConn) - result, err := svc.DescribeTargetGroupAttributes(input, accountID) - if err != nil { - return output, err - } - - return *result, nil -} diff --git a/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go b/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go deleted file mode 100644 index 0e57ca24e..000000000 --- a/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go +++ /dev/null @@ -1,40 +0,0 @@ -package gateway_elbv2 - -import ( - "errors" - - "github.com/aws/aws-sdk-go/service/elbv2" - "github.com/mulgadc/spinifex/spinifex/awserrors" - handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" - "github.com/nats-io/nats.go" -) - -func ValidateModifyLoadBalancerAttributesInput(input *elbv2.ModifyLoadBalancerAttributesInput) error { - if input == nil { - return errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Attributes) == 0 { - return errors.New(awserrors.ErrorMissingParameter) - } - return nil -} - -// ModifyLoadBalancerAttributes handles the ELBv2 ModifyLoadBalancerAttributes API call. -func ModifyLoadBalancerAttributes(input *elbv2.ModifyLoadBalancerAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.ModifyLoadBalancerAttributesOutput, error) { - var output elbv2.ModifyLoadBalancerAttributesOutput - - if err := ValidateModifyLoadBalancerAttributesInput(input); err != nil { - return output, err - } - - svc := handlers_elbv2.NewNATSELBv2Service(natsConn) - result, err := svc.ModifyLoadBalancerAttributes(input, accountID) - if err != nil { - return output, err - } - - return *result, nil -} diff --git a/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go b/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go deleted file mode 100644 index f759608db..000000000 --- a/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go +++ /dev/null @@ -1,40 +0,0 @@ -package gateway_elbv2 - -import ( - "errors" - - "github.com/aws/aws-sdk-go/service/elbv2" - "github.com/mulgadc/spinifex/spinifex/awserrors" - handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" - "github.com/nats-io/nats.go" -) - -func ValidateModifyTargetGroupAttributesInput(input *elbv2.ModifyTargetGroupAttributesInput) error { - if input == nil { - return errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Attributes) == 0 { - return errors.New(awserrors.ErrorMissingParameter) - } - return nil -} - -// ModifyTargetGroupAttributes handles the ELBv2 ModifyTargetGroupAttributes API call. -func ModifyTargetGroupAttributes(input *elbv2.ModifyTargetGroupAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.ModifyTargetGroupAttributesOutput, error) { - var output elbv2.ModifyTargetGroupAttributesOutput - - if err := ValidateModifyTargetGroupAttributesInput(input); err != nil { - return output, err - } - - svc := handlers_elbv2.NewNATSELBv2Service(natsConn) - result, err := svc.ModifyTargetGroupAttributes(input, accountID) - if err != nil { - return output, err - } - - return *result, nil -} diff --git a/spinifex/gateway/elbv2/RemoveTags.go b/spinifex/gateway/elbv2/RemoveTags.go new file mode 100644 index 000000000..e40ac1b0d --- /dev/null +++ b/spinifex/gateway/elbv2/RemoveTags.go @@ -0,0 +1,29 @@ +package gateway_elbv2 + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" +) + +// RemoveTags handles the ELBv2 RemoveTags API call. It removes the given tag +// keys from each ELBv2 resource (load balancer, target group, listener, +// listener rule) named in ResourceArns. Removal is idempotent. +func RemoveTags(input *elbv2.RemoveTagsInput, natsConn *nats.Conn, accountID string) (elbv2.RemoveTagsOutput, error) { + var output elbv2.RemoveTagsOutput + + if input == nil || len(input.ResourceArns) == 0 || len(input.TagKeys) == 0 { + return output, errors.New(awserrors.ErrorMissingParameter) + } + + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + result, err := svc.RemoveTags(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/elbv2/SetIpAddressType.go b/spinifex/gateway/elbv2/SetIpAddressType.go new file mode 100644 index 000000000..87f7a2bd8 --- /dev/null +++ b/spinifex/gateway/elbv2/SetIpAddressType.go @@ -0,0 +1,31 @@ +package gateway_elbv2 + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" +) + +// SetIpAddressType handles the ELBv2 SetIpAddressType API call. It sets the IP +// address type of the load balancer. +func SetIpAddressType(input *elbv2.SetIpAddressTypeInput, natsConn *nats.Conn, accountID string) (elbv2.SetIpAddressTypeOutput, error) { + var output elbv2.SetIpAddressTypeOutput + + if input == nil || input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return output, errors.New(awserrors.ErrorMissingParameter) + } + if input.IpAddressType == nil || *input.IpAddressType == "" { + return output, errors.New(awserrors.ErrorMissingParameter) + } + + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + result, err := svc.SetIpAddressType(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/elbv2/SetSecurityGroups.go b/spinifex/gateway/elbv2/SetSecurityGroups.go new file mode 100644 index 000000000..893f32e2c --- /dev/null +++ b/spinifex/gateway/elbv2/SetSecurityGroups.go @@ -0,0 +1,31 @@ +package gateway_elbv2 + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" +) + +// SetSecurityGroups handles the ELBv2 SetSecurityGroups API call. It replaces +// the security groups associated with the load balancer. +func SetSecurityGroups(input *elbv2.SetSecurityGroupsInput, natsConn *nats.Conn, accountID string) (elbv2.SetSecurityGroupsOutput, error) { + var output elbv2.SetSecurityGroupsOutput + + if input == nil || input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return output, errors.New(awserrors.ErrorMissingParameter) + } + if len(input.SecurityGroups) == 0 { + return output, errors.New(awserrors.ErrorMissingParameter) + } + + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + result, err := svc.SetSecurityGroups(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/elbv2/attributes.go b/spinifex/gateway/elbv2/attributes.go new file mode 100644 index 000000000..bd245d3cb --- /dev/null +++ b/spinifex/gateway/elbv2/attributes.go @@ -0,0 +1,106 @@ +package gateway_elbv2 + +import ( + "errors" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" +) + +// validateAttrArn rejects a nil or empty resource ARN, shared by every +// attribute Modify/Describe validator. +func validateAttrArn(arn *string) error { + if arn == nil || *arn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + +// attributeOp validates an attribute request and dispatches it to its NATS +// service call, returning a zero output on any error. Shared by all four +// Modify/Describe attribute handlers. +func attributeOp[I, O any](input *I, validate func(*I) error, call func(*I) (*O, error)) (O, error) { + var output O + if err := validate(input); err != nil { + return output, err + } + result, err := call(input) + if err != nil { + return output, err + } + return *result, nil +} + +func ValidateModifyTargetGroupAttributesInput(input *elbv2.ModifyTargetGroupAttributesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if err := validateAttrArn(input.TargetGroupArn); err != nil { + return err + } + if len(input.Attributes) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + +func ValidateDescribeTargetGroupAttributesInput(input *elbv2.DescribeTargetGroupAttributesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + return validateAttrArn(input.TargetGroupArn) +} + +func ValidateModifyLoadBalancerAttributesInput(input *elbv2.ModifyLoadBalancerAttributesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if err := validateAttrArn(input.LoadBalancerArn); err != nil { + return err + } + if len(input.Attributes) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + +func ValidateDescribeLoadBalancerAttributesInput(input *elbv2.DescribeLoadBalancerAttributesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + return validateAttrArn(input.LoadBalancerArn) +} + +func ModifyTargetGroupAttributes(input *elbv2.ModifyTargetGroupAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.ModifyTargetGroupAttributesOutput, error) { + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + return attributeOp(input, ValidateModifyTargetGroupAttributesInput, + func(i *elbv2.ModifyTargetGroupAttributesInput) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + return svc.ModifyTargetGroupAttributes(i, accountID) + }) +} + +func DescribeTargetGroupAttributes(input *elbv2.DescribeTargetGroupAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.DescribeTargetGroupAttributesOutput, error) { + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + return attributeOp(input, ValidateDescribeTargetGroupAttributesInput, + func(i *elbv2.DescribeTargetGroupAttributesInput) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return svc.DescribeTargetGroupAttributes(i, accountID) + }) +} + +func ModifyLoadBalancerAttributes(input *elbv2.ModifyLoadBalancerAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.ModifyLoadBalancerAttributesOutput, error) { + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + return attributeOp(input, ValidateModifyLoadBalancerAttributesInput, + func(i *elbv2.ModifyLoadBalancerAttributesInput) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { + return svc.ModifyLoadBalancerAttributes(i, accountID) + }) +} + +func DescribeLoadBalancerAttributes(input *elbv2.DescribeLoadBalancerAttributesInput, natsConn *nats.Conn, accountID string) (elbv2.DescribeLoadBalancerAttributesOutput, error) { + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + return attributeOp(input, ValidateDescribeLoadBalancerAttributesInput, + func(i *elbv2.DescribeLoadBalancerAttributesInput) (*elbv2.DescribeLoadBalancerAttributesOutput, error) { + return svc.DescribeLoadBalancerAttributes(i, accountID) + }) +} diff --git a/spinifex/gateway/elbv2_test.go b/spinifex/gateway/elbv2_test.go index ad81cab0f..dacb82bc6 100644 --- a/spinifex/gateway/elbv2_test.go +++ b/spinifex/gateway/elbv2_test.go @@ -52,6 +52,10 @@ func TestELBv2ActionsMap_AllActionsRegistered(t *testing.T) { "ModifyListener", "DescribeListeners", "DescribeTags", + "AddTags", + "RemoveTags", + "SetSecurityGroups", + "SetIpAddressType", "LBAgentHeartbeat", "GetLBConfig", "ModifyTargetGroupAttributes", diff --git a/spinifex/handlers/elbv2/haproxy.go b/spinifex/handlers/elbv2/haproxy.go index 865b9915e..0b87950f9 100644 --- a/spinifex/handlers/elbv2/haproxy.go +++ b/spinifex/handlers/elbv2/haproxy.go @@ -45,7 +45,9 @@ frontend {{.Name}} {{end}} {{range .Backends}} backend {{.Name}} -{{- if .FixedResponse}} +{{- if .Redirect}} + http-request redirect {{if .Redirect.Scheme}}scheme {{.Redirect.Scheme}}{{else}}location {{.Redirect.Location}}{{end}} code {{.Redirect.Code}} +{{- else if .FixedResponse}} http-request return status {{.FixedResponse.StatusCode}}{{if .FixedResponse.ContentType}} content-type "{{.FixedResponse.ContentType}}"{{end}}{{if .FixedResponse.Body}} string "{{.FixedResponse.Body}}"{{end}} {{- else}} balance roundrobin{{if .HealthCheckPath}} @@ -129,7 +131,8 @@ type HAProxyACL struct { } // HAProxyBackend represents a target group backend, or a synthetic backend that -// emits a fixed response (when FixedResponse is set, the server fields are unused). +// emits a fixed response or a redirect (when FixedResponse / Redirect is set, +// the server fields are unused). type HAProxyBackend struct { Name string HealthCheckPath string @@ -138,6 +141,7 @@ type HAProxyBackend struct { HTTPCheck bool Servers []HAProxyServer FixedResponse *HAProxyFixedResponse + Redirect *HAProxyRedirect } // HAProxyFixedResponse renders an `http-request return` directive for a @@ -148,6 +152,15 @@ type HAProxyFixedResponse struct { Body string } +// HAProxyRedirect renders an `http-request redirect` directive. Exactly one of +// Scheme (the scheme-only fast path, e.g. HTTP→HTTPS) or Location (a full +// rebuilt URL with HAProxy log-format fetches) is set. +type HAProxyRedirect struct { + Scheme string + Location string + Code string +} + // HAProxyServer represents a single target server. type HAProxyServer struct { Name string @@ -273,6 +286,14 @@ func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgB backendAdded[name] = struct{}{} } + addRedirectBackend := func(name string, rd *HAProxyRedirect) { + if _, ok := backendAdded[name]; ok { + return + } + cfg.Backends = append(cfg.Backends, HAProxyBackend{Name: name, Redirect: rd}) + backendAdded[name] = struct{}{} + } + for _, l := range listeners { if len(l.DefaultActions) == 0 { continue @@ -285,9 +306,9 @@ func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgB if isForwardDefault { defaultBackendName = sanitizeName("bk", da.TargetGroupArn) } else { - // fixed-response (or any non-forward default, e.g. the shared-ingress - // 404): synthesize a backend that returns the canned reply so HAProxy - // has a valid default_backend instead of a dangling reference. + // redirect / fixed-response (or any non-forward default, e.g. the + // shared-ingress 404): synthesize a backend so HAProxy has a valid + // default_backend instead of a dangling reference. defaultBackendName = sanitizeName("bkdefault", l.ListenerArn) } @@ -299,25 +320,30 @@ func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgB } if !isTCP { - rules := rulesByListener[l.ListenerArn] - for _, r := range rules { - rule, err := buildHAProxyRule(r) + for _, r := range rulesByListener[l.ListenerArn] { + backendName, err := registerRuleBackend(r, l, addBackend, addFixedResponseBackend, addRedirectBackend) + if err != nil { + return HAProxyConfig{}, err + } + rule, err := buildHAProxyRule(r, backendName) if err != nil { return HAProxyConfig{}, err } frontend.Rules = append(frontend.Rules, rule) - for _, a := range r.Actions { - if a.Type == ActionTypeForward && a.TargetGroupArn != "" { - addBackend(a.TargetGroupArn) - } - } } } cfg.Frontends = append(cfg.Frontends, frontend) - if isForwardDefault { + switch { + case isForwardDefault: addBackend(da.TargetGroupArn) - } else { + case da.Type == ActionTypeRedirect: + rd, err := buildRedirect(da.Redirect, l) + if err != nil { + return HAProxyConfig{}, err + } + addRedirectBackend(defaultBackendName, rd) + default: addFixedResponseBackend(defaultBackendName, da.FixedResponse) } } @@ -325,15 +351,50 @@ func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgB return cfg, nil } -// buildHAProxyRule converts a RuleRecord into the rendered ACL+use_backend -// shape. Returns an error if any condition value fails the defence-in-depth -// re-validation (HAProxy meta-character rejection). -func buildHAProxyRule(r *RuleRecord) (HAProxyRule, error) { - if len(r.Actions) == 0 || r.Actions[0].Type != ActionTypeForward { - return HAProxyRule{}, fmt.Errorf("rule %s has no forward action", r.RuleID) +// registerRuleBackend resolves the HAProxy backend a rule's single action +// routes to, registering a synthetic backend for redirect / fixed-response +// rule actions, and returns its name. +func registerRuleBackend( + r *RuleRecord, + l *ListenerRecord, + addBackend func(string), + addFixedResponseBackend func(string, *FixedResponseAction), + addRedirectBackend func(string, *HAProxyRedirect), +) (string, error) { + if len(r.Actions) == 0 { + return "", fmt.Errorf("rule %s has no action", r.RuleID) } + a := r.Actions[0] + switch a.Type { + case ActionTypeForward: + if a.TargetGroupArn == "" { + return "", fmt.Errorf("rule %s forward action has no target group", r.RuleID) + } + addBackend(a.TargetGroupArn) + return sanitizeName("bk", a.TargetGroupArn), nil + case ActionTypeFixedResponse: + name := sanitizeName("bkrule", r.RuleID) + addFixedResponseBackend(name, a.FixedResponse) + return name, nil + case ActionTypeRedirect: + rd, err := buildRedirect(a.Redirect, l) + if err != nil { + return "", err + } + name := sanitizeName("bkrule", r.RuleID) + addRedirectBackend(name, rd) + return name, nil + default: + return "", fmt.Errorf("rule %s has unsupported action %q", r.RuleID, a.Type) + } +} - rule := HAProxyRule{Backend: sanitizeName("bk", r.Actions[0].TargetGroupArn)} +// buildHAProxyRule converts a RuleRecord's conditions into the rendered +// ACL+use_backend shape, routing to backendName (already resolved/registered +// by registerRuleBackend). Returns an error if any condition value fails the +// defence-in-depth re-validation (HAProxy meta-character rejection). +func buildHAProxyRule(r *RuleRecord, backendName string) (HAProxyRule, error) { + rule := HAProxyRule{Backend: backendName} for ci, c := range r.Conditions { aclName := fmt.Sprintf("r%s_c%d", sanitizeACLToken(r.RuleID), ci) @@ -481,6 +542,118 @@ func validFixedResponseBody(s string) bool { return true } +// buildRedirect translates a stored RedirectAction into an HAProxy redirect +// directive in the context of listener l (which resolves the `#{protocol}` and +// `#{port}` placeholders). The common scheme-only redirect (e.g. HTTP→HTTPS +// with every other field left at its AWS default) renders as `redirect scheme` +// so HAProxy preserves host/path/query; anything else rebuilds a `location`. +func buildRedirect(rd *RedirectAction, l *ListenerRecord) (*HAProxyRedirect, error) { + if rd == nil { + return nil, fmt.Errorf("redirect action missing config") + } + + var code string + switch rd.StatusCode { + case "HTTP_301": + code = "301" + case "HTTP_302": + code = "302" + default: + return nil, fmt.Errorf("redirect has invalid status code %q", rd.StatusCode) + } + + for _, f := range []string{rd.Protocol, rd.Host, rd.Port, rd.Path, rd.Query} { + if !validRedirectField(f) { + return nil, fmt.Errorf("redirect field contains forbidden characters") + } + } + + defaultHost := rd.Host == "" || rd.Host == "#{host}" + defaultPort := rd.Port == "" || rd.Port == "#{port}" + defaultPath := rd.Path == "" || rd.Path == "#{path}" || rd.Path == "/#{path}" + defaultQuery := rd.Query == "" || rd.Query == "#{query}" + explicitScheme := rd.Protocol == ProtocolHTTP || rd.Protocol == ProtocolHTTPS + + if explicitScheme && defaultHost && defaultPort && defaultPath && defaultQuery { + return &HAProxyRedirect{Scheme: strings.ToLower(rd.Protocol), Code: code}, nil + } + + scheme := redirectScheme(rd.Protocol, l) + host := redirectPlaceholders(orDefault(rd.Host, "#{host}"), l) + + port := "" + if rd.Port != "" && rd.Port != "#{port}" { + port = ":" + redirectPlaceholders(rd.Port, l) + } + + path := "%[path]" + if !defaultPath { + path = redirectPlaceholders(rd.Path, l) + } + + query := "" + if !defaultQuery { + query = "?" + redirectPlaceholders(rd.Query, l) + } else if rd.Query == "#{query}" { + query = "?%[query]" + } + + return &HAProxyRedirect{Location: scheme + "://" + host + port + path + query, Code: code}, nil +} + +func orDefault(s, def string) string { + if s == "" { + return def + } + return s +} + +// redirectScheme resolves the redirect scheme literal, mapping the `#{protocol}` +// placeholder (or an empty protocol) to the listener's own protocol. +func redirectScheme(proto string, l *ListenerRecord) string { + switch proto { + case ProtocolHTTP, ProtocolHTTPS: + return strings.ToLower(proto) + default: + return strings.ToLower(l.Protocol) + } +} + +// redirectPlaceholders substitutes the AWS redirect placeholders with HAProxy +// log-format fetches. `#{protocol}` and `#{port}` resolve from the listener. +func redirectPlaceholders(s string, l *ListenerRecord) string { + r := strings.NewReplacer( + "#{protocol}", strings.ToLower(l.Protocol), + "#{host}", "%[req.hdr(host)]", + "#{port}", strconv.FormatInt(l.Port, 10), + "#{path}", "%[path]", + "#{query}", "%[query]", + ) + return r.Replace(s) +} + +// validRedirectField rejects bytes that would break the HAProxy redirect +// directive. The known `#{...}` placeholders are stripped first, so any +// remaining `#`, `%`, bracket, quote, backslash, whitespace, or control byte +// (which would indicate an unknown placeholder or an injection attempt) fails. +func validRedirectField(s string) bool { + known := []string{"#{protocol}", "#{host}", "#{port}", "#{path}", "#{query}"} + tmp := s + for _, k := range known { + tmp = strings.ReplaceAll(tmp, k, "") + } + for _, r := range tmp { + if r < 0x20 || r == 0x7f { + return false + } + switch r { + case ' ', '"', '#', '%', '[', ']', '\\': + return false + } + } + return true +} + // validateRenderSafe rejects bytes that could break ACL parsing or inject new // directives. Mirrors the write-time validators in service_impl_rules.go; // duplicated here so a future migration that bypasses the API cannot leak a diff --git a/spinifex/handlers/elbv2/haproxy_test.go b/spinifex/handlers/elbv2/haproxy_test.go index 2c89d2480..2f325d135 100644 --- a/spinifex/handlers/elbv2/haproxy_test.go +++ b/spinifex/handlers/elbv2/haproxy_test.go @@ -248,6 +248,159 @@ func TestGenerateHAProxyConfig_FixedResponseRejectsUnsafeBody(t *testing.T) { assert.NotContains(t, config, `string "evil`) } +// TestGenerateHAProxyConfig_RedirectSchemeDefault renders the canonical +// HTTP→HTTPS redirect (scheme-only change, all other fields default) as a +// `redirect scheme https` directive so HAProxy preserves host/path/query. +func TestGenerateHAProxyConfig_RedirectSchemeDefault(t *testing.T) { + lb := &LoadBalancerRecord{LoadBalancerID: "lb-redir"} + listenerArn := "arn:aws:elasticloadbalancing:ap-southeast-2:1:listener/app/r/lb-redir/lst-aa" + listeners := []*ListenerRecord{ + { + ListenerArn: listenerArn, + Port: 80, + Protocol: ProtocolHTTP, + DefaultActions: []ListenerAction{ + {Type: ActionTypeRedirect, Redirect: &RedirectAction{ + Protocol: "HTTPS", + Host: "#{host}", + Path: "/#{path}", + Query: "#{query}", + StatusCode: "HTTP_301", + }}, + }, + }, + } + + config, err := GenerateHAProxyConfig(lb, listeners, nil, nil, "0.0.0.0") + require.NoError(t, err) + + assert.Contains(t, config, "default_backend bkdefault_lst-aa") + assert.Contains(t, config, "http-request redirect scheme https code 301") + assert.NotContains(t, config, "location") +} + +// TestGenerateHAProxyConfig_RedirectLocation renders a redirect that changes +// the host as a rebuilt `location` with HAProxy placeholders preserved. +func TestGenerateHAProxyConfig_RedirectLocation(t *testing.T) { + lb := &LoadBalancerRecord{LoadBalancerID: "lb-redir2"} + listenerArn := "arn:aws:elasticloadbalancing:ap-southeast-2:1:listener/app/r/lb-redir2/lst-bb" + listeners := []*ListenerRecord{ + { + ListenerArn: listenerArn, + Port: 80, + Protocol: ProtocolHTTP, + DefaultActions: []ListenerAction{ + {Type: ActionTypeRedirect, Redirect: &RedirectAction{ + Protocol: "HTTPS", + Host: "www.example.com", + Path: "/#{path}", + Query: "#{query}", + StatusCode: "HTTP_302", + }}, + }, + }, + } + + config, err := GenerateHAProxyConfig(lb, listeners, nil, nil, "0.0.0.0") + require.NoError(t, err) + + assert.Contains(t, config, "http-request redirect location https://www.example.com%[path]?%[query] code 302") +} + +// TestGenerateHAProxyConfig_RedirectCustomAll exercises the location form with +// a placeholder protocol (resolved from the listener), an explicit port, and a +// custom literal path + query. +func TestGenerateHAProxyConfig_RedirectCustomAll(t *testing.T) { + lb := &LoadBalancerRecord{LoadBalancerID: "lb-redir3"} + listenerArn := "arn:aws:elasticloadbalancing:ap-southeast-2:1:listener/app/r/lb-redir3/lst-dd" + listeners := []*ListenerRecord{ + { + ListenerArn: listenerArn, + Port: 80, + Protocol: ProtocolHTTPS, + DefaultActions: []ListenerAction{ + {Type: ActionTypeRedirect, Redirect: &RedirectAction{ + Protocol: "#{protocol}", + Host: "alt.example.com", + Port: "8443", + Path: "/moved", + Query: "ref=1", + StatusCode: "HTTP_302", + }}, + }, + }, + } + + config, err := GenerateHAProxyConfig(lb, listeners, nil, nil, "0.0.0.0") + require.NoError(t, err) + + assert.Contains(t, config, "http-request redirect location https://alt.example.com:8443/moved?ref=1 code 302") +} + +// TestGenerateHAProxyConfig_RedirectRule renders a per-rule redirect action via +// a synthetic backend reached by use_backend. +func TestGenerateHAProxyConfig_RedirectRule(t *testing.T) { + lb := &LoadBalancerRecord{LoadBalancerID: "lb-rr"} + listenerArn := "arn:aws:elasticloadbalancing:ap-southeast-2:1:listener/app/r/lb-rr/lst-cc" + appTG := "arn:aws:elasticloadbalancing:ap-southeast-2:1:targetgroup/app/tg-app" + listeners := []*ListenerRecord{ + { + ListenerArn: listenerArn, + Port: 80, + Protocol: ProtocolHTTP, + DefaultActions: []ListenerAction{ + {Type: ActionTypeForward, TargetGroupArn: appTG}, + }, + }, + } + rulesByListener := map[string][]*RuleRecord{ + listenerArn: { + { + RuleID: "ruleR", + ListenerArn: listenerArn, + Priority: 1, + Conditions: []RuleCondition{{Field: RuleFieldPathPattern, Values: []string{"/old/*"}}}, + Actions: []ListenerAction{{Type: ActionTypeRedirect, Redirect: &RedirectAction{ + Protocol: "HTTPS", + StatusCode: "HTTP_301", + }}}, + }, + }, + } + tgByArn := map[string]*TargetGroupRecord{ + appTG: {TargetGroupArn: appTG, HealthCheck: DefaultHealthCheck()}, + } + + config, err := GenerateHAProxyConfig(lb, listeners, tgByArn, rulesByListener, "0.0.0.0") + require.NoError(t, err) + + assert.Contains(t, config, "use_backend bkrule_ruleR if") + assert.Contains(t, config, "backend bkrule_ruleR") + assert.Contains(t, config, "http-request redirect scheme https code 301") +} + +// TestGenerateHAProxyConfig_RedirectRejectsInjection fails the render when a +// redirect field carries HAProxy meta-characters that survived to the store. +func TestGenerateHAProxyConfig_RedirectRejectsInjection(t *testing.T) { + lb := &LoadBalancerRecord{LoadBalancerID: "lb-evil2"} + listeners := []*ListenerRecord{ + { + ListenerArn: "arn:lst-evil2", + Port: 80, + Protocol: ProtocolHTTP, + DefaultActions: []ListenerAction{ + {Type: ActionTypeRedirect, Redirect: &RedirectAction{ + Host: "evil\"\n acl x always_true", + StatusCode: "HTTP_301", + }}, + }, + }, + } + + _, err := GenerateHAProxyConfig(lb, listeners, nil, nil, "0.0.0.0") + require.Error(t, err) +} + func TestGenerateHAProxyConfig_NoListeners(t *testing.T) { lb := &LoadBalancerRecord{LoadBalancerID: "lb-empty"} config, err := GenerateHAProxyConfig(lb, nil, nil, nil, "0.0.0.0") diff --git a/spinifex/handlers/elbv2/lifecycle_invariants_test.go b/spinifex/handlers/elbv2/lifecycle_invariants_test.go new file mode 100644 index 000000000..b2ac8b6e6 --- /dev/null +++ b/spinifex/handlers/elbv2/lifecycle_invariants_test.go @@ -0,0 +1,187 @@ +package handlers_elbv2 + +import ( + "os" + "strings" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRLC1_ELBv2DeleteIdempotentOnAbsent enforces Common Resource Lifecycle +// Contract rule #1 (idempotent delete) across the ELBv2 surface: deleting an +// absent resource is success, not a NotFound error. This mirrors AWS ELBv2 +// delete semantics and is required for tofu destroy to converge on retry. +// +// Every ELBv2 delete endpoint MUST have a case here. When a new delete +// endpoint lands, add it; a missing case is an idempotency gap. +func TestRLC1_ELBv2DeleteIdempotentOnAbsent(t *testing.T) { + const arnBase = "arn:aws:elasticloadbalancing:us-east-1:123456789012:" + + cases := []struct { + name string + call func(svc *ELBv2ServiceImpl) (any, error) + }{ + {"DeleteLoadBalancer", func(svc *ELBv2ServiceImpl) (any, error) { + return svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{ + LoadBalancerArn: aws.String(arnBase + "loadbalancer/app/absent/0000000000000000"), + }, testAccountID) + }}, + {"DeleteTargetGroup", func(svc *ELBv2ServiceImpl) (any, error) { + return svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{ + TargetGroupArn: aws.String(arnBase + "targetgroup/absent/0000000000000000"), + }, testAccountID) + }}, + {"DeleteListener", func(svc *ELBv2ServiceImpl) (any, error) { + return svc.DeleteListener(&elbv2.DeleteListenerInput{ + ListenerArn: aws.String(arnBase + "listener/app/absent/0000000000000000/1111111111111111"), + }, testAccountID) + }}, + {"DeleteRule", func(svc *ELBv2ServiceImpl) (any, error) { + return svc.DeleteRule(&elbv2.DeleteRuleInput{ + RuleArn: aws.String(arnBase + "listener-rule/app/absent/0000000000000000/1111111111111111/2222222222222222"), + }, testAccountID) + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + svc := setupTestService(t) + out, err := tc.call(svc) + require.NoErrorf(t, err, "%s on an absent resource must return success (RLC rule #1)", tc.name) + assert.NotNilf(t, out, "%s must return a non-nil output on absent", tc.name) + }) + } +} + +// TestRLC2_ELBv2NoOrphanAfterDeleteLB enforces ADR-0002 §5 no-orphan +// completeness: after DeleteLoadBalancer, no listener or rule owned by that LB +// may survive in the store. Uses two listeners (each with a rule) so a +// single-listener cascade can't pass by accident. +func TestRLC2_ELBv2NoOrphanAfterDeleteLB(t *testing.T) { + svc := setupTestService(t) + + lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("rlc2-lb")}, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("rlc2-tg")}, testAccountID) + require.NoError(t, err) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + + for i, port := range []int64{80, 8080} { + lst, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(port), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgArn}}, + }, testAccountID) + require.NoError(t, err) + + _, err = svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: lst.Listeners[0].ListenerArn, + Priority: aws.Int64(int64(10 + i)), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/api/*"})}}, + Actions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgArn}}, + }, testAccountID) + require.NoError(t, err) + } + + _, err = svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{LoadBalancerArn: lbArn}, testAccountID) + require.NoError(t, err) + + listeners, err := svc.store.ListListenersByLB(*lbArn) + require.NoError(t, err) + require.Emptyf(t, listeners, "ADR-0002 §5 no-orphan completeness: no listener owned by a deleted LB may remain") + + rules, err := svc.store.ListRules() + require.NoError(t, err) + require.Emptyf(t, rules, "ADR-0002 §5 no-orphan completeness: no rule owned by a deleted LB may remain") +} + +// TestRLC3_ELBv2DeleteLBDoesNotBypassListenerCascade enforces ADR-0002 §5 no +// store-bypass cascade: DeleteLoadBalancer must tear listeners (and their +// rules) down through the shared deleteListenerCascade helper, never by +// calling store.DeleteListener directly — a direct store delete orphans the +// listener's rules. The store field is a concrete type with no injection seam, +// so the contract is enforced structurally against the source rather than with +// a runtime spy. +func TestRLC3_ELBv2DeleteLBDoesNotBypassListenerCascade(t *testing.T) { + src, err := os.ReadFile("service_impl.go") + require.NoError(t, err) + + body := stripComments(deleteLBFuncBody(t, string(src))) + assert.Containsf(t, body, "deleteListenerCascade", + "ADR-0002 §5 no store-bypass cascade: DeleteLoadBalancer must route listener teardown through deleteListenerCascade") + assert.NotContainsf(t, body, "s.store.DeleteListener(", + "ADR-0002 §5 no store-bypass cascade: DeleteLoadBalancer must not call store.DeleteListener directly") +} + +// deleteLBFuncBody returns the source text of the DeleteLoadBalancer method, +// from its signature up to the start of the next top-level func. +func deleteLBFuncBody(t *testing.T, src string) string { + t.Helper() + const sig = "func (s *ELBv2ServiceImpl) DeleteLoadBalancer(" + start := strings.Index(src, sig) + require.GreaterOrEqual(t, start, 0, "DeleteLoadBalancer not found in service_impl.go") + rest := src[start+len(sig):] + if before, _, found := strings.Cut(rest, "\nfunc "); found { + return before + } + return rest +} + +// stripComments removes // line comments so source-guard assertions match real +// code, not the prose that documents it. +func stripComments(src string) string { + var b strings.Builder + for line := range strings.SplitSeq(src, "\n") { + if i := strings.Index(line, "//"); i >= 0 { + line = line[:i] + } + b.WriteString(line) + b.WriteByte('\n') + } + return b.String() +} + +// TestRLC4_ELBv2TGDeletableAfterLBTeardown enforces ADR-0002 §5 TG deletability +// after LB teardown: create LB+listener+rule→TG, delete the LB, then the target +// group must delete successfully rather than staying pinned as ResourceInUse by +// an orphaned rule or stale listener default action. +func TestRLC4_ELBv2TGDeletableAfterLBTeardown(t *testing.T) { + svc := setupTestService(t) + + lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("rlc4-lb")}, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("rlc4-tg")}, testAccountID) + require.NoError(t, err) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + + lst, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgArn}}, + }, testAccountID) + require.NoError(t, err) + + _, err = svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: lst.Listeners[0].ListenerArn, + Priority: aws.Int64(10), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/api/*"})}}, + Actions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgArn}}, + }, testAccountID) + require.NoError(t, err) + + _, err = svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{LoadBalancerArn: lbArn}, testAccountID) + require.NoError(t, err) + + _, err = svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{TargetGroupArn: tgArn}, testAccountID) + require.NoErrorf(t, err, "ADR-0002 §5 TG deletability after LB teardown: target group must not stay pinned as ResourceInUse") +} diff --git a/spinifex/handlers/elbv2/service.go b/spinifex/handlers/elbv2/service.go index 4a6276c46..b2784d9ba 100644 --- a/spinifex/handlers/elbv2/service.go +++ b/spinifex/handlers/elbv2/service.go @@ -9,6 +9,8 @@ type ELBv2Service interface { DescribeLoadBalancers(input *elbv2.DescribeLoadBalancersInput, accountID string) (*elbv2.DescribeLoadBalancersOutput, error) ModifyLoadBalancerAttributes(input *elbv2.ModifyLoadBalancerAttributesInput, accountID string) (*elbv2.ModifyLoadBalancerAttributesOutput, error) DescribeLoadBalancerAttributes(input *elbv2.DescribeLoadBalancerAttributesInput, accountID string) (*elbv2.DescribeLoadBalancerAttributesOutput, error) + SetSecurityGroups(input *elbv2.SetSecurityGroupsInput, accountID string) (*elbv2.SetSecurityGroupsOutput, error) + SetIpAddressType(input *elbv2.SetIpAddressTypeInput, accountID string) (*elbv2.SetIpAddressTypeOutput, error) CreateTargetGroup(input *elbv2.CreateTargetGroupInput, accountID string) (*elbv2.CreateTargetGroupOutput, error) ModifyTargetGroup(input *elbv2.ModifyTargetGroupInput, accountID string) (*elbv2.ModifyTargetGroupOutput, error) @@ -33,6 +35,8 @@ type ELBv2Service interface { SetRulePriorities(input *elbv2.SetRulePrioritiesInput, accountID string) (*elbv2.SetRulePrioritiesOutput, error) DescribeTags(input *elbv2.DescribeTagsInput, accountID string) (*elbv2.DescribeTagsOutput, error) + AddTags(input *elbv2.AddTagsInput, accountID string) (*elbv2.AddTagsOutput, error) + RemoveTags(input *elbv2.RemoveTagsInput, accountID string) (*elbv2.RemoveTagsOutput, error) LBAgentHeartbeat(input *LBAgentHeartbeatInput, accountID string) (*LBAgentHeartbeatOutput, error) GetLBConfig(input *GetLBConfigInput, accountID string) (*GetLBConfigOutput, error) diff --git a/spinifex/handlers/elbv2/service_impl.go b/spinifex/handlers/elbv2/service_impl.go index 7bb7f796a..4d2492bfb 100644 --- a/spinifex/handlers/elbv2/service_impl.go +++ b/spinifex/handlers/elbv2/service_impl.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "log/slog" - "maps" "net" "regexp" "slices" @@ -183,8 +182,6 @@ func (s *ELBv2ServiceImpl) Close() { // heartbeat re-converges. Acceptable trade-off — heartbeats arrive within // one health-check interval (default 5-30s) and round-robin transiently // excluding a backend is safer than serving traffic to a dead one. -// -// Refs mulga-siv-119. func (s *ELBv2ServiceImpl) ResetTargetHealthOnStartup(ctx context.Context) error { if s == nil || s.store == nil { return nil @@ -762,6 +759,28 @@ func (s *ELBv2ServiceImpl) resolveTargetIP(instanceID, accountID string) string return "" } +// resolveRegisteredTargetIP returns the private IP to route to for a target +// being registered, based on the target group's target type. For ip targets the +// supplied ID is the IP itself; for instance targets it is resolved via the +// instance's primary ENI. Returns an error when the ID's shape does not match +// the target type. +func (s *ELBv2ServiceImpl) resolveRegisteredTargetIP(targetType, id, accountID string) (string, error) { + switch targetType { + case TargetTypeIP: + if net.ParseIP(id) == nil { + slog.Warn("RegisterTargets: ip target id is not a valid IP", "id", id) + return "", errors.New(awserrors.ErrorInvalidParameterValue) + } + return id, nil + default: // instance + if net.ParseIP(id) != nil { + slog.Warn("RegisterTargets: instance target group given an IP address", "id", id) + return "", errors.New(awserrors.ErrorInvalidParameterValue) + } + return s.resolveTargetIP(id, accountID), nil + } +} + // buildTGArn constructs a target group ARN from components. func buildTGArn(region, accountID, name, tgID string) string { return fmt.Sprintf("arn:aws:elasticloadbalancing:%s:%s:targetgroup/%s/%s", region, accountID, name, tgID) @@ -1078,7 +1097,27 @@ func (s *ELBv2ServiceImpl) DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInp return nil, errors.New(awserrors.ErrorServerInternal) } if lb == nil || lb.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) + // Idempotent: AWS ELBv2 delete returns success on an absent (or + // not-owned) load balancer, so tofu destroy retries converge. + return &elbv2.DeleteLoadBalancerOutput{}, nil + } + + // Delete all listeners (and their rules) for this LB before tearing down + // backing artifacts. Cascade through the shared helper, not + // store.DeleteListener directly, so rules don't survive and pin their + // target groups as ResourceInUse after the LB is gone. A list or cascade + // failure aborts the delete with the record intact, so tofu retries + // converge instead of leaving orphaned rules behind. + listeners, err := s.store.ListListenersByLB(lb.LoadBalancerArn) + if err != nil { + slog.Error("DeleteLoadBalancer: failed to list listeners for cascade", "lbArn", lb.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + for _, l := range listeners { + if err := s.deleteListenerCascade(l); err != nil { + slog.Error("DeleteLoadBalancer: failed to cascade listener delete", "listenerID", l.ListenerID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } } // Terminate ALB VM in background (VM termination also cleans up tap device). @@ -1093,17 +1132,6 @@ func (s *ELBv2ServiceImpl) DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInp }() } - // Delete all listeners for this LB - listeners, err := s.store.ListListenersByLB(lb.LoadBalancerArn) - if err != nil { - slog.Warn("Failed to list listeners for cleanup", "lbArn", lb.LoadBalancerArn, "err", err) - } - for _, l := range listeners { - if err := s.store.DeleteListener(l.ListenerID); err != nil { - slog.Warn("Failed to delete listener during LB cleanup", "listenerID", l.ListenerID, "err", err) - } - } - // Delete system-managed ENIs. Detach first to clear in-use status. if s.VPCService != nil { for _, eniID := range lb.ENIs { @@ -1210,10 +1238,14 @@ func (s *ELBv2ServiceImpl) CreateTargetGroup(input *elbv2.CreateTargetGroupInput return nil, errors.New(awserrors.ErrorELBv2DuplicateTargetGroup) } - targetType := "instance" + targetType := TargetTypeInstance if input.TargetType != nil && *input.TargetType != "" { targetType = *input.TargetType } + if targetType != TargetTypeInstance && targetType != TargetTypeIP { + slog.Warn("CreateTargetGroup: unsupported target type", "targetType", targetType) + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } // Use NLB health check defaults for NLB protocols. var hc HealthCheckConfig @@ -1366,16 +1398,37 @@ func (s *ELBv2ServiceImpl) DeleteTargetGroup(input *elbv2.DeleteTargetGroupInput return nil, errors.New(awserrors.ErrorServerInternal) } if tg == nil || tg.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) + // Idempotent: AWS ELBv2 delete returns success on an absent (or + // not-owned) target group, so tofu destroy retries converge. + return &elbv2.DeleteTargetGroupOutput{}, nil } - // Check if any listener references this target group + // Only a *live* listener or rule pins the target group. A listener (or + // rule) whose owning load balancer no longer exists is treated as already + // torn down, not a live reference, so an orphan left by a partial teardown + // can never pin this target group as ResourceInUse permanently. + lbs, err := s.store.ListLoadBalancers() + if err != nil { + slog.Error("DeleteTargetGroup: failed to list load balancers for in-use check", "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + liveLB := make(map[string]bool, len(lbs)) + for _, lb := range lbs { + liveLB[lb.LoadBalancerArn] = true + } + + // Check if any live listener references this target group. listeners, err := s.store.ListListeners() if err != nil { slog.Error("DeleteTargetGroup: failed to list listeners for in-use check", "err", err) return nil, errors.New(awserrors.ErrorServerInternal) } + liveListener := make(map[string]bool, len(listeners)) for _, l := range listeners { + if !liveLB[l.LoadBalancerArn] { + continue + } + liveListener[l.ListenerArn] = true for _, action := range l.DefaultActions { if action.TargetGroupArn == tg.TargetGroupArn { return nil, errors.New(awserrors.ErrorELBv2TargetGroupInUse) @@ -1383,13 +1436,16 @@ func (s *ELBv2ServiceImpl) DeleteTargetGroup(input *elbv2.DeleteTargetGroupInput } } - // Block deletion when a rule still forwards to the target group. + // Block deletion when a live rule still forwards to the target group. allRules, err := s.store.ListRules() if err != nil { slog.Error("DeleteTargetGroup: failed to list rules for in-use check", "err", err) return nil, errors.New(awserrors.ErrorServerInternal) } for _, r := range allRules { + if !liveListener[r.ListenerArn] { + continue + } for _, action := range r.Actions { if action.TargetGroupArn == tg.TargetGroupArn { return nil, errors.New(awserrors.ErrorELBv2TargetGroupInUse) @@ -1498,8 +1554,11 @@ func (s *ELBv2ServiceImpl) RegisterTargets(input *elbv2.RegisterTargetsInput, ac continue // Already registered } - // Resolve instance ID → private IP via ENI lookup - privateIP := s.resolveTargetIP(*td.Id, accountID) + // Resolve target ID → private IP (instance ENI lookup, or raw IP for ip-type TGs) + privateIP, err := s.resolveRegisteredTargetIP(tg.TargetType, *td.Id, accountID) + if err != nil { + return nil, err + } tg.Targets = append(tg.Targets, Target{ Id: *td.Id, @@ -1653,9 +1712,66 @@ func listenerActionFromSDK(a *elbv2.Action) ListenerAction { } action.FixedResponse = fr } + if a.RedirectConfig != nil { + rd := &RedirectAction{} + if a.RedirectConfig.Protocol != nil { + rd.Protocol = *a.RedirectConfig.Protocol + } + if a.RedirectConfig.Host != nil { + rd.Host = *a.RedirectConfig.Host + } + if a.RedirectConfig.Port != nil { + rd.Port = *a.RedirectConfig.Port + } + if a.RedirectConfig.Path != nil { + rd.Path = *a.RedirectConfig.Path + } + if a.RedirectConfig.Query != nil { + rd.Query = *a.RedirectConfig.Query + } + if a.RedirectConfig.StatusCode != nil { + rd.StatusCode = *a.RedirectConfig.StatusCode + } + action.Redirect = rd + } return action } +// validateListenerAction enforces the per-type action contract for listener +// default actions and rule actions. Forward actions are validated against the +// target group elsewhere; this covers redirect (status code + render-safe +// fields) so invalid input fails at the API instead of being silently +// defaulted by the renderer. +func validateListenerAction(a ListenerAction) error { + if a.Type == ActionTypeRedirect { + if a.Redirect == nil { + return errors.New(awserrors.ErrorMissingParameter) + } + return validateRedirectAction(a.Redirect) + } + return nil +} + +// validateRedirectAction rejects an unsupported status code or any field that +// would break the HAProxy redirect directive once the known AWS placeholders +// are stripped. +func validateRedirectAction(rd *RedirectAction) error { + if rd == nil { + return errors.New(awserrors.ErrorMissingParameter) + } + switch rd.StatusCode { + case "HTTP_301", "HTTP_302": + default: + return errors.New(awserrors.ErrorInvalidParameterValue) + } + for _, f := range []string{rd.Protocol, rd.Host, rd.Port, rd.Path, rd.Query} { + if !validRedirectField(f) { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + } + return nil +} + func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, accountID string) (*elbv2.CreateListenerOutput, error) { if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { return nil, errors.New(awserrors.ErrorMissingParameter) @@ -1735,7 +1851,18 @@ func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, acco var actions []ListenerAction for _, a := range input.DefaultActions { - actions = append(actions, listenerActionFromSDK(a)) + action := listenerActionFromSDK(a) + if err := validateListenerAction(action); err != nil { + return nil, err + } + actions = append(actions, action) + } + + tags := make(map[string]string) + for _, tag := range input.Tags { + if tag.Key != nil && tag.Value != nil { + tags[*tag.Key] = *tag.Value + } } record := &ListenerRecord{ @@ -1747,6 +1874,7 @@ func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, acco DefaultActions: actions, AccountID: accountID, CreatedAt: time.Now().UTC(), + Tags: tags, } if err := s.store.PutListener(record); err != nil { @@ -1767,6 +1895,25 @@ func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, acco }, nil } +// deleteListenerCascade removes a listener and all of its rules. Shared by +// DeleteListener and DeleteLoadBalancer so LB teardown never bypasses the rule +// cascade and leaves orphan rules that pin a target group as ResourceInUse. +func (s *ELBv2ServiceImpl) deleteListenerCascade(listener *ListenerRecord) error { + rules, err := s.store.ListRulesByListener(listener.ListenerArn) + if err != nil { + return fmt.Errorf("list rules for listener %s: %w", listener.ListenerArn, err) + } + for _, r := range rules { + if err := s.store.DeleteRule(r.RuleID); err != nil { + return fmt.Errorf("delete rule %s: %w", r.RuleID, err) + } + } + if err := s.store.DeleteListener(listener.ListenerID); err != nil { + return fmt.Errorf("delete listener %s: %w", listener.ListenerID, err) + } + return nil +} + func (s *ELBv2ServiceImpl) DeleteListener(input *elbv2.DeleteListenerInput, accountID string) (*elbv2.DeleteListenerOutput, error) { if input.ListenerArn == nil || *input.ListenerArn == "" { return nil, errors.New(awserrors.ErrorMissingParameter) @@ -1778,24 +1925,14 @@ func (s *ELBv2ServiceImpl) DeleteListener(input *elbv2.DeleteListenerInput, acco return nil, errors.New(awserrors.ErrorServerInternal) } if listener == nil || listener.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2ListenerNotFound) + // Idempotent: AWS ELBv2 delete returns success on an absent (or + // not-owned) listener, so tofu destroy retries converge. + return &elbv2.DeleteListenerOutput{}, nil } // Cascade-delete rules so a recreated listener doesn't inherit orphans. - rules, ruleErr := s.store.ListRulesByListener(listener.ListenerArn) - if ruleErr != nil { - slog.Error("DeleteListener: failed to list rules", "listenerArn", listener.ListenerArn, "err", ruleErr) - return nil, errors.New(awserrors.ErrorServerInternal) - } - for _, r := range rules { - if err := s.store.DeleteRule(r.RuleID); err != nil { - slog.Error("DeleteListener: failed to delete rule", "ruleId", r.RuleID, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - } - - if err := s.store.DeleteListener(listener.ListenerID); err != nil { - slog.Error("DeleteListener: failed to delete record", "listenerId", listener.ListenerID, "err", err) + if err := s.deleteListenerCascade(listener); err != nil { + slog.Error("DeleteListener: failed to cascade-delete listener", "listenerArn", listener.ListenerArn, "err", err) return nil, errors.New(awserrors.ErrorServerInternal) } @@ -1881,6 +2018,9 @@ func (s *ELBv2ServiceImpl) ModifyListener(input *elbv2.ModifyListenerInput, acco var actions []ListenerAction for _, a := range input.DefaultActions { action := listenerActionFromSDK(a) + if err := validateListenerAction(action); err != nil { + return nil, err + } if action.Type == ActionTypeForward && action.TargetGroupArn != "" { tg, tgErr := s.store.GetTargetGroupByArn(action.TargetGroupArn) if tgErr != nil { @@ -1975,10 +2115,9 @@ func (s *ELBv2ServiceImpl) DescribeListeners(input *elbv2.DescribeListenersInput // DescribeTags returns tags for one or more ELBv2 resources (load balancers, // target groups, listeners, listener rules). Tag data is read from the existing -// record stores — Spinifex doesn't have a separate tag KV. Listeners and rules -// currently never store tags, so they always return an empty Tags slice -// (matches AWS behaviour for untagged resources). Cross-account or unknown ARNs -// return the per-resource not-found error so existence isn't leaked across +// record stores — Spinifex doesn't have a separate tag KV. Untagged resources +// return an empty Tags slice (matches AWS behaviour). Cross-account or unknown +// ARNs return the per-resource not-found error so existence isn't leaked across // accounts. func (s *ELBv2ServiceImpl) DescribeTags(input *elbv2.DescribeTagsInput, accountID string) (*elbv2.DescribeTagsOutput, error) { if input == nil || len(input.ResourceArns) == 0 { @@ -2037,8 +2176,8 @@ func (s *ELBv2ServiceImpl) DescribeTags(input *elbv2.DescribeTagsInput, accountI } if l != nil { found = true + tags = l.Tags ownerAccount = l.AccountID - // Listeners don't store tags yet — leave map nil. } case elbv2ResourceListenerRule: notFoundError = awserrors.ErrorELBv2RuleNotFound @@ -2049,8 +2188,8 @@ func (s *ELBv2ServiceImpl) DescribeTags(input *elbv2.DescribeTagsInput, accountI } if r != nil { found = true + tags = r.Tags ownerAccount = r.AccountID - // Rules don't store tags yet — leave map nil. } } @@ -2088,210 +2227,6 @@ func tagsMapToSDK(tags map[string]string) []*elbv2.Tag { return out } -// --- SDK type conversion helpers --- - -func (s *ELBv2ServiceImpl) ModifyTargetGroupAttributes(input *elbv2.ModifyTargetGroupAttributesInput, accountID string) (*elbv2.ModifyTargetGroupAttributesOutput, error) { - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return nil, errors.New(awserrors.ErrorMissingParameter) - } - - tg, err := s.store.GetTargetGroupByArn(*input.TargetGroupArn) - if err != nil { - slog.Error("ModifyTargetGroupAttributes: failed to get TG", "arn", *input.TargetGroupArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - if tg == nil || tg.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) - } - - knownTGAttrs := DefaultTargetGroupAttributes() - var submitted []*elbv2.TargetGroupAttribute - dirty := false - for _, attr := range input.Attributes { - if attr == nil { - slog.Warn("ModifyTargetGroupAttributes: skipping nil attribute element", "arn", *input.TargetGroupArn) - continue - } - if attr.Key == nil || attr.Value == nil { - slog.Warn("ModifyTargetGroupAttributes: skipping attribute with nil Key or Value", "arn", *input.TargetGroupArn) - continue - } - if _, ok := knownTGAttrs[*attr.Key]; !ok { - slog.Warn("ModifyTargetGroupAttributes: rejecting unknown attribute key", "arn", *input.TargetGroupArn, "key", *attr.Key) - return nil, errors.New(awserrors.ErrorValidationError) - } - submitted = append(submitted, &elbv2.TargetGroupAttribute{ - Key: attr.Key, - Value: attr.Value, - }) - existing, exists := tg.Attributes[*attr.Key] - if exists && existing == *attr.Value { - continue // value already matches stored — no mutation needed - } - if tg.Attributes == nil { - tg.Attributes = make(map[string]string) - } - tg.Attributes[*attr.Key] = *attr.Value - dirty = true - } - - // If the caller sent attributes but every single one was rejected by the - // nil guard above, surface that as an error instead of silently returning - // a successful empty response — otherwise the caller thinks the write - // landed when nothing was actually applied. - if len(input.Attributes) > 0 && len(submitted) == 0 { - slog.Warn("ModifyTargetGroupAttributes: all submitted attributes were invalid", "arn", *input.TargetGroupArn, "submitted_count", len(input.Attributes)) - return nil, errors.New(awserrors.ErrorInvalidParameterValue) - } - - // Skip the NATS/KV write when nothing changed. Terraform re-applies the same - // attribute set on every drift check, so this kills ~all Modify traffic - // during steady state and narrows the read-modify-write race window. - if dirty { - if err := s.store.PutTargetGroup(tg); err != nil { - slog.Error("ModifyTargetGroupAttributes: failed to persist TG", "arn", *input.TargetGroupArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - } - - return &elbv2.ModifyTargetGroupAttributesOutput{ - Attributes: submitted, - }, nil -} - -func (s *ELBv2ServiceImpl) DescribeTargetGroupAttributes(input *elbv2.DescribeTargetGroupAttributesInput, accountID string) (*elbv2.DescribeTargetGroupAttributesOutput, error) { - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return nil, errors.New(awserrors.ErrorMissingParameter) - } - - tg, err := s.store.GetTargetGroupByArn(*input.TargetGroupArn) - if err != nil { - slog.Error("DescribeTargetGroupAttributes: failed to get TG", "arn", *input.TargetGroupArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - if tg == nil || tg.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) - } - - merged := DefaultTargetGroupAttributes() - maps.Copy(merged, tg.Attributes) - - // Sort keys for deterministic output — Terraform diffs and snapshot tests - // depend on stable attribute ordering. - keys := slices.Sorted(maps.Keys(merged)) - attrs := make([]*elbv2.TargetGroupAttribute, 0, len(keys)) - for _, k := range keys { - attrs = append(attrs, &elbv2.TargetGroupAttribute{ - Key: aws.String(k), - Value: aws.String(merged[k]), - }) - } - - return &elbv2.DescribeTargetGroupAttributesOutput{ - Attributes: attrs, - }, nil -} - -func (s *ELBv2ServiceImpl) ModifyLoadBalancerAttributes(input *elbv2.ModifyLoadBalancerAttributesInput, accountID string) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return nil, errors.New(awserrors.ErrorMissingParameter) - } - - lb, err := s.store.GetLoadBalancerByArn(*input.LoadBalancerArn) - if err != nil { - slog.Error("ModifyLoadBalancerAttributes: failed to get LB", "arn", *input.LoadBalancerArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - if lb == nil || lb.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) - } - - knownLBAttrs := DefaultLoadBalancerAttributes(lb.Type) - var submitted []*elbv2.LoadBalancerAttribute - dirty := false - for _, attr := range input.Attributes { - if attr == nil { - slog.Warn("ModifyLoadBalancerAttributes: skipping nil attribute element", "arn", *input.LoadBalancerArn) - continue - } - if attr.Key == nil || attr.Value == nil { - slog.Warn("ModifyLoadBalancerAttributes: skipping attribute with nil Key or Value", "arn", *input.LoadBalancerArn) - continue - } - if _, ok := knownLBAttrs[*attr.Key]; !ok { - slog.Warn("ModifyLoadBalancerAttributes: rejecting unknown attribute key", "arn", *input.LoadBalancerArn, "key", *attr.Key, "lbType", lb.Type) - return nil, errors.New(awserrors.ErrorValidationError) - } - submitted = append(submitted, &elbv2.LoadBalancerAttribute{ - Key: attr.Key, - Value: attr.Value, - }) - existing, exists := lb.Attributes[*attr.Key] - if exists && existing == *attr.Value { - continue - } - if lb.Attributes == nil { - lb.Attributes = make(map[string]string) - } - lb.Attributes[*attr.Key] = *attr.Value - dirty = true - } - - // If the caller sent attributes but every single one was rejected by the - // nil guard above, surface that as an error instead of silently returning - // a successful empty response. - if len(input.Attributes) > 0 && len(submitted) == 0 { - slog.Warn("ModifyLoadBalancerAttributes: all submitted attributes were invalid", "arn", *input.LoadBalancerArn, "submitted_count", len(input.Attributes)) - return nil, errors.New(awserrors.ErrorInvalidParameterValue) - } - - // Skip the NATS/KV write when nothing changed. See - // ModifyTargetGroupAttributes for the Terraform-drift-check motivation. - if dirty { - if err := s.store.PutLoadBalancer(lb); err != nil { - slog.Error("ModifyLoadBalancerAttributes: failed to persist LB", "arn", *input.LoadBalancerArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - } - - return &elbv2.ModifyLoadBalancerAttributesOutput{ - Attributes: submitted, - }, nil -} - -func (s *ELBv2ServiceImpl) DescribeLoadBalancerAttributes(input *elbv2.DescribeLoadBalancerAttributesInput, accountID string) (*elbv2.DescribeLoadBalancerAttributesOutput, error) { - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return nil, errors.New(awserrors.ErrorMissingParameter) - } - - lb, err := s.store.GetLoadBalancerByArn(*input.LoadBalancerArn) - if err != nil { - slog.Error("DescribeLoadBalancerAttributes: failed to get LB", "arn", *input.LoadBalancerArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - if lb == nil || lb.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) - } - - merged := DefaultLoadBalancerAttributes(lb.Type) - maps.Copy(merged, lb.Attributes) - - // Sort keys for deterministic output — Terraform diffs and snapshot tests - // depend on stable attribute ordering. - keys := slices.Sorted(maps.Keys(merged)) - attrs := make([]*elbv2.LoadBalancerAttribute, 0, len(keys)) - for _, k := range keys { - attrs = append(attrs, &elbv2.LoadBalancerAttribute{ - Key: aws.String(k), - Value: aws.String(merged[k]), - }) - } - - return &elbv2.DescribeLoadBalancerAttributesOutput{ - Attributes: attrs, - }, nil -} - func (s *ELBv2ServiceImpl) lbRecordToSDK(r *LoadBalancerRecord) *elbv2.LoadBalancer { lb := &elbv2.LoadBalancer{ LoadBalancerArn: aws.String(r.LoadBalancerArn), @@ -2387,24 +2322,52 @@ func (s *ELBv2ServiceImpl) listenerRecordToSDK(r *ListenerRecord) *elbv2.Listene } for _, a := range r.DefaultActions { - action := &elbv2.Action{Type: aws.String(a.Type)} - if a.TargetGroupArn != "" { - action.TargetGroupArn = aws.String(a.TargetGroupArn) - } - if a.FixedResponse != nil { - fr := &elbv2.FixedResponseActionConfig{ - StatusCode: aws.String(a.FixedResponse.StatusCode), - } - if a.FixedResponse.ContentType != "" { - fr.ContentType = aws.String(a.FixedResponse.ContentType) - } - if a.FixedResponse.MessageBody != "" { - fr.MessageBody = aws.String(a.FixedResponse.MessageBody) - } - action.FixedResponseConfig = fr - } - listener.DefaultActions = append(listener.DefaultActions, action) + listener.DefaultActions = append(listener.DefaultActions, listenerActionToSDK(a)) } return listener } + +// listenerActionToSDK converts a stored action into the AWS SDK shape, +// preserving forward / fixed-response / redirect detail so DescribeListeners +// and DescribeRules round-trip cleanly. Shared by listeners and rules. +func listenerActionToSDK(a ListenerAction) *elbv2.Action { + action := &elbv2.Action{Type: aws.String(a.Type)} + if a.TargetGroupArn != "" { + action.TargetGroupArn = aws.String(a.TargetGroupArn) + } + if a.FixedResponse != nil { + fr := &elbv2.FixedResponseActionConfig{ + StatusCode: aws.String(a.FixedResponse.StatusCode), + } + if a.FixedResponse.ContentType != "" { + fr.ContentType = aws.String(a.FixedResponse.ContentType) + } + if a.FixedResponse.MessageBody != "" { + fr.MessageBody = aws.String(a.FixedResponse.MessageBody) + } + action.FixedResponseConfig = fr + } + if a.Redirect != nil { + rd := &elbv2.RedirectActionConfig{ + StatusCode: aws.String(a.Redirect.StatusCode), + } + if a.Redirect.Protocol != "" { + rd.Protocol = aws.String(a.Redirect.Protocol) + } + if a.Redirect.Host != "" { + rd.Host = aws.String(a.Redirect.Host) + } + if a.Redirect.Port != "" { + rd.Port = aws.String(a.Redirect.Port) + } + if a.Redirect.Path != "" { + rd.Path = aws.String(a.Redirect.Path) + } + if a.Redirect.Query != "" { + rd.Query = aws.String(a.Redirect.Query) + } + action.RedirectConfig = rd + } + return action +} diff --git a/spinifex/handlers/elbv2/service_impl_attributes.go b/spinifex/handlers/elbv2/service_impl_attributes.go new file mode 100644 index 000000000..362c469e5 --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_attributes.go @@ -0,0 +1,265 @@ +package handlers_elbv2 + +import ( + "errors" + "log/slog" + "maps" + "slices" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// attrPair is the internal key/value shape the generic attribute handlers work +// in, decoupled from the two near-identical SDK attribute types +// (*elbv2.TargetGroupAttribute and *elbv2.LoadBalancerAttribute). +type attrPair struct { + Key string + Value string +} + +// attrResource bundles the per-resource-type plumbing the generic Modify/ +// Describe attribute handlers need, so the target-group and load-balancer +// paths share one implementation. +type attrResource[R any] struct { + arn string + accountID string + opName string // slog prefix / error context + notFound string // awserrors code returned for missing/cross-account + fetch func(string) (*R, error) + save func(*R) error + accountIDOf func(*R) string + defaults func(*R) map[string]string + attrsOf func(*R) map[string]string + setAttrs func(*R, map[string]string) +} + +// load fetches and authorises the record. An empty accountID is rejected +// explicitly so a direct NATS/internal caller cannot match a record whose +// AccountID is also empty. +func (r attrResource[R]) load() (*R, error) { + rec, err := r.fetch(r.arn) + if err != nil { + slog.Error(r.opName+": failed to load record", "arn", r.arn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if rec == nil || r.accountID == "" || r.accountIDOf(rec) != r.accountID { + return nil, errors.New(r.notFound) + } + return rec, nil +} + +// modifyResourceAttributes validates and merges submitted attributes into the +// record, persisting only when something actually changed. rawCount is the +// number of attributes the caller submitted before nil filtering, used to +// distinguish "nothing sent" from "everything sent was invalid". +func modifyResourceAttributes[R any](res attrResource[R], pairs []attrPair, rawCount int) ([]attrPair, error) { + if res.arn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + rec, err := res.load() + if err != nil { + return nil, err + } + + known := res.defaults(rec) + attrs := res.attrsOf(rec) + submitted := make([]attrPair, 0, len(pairs)) + dirty := false + for _, p := range pairs { + if _, ok := known[p.Key]; !ok { + slog.Warn(res.opName+": rejecting unknown attribute key", "arn", res.arn, "key", p.Key) + return nil, errors.New(awserrors.ErrorValidationError) + } + submitted = append(submitted, p) + if existing, ok := attrs[p.Key]; ok && existing == p.Value { + continue // value already matches stored — no mutation needed + } + if attrs == nil { + attrs = make(map[string]string) + } + attrs[p.Key] = p.Value + dirty = true + } + + // Caller sent attributes but every one was dropped by nil filtering: surface + // an error rather than a successful empty response the caller misreads as a + // landed write. + if rawCount > 0 && len(submitted) == 0 { + slog.Warn(res.opName+": all submitted attributes were invalid", "arn", res.arn, "submitted_count", rawCount) + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + + // Skip the NATS/KV write when nothing changed. Terraform re-applies the same + // attribute set on every drift check, so this kills ~all Modify traffic + // during steady state and narrows the read-modify-write race window. + if dirty { + res.setAttrs(rec, attrs) + if err := res.save(rec); err != nil { + slog.Error(res.opName+": failed to persist", "arn", res.arn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + } + + return submitted, nil +} + +// describeResourceAttributes returns the record's attributes merged over the +// per-type defaults, sorted by key for deterministic output (Terraform diffs +// and snapshot tests depend on stable ordering). +func describeResourceAttributes[R any](res attrResource[R]) ([]attrPair, error) { + if res.arn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + rec, err := res.load() + if err != nil { + return nil, err + } + + merged := res.defaults(rec) + maps.Copy(merged, res.attrsOf(rec)) + + keys := slices.Sorted(maps.Keys(merged)) + out := make([]attrPair, 0, len(keys)) + for _, k := range keys { + out = append(out, attrPair{Key: k, Value: merged[k]}) + } + return out, nil +} + +// sdkAttrsIn converts an SDK attribute slice into internal pairs, dropping nil +// elements and elements with a nil Key/Value. Returns the cleaned pairs and the +// raw submitted count. +func sdkAttrsIn[A any](in []A, keyVal func(A) (*string, *string), arn, opName string) ([]attrPair, int) { + out := make([]attrPair, 0, len(in)) + for _, a := range in { + k, v := keyVal(a) + if k == nil || v == nil { + slog.Warn(opName+": skipping invalid attribute element", "arn", arn) + continue + } + out = append(out, attrPair{Key: *k, Value: *v}) + } + return out, len(in) +} + +// sdkAttrsOut converts internal pairs back into a fresh SDK attribute slice via +// mk. Fresh strings are allocated (never aliasing caller input). Returns nil for +// an empty set to match AWS for resources with no submitted attributes. +func sdkAttrsOut[A any](pairs []attrPair, mk func(key, value string) A) []A { + if len(pairs) == 0 { + return nil + } + out := make([]A, 0, len(pairs)) + for _, p := range pairs { + out = append(out, mk(p.Key, p.Value)) + } + return out +} + +func (s *ELBv2ServiceImpl) tgAttrResource(arn, accountID, opName string) attrResource[TargetGroupRecord] { + return attrResource[TargetGroupRecord]{ + arn: arn, + accountID: accountID, + opName: opName, + notFound: awserrors.ErrorELBv2TargetGroupNotFound, + fetch: s.store.GetTargetGroupByArn, + save: s.store.PutTargetGroup, + accountIDOf: func(r *TargetGroupRecord) string { return r.AccountID }, + defaults: func(*TargetGroupRecord) map[string]string { return DefaultTargetGroupAttributes() }, + attrsOf: func(r *TargetGroupRecord) map[string]string { return r.Attributes }, + setAttrs: func(r *TargetGroupRecord, m map[string]string) { r.Attributes = m }, + } +} + +func (s *ELBv2ServiceImpl) lbAttrResource(arn, accountID, opName string) attrResource[LoadBalancerRecord] { + return attrResource[LoadBalancerRecord]{ + arn: arn, + accountID: accountID, + opName: opName, + notFound: awserrors.ErrorELBv2LoadBalancerNotFound, + fetch: s.store.GetLoadBalancerByArn, + save: s.store.PutLoadBalancer, + accountIDOf: func(r *LoadBalancerRecord) string { return r.AccountID }, + defaults: func(r *LoadBalancerRecord) map[string]string { return DefaultLoadBalancerAttributes(r.Type) }, + attrsOf: func(r *LoadBalancerRecord) map[string]string { return r.Attributes }, + setAttrs: func(r *LoadBalancerRecord, m map[string]string) { r.Attributes = m }, + } +} + +func tgAttrKeyVal(a *elbv2.TargetGroupAttribute) (*string, *string) { + if a == nil { + return nil, nil + } + return a.Key, a.Value +} + +func lbAttrKeyVal(a *elbv2.LoadBalancerAttribute) (*string, *string) { + if a == nil { + return nil, nil + } + return a.Key, a.Value +} + +func mkTGAttr(key, value string) *elbv2.TargetGroupAttribute { + return &elbv2.TargetGroupAttribute{Key: aws.String(key), Value: aws.String(value)} +} + +func mkLBAttr(key, value string) *elbv2.LoadBalancerAttribute { + return &elbv2.LoadBalancerAttribute{Key: aws.String(key), Value: aws.String(value)} +} + +func (s *ELBv2ServiceImpl) ModifyTargetGroupAttributes(input *elbv2.ModifyTargetGroupAttributesInput, accountID string) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + if input == nil { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + const op = "ModifyTargetGroupAttributes" + arn := aws.StringValue(input.TargetGroupArn) + pairs, raw := sdkAttrsIn(input.Attributes, tgAttrKeyVal, arn, op) + submitted, err := modifyResourceAttributes(s.tgAttrResource(arn, accountID, op), pairs, raw) + if err != nil { + return nil, err + } + return &elbv2.ModifyTargetGroupAttributesOutput{Attributes: sdkAttrsOut(submitted, mkTGAttr)}, nil +} + +func (s *ELBv2ServiceImpl) DescribeTargetGroupAttributes(input *elbv2.DescribeTargetGroupAttributesInput, accountID string) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + if input == nil { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + const op = "DescribeTargetGroupAttributes" + arn := aws.StringValue(input.TargetGroupArn) + pairs, err := describeResourceAttributes(s.tgAttrResource(arn, accountID, op)) + if err != nil { + return nil, err + } + return &elbv2.DescribeTargetGroupAttributesOutput{Attributes: sdkAttrsOut(pairs, mkTGAttr)}, nil +} + +func (s *ELBv2ServiceImpl) ModifyLoadBalancerAttributes(input *elbv2.ModifyLoadBalancerAttributesInput, accountID string) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { + if input == nil { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + const op = "ModifyLoadBalancerAttributes" + arn := aws.StringValue(input.LoadBalancerArn) + pairs, raw := sdkAttrsIn(input.Attributes, lbAttrKeyVal, arn, op) + submitted, err := modifyResourceAttributes(s.lbAttrResource(arn, accountID, op), pairs, raw) + if err != nil { + return nil, err + } + return &elbv2.ModifyLoadBalancerAttributesOutput{Attributes: sdkAttrsOut(submitted, mkLBAttr)}, nil +} + +func (s *ELBv2ServiceImpl) DescribeLoadBalancerAttributes(input *elbv2.DescribeLoadBalancerAttributesInput, accountID string) (*elbv2.DescribeLoadBalancerAttributesOutput, error) { + if input == nil { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + const op = "DescribeLoadBalancerAttributes" + arn := aws.StringValue(input.LoadBalancerArn) + pairs, err := describeResourceAttributes(s.lbAttrResource(arn, accountID, op)) + if err != nil { + return nil, err + } + return &elbv2.DescribeLoadBalancerAttributesOutput{Attributes: sdkAttrsOut(pairs, mkLBAttr)}, nil +} diff --git a/spinifex/handlers/elbv2/service_impl_health_test.go b/spinifex/handlers/elbv2/service_impl_health_test.go index 1ea659b14..19553fe75 100644 --- a/spinifex/handlers/elbv2/service_impl_health_test.go +++ b/spinifex/handlers/elbv2/service_impl_health_test.go @@ -164,7 +164,7 @@ func TestRegisterTargets_MultipleInstances(t *testing.T) { // TestResetTargetHealthOnStartup_TransitionsAllNonDrainingToInitial verifies // that the startup reset rewrites every non-draining target's HealthState // back to "initial" so a daemon restart can't surface stale "healthy" claims -// before the lb-agent has posted a fresh report. Refs mulga-siv-119. +// before the lb-agent has posted a fresh report. func TestResetTargetHealthOnStartup_TransitionsAllNonDrainingToInitial(t *testing.T) { store := setupTestNATS(t) svc := &ELBv2ServiceImpl{store: store} diff --git a/spinifex/handlers/elbv2/service_impl_lb_network.go b/spinifex/handlers/elbv2/service_impl_lb_network.go new file mode 100644 index 000000000..85d648fe4 --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_lb_network.go @@ -0,0 +1,109 @@ +package handlers_elbv2 + +import ( + "errors" + "log/slog" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// SetIpAddressType sets the load balancer's IP address type. Spinifex ALBs are +// IPv4-only, so the only accepted value is "ipv4"; dualstack variants are +// rejected with InvalidConfigurationRequest. The call is idempotent. +func (s *ELBv2ServiceImpl) SetIpAddressType(input *elbv2.SetIpAddressTypeInput, accountID string) (*elbv2.SetIpAddressTypeOutput, error) { + if input == nil || input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + if input.IpAddressType == nil || *input.IpAddressType == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + if *input.IpAddressType != IPAddressTypeIPv4 { + return nil, errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest) + } + + lb, err := s.store.GetLoadBalancerByArn(*input.LoadBalancerArn) + if err != nil { + slog.Error("SetIpAddressType: failed to get LB", "arn", *input.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if lb == nil || lb.AccountID != accountID { + return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) + } + + if lb.IPAddressType != IPAddressTypeIPv4 { + lb.IPAddressType = IPAddressTypeIPv4 + if err := s.store.PutLoadBalancer(lb); err != nil { + slog.Error("SetIpAddressType: failed to persist LB", "arn", *input.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + } + + return &elbv2.SetIpAddressTypeOutput{ + IpAddressType: aws.String(lb.IPAddressType), + }, nil +} + +// SetSecurityGroups replaces the security groups associated with an +// (application) load balancer. The new groups are re-attached to every ENI the +// ALB spans via ModifyNetworkInterfaceAttribute, which validates them against +// the ENI's VPC and pushes the change to the live data-plane port before the +// record is persisted. +func (s *ELBv2ServiceImpl) SetSecurityGroups(input *elbv2.SetSecurityGroupsInput, accountID string) (*elbv2.SetSecurityGroupsOutput, error) { + if input == nil || input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + if len(input.SecurityGroups) == 0 { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + + lb, err := s.store.GetLoadBalancerByArn(*input.LoadBalancerArn) + if err != nil { + slog.Error("SetSecurityGroups: failed to get LB", "arn", *input.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if lb == nil || lb.AccountID != accountID { + return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) + } + + // NLBs do not support security groups (mirrors CreateLoadBalancer). + if lb.Type == LoadBalancerTypeNetwork { + return nil, errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest) + } + + sgs := make([]string, 0, len(input.SecurityGroups)) + for _, sg := range input.SecurityGroups { + if sg == nil || *sg == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + sgs = append(sgs, *sg) + } + + // Re-attach the new groups to each ALB ENI. This validates the groups + // against the ENI's VPC and fires the live port-SG update; a failure here + // (e.g. unknown SG) aborts before the record is persisted. All ENIs share + // the LB's VPC and groups, so a successful first apply implies the rest. + if s.VPCService != nil { + for _, eniID := range lb.ENIs { + if _, err := s.VPCService.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String(eniID), + Groups: aws.StringSlice(sgs), + }, accountID); err != nil { + slog.Error("SetSecurityGroups: failed to update ENI groups", "arn", *input.LoadBalancerArn, "eni", eniID, "err", err) + return nil, err + } + } + } + + lb.SecurityGroups = sgs + if err := s.store.PutLoadBalancer(lb); err != nil { + slog.Error("SetSecurityGroups: failed to persist LB", "arn", *input.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + return &elbv2.SetSecurityGroupsOutput{ + SecurityGroupIds: aws.StringSlice(sgs), + }, nil +} diff --git a/spinifex/handlers/elbv2/service_impl_lb_network_test.go b/spinifex/handlers/elbv2/service_impl_lb_network_test.go new file mode 100644 index 000000000..ad6906a5a --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_lb_network_test.go @@ -0,0 +1,136 @@ +package handlers_elbv2 + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createLBArn(t *testing.T, svc *ELBv2ServiceImpl, name string) string { + t.Helper() + out, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String(name), + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.LoadBalancers, 1) + return *out.LoadBalancers[0].LoadBalancerArn +} + +func describeLB(t *testing.T, svc *ELBv2ServiceImpl, arn string) *elbv2.LoadBalancer { + t.Helper() + out, err := svc.DescribeLoadBalancers(&elbv2.DescribeLoadBalancersInput{ + LoadBalancerArns: []*string{aws.String(arn)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.LoadBalancers, 1) + return out.LoadBalancers[0] +} + +func TestSetIpAddressType_IPv4Idempotent(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "ipt-lb") + + out, err := svc.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: aws.String(arn), + IpAddressType: aws.String("ipv4"), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, "ipv4", *out.IpAddressType) + assert.Equal(t, "ipv4", *describeLB(t, svc, arn).IpAddressType) +} + +func TestSetIpAddressType_DualstackRejected(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "ipt-ds") + + _, err := svc.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: aws.String(arn), + IpAddressType: aws.String("dualstack"), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2InvalidConfigurationRequest) +} + +func TestSetIpAddressType_MissingParams(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "ipt-mp") + + _, err := svc.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: aws.String(arn), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) + + _, err = svc.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + IpAddressType: aws.String("ipv4"), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) +} + +func TestSetIpAddressType_NotFound(t *testing.T) { + svc := setupTestService(t) + _, err := svc.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/missing/lb-deadbeef"), + IpAddressType: aws.String("ipv4"), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2LoadBalancerNotFound) +} + +func TestSetSecurityGroups_UpdatesRecord(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "sg-lb") + + out, err := svc.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: aws.String(arn), + SecurityGroups: aws.StringSlice([]string{"sg-aaa", "sg-bbb"}), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{"sg-aaa", "sg-bbb"}, aws.StringValueSlice(out.SecurityGroupIds)) + assert.Equal(t, []string{"sg-aaa", "sg-bbb"}, aws.StringValueSlice(describeLB(t, svc, arn).SecurityGroups)) +} + +func TestSetSecurityGroups_EmptyRejected(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "sg-empty") + + _, err := svc.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: aws.String(arn), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) +} + +func TestSetSecurityGroups_NLBRejected(t *testing.T) { + svc := setupTestService(t) + out, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("sg-nlb"), + Type: aws.String("network"), + }, testAccountID) + require.NoError(t, err) + arn := *out.LoadBalancers[0].LoadBalancerArn + + _, err = svc.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: aws.String(arn), + SecurityGroups: aws.StringSlice([]string{"sg-aaa"}), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2InvalidConfigurationRequest) +} + +func TestSetSecurityGroups_CrossAccount(t *testing.T) { + svc := setupTestService(t) + arn := createLBArn(t, svc, "sg-xacct") + + _, err := svc.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: aws.String(arn), + SecurityGroups: aws.StringSlice([]string{"sg-aaa"}), + }, "999999999999") + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2LoadBalancerNotFound) +} diff --git a/spinifex/handlers/elbv2/service_impl_rules.go b/spinifex/handlers/elbv2/service_impl_rules.go index 476dc8d51..56ec4e0da 100644 --- a/spinifex/handlers/elbv2/service_impl_rules.go +++ b/spinifex/handlers/elbv2/service_impl_rules.go @@ -86,6 +86,13 @@ func (s *ELBv2ServiceImpl) CreateRule(input *elbv2.CreateRuleInput, accountID st return nil, errors.New(awserrors.ErrorServerInternal) } + tags := make(map[string]string) + for _, tag := range input.Tags { + if tag.Key != nil && tag.Value != nil { + tags[*tag.Key] = *tag.Value + } + } + record := &RuleRecord{ RuleArn: ruleArn, RuleID: ruleID, @@ -95,6 +102,7 @@ func (s *ELBv2ServiceImpl) CreateRule(input *elbv2.CreateRuleInput, accountID st Actions: actions, AccountID: accountID, CreatedAt: time.Now().UTC(), + Tags: tags, } if err := s.store.PutRule(record); err != nil { @@ -176,10 +184,16 @@ func (s *ELBv2ServiceImpl) DeleteRule(input *elbv2.DeleteRuleInput, accountID st return nil, errors.New(awserrors.ErrorServerInternal) } if rule == nil || rule.AccountID != accountID { - return nil, errors.New(awserrors.ErrorELBv2RuleNotFound) + // Idempotent: AWS ELBv2 delete returns success on an absent (or + // not-owned) rule, so tofu destroy retries converge. + return &elbv2.DeleteRuleOutput{}, nil } - listener, _ := s.store.GetListenerByArn(rule.ListenerArn) + listener, err := s.store.GetListenerByArn(rule.ListenerArn) + if err != nil { + slog.Error("DeleteRule: failed to get listener for reload", "listenerArn", rule.ListenerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } if err := s.store.DeleteRule(rule.RuleID); err != nil { slog.Error("DeleteRule: failed to delete", "ruleId", rule.RuleID, "err", err) @@ -549,25 +563,35 @@ func (s *ELBv2ServiceImpl) validateAndConvertRuleActions(in []*elbv2.Action, lis if a == nil || a.Type == nil { return nil, errors.New(awserrors.ErrorMissingParameter) } - if *a.Type != ActionTypeForward { - // redirect / fixed-response / authenticate-* deferred to mulga-951. + action := listenerActionFromSDK(a) + switch action.Type { + case ActionTypeForward: + if action.TargetGroupArn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + tg, err := s.store.GetTargetGroupByArn(action.TargetGroupArn) + if err != nil { + slog.Error("rule action: failed to get target group", "arn", action.TargetGroupArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if tg == nil { + return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) + } + if !isCompatibleProtocol(listenerProto, tg.Protocol) { + return nil, errors.New(awserrors.ErrorELBv2IncompatibleProtocols) + } + case ActionTypeRedirect: + if err := validateRedirectAction(action.Redirect); err != nil { + return nil, err + } + case ActionTypeFixedResponse: + if action.FixedResponse == nil { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + default: return nil, errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest) } - if a.TargetGroupArn == nil || *a.TargetGroupArn == "" { - return nil, errors.New(awserrors.ErrorMissingParameter) - } - tg, err := s.store.GetTargetGroupByArn(*a.TargetGroupArn) - if err != nil { - slog.Error("rule action: failed to get target group", "arn", *a.TargetGroupArn, "err", err) - return nil, errors.New(awserrors.ErrorServerInternal) - } - if tg == nil { - return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) - } - if !isCompatibleProtocol(listenerProto, tg.Protocol) { - return nil, errors.New(awserrors.ErrorELBv2IncompatibleProtocols) - } - out = append(out, ListenerAction{Type: *a.Type, TargetGroupArn: *a.TargetGroupArn}) + out = append(out, action) } return out, nil } @@ -596,10 +620,7 @@ func ruleRecordToSDK(r *RuleRecord) *elbv2.Rule { rule.Conditions = append(rule.Conditions, ruleConditionToSDK(c)) } for _, a := range r.Actions { - rule.Actions = append(rule.Actions, &elbv2.Action{ - Type: aws.String(a.Type), - TargetGroupArn: aws.String(a.TargetGroupArn), - }) + rule.Actions = append(rule.Actions, listenerActionToSDK(a)) } return rule } @@ -647,10 +668,7 @@ func defaultRuleFromListener(l *ListenerRecord) *elbv2.Rule { IsDefault: aws.Bool(true), } for _, a := range l.DefaultActions { - rule.Actions = append(rule.Actions, &elbv2.Action{ - Type: aws.String(a.Type), - TargetGroupArn: aws.String(a.TargetGroupArn), - }) + rule.Actions = append(rule.Actions, listenerActionToSDK(a)) } return rule } diff --git a/spinifex/handlers/elbv2/service_impl_rules_extra_test.go b/spinifex/handlers/elbv2/service_impl_rules_extra_test.go index 3a1ba79ae..1f8c03f21 100644 --- a/spinifex/handlers/elbv2/service_impl_rules_extra_test.go +++ b/spinifex/handlers/elbv2/service_impl_rules_extra_test.go @@ -87,9 +87,10 @@ func TestDeleteRule_InputValidation(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "MissingParameter") - _, err = env.svc.DeleteRule(&elbv2.DeleteRuleInput{RuleArn: aws.String("arn:none")}, testAccountID) - require.Error(t, err) - assert.Contains(t, err.Error(), "RuleNotFound") + // AWS ELBv2 delete is idempotent: absent rule -> success, not NotFound. + out, err := env.svc.DeleteRule(&elbv2.DeleteRuleInput{RuleArn: aws.String("arn:none")}, testAccountID) + require.NoError(t, err) + assert.NotNil(t, out) } func TestDescribeRules_InputValidation(t *testing.T) { @@ -377,11 +378,23 @@ func TestHAProxyRender_UnsupportedField(t *testing.T) { require.Error(t, err) } -func TestBuildHAProxyRule_RejectsNonForward(t *testing.T) { - _, err := buildHAProxyRule(&RuleRecord{ +func TestBuildHAProxyRule_RoutesToBackend(t *testing.T) { + rule, err := buildHAProxyRule(&RuleRecord{ + RuleID: "r1", + Conditions: []RuleCondition{{Field: RuleFieldPathPattern, Values: []string{"/a"}}}, + }, "bk_target") + require.NoError(t, err) + assert.Equal(t, "bk_target", rule.Backend) +} + +func TestRegisterRuleBackend_RejectsUnknownAction(t *testing.T) { + noopBackend := func(string) {} + noopFixed := func(string, *FixedResponseAction) {} + noopRedirect := func(string, *HAProxyRedirect) {} + _, err := registerRuleBackend(&RuleRecord{ RuleID: "r1", - Actions: []ListenerAction{{Type: "redirect"}}, - }) + Actions: []ListenerAction{{Type: "authenticate-cognito"}}, + }, &ListenerRecord{Protocol: ProtocolHTTP}, noopBackend, noopFixed, noopRedirect) require.Error(t, err) } diff --git a/spinifex/handlers/elbv2/service_impl_rules_test.go b/spinifex/handlers/elbv2/service_impl_rules_test.go index 384d5951f..4bc334d7a 100644 --- a/spinifex/handlers/elbv2/service_impl_rules_test.go +++ b/spinifex/handlers/elbv2/service_impl_rules_test.go @@ -70,6 +70,46 @@ func TestCreateRule_PathPattern(t *testing.T) { assert.Contains(t, *out.Rules[0].RuleArn, ":listener-rule/") } +// TestDeleteLoadBalancer_CascadesRuleDeletion is a regression guard: +// DeleteLoadBalancer must cascade through listener rule deletion. Previously it +// called store.DeleteListener directly, bypassing the rule cascade, so a rule's +// target group stayed pinned as ResourceInUse and was permanently undeletable. +func TestDeleteLoadBalancer_CascadesRuleDeletion(t *testing.T) { + env := newRuleTestEnv(t, "del-cascade") + + // Rule on the listener forwards to the alt TG. + _, err := env.svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: aws.String(env.listenerArn), + Priority: aws.Int64(10), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/api/*"})}}, + Actions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: aws.String(env.tgAltArn)}}, + }, testAccountID) + require.NoError(t, err) + + // Tear down the LB. + _, err = env.svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{ + LoadBalancerArn: aws.String(env.lbArn), + }, testAccountID) + require.NoError(t, err) + + // LB deletion must cascade rule deletion — no rule may survive. + rules, err := env.svc.store.ListRules() + require.NoError(t, err) + assert.Empty(t, rules, "LB deletion must cascade rule deletion") + + // Both target groups must now be deletable: neither an orphan rule nor a + // stale listener default action may pin them as ResourceInUse. + _, err = env.svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{ + TargetGroupArn: aws.String(env.tgAltArn), + }, testAccountID) + require.NoError(t, err, "rule target group must not be pinned after LB deletion") + + _, err = env.svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{ + TargetGroupArn: aws.String(env.tgArn), + }, testAccountID) + require.NoError(t, err, "listener default-action target group must not be pinned after LB deletion") +} + func TestCreateRule_PriorityInUse(t *testing.T) { env := newRuleTestEnv(t, "cr-pri") @@ -107,14 +147,75 @@ func TestCreateRule_InvalidPriority(t *testing.T) { } } -func TestCreateRule_RejectsRedirect(t *testing.T) { +func TestCreateRule_RedirectAction(t *testing.T) { + env := newRuleTestEnv(t, "cr-redir") + + out, err := env.svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: aws.String(env.listenerArn), + Priority: aws.Int64(1), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/old/*"})}}, + Actions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{ + Protocol: aws.String("HTTPS"), + StatusCode: aws.String("HTTP_301"), + }, + }}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Rules, 1) + require.Len(t, out.Rules[0].Actions, 1) + assert.Equal(t, "redirect", *out.Rules[0].Actions[0].Type) + require.NotNil(t, out.Rules[0].Actions[0].RedirectConfig) + assert.Equal(t, "HTTP_301", *out.Rules[0].Actions[0].RedirectConfig.StatusCode) +} + +func TestCreateRule_FixedResponseAction(t *testing.T) { + env := newRuleTestEnv(t, "cr-fixed") + + out, err := env.svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: aws.String(env.listenerArn), + Priority: aws.Int64(1), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/down"})}}, + Actions: []*elbv2.Action{{ + Type: aws.String("fixed-response"), + FixedResponseConfig: &elbv2.FixedResponseActionConfig{ + StatusCode: aws.String("503"), + ContentType: aws.String("text/plain"), + MessageBody: aws.String("maintenance"), + }, + }}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Rules, 1) + assert.Equal(t, "fixed-response", *out.Rules[0].Actions[0].Type) + require.NotNil(t, out.Rules[0].Actions[0].FixedResponseConfig) +} + +func TestCreateRule_RejectsBadRedirectStatus(t *testing.T) { + env := newRuleTestEnv(t, "cr-badredir") + + _, err := env.svc.CreateRule(&elbv2.CreateRuleInput{ + ListenerArn: aws.String(env.listenerArn), + Priority: aws.Int64(1), + Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/a"})}}, + Actions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{StatusCode: aws.String("HTTP_418")}, + }}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + +func TestCreateRule_RejectsUnknownAction(t *testing.T) { env := newRuleTestEnv(t, "cr-rej") _, err := env.svc.CreateRule(&elbv2.CreateRuleInput{ ListenerArn: aws.String(env.listenerArn), Priority: aws.Int64(1), Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/a"})}}, - Actions: []*elbv2.Action{{Type: aws.String("redirect")}}, + Actions: []*elbv2.Action{{Type: aws.String("authenticate-cognito")}}, }, testAccountID) require.Error(t, err) assert.Contains(t, err.Error(), "InvalidConfigurationRequest") diff --git a/spinifex/handlers/elbv2/service_impl_tags.go b/spinifex/handlers/elbv2/service_impl_tags.go new file mode 100644 index 000000000..5238397d2 --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_tags.go @@ -0,0 +1,185 @@ +package handlers_elbv2 + +import ( + "errors" + "log/slog" + "maps" + + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// taggable bundles a resource's current tags with a persist closure so the +// AddTags/RemoveTags mutation logic is identical across resource types. +type taggable struct { + tags map[string]string + owner string + save func(map[string]string) error +} + +// resolveTaggable loads the ELBv2 resource named by arn and returns a handle for +// reading and persisting its tags. found is false when the resource doesn't +// exist; notFoundError carries the per-type API error for that case. Store +// failures are logged and returned as ErrorServerInternal. +func (s *ELBv2ServiceImpl) resolveTaggable(arn string) (h taggable, notFoundError string, found bool, err error) { + resourceType, terr := elbv2ResourceTypeFromArn(arn) + if terr != nil { + return taggable{}, "", false, terr + } + + switch resourceType { + case elbv2ResourceLoadBalancer: + notFoundError = awserrors.ErrorELBv2LoadBalancerNotFound + lb, e := s.store.GetLoadBalancerByArn(arn) + if e != nil { + slog.Error("resolveTaggable: failed to get LB", "arn", arn, "err", e) + return taggable{}, notFoundError, false, errors.New(awserrors.ErrorServerInternal) + } + if lb != nil { + found = true + h = taggable{tags: lb.Tags, owner: lb.AccountID, save: func(t map[string]string) error { + lb.Tags = t + return s.store.PutLoadBalancer(lb) + }} + } + case elbv2ResourceTargetGroup: + notFoundError = awserrors.ErrorELBv2TargetGroupNotFound + tg, e := s.store.GetTargetGroupByArn(arn) + if e != nil { + slog.Error("resolveTaggable: failed to get target group", "arn", arn, "err", e) + return taggable{}, notFoundError, false, errors.New(awserrors.ErrorServerInternal) + } + if tg != nil { + found = true + h = taggable{tags: tg.Tags, owner: tg.AccountID, save: func(t map[string]string) error { + tg.Tags = t + return s.store.PutTargetGroup(tg) + }} + } + case elbv2ResourceListener: + notFoundError = awserrors.ErrorELBv2ListenerNotFound + l, e := s.store.GetListenerByArn(arn) + if e != nil { + slog.Error("resolveTaggable: failed to get listener", "arn", arn, "err", e) + return taggable{}, notFoundError, false, errors.New(awserrors.ErrorServerInternal) + } + if l != nil { + found = true + h = taggable{tags: l.Tags, owner: l.AccountID, save: func(t map[string]string) error { + l.Tags = t + return s.store.PutListener(l) + }} + } + case elbv2ResourceListenerRule: + notFoundError = awserrors.ErrorELBv2RuleNotFound + r, e := s.store.GetRuleByArn(arn) + if e != nil { + slog.Error("resolveTaggable: failed to get rule", "arn", arn, "err", e) + return taggable{}, notFoundError, false, errors.New(awserrors.ErrorServerInternal) + } + if r != nil { + found = true + h = taggable{tags: r.Tags, owner: r.AccountID, save: func(t map[string]string) error { + r.Tags = t + return s.store.PutRule(r) + }} + } + } + + return h, notFoundError, found, nil +} + +// AddTags adds or overwrites tags on one or more ELBv2 resources. Tags are +// validated up-front so a malformed key can't leave a partial apply spanning +// the ResourceArns. Cross-account or unknown ARNs yield the per-resource +// not-found error. +func (s *ELBv2ServiceImpl) AddTags(input *elbv2.AddTagsInput, accountID string) (*elbv2.AddTagsOutput, error) { + if input == nil || len(input.ResourceArns) == 0 || len(input.Tags) == 0 { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + + newTags := make(map[string]string, len(input.Tags)) + for _, t := range input.Tags { + if t == nil || t.Key == nil || *t.Key == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + v := "" + if t.Value != nil { + v = *t.Value + } + newTags[*t.Key] = v + } + + for _, arnPtr := range input.ResourceArns { + if arnPtr == nil || *arnPtr == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + arn := *arnPtr + + h, notFoundError, found, err := s.resolveTaggable(arn) + if err != nil { + return nil, err + } + if !found || h.owner != accountID { + return nil, errors.New(notFoundError) + } + + merged := h.tags + if merged == nil { + merged = make(map[string]string, len(newTags)) + } + maps.Copy(merged, newTags) + if err := h.save(merged); err != nil { + slog.Error("AddTags: failed to persist", "arn", arn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + } + + return &elbv2.AddTagsOutput{}, nil +} + +// RemoveTags removes the named tag keys from one or more ELBv2 resources. +// Removal is idempotent — keys absent from a resource are silently ignored, +// matching AWS. Cross-account or unknown ARNs yield the per-resource not-found +// error. +func (s *ELBv2ServiceImpl) RemoveTags(input *elbv2.RemoveTagsInput, accountID string) (*elbv2.RemoveTagsOutput, error) { + if input == nil || len(input.ResourceArns) == 0 || len(input.TagKeys) == 0 { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + + keys := make([]string, 0, len(input.TagKeys)) + for _, k := range input.TagKeys { + if k == nil || *k == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + keys = append(keys, *k) + } + + for _, arnPtr := range input.ResourceArns { + if arnPtr == nil || *arnPtr == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + arn := *arnPtr + + h, notFoundError, found, err := s.resolveTaggable(arn) + if err != nil { + return nil, err + } + if !found || h.owner != accountID { + return nil, errors.New(notFoundError) + } + + if len(h.tags) == 0 { + continue + } + for _, k := range keys { + delete(h.tags, k) + } + if err := h.save(h.tags); err != nil { + slog.Error("RemoveTags: failed to persist", "arn", arn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + } + + return &elbv2.RemoveTagsOutput{}, nil +} diff --git a/spinifex/handlers/elbv2/service_impl_tags_test.go b/spinifex/handlers/elbv2/service_impl_tags_test.go new file mode 100644 index 000000000..bd54509e6 --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_tags_test.go @@ -0,0 +1,200 @@ +package handlers_elbv2 + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// tagsFor is a small helper that reads back a resource's tags via DescribeTags +// as a plain map for assertions. +func tagsFor(t *testing.T, svc *ELBv2ServiceImpl, arn string) map[string]string { + t.Helper() + out, err := svc.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: []*string{aws.String(arn)}}, testAccountID) + require.NoError(t, err) + require.Len(t, out.TagDescriptions, 1) + m := make(map[string]string) + for _, tag := range out.TagDescriptions[0].Tags { + m[*tag.Key] = *tag.Value + } + return m +} + +func TestAddTags_LoadBalancer(t *testing.T) { + svc := setupTestService(t) + lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("addtags-lb"), + Tags: []*elbv2.Tag{{Key: aws.String("Env"), Value: aws.String("dev")}}, + }, testAccountID) + require.NoError(t, err) + arn := *lbOut.LoadBalancers[0].LoadBalancerArn + + _, err = svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + Tags: []*elbv2.Tag{ + {Key: aws.String("App"), Value: aws.String("nginx")}, + {Key: aws.String("Env"), Value: aws.String("prod")}, // overwrite + }, + }, testAccountID) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"App": "nginx", "Env": "prod"}, tagsFor(t, svc, arn)) +} + +func TestAddTags_Listener(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-lst-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("addtags-lst-tg")}, testAccountID) + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{ + {Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}, + }, + }, testAccountID) + require.NoError(t, err) + arn := *lstOut.Listeners[0].ListenerArn + + _, err = svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + Tags: []*elbv2.Tag{{Key: aws.String("team"), Value: aws.String("platform")}}, + }, testAccountID) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"team": "platform"}, tagsFor(t, svc, arn)) +} + +func TestAddTags_CreateTimeTagsPreserved(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-tg-lb")}, testAccountID) + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("addtags-tg"), + Tags: []*elbv2.Tag{{Key: aws.String("owner"), Value: aws.String("alice")}}, + }, testAccountID) + require.NoError(t, err) + _ = lbOut + arn := *tgOut.TargetGroups[0].TargetGroupArn + + _, err = svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + Tags: []*elbv2.Tag{{Key: aws.String("cost-center"), Value: aws.String("42")}}, + }, testAccountID) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"owner": "alice", "cost-center": "42"}, tagsFor(t, svc, arn)) +} + +func TestAddTags_NotFound(t *testing.T) { + svc := setupTestService(t) + _, err := svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/missing/lb-deadbeef")}, + Tags: []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2LoadBalancerNotFound) +} + +func TestAddTags_CrossAccount(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-xacct")}, testAccountID) + arn := *lbOut.LoadBalancers[0].LoadBalancerArn + + _, err := svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + Tags: []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, "999999999999") + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2LoadBalancerNotFound) + + // Owner's tags untouched. + assert.Empty(t, tagsFor(t, svc, arn)) +} + +func TestAddTags_MissingParams(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-mp")}, testAccountID) + arn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err := svc.AddTags(&elbv2.AddTagsInput{ResourceArns: []*string{arn}}, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) + + _, err = svc.AddTags(&elbv2.AddTagsInput{Tags: []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}}, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) +} + +func TestAddTags_EmptyKeyRejected(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-ek")}, testAccountID) + arn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err := svc.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{arn}, + Tags: []*elbv2.Tag{{Key: aws.String(""), Value: aws.String("v")}}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) +} + +func TestRemoveTags_LoadBalancer(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("rmtags-lb"), + Tags: []*elbv2.Tag{ + {Key: aws.String("App"), Value: aws.String("nginx")}, + {Key: aws.String("Env"), Value: aws.String("prod")}, + }, + }, testAccountID) + arn := *lbOut.LoadBalancers[0].LoadBalancerArn + + _, err := svc.RemoveTags(&elbv2.RemoveTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + TagKeys: []*string{aws.String("Env")}, + }, testAccountID) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"App": "nginx"}, tagsFor(t, svc, arn)) +} + +func TestRemoveTags_Idempotent(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("rmtags-idem"), + Tags: []*elbv2.Tag{{Key: aws.String("App"), Value: aws.String("nginx")}}, + }, testAccountID) + arn := *lbOut.LoadBalancers[0].LoadBalancerArn + + // Removing an absent key is a no-op, not an error. + _, err := svc.RemoveTags(&elbv2.RemoveTagsInput{ + ResourceArns: []*string{aws.String(arn)}, + TagKeys: []*string{aws.String("DoesNotExist")}, + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, map[string]string{"App": "nginx"}, tagsFor(t, svc, arn)) +} + +func TestRemoveTags_NotFound(t *testing.T) { + svc := setupTestService(t) + _, err := svc.RemoveTags(&elbv2.RemoveTagsInput{ + ResourceArns: []*string{aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/missing/tg-deadbeef")}, + TagKeys: []*string{aws.String("k")}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorELBv2TargetGroupNotFound) +} + +func TestRemoveTags_MissingParams(t *testing.T) { + svc := setupTestService(t) + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("rmtags-mp")}, testAccountID) + arn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err := svc.RemoveTags(&elbv2.RemoveTagsInput{ResourceArns: []*string{arn}}, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter) +} diff --git a/spinifex/handlers/elbv2/service_impl_test.go b/spinifex/handlers/elbv2/service_impl_test.go index 968b86a65..3ffbbe7cf 100644 --- a/spinifex/handlers/elbv2/service_impl_test.go +++ b/spinifex/handlers/elbv2/service_impl_test.go @@ -142,7 +142,7 @@ func TestCreateLoadBalancer_NetworkType_RejectsSecurityGroups(t *testing.T) { assert.Contains(t, err.Error(), "InvalidConfigurationRequest") } -func TestCreateLoadBalancer_CrossZoneAttributes(t *testing.T) { +func TestCreateLoadBalancer_NLBCrossZoneAttribute(t *testing.T) { svc := setupTestService(t) // NLB: no stored attributes, Describe should return default cross-zone=false. @@ -164,6 +164,10 @@ func TestCreateLoadBalancer_CrossZoneAttributes(t *testing.T) { nlbAttrs[*a.Key] = *a.Value } assert.Equal(t, "false", nlbAttrs["load_balancing.cross_zone.enabled"]) +} + +func TestCreateLoadBalancer_ALBCrossZoneAttribute(t *testing.T) { + svc := setupTestService(t) // ALB: no stored attributes either, Describe should return default cross-zone=true. albOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ @@ -229,15 +233,16 @@ func TestDeleteLoadBalancer(t *testing.T) { assert.Empty(t, desc.LoadBalancers) } -func TestDeleteLoadBalancer_NotFound(t *testing.T) { +func TestDeleteLoadBalancer_IdempotentOnAbsent(t *testing.T) { svc := setupTestService(t) - _, err := svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{ + out, err := svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{ LoadBalancerArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/nope/xyz"), }, testAccountID) - assert.Error(t, err) - assert.Contains(t, err.Error(), "LoadBalancerNotFound") + // AWS ELBv2 delete is idempotent: absent LB -> success, not NotFound. + require.NoError(t, err) + assert.NotNil(t, out) } func TestDeleteLoadBalancer_CleansUpListeners(t *testing.T) { @@ -696,14 +701,15 @@ func TestDeleteTargetGroup_InUse(t *testing.T) { assert.Contains(t, err.Error(), "ResourceInUse") } -func TestDeleteTargetGroup_NotFound(t *testing.T) { +func TestDeleteTargetGroup_IdempotentOnAbsent(t *testing.T) { svc := setupTestService(t) - _, err := svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{ + out, err := svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{ TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/nope/xyz"), }, testAccountID) - assert.Error(t, err) - assert.Contains(t, err.Error(), "TargetGroupNotFound") + // AWS ELBv2 delete is idempotent: absent TG -> success, not NotFound. + require.NoError(t, err) + assert.NotNil(t, out) } func TestDescribeTargetGroups_FilterByLBArn(t *testing.T) { @@ -763,6 +769,93 @@ func TestRegisterTargets(t *testing.T) { assert.Equal(t, int64(8080), *health.TargetHealthDescriptions[1].Target.Port) } +func TestRegisterTargets_IPType(t *testing.T) { + svc := setupTestService(t) + + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("ip-tg"), + Port: aws.Int64(80), + TargetType: aws.String("ip"), + }, testAccountID) + require.NoError(t, err) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + assert.Equal(t, "ip", *tgOut.TargetGroups[0].TargetType) + + _, err = svc.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: tgArn, + Targets: []*elbv2.TargetDescription{ + {Id: aws.String("10.0.1.20")}, + {Id: aws.String("10.0.1.21"), Port: aws.Int64(8080)}, + }, + }, testAccountID) + require.NoError(t, err) + + // ip targets must carry the supplied IP as PrivateIP, not an empty + // ENI-resolution result — otherwise HAProxy/health-check silently drop them. + tg, err := svc.store.GetTargetGroupByArn(*tgArn) + require.NoError(t, err) + require.Len(t, tg.Targets, 2) + ipByID := make(map[string]string) + for _, target := range tg.Targets { + ipByID[target.Id] = target.PrivateIP + } + assert.Equal(t, "10.0.1.20", ipByID["10.0.1.20"]) + assert.Equal(t, "10.0.1.21", ipByID["10.0.1.21"]) + + health, err := svc.DescribeTargetHealth(&elbv2.DescribeTargetHealthInput{ + TargetGroupArn: tgArn, + }, testAccountID) + require.NoError(t, err) + require.Len(t, health.TargetHealthDescriptions, 2) +} + +func TestRegisterTargets_IPType_RejectsNonIP(t *testing.T) { + svc := setupTestService(t) + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("ip-tg-bad"), + TargetType: aws.String("ip"), + }, testAccountID) + require.NoError(t, err) + + _, err = svc.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, + Targets: []*elbv2.TargetDescription{{Id: aws.String("i-notanip")}}, + }, testAccountID) + assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) +} + +func TestRegisterTargets_InstanceType_RejectsIP(t *testing.T) { + svc := setupTestService(t) + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("inst-tg-badid"), + }, testAccountID) + require.NoError(t, err) + + _, err = svc.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, + Targets: []*elbv2.TargetDescription{{Id: aws.String("10.0.0.5")}}, + }, testAccountID) + assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) +} + +func TestCreateTargetGroup_RejectsUnsupportedTargetType(t *testing.T) { + svc := setupTestService(t) + _, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("lambda-tg"), + TargetType: aws.String("lambda"), + }, testAccountID) + assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) +} + +func TestCreateTargetGroup_DefaultsToInstanceType(t *testing.T) { + svc := setupTestService(t) + tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("default-tt"), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, "instance", *tgOut.TargetGroups[0].TargetType) +} + func TestRegisterTargets_Idempotent(t *testing.T) { svc := setupTestService(t) @@ -865,6 +958,127 @@ func TestCreateListener(t *testing.T) { assert.Equal(t, "forward", *l.DefaultActions[0].Type) } +func TestCreateListener_RedirectDefault(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("redir-lb")}, testAccountID) + + out, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{ + Protocol: aws.String("HTTPS"), + Port: aws.String("443"), + StatusCode: aws.String("HTTP_301"), + }, + }}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Listeners, 1) + require.Len(t, out.Listeners[0].DefaultActions, 1) + a := out.Listeners[0].DefaultActions[0] + assert.Equal(t, "redirect", *a.Type) + require.NotNil(t, a.RedirectConfig) + assert.Equal(t, "HTTPS", *a.RedirectConfig.Protocol) + assert.Equal(t, "HTTP_301", *a.RedirectConfig.StatusCode) +} + +func TestCreateListener_RedirectFullFields(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("redirfull-lb")}, testAccountID) + + out, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{ + Protocol: aws.String("HTTPS"), + Host: aws.String("new.example.com"), + Port: aws.String("8443"), + Path: aws.String("/moved"), + Query: aws.String("ref=1"), + StatusCode: aws.String("HTTP_302"), + }, + }}, + }, testAccountID) + require.NoError(t, err) + + rc := out.Listeners[0].DefaultActions[0].RedirectConfig + require.NotNil(t, rc) + assert.Equal(t, "new.example.com", *rc.Host) + assert.Equal(t, "8443", *rc.Port) + assert.Equal(t, "/moved", *rc.Path) + assert.Equal(t, "ref=1", *rc.Query) + assert.Equal(t, "HTTP_302", *rc.StatusCode) + + // Read back through Describe to exercise the stored→SDK path. + desc, err := svc.DescribeListeners(&elbv2.DescribeListenersInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.Listeners, 1) + assert.Equal(t, "new.example.com", *desc.Listeners[0].DefaultActions[0].RedirectConfig.Host) +} + +func TestModifyListener_ToRedirect(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-redir-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-redir-tg")}, testAccountID) + + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + require.NoError(t, err) + + out, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{Protocol: aws.String("HTTPS"), StatusCode: aws.String("HTTP_301")}, + }}, + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, "redirect", *out.Listeners[0].DefaultActions[0].Type) + + // A bad redirect on modify is rejected. + _, err = svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{StatusCode: aws.String("HTTP_999")}, + }}, + }, testAccountID) + require.Error(t, err) +} + +func TestCreateListener_RejectsBadRedirect(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("badredir-lb")}, testAccountID) + + _, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{StatusCode: aws.String("HTTP_500")}, + }}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + func TestCreateListener_DuplicatePort(t *testing.T) { svc := setupTestService(t) @@ -924,14 +1138,15 @@ func TestDeleteListener(t *testing.T) { assert.Empty(t, desc.Listeners) } -func TestDeleteListener_NotFound(t *testing.T) { +func TestDeleteListener_IdempotentOnAbsent(t *testing.T) { svc := setupTestService(t) - _, err := svc.DeleteListener(&elbv2.DeleteListenerInput{ + out, err := svc.DeleteListener(&elbv2.DeleteListenerInput{ ListenerArn: aws.String("arn:nonexistent"), }, testAccountID) - assert.Error(t, err) - assert.Contains(t, err.Error(), "ListenerNotFound") + // AWS ELBv2 delete is idempotent: absent listener -> success, not NotFound. + require.NoError(t, err) + assert.NotNil(t, out) } func TestModifyListener_Port(t *testing.T) { @@ -2181,7 +2396,11 @@ func TestDescribeLoadBalancerAttributes_ALBDefaults(t *testing.T) { for _, a := range out.Attributes { attrMap[*a.Key] = *a.Value } - // ALB default cross-zone is true — comes from the per-type default, not seeding. + // Every default key/value must round-trip through Describe, not just + // cross-zone. ALB default cross-zone is true — from the per-type default. + for k, v := range defaults { + assert.Equal(t, v, attrMap[k], "default mismatch for key %s", k) + } assert.Equal(t, "true", attrMap["load_balancing.cross_zone.enabled"]) } @@ -2273,8 +2492,8 @@ func TestDefaultLoadBalancerAttributes_NLBCoversExpectedKeys(t *testing.T) { } // TestModifyLoadBalancerAttributes_AcceptsConnectionLogsKey is a regression -// guard for mulga-931: terraform sends connection_logs.s3.enabled on every -// aws_lb apply and the handler must accept it. +// guard: terraform sends connection_logs.s3.enabled on every aws_lb apply and +// the handler must accept it. func TestModifyLoadBalancerAttributes_AcceptsConnectionLogsKey(t *testing.T) { svc := setupTestService(t) lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ @@ -2295,70 +2514,33 @@ func TestModifyLoadBalancerAttributes_AcceptsConnectionLogsKey(t *testing.T) { require.NoError(t, err, "terraform-sent attribute keys must be accepted") } -func TestModifyDescribeTargetGroupAttributes_RoundTrip(t *testing.T) { +// TestModifyLoadBalancerAttributes_EmptyStringClears verifies that a non-nil +// empty-string Value is treated as a real value, not skipped as invalid. AWS +// uses "" to clear access_logs.s3.bucket, so the handler must persist it. +func TestModifyLoadBalancerAttributes_EmptyStringClears(t *testing.T) { svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-attr-rt")}, testAccountID) + lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("alb-clear")}, testAccountID) require.NoError(t, err) - arn := tgOut.TargetGroups[0].TargetGroupArn + arn := lbOut.LoadBalancers[0].LoadBalancerArn - modOut, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("deregistration_delay.timeout_seconds"), Value: aws.String("60")}, - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, + // Set a bucket, then clear it with an empty string. + _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ + LoadBalancerArn: arn, + Attributes: []*elbv2.LoadBalancerAttribute{ + {Key: aws.String("access_logs.s3.bucket"), Value: aws.String("my-logs")}, }, }, testAccountID) require.NoError(t, err) - // Assert the exact echoed key/value pairs, not just the length — a regression - // that pointed at the wrong source or dropped Value would pass a length check. - require.Len(t, modOut.Attributes, 2) - modMap := make(map[string]string, len(modOut.Attributes)) - for _, a := range modOut.Attributes { - require.NotNil(t, a.Key) - require.NotNil(t, a.Value) - modMap[*a.Key] = *a.Value - } - assert.Equal(t, "60", modMap["deregistration_delay.timeout_seconds"]) - assert.Equal(t, "true", modMap["stickiness.enabled"]) - - descOut, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap := make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value - } - assert.Equal(t, "60", attrMap["deregistration_delay.timeout_seconds"]) - assert.Equal(t, "true", attrMap["stickiness.enabled"]) - // Unmodified defaults should still be present - assert.Equal(t, "lb_cookie", attrMap["stickiness.type"]) -} - -func TestModifyDescribeLoadBalancerAttributes_RoundTrip(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-attr-rt")}, testAccountID) - require.NoError(t, err) - arn := lbOut.LoadBalancers[0].LoadBalancerArn modOut, err := svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ LoadBalancerArn: arn, Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("120")}, - {Key: aws.String("deletion_protection.enabled"), Value: aws.String("true")}, + {Key: aws.String("access_logs.s3.bucket"), Value: aws.String("")}, }, }, testAccountID) require.NoError(t, err) - // Assert the exact echoed key/value pairs, not just the length. - require.Len(t, modOut.Attributes, 2) - modMap := make(map[string]string, len(modOut.Attributes)) - for _, a := range modOut.Attributes { - require.NotNil(t, a.Key) - require.NotNil(t, a.Value) - modMap[*a.Key] = *a.Value - } - assert.Equal(t, "120", modMap["idle_timeout.timeout_seconds"]) - assert.Equal(t, "true", modMap["deletion_protection.enabled"]) + require.Len(t, modOut.Attributes, 1, "empty-string Value must be echoed, not skipped") + assert.Equal(t, "", *modOut.Attributes[0].Value) descOut, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ LoadBalancerArn: arn, @@ -2368,560 +2550,455 @@ func TestModifyDescribeLoadBalancerAttributes_RoundTrip(t *testing.T) { for _, a := range descOut.Attributes { attrMap[*a.Key] = *a.Value } - assert.Equal(t, "120", attrMap["idle_timeout.timeout_seconds"]) - assert.Equal(t, "true", attrMap["deletion_protection.enabled"]) - // Unmodified defaults should still be present - assert.Equal(t, "true", attrMap["routing.http2.enabled"]) + assert.Equal(t, "", attrMap["access_logs.s3.bucket"], "empty-string clear must persist") } -func TestModifyTargetGroupAttributes_NotFound(t *testing.T) { - svc := setupTestService(t) - _, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/missing/tg-missing"), - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorELBv2TargetGroupNotFound) +// --- Attribute mirror-pair tests (table-driven over TG/LB) --- +// +// ModifyTargetGroupAttributes/ModifyLoadBalancerAttributes (and their Describe +// counterparts) are mirror pairs differing only by record type, store methods, +// default set and not-found error. The behavioural tests below run once per +// kind via t.Run instead of being hand-copied per resource. + +// rawAttr models one submitted SDK attribute, including the invalid shapes a +// handler must skip: a nil slice element, a nil Key, or a nil Value. +type rawAttr struct { + nilElem bool + key *string + val *string } -func TestDescribeTargetGroupAttributes_NotFound(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/missing/tg-missing"), - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorELBv2TargetGroupNotFound) -} +func kvAttr(k, v string) rawAttr { return rawAttr{key: aws.String(k), val: aws.String(v)} } -func TestModifyLoadBalancerAttributes_NotFound(t *testing.T) { - svc := setupTestService(t) - _, err := svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/missing/lb-missing"), - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("30")}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorELBv2LoadBalancerNotFound) -} - -func TestDescribeLoadBalancerAttributes_NotFound(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/missing/lb-missing"), - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorELBv2LoadBalancerNotFound) -} - -func TestModifyTargetGroupAttributes_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorMissingParameter) -} - -func TestDescribeTargetGroupAttributes_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{}, testAccountID) - assert.EqualError(t, err, awserrors.ErrorMissingParameter) -} - -func TestModifyLoadBalancerAttributes_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("30")}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorMissingParameter) -} - -func TestDescribeLoadBalancerAttributes_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{}, testAccountID) - assert.EqualError(t, err, awserrors.ErrorMissingParameter) -} - -func TestModifyTargetGroupAttributes_WrongAccount(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-attr-wrong-acct")}, testAccountID) - require.NoError(t, err) - - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, "999999999999") - assert.EqualError(t, err, awserrors.ErrorELBv2TargetGroupNotFound) -} - -func TestDescribeTargetGroupAttributes_WrongAccount(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-desc-attr-wrong-acct")}, testAccountID) - require.NoError(t, err) - - _, err = svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, - }, "999999999999") - assert.EqualError(t, err, awserrors.ErrorELBv2TargetGroupNotFound) -} - -func TestModifyLoadBalancerAttributes_WrongAccount(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-attr-wrong-acct")}, testAccountID) - require.NoError(t, err) - - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("30")}, - }, - }, "999999999999") - assert.EqualError(t, err, awserrors.ErrorELBv2LoadBalancerNotFound) -} - -func TestDescribeLoadBalancerAttributes_WrongAccount(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-desc-attr-wrong-acct")}, testAccountID) - require.NoError(t, err) - - _, err = svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, - }, "999999999999") - assert.EqualError(t, err, awserrors.ErrorELBv2LoadBalancerNotFound) -} - -// TestModifyTargetGroupAttributes_SkipsInvalidEntries verifies that nil slice -// elements, nil Keys, and nil Values are skipped (with a warning) rather than -// panicking or being silently dropped. Valid attributes in the same call must -// still be applied. -func TestModifyTargetGroupAttributes_SkipsInvalidEntries(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-attr-skip")}, testAccountID) - require.NoError(t, err) - arn := tgOut.TargetGroups[0].TargetGroupArn - - modOut, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - nil, // nil element must not panic - {Key: nil, Value: aws.String("v")}, - {Key: aws.String("k"), Value: nil}, - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - require.NoError(t, err) - // Only the one valid attribute should be returned. - require.Len(t, modOut.Attributes, 1) - assert.Equal(t, "stickiness.enabled", *modOut.Attributes[0].Key) - assert.Equal(t, "true", *modOut.Attributes[0].Value) - - // The valid attribute should have been persisted. - descOut, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap := make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value +func kvAttrs(kv ...[2]string) []rawAttr { + out := make([]rawAttr, len(kv)) + for i, p := range kv { + out[i] = kvAttr(p[0], p[1]) } - assert.Equal(t, "true", attrMap["stickiness.enabled"]) + return out } -// TestModifyLoadBalancerAttributes_SkipsInvalidEntries mirrors the TG case for -// the LB handler: nil elements and nil Key/Value fields must be skipped, not -// panic or swallow valid attributes in the same request. -func TestModifyLoadBalancerAttributes_SkipsInvalidEntries(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-attr-skip")}, testAccountID) - require.NoError(t, err) - arn := lbOut.LoadBalancers[0].LoadBalancerArn +type echoPair struct{ Key, Val string } - modOut, err := svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - Attributes: []*elbv2.LoadBalancerAttribute{ - nil, // nil element must not panic - {Key: nil, Value: aws.String("v")}, - {Key: aws.String("k"), Value: nil}, - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("75")}, - }, - }, testAccountID) - require.NoError(t, err) - require.Len(t, modOut.Attributes, 1) - assert.Equal(t, "idle_timeout.timeout_seconds", *modOut.Attributes[0].Key) - assert.Equal(t, "75", *modOut.Attributes[0].Value) - - descOut, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap := make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value +func echoMap(pairs []echoPair) map[string]string { + m := make(map[string]string, len(pairs)) + for _, p := range pairs { + m[p.Key] = p.Val } - assert.Equal(t, "75", attrMap["idle_timeout.timeout_seconds"]) + return m } -// TestModifyTargetGroupAttributes_AllInvalidReturnsError guards against the -// silent-success case where every submitted attribute trips the nil guard and -// the handler returned 200 OK with an empty response body — the caller would -// think the write landed when nothing was actually applied. -func TestModifyTargetGroupAttributes_AllInvalidReturnsError(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-all-invalid")}, testAccountID) - require.NoError(t, err) - - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, - Attributes: []*elbv2.TargetGroupAttribute{ - nil, - {Key: nil, Value: aws.String("v")}, - {Key: aws.String("k"), Value: nil}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) -} - -// TestModifyLoadBalancerAttributes_AllInvalidReturnsError mirrors the TG case. -func TestModifyLoadBalancerAttributes_AllInvalidReturnsError(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-all-invalid")}, testAccountID) - require.NoError(t, err) - - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, - Attributes: []*elbv2.LoadBalancerAttribute{ - nil, - {Key: nil, Value: aws.String("v")}, - {Key: aws.String("k"), Value: nil}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) -} - -// TestModifyTargetGroupAttributes_SequentialMerge verifies that successive -// Modify calls accumulate keys instead of replacing the entire attribute map. -// A future refactor that did `tg.Attributes = newMap` would pass every other -// test (single-call round-trip covers the happy path) but silently wipe -// previous attributes on every subsequent Modify — the most likely real-world -// ALB/TG bug. -func TestModifyTargetGroupAttributes_SequentialMerge(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-seq-merge")}, testAccountID) - require.NoError(t, err) - arn := tgOut.TargetGroups[0].TargetGroupArn - - // Call 1: set deregistration_delay.timeout_seconds. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("deregistration_delay.timeout_seconds"), Value: aws.String("45")}, - }, - }, testAccountID) - require.NoError(t, err) - - // Call 2: set a different key; must not wipe the first one. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - require.NoError(t, err) - - descOut, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap := make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value +// attrArnPtr returns nil for an empty ARN so the MissingArn cases exercise the +// real "no ARN supplied" path rather than an empty-string ARN. +func attrArnPtr(arn string) *string { + if arn == "" { + return nil } - assert.Equal(t, "45", attrMap["deregistration_delay.timeout_seconds"], "first-call key must survive second Modify") - assert.Equal(t, "true", attrMap["stickiness.enabled"], "second-call key must be present") - - // Call 3: overwrite the same key with a new value. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("false")}, - }, - }, testAccountID) - require.NoError(t, err) - - descOut, err = svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap = make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value - } - assert.Equal(t, "false", attrMap["stickiness.enabled"], "same-key overwrite must replace the value") - assert.Equal(t, "45", attrMap["deregistration_delay.timeout_seconds"], "unrelated key must still survive") + return aws.String(arn) } -// TestModifyLoadBalancerAttributes_SequentialMerge mirrors the TG case. -func TestModifyLoadBalancerAttributes_SequentialMerge(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-seq-merge")}, testAccountID) - require.NoError(t, err) - arn := lbOut.LoadBalancers[0].LoadBalancerArn +// attrKind bundles everything the table-driven attribute tests need to drive +// one resource type (target group or load balancer). +type attrKind struct { + name string + notFound string + missingArn string - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("120")}, - }, - }, testAccountID) - require.NoError(t, err) + keyA, valA, valA2 string // primary key, value, and an overwrite value + keyB, valB string // secondary key/value + defaultKey string // a default present after create + defaultVal string + unknownKey string // a key the handler must reject - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("deletion_protection.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - require.NoError(t, err) - - descOut, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap := make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value - } - assert.Equal(t, "120", attrMap["idle_timeout.timeout_seconds"], "first-call key must survive second Modify") - assert.Equal(t, "true", attrMap["deletion_protection.enabled"], "second-call key must be present") - - // Same-key overwrite. - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("90")}, - }, - }, testAccountID) - require.NoError(t, err) - - descOut, err = svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - }, testAccountID) - require.NoError(t, err) - attrMap = make(map[string]string) - for _, a := range descOut.Attributes { - attrMap[*a.Key] = *a.Value - } - assert.Equal(t, "90", attrMap["idle_timeout.timeout_seconds"], "same-key overwrite must replace the value") - assert.Equal(t, "true", attrMap["deletion_protection.enabled"], "unrelated key must still survive") + create func(t *testing.T, svc *ELBv2ServiceImpl, name string) string + modify func(svc *ELBv2ServiceImpl, arn, accountID string, attrs []rawAttr) ([]echoPair, error) + describe func(svc *ELBv2ServiceImpl, arn, accountID string) ([]echoPair, error) + revision func(t *testing.T, svc *ELBv2ServiceImpl, arn string) uint64 } -// TestModifyTargetGroupAttributes_NoopSkipsPersist verifies that a -// re-submission of the same attribute values does not bump the underlying KV -// revision — Terraform's drift check hits Modify on every apply, so the -// steady-state path must be a no-op at the storage layer. -func TestModifyTargetGroupAttributes_NoopSkipsPersist(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-noop")}, testAccountID) - require.NoError(t, err) - tgArn := tgOut.TargetGroups[0].TargetGroupArn - - // First modify — must hit the store. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgArn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, +func attrKinds() []attrKind { + return []attrKind{ + { + name: "TargetGroup", + notFound: awserrors.ErrorELBv2TargetGroupNotFound, + missingArn: "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/missing/tg-missing", + keyA: "deregistration_delay.timeout_seconds", valA: "45", valA2: "30", + keyB: "stickiness.enabled", valB: "true", + defaultKey: "stickiness.type", defaultVal: "lb_cookie", + unknownKey: "stickness.enabled", // typo of "stickiness" + create: func(t *testing.T, svc *ELBv2ServiceImpl, name string) string { + out, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String(name)}, testAccountID) + require.NoError(t, err) + return *out.TargetGroups[0].TargetGroupArn + }, + modify: func(svc *ELBv2ServiceImpl, arn, accountID string, attrs []rawAttr) ([]echoPair, error) { + sdk := make([]*elbv2.TargetGroupAttribute, len(attrs)) + for i, a := range attrs { + if a.nilElem { + continue + } + sdk[i] = &elbv2.TargetGroupAttribute{Key: a.key, Value: a.val} + } + out, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ + TargetGroupArn: attrArnPtr(arn), Attributes: sdk, + }, accountID) + if err != nil { + return nil, err + } + pairs := make([]echoPair, len(out.Attributes)) + for i, a := range out.Attributes { + pairs[i] = echoPair{*a.Key, *a.Value} + } + return pairs, nil + }, + describe: func(svc *ELBv2ServiceImpl, arn, accountID string) ([]echoPair, error) { + out, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ + TargetGroupArn: attrArnPtr(arn), + }, accountID) + if err != nil { + return nil, err + } + pairs := make([]echoPair, len(out.Attributes)) + for i, a := range out.Attributes { + pairs[i] = echoPair{*a.Key, *a.Value} + } + return pairs, nil + }, + revision: func(t *testing.T, svc *ELBv2ServiceImpl, arn string) uint64 { + tg, err := svc.store.GetTargetGroupByArn(arn) + require.NoError(t, err) + entry, err := svc.store.kv.Get(KeyPrefixTG + tg.TargetGroupID) + require.NoError(t, err) + return entry.Revision() + }, }, - }, testAccountID) - require.NoError(t, err) - - // Capture KV revision after the first write. - tg, err := svc.store.GetTargetGroupByArn(*tgArn) - require.NoError(t, err) - entry, err := svc.store.kv.Get(KeyPrefixTG + tg.TargetGroupID) - require.NoError(t, err) - revBefore := entry.Revision() - - // Second modify with identical values — must skip the Put. - modOut, err := svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgArn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, + { + name: "LoadBalancer", + notFound: awserrors.ErrorELBv2LoadBalancerNotFound, + missingArn: "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/missing/lb-missing", + keyA: "idle_timeout.timeout_seconds", valA: "120", valA2: "90", + keyB: "deletion_protection.enabled", valB: "true", + defaultKey: "routing.http2.enabled", defaultVal: "true", + unknownKey: "stickiness.enabled", // valid TG key — cross-product mistake + create: func(t *testing.T, svc *ELBv2ServiceImpl, name string) string { + out, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String(name)}, testAccountID) + require.NoError(t, err) + return *out.LoadBalancers[0].LoadBalancerArn + }, + modify: func(svc *ELBv2ServiceImpl, arn, accountID string, attrs []rawAttr) ([]echoPair, error) { + sdk := make([]*elbv2.LoadBalancerAttribute, len(attrs)) + for i, a := range attrs { + if a.nilElem { + continue + } + sdk[i] = &elbv2.LoadBalancerAttribute{Key: a.key, Value: a.val} + } + out, err := svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ + LoadBalancerArn: attrArnPtr(arn), Attributes: sdk, + }, accountID) + if err != nil { + return nil, err + } + pairs := make([]echoPair, len(out.Attributes)) + for i, a := range out.Attributes { + pairs[i] = echoPair{*a.Key, *a.Value} + } + return pairs, nil + }, + describe: func(svc *ELBv2ServiceImpl, arn, accountID string) ([]echoPair, error) { + out, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ + LoadBalancerArn: attrArnPtr(arn), + }, accountID) + if err != nil { + return nil, err + } + pairs := make([]echoPair, len(out.Attributes)) + for i, a := range out.Attributes { + pairs[i] = echoPair{*a.Key, *a.Value} + } + return pairs, nil + }, + revision: func(t *testing.T, svc *ELBv2ServiceImpl, arn string) uint64 { + lb, err := svc.store.GetLoadBalancerByArn(arn) + require.NoError(t, err) + entry, err := svc.store.kv.Get(KeyPrefixLB + lb.LoadBalancerID) + require.NoError(t, err) + return entry.Revision() + }, }, - }, testAccountID) - require.NoError(t, err) - require.Len(t, modOut.Attributes, 1) - - entry, err = svc.store.kv.Get(KeyPrefixTG + tg.TargetGroupID) - require.NoError(t, err) - assert.Equal(t, revBefore, entry.Revision(), "identical modify must not increment KV revision") - - // Empty attribute list is also a no-op. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgArn, - }, testAccountID) - require.NoError(t, err) - entry, err = svc.store.kv.Get(KeyPrefixTG + tg.TargetGroupID) - require.NoError(t, err) - assert.Equal(t, revBefore, entry.Revision(), "empty modify must not increment KV revision") -} - -// TestModifyLoadBalancerAttributes_NoopSkipsPersist mirrors the TG case. -func TestModifyLoadBalancerAttributes_NoopSkipsPersist(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-noop")}, testAccountID) - require.NoError(t, err) - lbArn := lbOut.LoadBalancers[0].LoadBalancerArn - - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: lbArn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("75")}, - }, - }, testAccountID) - require.NoError(t, err) - - lb, err := svc.store.GetLoadBalancerByArn(*lbArn) - require.NoError(t, err) - entry, err := svc.store.kv.Get(KeyPrefixLB + lb.LoadBalancerID) - require.NoError(t, err) - revBefore := entry.Revision() - - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: lbArn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("75")}, - }, - }, testAccountID) - require.NoError(t, err) - - entry, err = svc.store.kv.Get(KeyPrefixLB + lb.LoadBalancerID) - require.NoError(t, err) - assert.Equal(t, revBefore, entry.Revision(), "identical modify must not increment KV revision") -} - -// TestModifyTargetGroupAttributes_RejectsUnknownKey guards against silently -// persisting typo'd or cross-product attribute keys. AWS rejects unknown keys -// with ValidationError; we must match that so Terraform surfaces the typo at -// plan time instead of letting it drift into KV forever. -func TestModifyTargetGroupAttributes_RejectsUnknownKey(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-unknown-key")}, testAccountID) - require.NoError(t, err) - - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickness.enabled"), Value: aws.String("true")}, // typo of "stickiness" - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorValidationError) - - // The rejected key must not have been persisted. - descOut, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn, - }, testAccountID) - require.NoError(t, err) - for _, a := range descOut.Attributes { - assert.NotEqual(t, "stickness.enabled", *a.Key, "unknown key must not appear in Describe") } } -// TestModifyLoadBalancerAttributes_RejectsUnknownKey mirrors the TG case. -func TestModifyLoadBalancerAttributes_RejectsUnknownKey(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-unknown-key")}, testAccountID) - require.NoError(t, err) +func TestAttributeRoundTrip(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-rt") - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, - Attributes: []*elbv2.LoadBalancerAttribute{ - // Valid TG attribute key sent to LB handler — common cross-product mistake. - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - assert.EqualError(t, err, awserrors.ErrorValidationError) -} + // Assert the exact echoed key/value pairs, not just the length — a + // regression that pointed at the wrong source or dropped Value would + // pass a length check. + echoed, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA}, [2]string{k.keyB, k.valB})) + require.NoError(t, err) + require.Len(t, echoed, 2) + em := echoMap(echoed) + assert.Equal(t, k.valA, em[k.keyA]) + assert.Equal(t, k.valB, em[k.keyB]) -// TestDescribeTargetGroupAttributes_SortedOrder verifies that attributes are -// returned in a stable, sorted-by-key order. Go map iteration is randomised, -// so without explicit sorting Terraform would see spurious plan diffs between -// back-to-back describe calls. -func TestDescribeTargetGroupAttributes_SortedOrder(t *testing.T) { - svc := setupTestService(t) - tgOut, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-attr-sorted")}, testAccountID) - require.NoError(t, err) - arn := tgOut.TargetGroups[0].TargetGroupArn - - // Modify a few attributes to ensure the merged map contains both defaults - // and overrides. - _, err = svc.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ - TargetGroupArn: arn, - Attributes: []*elbv2.TargetGroupAttribute{ - {Key: aws.String("stickiness.enabled"), Value: aws.String("true")}, - {Key: aws.String("deregistration_delay.timeout_seconds"), Value: aws.String("45")}, - }, - }, testAccountID) - require.NoError(t, err) - - // Call Describe multiple times; each result must be identical and sorted. - var firstKeys []string - for i := range 5 { - descOut, err := svc.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{ - TargetGroupArn: arn, - }, testAccountID) - require.NoError(t, err) - - keys := make([]string, len(descOut.Attributes)) - for j, a := range descOut.Attributes { - keys[j] = *a.Key - } - assert.True(t, sort.StringsAreSorted(keys), "attributes must be sorted by key, got %v", keys) - if i == 0 { - firstKeys = keys - } else { - assert.Equal(t, firstKeys, keys, "attribute order must be stable across calls") - } + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + dm := echoMap(desc) + assert.Equal(t, k.valA, dm[k.keyA]) + assert.Equal(t, k.valB, dm[k.keyB]) + // Unmodified defaults should still be present. + assert.Equal(t, k.defaultVal, dm[k.defaultKey]) + }) } } -// TestDescribeLoadBalancerAttributes_SortedOrder mirrors the TG test: LB -// describe responses must be sorted and stable across repeated calls. -func TestDescribeLoadBalancerAttributes_SortedOrder(t *testing.T) { - svc := setupTestService(t) - lbOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-attr-sorted")}, testAccountID) - require.NoError(t, err) - arn := lbOut.LoadBalancers[0].LoadBalancerArn +func TestAttributeNotFound(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) - _, err = svc.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - Attributes: []*elbv2.LoadBalancerAttribute{ - {Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String("90")}, - {Key: aws.String("deletion_protection.enabled"), Value: aws.String("true")}, - }, - }, testAccountID) - require.NoError(t, err) + _, err := k.modify(svc, k.missingArn, testAccountID, kvAttrs([2]string{k.keyB, k.valB})) + assert.EqualError(t, err, k.notFound) - var firstKeys []string - for i := range 5 { - descOut, err := svc.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ - LoadBalancerArn: arn, - }, testAccountID) - require.NoError(t, err) + _, err = k.describe(svc, k.missingArn, testAccountID) + assert.EqualError(t, err, k.notFound) + }) + } +} - keys := make([]string, len(descOut.Attributes)) - for j, a := range descOut.Attributes { - keys[j] = *a.Key - } - assert.True(t, sort.StringsAreSorted(keys), "attributes must be sorted by key, got %v", keys) - if i == 0 { - firstKeys = keys - } else { - assert.Equal(t, firstKeys, keys, "attribute order must be stable across calls") - } +func TestAttributeMissingArn(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + + _, err := k.modify(svc, "", testAccountID, kvAttrs([2]string{k.keyB, k.valB})) + assert.EqualError(t, err, awserrors.ErrorMissingParameter) + + _, err = k.describe(svc, "", testAccountID) + assert.EqualError(t, err, awserrors.ErrorMissingParameter) + }) + } +} + +func TestAttributeWrongAccount(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-wrong-acct") + + _, err := k.modify(svc, arn, "999999999999", kvAttrs([2]string{k.keyB, k.valB})) + assert.EqualError(t, err, k.notFound) + + _, err = k.describe(svc, arn, "999999999999") + assert.EqualError(t, err, k.notFound) + + // The rejected modify must not have mutated the record — read back as + // the real owner and confirm keyB is still at its default, not valB. + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + assert.NotEqual(t, k.valB, echoMap(desc)[k.keyB], "wrong-account modify must not mutate the record") + }) + } +} + +// TestAttributeCrossAccountIsolation verifies that two accounts each holding a +// resource never see each other's attributes: a Modify by account A must not be +// visible to a Describe by account B, even for similarly-named resources. +func TestAttributeCrossAccountIsolation(t *testing.T) { + const accountB = "222222222222" + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svcA := setupTestService(t) + arnA := k.create(t, svcA, "attr-iso") + + // Account A sets keyB. + _, err := k.modify(svcA, arnA, testAccountID, kvAttrs([2]string{k.keyB, k.valB})) + require.NoError(t, err) + + // Account B cannot see or describe A's resource at all. + _, err = k.describe(svcA, arnA, accountB) + assert.EqualError(t, err, k.notFound) + _, err = k.modify(svcA, arnA, accountB, kvAttrs([2]string{k.keyB, "false"})) + assert.EqualError(t, err, k.notFound) + + // A's value is untouched. + desc, err := k.describe(svcA, arnA, testAccountID) + require.NoError(t, err) + assert.Equal(t, k.valB, echoMap(desc)[k.keyB]) + }) + } +} + +// TestAttributeSkipsInvalidEntries verifies that nil slice elements, nil Keys, +// and nil Values are skipped (with a warning) rather than panicking or being +// silently dropped. Valid attributes in the same call must still be applied. +func TestAttributeSkipsInvalidEntries(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-skip") + + echoed, err := k.modify(svc, arn, testAccountID, []rawAttr{ + {nilElem: true}, // nil element must not panic + {key: nil, val: aws.String("v")}, + {key: aws.String("k"), val: nil}, + kvAttr(k.keyB, k.valB), + }) + require.NoError(t, err) + // Only the one valid attribute should be returned. + require.Len(t, echoed, 1) + assert.Equal(t, k.keyB, echoed[0].Key) + assert.Equal(t, k.valB, echoed[0].Val) + + // The valid attribute should have been persisted. + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + assert.Equal(t, k.valB, echoMap(desc)[k.keyB]) + }) + } +} + +// TestAttributeAllInvalidReturnsError guards against the silent-success case +// where every submitted attribute trips the nil guard and the handler returns +// 200 OK with an empty body — the caller would think the write landed when +// nothing was applied. +func TestAttributeAllInvalidReturnsError(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-all-invalid") + + _, err := k.modify(svc, arn, testAccountID, []rawAttr{ + {nilElem: true}, + {key: nil, val: aws.String("v")}, + {key: aws.String("k"), val: nil}, + }) + assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) + }) + } +} + +// TestAttributeSequentialMerge verifies that successive Modify calls accumulate +// keys instead of replacing the entire attribute map. A refactor that did +// `rec.Attributes = newMap` would pass every other test (single-call round-trip +// covers the happy path) but silently wipe previous attributes on every +// subsequent Modify — the most likely real-world bug. +func TestAttributeSequentialMerge(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-seq-merge") + + // Call 1: set keyA. + _, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA})) + require.NoError(t, err) + // Call 2: set a different key; must not wipe the first. + _, err = k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyB, k.valB})) + require.NoError(t, err) + + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + dm := echoMap(desc) + assert.Equal(t, k.valA, dm[k.keyA], "first-call key must survive second Modify") + assert.Equal(t, k.valB, dm[k.keyB], "second-call key must be present") + + // Call 3: overwrite keyA with a new value. + _, err = k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA2})) + require.NoError(t, err) + + desc, err = k.describe(svc, arn, testAccountID) + require.NoError(t, err) + dm = echoMap(desc) + assert.Equal(t, k.valA2, dm[k.keyA], "same-key overwrite must replace the value") + assert.Equal(t, k.valB, dm[k.keyB], "unrelated key must still survive") + }) + } +} + +// TestAttributeNoopSkipsPersist verifies that re-submitting identical values +// does not bump the KV revision — Terraform's drift check hits Modify on every +// apply, so the steady-state path must be a storage-layer no-op. +func TestAttributeNoopSkipsPersist(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-noop") + + // First modify — must hit the store. + _, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA})) + require.NoError(t, err) + revBefore := k.revision(t, svc, arn) + + // Second modify with identical values — must skip the Put. + echoed, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA})) + require.NoError(t, err) + require.Len(t, echoed, 1) + assert.Equal(t, revBefore, k.revision(t, svc, arn), "identical modify must not increment KV revision") + + // Empty attribute list is also a no-op. + _, err = k.modify(svc, arn, testAccountID, nil) + require.NoError(t, err) + assert.Equal(t, revBefore, k.revision(t, svc, arn), "empty modify must not increment KV revision") + }) + } +} + +// TestAttributeRejectsUnknownKey guards against silently persisting typo'd or +// cross-product attribute keys. AWS rejects unknown keys with ValidationError; +// we match that so Terraform surfaces the typo at plan time instead of letting +// it drift into KV forever. +func TestAttributeRejectsUnknownKey(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-unknown-key") + + _, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.unknownKey, "true"})) + assert.EqualError(t, err, awserrors.ErrorValidationError) + + // The rejected key must not have been persisted. + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + _, present := echoMap(desc)[k.unknownKey] + assert.False(t, present, "unknown key must not appear in Describe") + }) + } +} + +// TestAttributeSortedOrder verifies attributes are returned in a stable, +// sorted-by-key order. Go map iteration is randomised, so without explicit +// sorting Terraform would see spurious plan diffs between describe calls. +func TestAttributeSortedOrder(t *testing.T) { + for _, k := range attrKinds() { + t.Run(k.name, func(t *testing.T) { + svc := setupTestService(t) + arn := k.create(t, svc, "attr-sorted") + + // Modify a couple of attributes so the merged map mixes defaults and + // overrides. + _, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA}, [2]string{k.keyB, k.valB})) + require.NoError(t, err) + + var firstKeys []string + for i := range 5 { + desc, err := k.describe(svc, arn, testAccountID) + require.NoError(t, err) + keys := make([]string, len(desc)) + for j, p := range desc { + keys[j] = p.Key + } + assert.True(t, sort.StringsAreSorted(keys), "attributes must be sorted by key, got %v", keys) + if i == 0 { + firstKeys = keys + } else { + assert.Equal(t, firstKeys, keys, "attribute order must be stable across calls") + } + } + }) } } @@ -2968,9 +3045,9 @@ func TestDescribeTags_Listener_NoTags(t *testing.T) { }, testAccountID) require.NoError(t, err) - // Listeners don't store tags yet — must still return a TagDescription - // (with an empty Tags slice), not an error. This is the case the - // Terraform AWS provider hits during post-create refresh. + // Listener created without tags must still return a TagDescription (with an + // empty Tags slice), not an error. This is the case the Terraform AWS + // provider hits during post-create refresh. out, err := svc.DescribeTags(&elbv2.DescribeTagsInput{ ResourceArns: []*string{lstOut.Listeners[0].ListenerArn}, }, testAccountID) @@ -3006,7 +3083,7 @@ func TestDescribeTags_ListenerRule_NoTags(t *testing.T) { }, testAccountID) require.NoError(t, err) - // Rules don't store tags yet — must still return a TagDescription (with an + // Rule created without tags must still return a TagDescription (with an // empty Tags slice), not an error. This is the case the Terraform AWS // provider hits during post-create refresh of aws_lb_listener_rule. out, err := svc.DescribeTags(&elbv2.DescribeTagsInput{ diff --git a/spinifex/handlers/elbv2/service_impl_vpc_test.go b/spinifex/handlers/elbv2/service_impl_vpc_test.go index 652f12227..2e316ed50 100644 --- a/spinifex/handlers/elbv2/service_impl_vpc_test.go +++ b/spinifex/handlers/elbv2/service_impl_vpc_test.go @@ -139,7 +139,7 @@ func TestCreateLoadBalancer_MultipleSubnets(t *testing.T) { // TestCreateLoadBalancer_MultiSubnet_AllENIsPassedToLauncher verifies that // every subnet's ENI is threaded through to SystemInstanceInput, not just -// the first one. Regression guard for mulga-929. +// the first one. func TestCreateLoadBalancer_MultiSubnet_AllENIsPassedToLauncher(t *testing.T) { svc, vpcSvc := setupTestServiceWithVPC(t) diff --git a/spinifex/handlers/elbv2/service_nats.go b/spinifex/handlers/elbv2/service_nats.go index 605ac2fbf..00797d140 100644 --- a/spinifex/handlers/elbv2/service_nats.go +++ b/spinifex/handlers/elbv2/service_nats.go @@ -111,6 +111,14 @@ func (s *NATSELBv2Service) DescribeTags(input *elbv2.DescribeTagsInput, accountI return utils.NATSRequest[elbv2.DescribeTagsOutput](s.natsConn, "elbv2.DescribeTags", input, defaultTimeout, accountID) } +func (s *NATSELBv2Service) AddTags(input *elbv2.AddTagsInput, accountID string) (*elbv2.AddTagsOutput, error) { + return utils.NATSRequest[elbv2.AddTagsOutput](s.natsConn, "elbv2.AddTags", input, defaultTimeout, accountID) +} + +func (s *NATSELBv2Service) RemoveTags(input *elbv2.RemoveTagsInput, accountID string) (*elbv2.RemoveTagsOutput, error) { + return utils.NATSRequest[elbv2.RemoveTagsOutput](s.natsConn, "elbv2.RemoveTags", input, defaultTimeout, accountID) +} + func (s *NATSELBv2Service) ModifyTargetGroupAttributes(input *elbv2.ModifyTargetGroupAttributesInput, accountID string) (*elbv2.ModifyTargetGroupAttributesOutput, error) { return utils.NATSRequest[elbv2.ModifyTargetGroupAttributesOutput](s.natsConn, "elbv2.ModifyTargetGroupAttributes", input, defaultTimeout, accountID) } @@ -127,6 +135,14 @@ func (s *NATSELBv2Service) DescribeLoadBalancerAttributes(input *elbv2.DescribeL return utils.NATSRequest[elbv2.DescribeLoadBalancerAttributesOutput](s.natsConn, "elbv2.DescribeLoadBalancerAttributes", input, defaultTimeout, accountID) } +func (s *NATSELBv2Service) SetSecurityGroups(input *elbv2.SetSecurityGroupsInput, accountID string) (*elbv2.SetSecurityGroupsOutput, error) { + return utils.NATSRequest[elbv2.SetSecurityGroupsOutput](s.natsConn, "elbv2.SetSecurityGroups", input, defaultTimeout, accountID) +} + +func (s *NATSELBv2Service) SetIpAddressType(input *elbv2.SetIpAddressTypeInput, accountID string) (*elbv2.SetIpAddressTypeOutput, error) { + return utils.NATSRequest[elbv2.SetIpAddressTypeOutput](s.natsConn, "elbv2.SetIpAddressType", input, defaultTimeout, accountID) +} + func (s *NATSELBv2Service) LBAgentHeartbeat(input *LBAgentHeartbeatInput, accountID string) (*LBAgentHeartbeatOutput, error) { return utils.NATSRequest[LBAgentHeartbeatOutput](s.natsConn, "elbv2.LBAgentHeartbeat", input, defaultTimeout, accountID) } diff --git a/spinifex/handlers/elbv2/service_nats_e2e_test.go b/spinifex/handlers/elbv2/service_nats_e2e_test.go index 243772e79..604f1ae3c 100644 --- a/spinifex/handlers/elbv2/service_nats_e2e_test.go +++ b/spinifex/handlers/elbv2/service_nats_e2e_test.go @@ -38,6 +38,20 @@ func setupNATSELBv2Test(t *testing.T) (ELBv2Service, *ELBv2ServiceImpl) { "elbv2.DeleteListener": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DeleteListener) }, "elbv2.ModifyListener": func(msg *nats.Msg) { handleNATSMsg(msg, backend.ModifyListener) }, "elbv2.DescribeListeners": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DescribeListeners) }, + "elbv2.CreateRule": func(msg *nats.Msg) { handleNATSMsg(msg, backend.CreateRule) }, + "elbv2.DescribeRules": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DescribeRules) }, + "elbv2.AddTags": func(msg *nats.Msg) { handleNATSMsg(msg, backend.AddTags) }, + "elbv2.RemoveTags": func(msg *nats.Msg) { handleNATSMsg(msg, backend.RemoveTags) }, + "elbv2.DescribeTags": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DescribeTags) }, + "elbv2.SetSecurityGroups": func(msg *nats.Msg) { handleNATSMsg(msg, backend.SetSecurityGroups) }, + "elbv2.SetIpAddressType": func(msg *nats.Msg) { handleNATSMsg(msg, backend.SetIpAddressType) }, + + "elbv2.ModifyTargetGroupAttributes": func(msg *nats.Msg) { handleNATSMsg(msg, backend.ModifyTargetGroupAttributes) }, + "elbv2.DescribeTargetGroupAttributes": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DescribeTargetGroupAttributes) }, + "elbv2.ModifyLoadBalancerAttributes": func(msg *nats.Msg) { handleNATSMsg(msg, backend.ModifyLoadBalancerAttributes) }, + "elbv2.DescribeLoadBalancerAttributes": func(msg *nats.Msg) { + handleNATSMsg(msg, backend.DescribeLoadBalancerAttributes) + }, } for topic, handler := range topics { @@ -281,3 +295,192 @@ func TestNATSE2E_TargetGroupInUseProtection(t *testing.T) { }, testAccountID) require.Error(t, err) } + +func TestNATSE2E_TagsLifecycle(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + lbOut, err := client.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("e2e-tags-lb")}, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err = client.AddTags(&elbv2.AddTagsInput{ + ResourceArns: []*string{lbArn}, + Tags: []*elbv2.Tag{ + {Key: aws.String("App"), Value: aws.String("nginx")}, + {Key: aws.String("Env"), Value: aws.String("prod")}, + }, + }, testAccountID) + require.NoError(t, err) + + desc, err := client.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: []*string{lbArn}}, testAccountID) + require.NoError(t, err) + require.Len(t, desc.TagDescriptions, 1) + got := map[string]string{} + for _, tag := range desc.TagDescriptions[0].Tags { + got[*tag.Key] = *tag.Value + } + assert.Equal(t, map[string]string{"App": "nginx", "Env": "prod"}, got) + + _, err = client.RemoveTags(&elbv2.RemoveTagsInput{ + ResourceArns: []*string{lbArn}, + TagKeys: []*string{aws.String("Env")}, + }, testAccountID) + require.NoError(t, err) + + desc2, err := client.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: []*string{lbArn}}, testAccountID) + require.NoError(t, err) + require.Len(t, desc2.TagDescriptions, 1) + got2 := map[string]string{} + for _, tag := range desc2.TagDescriptions[0].Tags { + got2[*tag.Key] = *tag.Value + } + assert.Equal(t, map[string]string{"App": "nginx"}, got2) +} + +func TestNATSE2E_AttributeRoundTrip(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + tgOut, err := client.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("e2e-attr-tg")}, testAccountID) + require.NoError(t, err) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + + _, err = client.ModifyTargetGroupAttributes(&elbv2.ModifyTargetGroupAttributesInput{ + TargetGroupArn: tgArn, + Attributes: []*elbv2.TargetGroupAttribute{ + {Key: aws.String("deregistration_delay.timeout_seconds"), Value: aws.String("120")}, + }, + }, testAccountID) + require.NoError(t, err) + + tgAttrs, err := client.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{TargetGroupArn: tgArn}, testAccountID) + require.NoError(t, err) + tgGot := map[string]string{} + for _, a := range tgAttrs.Attributes { + tgGot[*a.Key] = *a.Value + } + assert.Equal(t, "120", tgGot["deregistration_delay.timeout_seconds"]) + + lbOut, err := client.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("e2e-attr-lb")}, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err = client.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{ + LoadBalancerArn: lbArn, + Attributes: []*elbv2.LoadBalancerAttribute{ + {Key: aws.String("deletion_protection.enabled"), Value: aws.String("true")}, + }, + }, testAccountID) + require.NoError(t, err) + + lbAttrs, err := client.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: lbArn}, testAccountID) + require.NoError(t, err) + lbGot := map[string]string{} + for _, a := range lbAttrs.Attributes { + lbGot[*a.Key] = *a.Value + } + assert.Equal(t, "true", lbGot["deletion_protection.enabled"]) +} + +func TestNATSE2E_RedirectListener(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + lbOut, err := client.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("e2e-redir-lb")}, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err = client.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbArn, + Protocol: aws.String("HTTP"), + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{ + Type: aws.String("redirect"), + RedirectConfig: &elbv2.RedirectActionConfig{ + Protocol: aws.String("HTTPS"), + Port: aws.String("443"), + StatusCode: aws.String("HTTP_301"), + }, + }}, + }, testAccountID) + require.NoError(t, err) + + desc, err := client.DescribeListeners(&elbv2.DescribeListenersInput{LoadBalancerArn: lbArn}, testAccountID) + require.NoError(t, err) + require.Len(t, desc.Listeners, 1) + require.Len(t, desc.Listeners[0].DefaultActions, 1) + action := desc.Listeners[0].DefaultActions[0] + assert.Equal(t, "redirect", *action.Type) + require.NotNil(t, action.RedirectConfig, "redirect action config must survive the NATS round-trip") + assert.Equal(t, "HTTP_301", *action.RedirectConfig.StatusCode) + assert.Equal(t, "HTTPS", *action.RedirectConfig.Protocol) +} + +func TestNATSE2E_IPTargetType(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + tgOut, err := client.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("e2e-ip-tg"), + TargetType: aws.String("ip"), + }, testAccountID) + require.NoError(t, err) + require.Equal(t, "ip", *tgOut.TargetGroups[0].TargetType) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + + _, err = client.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: tgArn, + Targets: []*elbv2.TargetDescription{ + {Id: aws.String("10.0.1.20")}, + {Id: aws.String("10.0.1.21"), Port: aws.Int64(8080)}, + }, + }, testAccountID) + require.NoError(t, err) + + health, err := client.DescribeTargetHealth(&elbv2.DescribeTargetHealthInput{TargetGroupArn: tgArn}, testAccountID) + require.NoError(t, err) + require.Len(t, health.TargetHealthDescriptions, 2) + + // A non-IP id must be rejected for an ip target group, end-to-end. + _, err = client.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: tgArn, + Targets: []*elbv2.TargetDescription{{Id: aws.String("i-notanip")}}, + }, testAccountID) + require.Error(t, err) +} + +func TestNATSE2E_SetSecurityGroupsAndIpAddressType(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + lbOut, err := client.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("e2e-net-lb"), + SecurityGroups: []*string{aws.String("sg-aaa")}, + }, testAccountID) + require.NoError(t, err) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + _, err = client.SetSecurityGroups(&elbv2.SetSecurityGroupsInput{ + LoadBalancerArn: lbArn, + SecurityGroups: []*string{aws.String("sg-bbb"), aws.String("sg-ccc")}, + }, testAccountID) + require.NoError(t, err) + + // Spinifex ALBs are IPv4-only; "ipv4" is the only accepted value. + _, err = client.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: lbArn, + IpAddressType: aws.String("ipv4"), + }, testAccountID) + require.NoError(t, err) + + // A dualstack request must be rejected end-to-end. + _, err = client.SetIpAddressType(&elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: lbArn, + IpAddressType: aws.String("dualstack"), + }, testAccountID) + require.Error(t, err) + + desc, err := client.DescribeLoadBalancers(&elbv2.DescribeLoadBalancersInput{ + LoadBalancerArns: []*string{lbArn}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.LoadBalancers, 1) + assert.ElementsMatch(t, []string{"sg-bbb", "sg-ccc"}, aws.StringValueSlice(desc.LoadBalancers[0].SecurityGroups)) + assert.Equal(t, "ipv4", *desc.LoadBalancers[0].IpAddressType) +} diff --git a/spinifex/handlers/elbv2/store.go b/spinifex/handlers/elbv2/store.go index e852dce01..e2c04c012 100644 --- a/spinifex/handlers/elbv2/store.go +++ b/spinifex/handlers/elbv2/store.go @@ -92,9 +92,9 @@ func (s *Store) ListLoadBalancers() ([]*LoadBalancerRecord, error) { } // GetLoadBalancerByArn finds a load balancer by its ARN via a direct KV lookup -// on the short ID embedded in the ARN's final path segment. Falls back to a -// linear scan only if the ARN can't be parsed. Terraform hits Describe*Attributes -// on every plan/refresh so this must be O(1), not O(n). +// on the short ID embedded in the ARN's final path segment. Returns (nil, nil) +// if the ARN can't be parsed or no record matches. Terraform hits +// Describe*Attributes on every plan/refresh so this must be O(1), not O(n). func (s *Store) GetLoadBalancerByArn(arn string) (*LoadBalancerRecord, error) { // ELBv2 LB ARN: arn:aws:elasticloadbalancing:{region}:{account}:loadbalancer/{app,net}/{name}/{lbID} idx := strings.LastIndex(arn, "/") @@ -106,11 +106,16 @@ func (s *Store) GetLoadBalancerByArn(arn string) (*LoadBalancerRecord, error) { if err != nil { return nil, err } + if lb == nil { + return nil, nil + } // Defence-in-depth: ensure the record actually belongs to this ARN. // Short IDs are random hex, so collisions are effectively impossible, but // a mismatch here would indicate KV corruption and must not be silently // served as a successful lookup. - if lb == nil || lb.LoadBalancerArn != arn { + if lb.LoadBalancerArn != arn { + slog.Error("load balancer KV record ARN mismatch", + "requested_arn", arn, "stored_arn", lb.LoadBalancerArn, "lb_id", lbID) return nil, nil } return lb, nil @@ -187,7 +192,12 @@ func (s *Store) GetTargetGroupByArn(arn string) (*TargetGroupRecord, error) { if err != nil { return nil, err } - if tg == nil || tg.TargetGroupArn != arn { + if tg == nil { + return nil, nil + } + if tg.TargetGroupArn != arn { + slog.Error("target group KV record ARN mismatch", + "requested_arn", arn, "stored_arn", tg.TargetGroupArn, "tg_id", tgID) return nil, nil } return tg, nil @@ -322,18 +332,26 @@ func (s *Store) ListListenersByLB(lbArn string) ([]*ListenerRecord, error) { return result, nil } -// GetListenerByArn finds a listener by its ARN. +// GetListenerByArn finds a listener by its ARN via a direct KV lookup on the +// short ID embedded in the ARN's final path segment. See GetLoadBalancerByArn +// for the motivation — Terraform calls DescribeTags on every aws_lb_listener +// on every plan, and DescribeTags can carry many ARNs per call, so this must +// be O(1) per ARN, not an O(n) listener-KV scan. func (s *Store) GetListenerByArn(arn string) (*ListenerRecord, error) { - listeners, err := s.ListListeners() + // ELBv2 listener ARN: arn:aws:elasticloadbalancing:{region}:{account}:listener/{app,net}/{name}/{lbID}/{listenerID} + idx := strings.LastIndex(arn, "/") + if idx < 0 || idx == len(arn)-1 { + return nil, nil + } + listenerID := arn[idx+1:] + l, err := s.GetListener(listenerID) if err != nil { return nil, err } - for _, l := range listeners { - if l.ListenerArn == arn { - return l, nil - } + if l == nil || l.ListenerArn != arn { + return nil, nil } - return nil, nil + return l, nil } // --- Rule CRUD --- diff --git a/spinifex/handlers/elbv2/store_test.go b/spinifex/handlers/elbv2/store_test.go index ab9d4bf86..6c1285c90 100644 --- a/spinifex/handlers/elbv2/store_test.go +++ b/spinifex/handlers/elbv2/store_test.go @@ -350,6 +350,16 @@ func TestGetListenerByArn(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) assert.Equal(t, l.ListenerID, got.ListenerID) + + got, err = store.GetListenerByArn("arn:nonexistent") + require.NoError(t, err) + assert.Nil(t, got) + + // ARN that parses to a real listener ID but a different ARN must not match + // (defence-in-depth against the parse trick serving a wrong record). + got, err = store.GetListenerByArn("arn:aws:elasticloadbalancing:us-east-1:" + testAccountID + ":listener/app/other/lbX/" + l.ListenerID) + require.NoError(t, err) + assert.Nil(t, got) } // --- Cross-resource isolation: shared IDs across record types must not collide --- diff --git a/spinifex/handlers/elbv2/types.go b/spinifex/handlers/elbv2/types.go index 21fe3ae0f..c9137b169 100644 --- a/spinifex/handlers/elbv2/types.go +++ b/spinifex/handlers/elbv2/types.go @@ -1,6 +1,9 @@ package handlers_elbv2 -import "time" +import ( + "maps" + "time" +) const ( // LoadBalancer types @@ -16,6 +19,10 @@ const ( StateActive = "active" StateFailed = "failed" + // Target group target types (v1 supports instance and ip) + TargetTypeInstance = "instance" + TargetTypeIP = "ip" + // Target health states TargetHealthInitial = "initial" TargetHealthHealthy = "healthy" @@ -36,6 +43,7 @@ const ( // Listener action types ActionTypeForward = "forward" ActionTypeFixedResponse = "fixed-response" + ActionTypeRedirect = "redirect" // Rule condition fields RuleFieldHostHeader = "host-header" @@ -53,7 +61,7 @@ const ( MaxRulesPerListener = 100 MaxConditionsPerRule = 5 MaxValuesPerCondition = 5 - MaxActionsPerRule = 1 // forward only; mulga-951 may bump + MaxActionsPerRule = 1 // single terminal action: forward, redirect, or fixed-response MaxConditionValueLen = 128 MaxHTTPHeaderNameLen = 40 @@ -123,7 +131,7 @@ type TargetGroupRecord struct { Protocol string `json:"protocol"` // "HTTP" or "HTTPS" Port int64 `json:"port"` // Default target port VpcId string `json:"vpc_id"` - TargetType string `json:"target_type"` // "instance" for v1 + TargetType string `json:"target_type"` // TargetTypeInstance or TargetTypeIP HealthCheck HealthCheckConfig `json:"health_check"` Targets []Target `json:"targets"` Attributes map[string]string `json:"attributes,omitempty"` @@ -176,11 +184,11 @@ func DefaultNLBHealthCheck() HealthCheckConfig { // Target represents a registered target in a target group. type Target struct { - Id string `json:"id"` // Instance ID (e.g. i-xxxxx) + Id string `json:"id"` // Instance ID (i-xxxxx) or IP for ip-type TGs Port int64 `json:"port"` // Override port (0 = use TG default) HealthState string `json:"health_state"` // "initial", "healthy", "unhealthy", "draining" HealthDesc string `json:"health_desc"` // Reason for current state - PrivateIP string `json:"private_ip"` // Resolved from instance ENI + PrivateIP string `json:"private_ip"` // Instance ENI IP, or the target IP for ip-type TGs } // ListenerRecord represents a stored Listener. @@ -193,14 +201,18 @@ type ListenerRecord struct { DefaultActions []ListenerAction `json:"default_actions"` AccountID string `json:"account_id"` CreatedAt time.Time `json:"created_at"` + + Tags map[string]string `json:"tags,omitempty"` } -// ListenerAction defines a listener's default action. +// ListenerAction defines a listener default action or a rule action. type ListenerAction struct { - Type string `json:"type"` // "forward" or "fixed-response" + Type string `json:"type"` // "forward", "redirect", or "fixed-response" TargetGroupArn string `json:"target_group_arn"` // FixedResponse is populated when Type == "fixed-response" (no target group). FixedResponse *FixedResponseAction `json:"fixed_response,omitempty"` + // Redirect is populated when Type == "redirect". + Redirect *RedirectAction `json:"redirect,omitempty"` } // FixedResponseAction holds the canned reply for a "fixed-response" action. @@ -210,6 +222,18 @@ type FixedResponseAction struct { MessageBody string `json:"message_body,omitempty"` } +// RedirectAction holds the target for a "redirect" action. Fields mirror the +// AWS RedirectActionConfig and may contain the `#{protocol}`, `#{host}`, +// `#{port}`, `#{path}`, and `#{query}` placeholders. +type RedirectAction struct { + Protocol string `json:"protocol,omitempty"` + Host string `json:"host,omitempty"` + Port string `json:"port,omitempty"` + Path string `json:"path,omitempty"` + Query string `json:"query,omitempty"` + StatusCode string `json:"status_code"` // "HTTP_301" or "HTTP_302" +} + // RuleRecord represents a stored listener rule. type RuleRecord struct { RuleArn string `json:"rule_arn"` @@ -220,6 +244,8 @@ type RuleRecord struct { Actions []ListenerAction `json:"actions"` AccountID string `json:"account_id"` CreatedAt time.Time `json:"created_at"` + + Tags map[string]string `json:"tags,omitempty"` } // RuleCondition is one routing predicate on a rule. Only the field block @@ -250,50 +276,71 @@ type RuleQueryStringKV struct { // provider sends every attribute it knows about, and any key missing here // gets rejected with ValidationError, surfacing as "UnknownError" in tofu. func DefaultLoadBalancerAttributes(lbType string) map[string]string { - // Common to all load balancer types. - attrs := map[string]string{ - "deletion_protection.enabled": "false", - "load_balancing.cross_zone.enabled": "false", - } - switch lbType { case LoadBalancerTypeApplication: - // ALB-specific attributes. Defaults match real AWS values as of the - // aws-sdk-go 1.55 elbv2 API documentation. - attrs["load_balancing.cross_zone.enabled"] = "true" - attrs["access_logs.s3.enabled"] = "false" - attrs["access_logs.s3.bucket"] = "" - attrs["access_logs.s3.prefix"] = "" - attrs["connection_logs.s3.enabled"] = "false" - attrs["connection_logs.s3.bucket"] = "" - attrs["connection_logs.s3.prefix"] = "" - attrs["idle_timeout.timeout_seconds"] = "60" - attrs["client_keep_alive.seconds"] = "3600" - attrs["routing.http.desync_mitigation_mode"] = "defensive" - attrs["routing.http.drop_invalid_header_fields.enabled"] = "false" - attrs["routing.http.preserve_host_header.enabled"] = "false" - attrs["routing.http.x_amzn_tls_version_and_cipher_suite.enabled"] = "false" - attrs["routing.http.xff_client_port.enabled"] = "false" - attrs["routing.http.xff_header_processing.mode"] = "append" - attrs["routing.http2.enabled"] = "true" - attrs["waf.fail_open.enabled"] = "false" - attrs["zonal_shift.config.enabled"] = "false" + return maps.Clone(albAttributeDefaults) case LoadBalancerTypeNetwork: - // NLB-specific attributes. - attrs["access_logs.s3.enabled"] = "false" - attrs["access_logs.s3.bucket"] = "" - attrs["access_logs.s3.prefix"] = "" - attrs["dns_record.client_routing_policy"] = "any_availability_zone" - attrs["ipv6.deny_all_igw_traffic"] = "false" - attrs["zonal_shift.config.enabled"] = "false" + return maps.Clone(nlbAttributeDefaults) + default: + return maps.Clone(lbBaseAttributeDefaults) } - - return attrs } // DefaultTargetGroupAttributes returns the default attribute set for target groups. func DefaultTargetGroupAttributes() map[string]string { - return map[string]string{ + return maps.Clone(targetGroupAttributeDefaults) +} + +// Default attribute sets are package-level so Describe calls clone a prebuilt +// map instead of rebuilding it on every request. Callers receive a clone, never +// the shared map, so mutation cannot leak back into the defaults. +// +// access_logs.s3.bucket / .prefix (and the connection_logs equivalents) default +// to empty strings: that matches real AWS, where logging is disabled by default +// and the bucket/prefix are unset until the operator opts in. Terraform sends +// these empty strings on every apply, so they must be present as known keys. +var ( + // Common to all load balancer types; also the fallback for unknown types. + lbBaseAttributeDefaults = map[string]string{ + "deletion_protection.enabled": "false", + "load_balancing.cross_zone.enabled": "false", + } + + // ALB defaults match real AWS values as of the aws-sdk-go 1.55 elbv2 API. + albAttributeDefaults = map[string]string{ + "deletion_protection.enabled": "false", + "load_balancing.cross_zone.enabled": "true", + "access_logs.s3.enabled": "false", + "access_logs.s3.bucket": "", + "access_logs.s3.prefix": "", + "connection_logs.s3.enabled": "false", + "connection_logs.s3.bucket": "", + "connection_logs.s3.prefix": "", + "idle_timeout.timeout_seconds": "60", + "client_keep_alive.seconds": "3600", + "routing.http.desync_mitigation_mode": "defensive", + "routing.http.drop_invalid_header_fields.enabled": "false", + "routing.http.preserve_host_header.enabled": "false", + "routing.http.x_amzn_tls_version_and_cipher_suite.enabled": "false", + "routing.http.xff_client_port.enabled": "false", + "routing.http.xff_header_processing.mode": "append", + "routing.http2.enabled": "true", + "waf.fail_open.enabled": "false", + "zonal_shift.config.enabled": "false", + } + + 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", + } + + targetGroupAttributeDefaults = map[string]string{ "deregistration_delay.timeout_seconds": "300", "stickiness.enabled": "false", "stickiness.type": "lb_cookie", @@ -302,4 +349,4 @@ func DefaultTargetGroupAttributes() map[string]string { "load_balancing.algorithm.type": "round_robin", "slow_start.duration_seconds": "0", } -} +) diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.test.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.test.tsx new file mode 100644 index 000000000..4b2a45c7e --- /dev/null +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.test.tsx @@ -0,0 +1,114 @@ +import type { SecurityGroup } from "@aws-sdk/client-ec2" +import { render, screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { describe, expect, it, vi } from "vitest" + +import { SecurityGroupsEditor } from "./security-groups-editor" + +const AVAILABLE: SecurityGroup[] = [ + { GroupId: "sg-1", GroupName: "default", VpcId: "vpc-a" }, + { GroupId: "sg-2", GroupName: "web", VpcId: "vpc-a" }, + { GroupId: "sg-3", GroupName: "db", VpcId: "vpc-a" }, +] + +describe("SecurityGroupsEditor", () => { + it("seeds selection from current and disables Save with no changes", () => { + render( + {}} + />, + ) + + expect(screen.getByLabelText("Security group sg-1 (default)")).toBeChecked() + expect(screen.getByLabelText("Security group sg-2 (web)")).not.toBeChecked() + expect(screen.getByText("No changes")).toBeInTheDocument() + expect(screen.getByRole("button", { name: /save changes/i })).toBeDisabled() + }) + + it("submits the new selection set", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render( + , + ) + + await user.click(screen.getByLabelText("Security group sg-2 (web)")) + await user.click(screen.getByRole("button", { name: /save changes/i })) + + expect(onSubmit).toHaveBeenCalledWith(["sg-1", "sg-2"]) + }) + + it("blocks submit when no security group is selected", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render( + , + ) + + await user.click(screen.getByLabelText("Security group sg-1 (default)")) + + expect(screen.getByText(/at least one security group/i)).toBeInTheDocument() + expect(screen.getByRole("button", { name: /save changes/i })).toBeDisabled() + expect(onSubmit).not.toHaveBeenCalled() + }) + + it("Reset reverts selection back to initial", async () => { + const user = userEvent.setup() + render( + {}} + />, + ) + + await user.click(screen.getByLabelText("Security group sg-2 (web)")) + expect(screen.getByText("Unsaved changes")).toBeInTheDocument() + + await user.click(screen.getByRole("button", { name: /reset/i })) + + expect(screen.getByLabelText("Security group sg-2 (web)")).not.toBeChecked() + expect(screen.getByText("No changes")).toBeInTheDocument() + }) + + it("shows an empty state when the VPC has no security groups", () => { + render( + {}} + />, + ) + expect( + screen.getByText("No security groups in this VPC."), + ).toBeInTheDocument() + }) + + it("renders an error banner when the mutation fails", () => { + render( + {}} + />, + ) + expect(screen.getByRole("alert")).toHaveTextContent("boom") + }) +}) diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.tsx new file mode 100644 index 000000000..f8725bd3a --- /dev/null +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/security-groups-editor.tsx @@ -0,0 +1,127 @@ +import type { SecurityGroup } from "@aws-sdk/client-ec2" +import { useMemo, useState } from "react" + +import { Button } from "@/components/ui/button" + +interface SecurityGroupsEditorProps { + // Security groups in the load balancer's VPC, selectable for attachment. + available: SecurityGroup[] + // Currently attached security group IDs. + current: string[] + onSubmit: (securityGroupIds: string[]) => void + isPending: boolean + error?: Error | null + isSuccess?: boolean +} + +function sameSet(a: string[], b: string[]): boolean { + if (a.length !== b.length) { + return false + } + const setB = new Set(b) + return a.every((id) => setB.has(id)) +} + +export function SecurityGroupsEditor({ + available, + current, + onSubmit, + isPending, + error, + isSuccess, +}: SecurityGroupsEditorProps) { + const initial = useMemo(() => [...current], [current]) + const [selected, setSelected] = useState(initial) + + const toggle = (id: string) => { + setSelected((prev) => + prev.includes(id) ? prev.filter((x) => x !== id) : [...prev, id], + ) + } + const handleReset = () => setSelected(initial) + + const dirty = !sameSet(selected, initial) + // AWS requires at least one security group on an ALB. + const isEmpty = selected.length === 0 + + return ( +
{ + e.preventDefault() + if (!dirty || isEmpty) { + return + } + onSubmit(selected) + }} + > + {error && ( +
+ {error.message || "Failed to update security groups"} +
+ )} + {isSuccess && !dirty && ( +
+ Security groups updated. +
+ )} +
+
+ {available.length === 0 ? ( +

+ No security groups in this VPC. +

+ ) : ( + available.map((sg) => ( + + )) + )} + {isEmpty && ( +

+ At least one security group is required. +

+ )} +
+
+ + {dirty ? "Unsaved changes" : "No changes"} + + + +
+
+
+ ) +} diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.test.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.test.tsx new file mode 100644 index 000000000..da6a78849 --- /dev/null +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.test.tsx @@ -0,0 +1,128 @@ +import type { Tag } from "@aws-sdk/client-elastic-load-balancing-v2" +import { render, screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { describe, expect, it, vi } from "vitest" + +import { TagsEditor } from "./tags-editor" + +function at(arr: T[], i: number): T { + const v = arr[i] + if (v === undefined) { + throw new Error(`expected an element at index ${i}`) + } + return v +} + +const INITIAL: Tag[] = [ + { Key: "env", Value: "prod" }, + { Key: "team", Value: "platform" }, +] + +describe("TagsEditor", () => { + it("seeds rows from tags and disables Save with no changes", () => { + render( {}} tags={INITIAL} />) + + const keys = screen.getAllByLabelText("Tag key") + expect(keys[0]).toHaveValue("env") + expect(keys[1]).toHaveValue("team") + expect(screen.getByText("No changes")).toBeInTheDocument() + expect(screen.getByRole("button", { name: /save changes/i })).toBeDisabled() + }) + + it("submits the full desired tag set after an edit", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render() + + const value = at(screen.getAllByLabelText("Tag value"), 0) + await user.clear(value) + await user.type(value, "staging") + + await user.click(screen.getByRole("button", { name: /save changes/i })) + + expect(onSubmit).toHaveBeenCalledWith([ + { key: "env", value: "staging" }, + { key: "team", value: "platform" }, + ]) + }) + + it("adds and removes tag rows", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render() + + await user.click(screen.getByRole("button", { name: /add tag/i })) + await user.type(at(screen.getAllByLabelText("Tag key"), 2), "owner") + await user.type(at(screen.getAllByLabelText("Tag value"), 2), "alice") + + await user.click( + at(screen.getAllByRole("button", { name: /remove tag/i }), 1), + ) + + await user.click(screen.getByRole("button", { name: /save changes/i })) + + expect(onSubmit).toHaveBeenCalledWith([ + { key: "env", value: "prod" }, + { key: "owner", value: "alice" }, + ]) + }) + + it("blocks submit on a duplicate key", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render() + + const key = at(screen.getAllByLabelText("Tag key"), 1) + await user.clear(key) + await user.type(key, "env") + + expect(screen.getByText(/keys must be unique/i)).toBeInTheDocument() + expect(screen.getByRole("button", { name: /save changes/i })).toBeDisabled() + expect(onSubmit).not.toHaveBeenCalled() + }) + + it("blocks submit on a value without a key", async () => { + const user = userEvent.setup() + const onSubmit = vi.fn() + render() + + await user.click(screen.getByRole("button", { name: /add tag/i })) + await user.type(at(screen.getAllByLabelText("Tag value"), 0), "orphan") + + expect(screen.getByText(/needs a key/i)).toBeInTheDocument() + expect(screen.getByRole("button", { name: /save changes/i })).toBeDisabled() + expect(onSubmit).not.toHaveBeenCalled() + }) + + it("Reset reverts edits back to initial", async () => { + const user = userEvent.setup() + render( {}} tags={INITIAL} />) + + const value = at(screen.getAllByLabelText("Tag value"), 0) + await user.clear(value) + await user.type(value, "dev") + expect(screen.getByText("Unsaved changes")).toBeInTheDocument() + + await user.click(screen.getByRole("button", { name: /reset/i })) + + expect(at(screen.getAllByLabelText("Tag value"), 0)).toHaveValue("prod") + expect(screen.getByText("No changes")).toBeInTheDocument() + }) + + it("renders the Save button in pending state", () => { + render( {}} tags={INITIAL} />) + expect(screen.getByRole("button", { name: /saving/i })).toBeDisabled() + }) + + it("renders an error banner when the mutation fails", () => { + render( + {}} + tags={INITIAL} + />, + ) + expect(screen.getByRole("alert")).toHaveTextContent("boom") + }) +}) diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.tsx new file mode 100644 index 000000000..3dbe38b28 --- /dev/null +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/tags-editor.tsx @@ -0,0 +1,170 @@ +import type { Tag } from "@aws-sdk/client-elastic-load-balancing-v2" +import { Plus, Trash2 } from "lucide-react" +import { useMemo, useState } from "react" + +import { Button } from "@/components/ui/button" +import { Input } from "@/components/ui/input" + +interface TagRow { + // Stable id so React keys survive key/value edits and row removal. + id: number + key: string + value: string +} + +interface TagsEditorProps { + tags: Tag[] + onSubmit: (tags: { key: string; value: string }[]) => void + isPending: boolean + error?: Error | null + isSuccess?: boolean +} + +function buildInitialRows(tags: Tag[]): TagRow[] { + return tags.map((t, i) => ({ id: i, key: t.Key ?? "", value: t.Value ?? "" })) +} + +function normalize(rows: TagRow[]): { key: string; value: string }[] { + return rows + .map((r) => ({ key: r.key.trim(), value: r.value })) + .filter((r) => r.key.length > 0) +} + +export function TagsEditor({ + tags, + onSubmit, + isPending, + error, + isSuccess, +}: TagsEditorProps) { + const initialRows = useMemo(() => buildInitialRows(tags), [tags]) + + const [rows, setRows] = useState(initialRows) + const [nextId, setNextId] = useState(initialRows.length) + + const setRow = (id: number, patch: Partial) => { + setRows((prev) => prev.map((r) => (r.id === id ? { ...r, ...patch } : r))) + } + const addRow = () => { + setRows((prev) => [...prev, { id: nextId, key: "", value: "" }]) + setNextId((n) => n + 1) + } + const removeRow = (id: number) => { + setRows((prev) => prev.filter((r) => r.id !== id)) + } + const handleReset = () => { + setRows(initialRows) + } + + // A row with a value but no key is incomplete; duplicate keys are ambiguous. + const hasBlankKey = rows.some( + (r) => r.key.trim().length === 0 && r.value.length > 0, + ) + const normalized = normalize(rows) + const keys = normalized.map((r) => r.key) + const hasDuplicate = new Set(keys).size !== keys.length + const hasError = hasBlankKey || hasDuplicate + + const dirty = + JSON.stringify(normalized) !== JSON.stringify(normalize(initialRows)) + + return ( +
{ + e.preventDefault() + if (hasError || !dirty) { + return + } + onSubmit(normalized) + }} + > + {error && ( +
+ {error.message || "Failed to save tags"} +
+ )} + {isSuccess && !dirty && ( +
+ Tags saved. +
+ )} +
+
+ {rows.length === 0 && ( +

No tags.

+ )} + {rows.map((row) => ( +
+ setRow(row.id, { key: e.target.value })} + placeholder="Key" + value={row.key} + /> + setRow(row.id, { value: e.target.value })} + placeholder="Value" + value={row.value} + /> + +
+ ))} +
+ +
+ {hasBlankKey && ( +

+ Every tag with a value needs a key. +

+ )} + {hasDuplicate && ( +

Tag keys must be unique.

+ )} +
+
+ + {dirty ? "Unsaved changes" : "No changes"} + + + +
+
+
+ ) +} diff --git a/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts b/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts index 2f1ec077b..16c2fbfb1 100644 --- a/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts +++ b/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts @@ -4,6 +4,7 @@ import { type RulePriorityPair, type Tag, type TargetDescription, + AddTagsCommand, CreateListenerCommand, CreateLoadBalancerCommand, CreateRuleCommand, @@ -18,7 +19,9 @@ import { ModifyRuleCommand, ModifyTargetGroupAttributesCommand, RegisterTargetsCommand, + RemoveTagsCommand, SetRulePrioritiesCommand, + SetSecurityGroupsCommand, } from "@aws-sdk/client-elastic-load-balancing-v2" import { useMutation, useQueryClient } from "@tanstack/react-query" @@ -76,6 +79,76 @@ export function useModifyLoadBalancerAttributes() { }) } +export interface UpdateTagsParams { + // ARN of any taggable ELBv2 resource (load balancer, target group, listener, + // listener rule). + resourceArn: string + // Desired final tag set after the edit. + tags: { key: string; value: string }[] + // Tag keys present before the edit, used to compute removals. + initialKeys: string[] +} + +// useUpdateTags reconciles a resource's tags to the desired set by diffing +// against the prior keys: it overwrites the final tags via AddTags and deletes +// any dropped keys via RemoveTags. Either call is skipped when it has no work. +export function useUpdateTags() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: async (params: UpdateTagsParams) => { + const finalKeys = new Set(params.tags.map((t) => t.key)) + const toRemove = params.initialKeys.filter((k) => !finalKeys.has(k)) + const client = getElbv2Client() + + if (params.tags.length > 0) { + await client.send( + new AddTagsCommand({ + ResourceArns: [params.resourceArn], + Tags: params.tags.map((t) => ({ Key: t.key, Value: t.value })), + }), + ) + } + if (toRemove.length > 0) { + await client.send( + new RemoveTagsCommand({ + ResourceArns: [params.resourceArn], + TagKeys: toRemove, + }), + ) + } + }, + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: ["elbv2", "tags"] }) + }, + }) +} + +export interface SetSecurityGroupsParams { + loadBalancerArn: string + securityGroupIds: string[] +} + +// useSetSecurityGroups replaces the security groups associated with a load +// balancer (SetSecurityGroups). The new set takes effect on the live data plane +// server-side; the LB query is invalidated so the detail page reflects it. +export function useSetSecurityGroups() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: async (params: SetSecurityGroupsParams) => { + const command = new SetSecurityGroupsCommand({ + LoadBalancerArn: params.loadBalancerArn, + SecurityGroups: params.securityGroupIds, + }) + return await getElbv2Client().send(command) + }, + onSuccess: (_data, variables) => { + void queryClient.invalidateQueries({ + queryKey: ["elbv2", "loadBalancers", variables.loadBalancerArn], + }) + }, + }) +} + export function useCreateTargetGroup() { const queryClient = useQueryClient() return useMutation({ diff --git a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/-components/load-balancer-detail-page.tsx b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/-components/load-balancer-detail-page.tsx index 59f2e8f56..ca2067cbd 100644 --- a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/-components/load-balancer-detail-page.tsx +++ b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/-components/load-balancer-detail-page.tsx @@ -1,5 +1,4 @@ import type { Subnet } from "@aws-sdk/client-ec2" -import type { Tag } from "@aws-sdk/client-elastic-load-balancing-v2" import { useSuspenseQuery } from "@tanstack/react-query" import { useNavigate } from "@tanstack/react-router" import { Trash2 } from "lucide-react" @@ -14,6 +13,8 @@ import { AttributesEditor, } from "@/components/elbv2/attributes-editor" import { ListenersTab } from "@/components/elbv2/listeners-tab" +import { SecurityGroupsEditor } from "@/components/elbv2/security-groups-editor" +import { TagsEditor } from "@/components/elbv2/tags-editor" import { ErrorBanner } from "@/components/error-banner" import { PageHeading } from "@/components/page-heading" import { StateBadge } from "@/components/state-badge" @@ -23,8 +24,13 @@ import { formatDateTime, getNameTag } from "@/lib/utils" import { useDeleteLoadBalancer, useModifyLoadBalancerAttributes, + useSetSecurityGroups, + useUpdateTags, } from "@/mutations/elbv2" -import { ec2SubnetsQueryOptions } from "@/queries/ec2" +import { + ec2SecurityGroupsQueryOptions, + ec2SubnetsQueryOptions, +} from "@/queries/ec2" import { elbv2LoadBalancerAttributesQueryOptions, elbv2LoadBalancerQueryOptions, @@ -43,9 +49,12 @@ export function LoadBalancerDetailPage({ arn }: Props) { ) const { data: tagsData } = useSuspenseQuery(elbv2TagsQueryOptions([arn])) const { data: subnetsData } = useSuspenseQuery(ec2SubnetsQueryOptions) + const { data: sgsData } = useSuspenseQuery(ec2SecurityGroupsQueryOptions) const deleteMutation = useDeleteLoadBalancer() const modifyAttrsMutation = useModifyLoadBalancerAttributes() + const updateTagsMutation = useUpdateTags() + const setSecurityGroupsMutation = useSetSecurityGroups() const [showDeleteDialog, setShowDeleteDialog] = useState(false) const lb = lbData.LoadBalancers?.[0] @@ -84,6 +93,12 @@ export function LoadBalancerDetailPage({ arn }: Props) { const attributes = attrsData.Attributes ?? [] const lbTags = tagsData?.TagDescriptions?.find((td) => td.ResourceArn === arn)?.Tags ?? [] + const currentSecurityGroups = (lb.SecurityGroups ?? []).filter( + (id): id is string => id !== undefined, + ) + const vpcSecurityGroups = (sgsData.SecurityGroups ?? []).filter( + (sg) => sg.VpcId === lb.VpcId, + ) return ( <> @@ -121,6 +136,7 @@ export function LoadBalancerDetailPage({ arn }: Props) { Overview Listeners + Security groups Attributes Tags @@ -179,6 +195,22 @@ export function LoadBalancerDetailPage({ arn }: Props) { + + + setSecurityGroupsMutation.mutate({ + loadBalancerArn: arn, + securityGroupIds, + }) + } + /> + + - {lbTags.length > 0 ? ( - - Tags - - {lbTags.map((tag: Tag) => ( - - ))} - - - ) : ( -

No tags.

- )} + + updateTagsMutation.mutate({ + resourceArn: arn, + tags, + initialKeys: lbTags + .map((t) => t.Key ?? "") + .filter((k) => k.length > 0), + }) + } + tags={lbTags} + />
diff --git a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/describe-load-balancers/$id.test.tsx b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/describe-load-balancers/$id.test.tsx index cd0ebc56c..b10763eaf 100644 --- a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/describe-load-balancers/$id.test.tsx +++ b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(load-balancers)/describe-load-balancers/$id.test.tsx @@ -105,6 +105,11 @@ describe("load-balancer detail route", () => { qc.setQueryData(["ec2", "subnets"], { Subnets: [{ SubnetId: "subnet-a", CidrBlock: "10.0.1.0/24", Tags: [] }], }) + qc.setQueryData(["ec2", "securityGroups"], { + SecurityGroups: [ + { GroupId: "sg-1", GroupName: "default", VpcId: "vpc-aaa" }, + ], + }) return qc } @@ -126,6 +131,7 @@ describe("load-balancer detail route", () => { }) qc.setQueryData(["elbv2", "tags", LB_ARN], { TagDescriptions: [] }) qc.setQueryData(["ec2", "subnets"], { Subnets: [] }) + qc.setQueryData(["ec2", "securityGroups"], { SecurityGroups: [] }) renderWithClient(, qc) diff --git a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(target-groups)/-components/target-group-detail-page.tsx b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(target-groups)/-components/target-group-detail-page.tsx index 6d3c8dd7b..da1fbcdf9 100644 --- a/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(target-groups)/-components/target-group-detail-page.tsx +++ b/spinifex/services/spinifexui/frontend/src/routes/_auth/ec2/(target-groups)/-components/target-group-detail-page.tsx @@ -1,4 +1,3 @@ -import type { Tag } from "@aws-sdk/client-elastic-load-balancing-v2" import { useSuspenseQuery } from "@tanstack/react-query" import { useNavigate } from "@tanstack/react-router" import { Trash2 } from "lucide-react" @@ -12,6 +11,7 @@ import { AttributesEditor, targetGroupAttributeSpecs, } from "@/components/elbv2/attributes-editor" +import { TagsEditor } from "@/components/elbv2/tags-editor" import { TargetsTab } from "@/components/elbv2/targets-tab" import { ErrorBanner } from "@/components/error-banner" import { PageHeading } from "@/components/page-heading" @@ -20,6 +20,7 @@ import { Tabs, TabsList, TabsPanel, TabsTab } from "@/components/ui/tabs" import { useDeleteTargetGroup, useModifyTargetGroupAttributes, + useUpdateTags, } from "@/mutations/elbv2" import { elbv2TagsQueryOptions, @@ -41,6 +42,7 @@ export function TargetGroupDetailPage({ arn }: Props) { const deleteMutation = useDeleteTargetGroup() const modifyAttrsMutation = useModifyTargetGroupAttributes() + const updateTagsMutation = useUpdateTags() const [showDeleteDialog, setShowDeleteDialog] = useState(false) const [activeTab, setActiveTab] = useState("overview") @@ -195,22 +197,21 @@ export function TargetGroupDetailPage({ arn }: Props) {
- {tgTags.length > 0 ? ( - - Tags - - {tgTags.map((tag: Tag) => ( - - ))} - - - ) : ( -

No tags.

- )} + + updateTagsMutation.mutate({ + resourceArn: arn, + tags, + initialKeys: tgTags + .map((t) => t.Key ?? "") + .filter((k) => k.length > 0), + }) + } + tags={tgTags} + />