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} + />