From 287487a4f2ffcc493be36fbeec369893c41d61ba Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Sat, 30 May 2026 09:18:52 +1000 Subject: [PATCH] feat(elbv2): implement ModifyListener (#268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(elbv2): implement ModifyListener Adds AWS-compatible ModifyListener to the spinifex ELBv2 stack so listeners can be edited in place — port, protocol, and default actions — without the current delete+create dance that briefly drops the listener from HAProxy. - Handler interface, impl, and NATS wrapper get ModifyListener; validation mirrors CreateListener (LB type ↔ protocol, dup-port excluding self, target-group existence + protocol compatibility). - Daemon subscribes to elbv2.ModifyListener on the spinifex-workers queue and a new gateway action dispatches it. - Frontend exposes a per-row edit button that pre-fills a dialog from the existing listener and calls ModifyListenerCommand, replacing the "Listeners cannot be edited" note. - Unit + NATS round-trip + frontend tests cover the new flow. Cert / SslPolicy / Alpn handling stays out of scope and lands with HTTPS listener support. * test(e2e): cover ModifyListener in lb harness Adds an Internal_ALB_ModifyListener subtest to TestLoadBalancer that exercises in-place listener edits end-to-end: - Provisions tgA + tgB sharing the same app instances. - Brings up an internal ALB with HTTP:80 → tgA, baselines round-robin. - ModifyListener port=8090, asserts DescribeListeners shows the new port and that probes on 8090 still round-robin — proves HAProxy is reconciled without dropping the listener (the regression mulga-944 fixes; the frontend previously delete+created on every edit). - ModifyListener DefaultActions → tgB, asserts traffic still flows. Also factors a probeAtPort helper so the order JSON can target any listener port, and updates TEST_COVERAGE.md with phase 3e. * fix(ui): widen listener add/edit dialog so content stays inside the box AlertDialogContent defaults to max-w-xs (20rem) which clipped the listener form on the right edge. Use a 32rem cap with grid-cols-[minmax(0,1fr)] + min-w-0 on the form so CSS grid tracks shrink instead of expanding to max-content. * test: cover ModifyListener + system instance dispatch Lifts ModifyListener diff coverage by exercising nil/empty arn paths, NLB protocol acceptance + rejection, protocol-only revalidation against existing target groups, and the launcher-nats wire format error paths (missing input, missing instance type, decode failures, no responder, default timeout fallback). Adds a new spinifex/daemon/daemon_system_dispatch_test.go covering splitSubjectTail edge cases, respondWithSystem{Launch,Terminate}{Output, OK,Error} envelope shape, handler entry points for invalid JSON and missing instance ID, and subscribeSystemTerminate nil-conn + idempotent re-subscribe behaviour. Fixes the flaky TestHandleEC2DescribeInstanceTypes/NoFilter… subtest by selecting a 2-vCPU instance type that also fits schedulable memory, so hosts with smaller schedulableMem (CI 4 vCPU / ~13 GiB) don't pick a 16 GiB-mem type that fails allocation. * test(e2e): wait for tgB after listener swap, not before Spinifex health-checks only target groups wired into a listener's HAProxy config. Waiting for tgB to report healthy before modifyListenerDefaultTG attaches it to the listener is impossible — the agent never probes its targets, so they stay in initial and the 2m EventuallyErr trips. Drop the pre-swap tgB wait. After modifyListenerDefaultTG, replace the fixed 3s sleep with WaitForTargetsHealthy on tgB (post-swap), which is now the listener's DefaultAction. * test(e2e): derive LB name from ARN in lbENI/deleteLB ENI filter The ENI description filter was building the LB-name token from a hard-coded "alb"/"nlb" template (e.g. `ELB app/lb-e2e-alb-int/`), which only matched LBs whose name happened to be `lb-e2e-alb-*` or `lb-e2e-nlb-*`. The ModifyListener suite names its LB `lb-e2e-mod`, so lbENI's EventuallyErr could never find the ENI and tripped after 30s. deleteLB had the same bug: sysInstanceID stayed empty and WaitForENICleanup waited for a non-existent filter, allowing the next suite's createLB to race the sys.micro VM reclaim. Parse the LB name straight out of lb.ARN (penultimate path segment, same shape createLB already relies on) so the filter matches any caller-chosen name. Drop the now-redundant scheme arg from lbENI. * test(e2e): poll for listener serving on new port after ModifyListener The post-modifyListenerPort settle was a hard 3s sleep, but lb-agent polls for new HAProxy config on a 5s tick. After the API returns, the agent has typically not yet detected the config-hash change, so HAProxy is still bound to the old port — every probe to the new port RSTs and probeAtPort sees 0/0 successful. Replace the sleep with waitForListenerServing: a polling loop that fires 2 probes via the shared trigger client every 2s until at least one returns a parsable instance_id, with a 60s ceiling. Factor the trigger-and-fetch out of probeAtPort into probeOnce so the polling helper can reuse it without the require.* asserts. --- spinifex/daemon/daemon.go | 1 + spinifex/daemon/daemon_handlers_elbv2.go | 4 + .../daemon/daemon_system_dispatch_test.go | 308 +++++++++++++++ spinifex/daemon/daemon_test.go | 16 +- spinifex/gateway/elbv2.go | 3 + spinifex/gateway/elbv2/ModifyListener.go | 37 ++ spinifex/gateway/elbv2/gateway_elbv2_test.go | 10 + spinifex/gateway/elbv2_test.go | 1 + spinifex/handlers/elbv2/service.go | 1 + spinifex/handlers/elbv2/service_impl.go | 126 ++++++ spinifex/handlers/elbv2/service_impl_test.go | 369 ++++++++++++++++++ spinifex/handlers/elbv2/service_nats.go | 4 + .../handlers/elbv2/service_nats_e2e_test.go | 31 ++ .../system_instance_launcher_nats_test.go | 101 +++++ .../components/elbv2/listeners-tab.test.tsx | 59 ++- .../src/components/elbv2/listeners-tab.tsx | 153 +++++++- .../frontend/src/mutations/elbv2.ts | 35 ++ tests/e2e/TEST_COVERAGE.md | 13 + tests/e2e/lb/lb_test.go | 211 +++++++++- 19 files changed, 1441 insertions(+), 42 deletions(-) create mode 100644 spinifex/daemon/daemon_system_dispatch_test.go create mode 100644 spinifex/gateway/elbv2/ModifyListener.go diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 91c497cc8..50d8e3156 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -825,6 +825,7 @@ func (d *Daemon) subscribeAll() error { natsSub{"elbv2.DescribeTargetHealth", d.handleELBv2DescribeTargetHealth, "spinifex-workers"}, natsSub{"elbv2.CreateListener", d.handleELBv2CreateListener, "spinifex-workers"}, natsSub{"elbv2.DeleteListener", d.handleELBv2DeleteListener, "spinifex-workers"}, + natsSub{"elbv2.ModifyListener", d.handleELBv2ModifyListener, "spinifex-workers"}, natsSub{"elbv2.DescribeListeners", d.handleELBv2DescribeListeners, "spinifex-workers"}, natsSub{"elbv2.DescribeTags", d.handleELBv2DescribeTags, "spinifex-workers"}, natsSub{"elbv2.LBAgentHeartbeat", d.handleELBv2LBAgentHeartbeat, "spinifex-workers"}, diff --git a/spinifex/daemon/daemon_handlers_elbv2.go b/spinifex/daemon/daemon_handlers_elbv2.go index 991b10a87..044f4a6e3 100644 --- a/spinifex/daemon/daemon_handlers_elbv2.go +++ b/spinifex/daemon/daemon_handlers_elbv2.go @@ -46,6 +46,10 @@ func (d *Daemon) handleELBv2DeleteListener(msg *nats.Msg) { handleNATSRequest(msg, d.elbv2Service.DeleteListener) } +func (d *Daemon) handleELBv2ModifyListener(msg *nats.Msg) { + handleNATSRequest(msg, d.elbv2Service.ModifyListener) +} + func (d *Daemon) handleELBv2DescribeListeners(msg *nats.Msg) { handleNATSRequest(msg, d.elbv2Service.DescribeListeners) } diff --git a/spinifex/daemon/daemon_system_dispatch_test.go b/spinifex/daemon/daemon_system_dispatch_test.go new file mode 100644 index 000000000..a98f390f7 --- /dev/null +++ b/spinifex/daemon/daemon_system_dispatch_test.go @@ -0,0 +1,308 @@ +package daemon + +import ( + "encoding/json" + "testing" + "time" + + handlers_elbv2 "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" + "github.com/nats-io/nats.go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSplitSubjectTail(t *testing.T) { + cases := []struct { + name string + subject string + prefix string + want string + }{ + {"happy", "system.TerminateInstance.i-abc", "system.TerminateInstance.", "i-abc"}, + {"empty tail equals prefix length", "system.TerminateInstance.", "system.TerminateInstance.", ""}, + {"prefix mismatch", "other.subject", "system.TerminateInstance.", ""}, + {"shorter than prefix", "sys", "system.TerminateInstance.", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, splitSubjectTail(tc.subject, tc.prefix)) + }) + } +} + +func TestRespondWithSystemLaunchOutput(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.launch.ok.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := &nats.Msg{Subject: "system.LaunchInstance.sys.micro", Reply: "test.launch.ok.reply", Sub: sub} + out := &handlers_elbv2.SystemInstanceOutput{InstanceID: "i-abc", PrivateIP: "10.0.0.5"} + msg = msgWithConn(nc, msg) + + respondWithSystemLaunchOutput(msg, out) + + select { + case payload := <-replyCh: + var env struct { + Output *handlers_elbv2.SystemInstanceOutput `json:"output,omitempty"` + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + require.NotNil(t, env.Output) + assert.Equal(t, "i-abc", env.Output.InstanceID) + assert.Empty(t, env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected response on reply subject") + } +} + +func TestRespondWithSystemLaunchError(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.launch.err.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{Subject: "system.LaunchInstance.sys.micro", Reply: "test.launch.err.reply", Sub: sub}) + + respondWithSystemLaunchError(msg, "boom") + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.Equal(t, "boom", env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected error response on reply subject") + } +} + +func TestRespondWithSystemLaunchError_EmptyDefaultsToServerInternal(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.launch.empty.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{Subject: "system.LaunchInstance.sys.micro", Reply: "test.launch.empty.reply", Sub: sub}) + + respondWithSystemLaunchError(msg, "") + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.NotEmpty(t, env.Error, "empty input must be replaced with default error") + case <-time.After(2 * time.Second): + t.Fatal("expected response on reply subject") + } +} + +func TestRespondWithSystemTerminateOK(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.term.ok.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{Subject: "system.TerminateInstance.i-x", Reply: "test.term.ok.reply", Sub: sub}) + + respondWithSystemTerminateOK(msg) + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.Empty(t, env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected response on reply subject") + } +} + +func TestRespondWithSystemTerminateError(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.term.err.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{Subject: "system.TerminateInstance.i-x", Reply: "test.term.err.reply", Sub: sub}) + + respondWithSystemTerminateError(msg, "not found") + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.Equal(t, "not found", env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected response on reply subject") + } +} + +func TestRespondWithSystemTerminateError_EmptyDefaultsToServerInternal(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.term.empty.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{Subject: "system.TerminateInstance.i-x", Reply: "test.term.empty.reply", Sub: sub}) + + respondWithSystemTerminateError(msg, "") + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.NotEmpty(t, env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected response on reply subject") + } +} + +func TestHandleSystemLaunchInstance_InvalidJSON(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + d := &Daemon{natsConn: nc, natsSubscriptions: make(map[string]*nats.Subscription)} + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.launch.bad.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{ + Subject: "system.LaunchInstance.sys.micro", + Reply: "test.launch.bad.reply", + Sub: sub, + Data: []byte("{not valid"), + }) + + d.handleSystemLaunchInstance(msg) + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.NotEmpty(t, env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected error response for invalid JSON") + } +} + +func TestHandleSystemTerminateInstance_MissingID(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + d := &Daemon{natsConn: nc, natsSubscriptions: make(map[string]*nats.Subscription)} + + replyCh := make(chan []byte, 1) + sub, err := nc.Subscribe("test.term.missing.reply", func(msg *nats.Msg) { + replyCh <- msg.Data + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + msg := msgWithConn(nc, &nats.Msg{ + Subject: "system.TerminateInstance.", + Reply: "test.term.missing.reply", + Sub: sub, + }) + + d.handleSystemTerminateInstance(msg) + + select { + case payload := <-replyCh: + var env struct { + Error string `json:"error,omitempty"` + } + require.NoError(t, json.Unmarshal(payload, &env)) + assert.NotEmpty(t, env.Error) + case <-time.After(2 * time.Second): + t.Fatal("expected error response for missing instance ID") + } +} + +func TestSubscribeSystemTerminate_NilConn(t *testing.T) { + d := &Daemon{natsSubscriptions: make(map[string]*nats.Subscription)} + assert.NoError(t, d.subscribeSystemTerminate("i-noop"), "nil natsConn must short-circuit without error") + assert.Empty(t, d.natsSubscriptions, "no subscription should be registered when conn is nil") +} + +func TestSubscribeSystemTerminate_Idempotent(t *testing.T) { + nc, err := nats.Connect(sharedNATSURL) + require.NoError(t, err) + t.Cleanup(nc.Close) + + d := &Daemon{natsConn: nc, natsSubscriptions: make(map[string]*nats.Subscription)} + + require.NoError(t, d.subscribeSystemTerminate("i-idem")) + first := d.natsSubscriptions["system.TerminateInstance.i-idem"] + require.NotNil(t, first) + + require.NoError(t, d.subscribeSystemTerminate("i-idem")) + second := d.natsSubscriptions["system.TerminateInstance.i-idem"] + assert.Same(t, first, second, "second call must be a no-op and keep the original subscription") +} + +// msgWithConn returns msg with its internal connection set so msg.Respond +// publishes through nc. nats.Msg.Respond requires a backing connection set +// either via Subscribe delivery or by hand for synthetic test messages. +func msgWithConn(nc *nats.Conn, msg *nats.Msg) *nats.Msg { + // The Subscribe-created subscription embeds the conn; reuse it. + if msg.Sub != nil { + return msg + } + sub, _ := nc.Subscribe("_test.unused.subject."+msg.Reply, func(*nats.Msg) {}) + msg.Sub = sub + return msg +} diff --git a/spinifex/daemon/daemon_test.go b/spinifex/daemon/daemon_test.go index c8d13037e..56e2af77b 100644 --- a/spinifex/daemon/daemon_test.go +++ b/spinifex/daemon/daemon_test.go @@ -486,14 +486,22 @@ func TestHandleEC2DescribeInstanceTypes(t *testing.T) { initialCount := len(initialOutput.InstanceTypes) t.Logf("Initial supported instance types: %d", initialCount) + schedulableMem := daemon.resourceMgr.hostMemGB - daemon.resourceMgr.reservedMem var instanceType2CPU *ec2.InstanceTypeInfo for _, it := range initialOutput.InstanceTypes { - if it.VCpuInfo != nil && it.VCpuInfo.DefaultVCpus != nil && *it.VCpuInfo.DefaultVCpus == 2 { - instanceType2CPU = it - break + if it.VCpuInfo == nil || it.VCpuInfo.DefaultVCpus == nil || *it.VCpuInfo.DefaultVCpus != 2 { + continue } + if it.MemoryInfo == nil || it.MemoryInfo.SizeInMiB == nil { + continue + } + if float64(*it.MemoryInfo.SizeInMiB)/1024.0 > schedulableMem { + continue + } + instanceType2CPU = it + break } - require.NotNil(t, instanceType2CPU, "Should find an instance type with 2 vCPUs") + require.NotNil(t, instanceType2CPU, "Should find a 2 vCPU instance type that fits schedulable memory") err = daemon.resourceMgr.allocate(instanceType2CPU) require.NoError(t, err, "Should be able to allocate 2 vCPU instance") diff --git a/spinifex/gateway/elbv2.go b/spinifex/gateway/elbv2.go index 1fda3f004..ff570a6fe 100644 --- a/spinifex/gateway/elbv2.go +++ b/spinifex/gateway/elbv2.go @@ -75,6 +75,9 @@ var elbv2Actions = map[string]ELBv2Handler{ "DeleteListener": elbv2Handler(func(input *elbv2.DeleteListenerInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.DeleteListener(input, gw.NATSConn, accountID) }), + "ModifyListener": elbv2Handler(func(input *elbv2.ModifyListenerInput, gw *GatewayConfig, accountID string) (any, error) { + return gateway_elbv2.ModifyListener(input, gw.NATSConn, accountID) + }), "DescribeListeners": elbv2Handler(func(input *elbv2.DescribeListenersInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_elbv2.DescribeListeners(input, gw.NATSConn, accountID) }), diff --git a/spinifex/gateway/elbv2/ModifyListener.go b/spinifex/gateway/elbv2/ModifyListener.go new file mode 100644 index 000000000..672e65f5a --- /dev/null +++ b/spinifex/gateway/elbv2/ModifyListener.go @@ -0,0 +1,37 @@ +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 ValidateModifyListenerInput(input *elbv2.ModifyListenerInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.ListenerArn == nil || *input.ListenerArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + +// ModifyListener handles the ELBv2 ModifyListener API call. +func ModifyListener(input *elbv2.ModifyListenerInput, natsConn *nats.Conn, accountID string) (elbv2.ModifyListenerOutput, error) { + var output elbv2.ModifyListenerOutput + + if err := ValidateModifyListenerInput(input); err != nil { + return output, err + } + + svc := handlers_elbv2.NewNATSELBv2Service(natsConn) + result, err := svc.ModifyListener(input, accountID) + if err != nil { + return output, err + } + + return *result, nil +} diff --git a/spinifex/gateway/elbv2/gateway_elbv2_test.go b/spinifex/gateway/elbv2/gateway_elbv2_test.go index 796802c77..c775b6122 100644 --- a/spinifex/gateway/elbv2/gateway_elbv2_test.go +++ b/spinifex/gateway/elbv2/gateway_elbv2_test.go @@ -140,6 +140,16 @@ func TestDeleteListener_MissingArn(t *testing.T) { assert.EqualError(t, err, awserrors.ErrorMissingParameter) } +func TestModifyListener_NilInput(t *testing.T) { + _, err := ModifyListener(nil, nil, "123456789012") + assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) +} + +func TestModifyListener_MissingArn(t *testing.T) { + _, err := ModifyListener(&elbv2.ModifyListenerInput{}, nil, "123456789012") + assert.EqualError(t, err, awserrors.ErrorMissingParameter) +} + func TestDescribeListeners_NilInput(t *testing.T) { _, err := DescribeListeners(nil, nil, "123456789012") assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue) diff --git a/spinifex/gateway/elbv2_test.go b/spinifex/gateway/elbv2_test.go index 001962c84..fa2a4149f 100644 --- a/spinifex/gateway/elbv2_test.go +++ b/spinifex/gateway/elbv2_test.go @@ -48,6 +48,7 @@ func TestELBv2ActionsMap_AllActionsRegistered(t *testing.T) { "DescribeTargetHealth", "CreateListener", "DeleteListener", + "ModifyListener", "DescribeListeners", "DescribeTags", "LBAgentHeartbeat", diff --git a/spinifex/handlers/elbv2/service.go b/spinifex/handlers/elbv2/service.go index 477c61c56..60d63809e 100644 --- a/spinifex/handlers/elbv2/service.go +++ b/spinifex/handlers/elbv2/service.go @@ -22,6 +22,7 @@ type ELBv2Service interface { CreateListener(input *elbv2.CreateListenerInput, accountID string) (*elbv2.CreateListenerOutput, error) DeleteListener(input *elbv2.DeleteListenerInput, accountID string) (*elbv2.DeleteListenerOutput, error) + ModifyListener(input *elbv2.ModifyListenerInput, accountID string) (*elbv2.ModifyListenerOutput, error) DescribeListeners(input *elbv2.DescribeListenersInput, accountID string) (*elbv2.DescribeListenersOutput, error) DescribeTags(input *elbv2.DescribeTagsInput, accountID string) (*elbv2.DescribeTagsOutput, error) diff --git a/spinifex/handlers/elbv2/service_impl.go b/spinifex/handlers/elbv2/service_impl.go index 4e955d0c0..9ce13fb7b 100644 --- a/spinifex/handlers/elbv2/service_impl.go +++ b/spinifex/handlers/elbv2/service_impl.go @@ -1658,6 +1658,132 @@ func (s *ELBv2ServiceImpl) DeleteListener(input *elbv2.DeleteListenerInput, acco return &elbv2.DeleteListenerOutput{}, nil } +func (s *ELBv2ServiceImpl) ModifyListener(input *elbv2.ModifyListenerInput, accountID string) (*elbv2.ModifyListenerOutput, error) { + if input == nil || input.ListenerArn == nil || *input.ListenerArn == "" { + return nil, errors.New(awserrors.ErrorMissingParameter) + } + + listener, err := s.store.GetListenerByArn(*input.ListenerArn) + if err != nil { + slog.Error("ModifyListener: failed to get listener", "arn", *input.ListenerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if listener == nil || listener.AccountID != accountID { + return nil, errors.New(awserrors.ErrorELBv2ListenerNotFound) + } + + lb, err := s.store.GetLoadBalancerByArn(listener.LoadBalancerArn) + if err != nil { + slog.Error("ModifyListener: failed to get LB", "arn", listener.LoadBalancerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if lb == nil { + return nil, errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) + } + + updated := *listener + + if input.Protocol != nil && *input.Protocol != "" { + proto := *input.Protocol + switch lb.Type { + case LoadBalancerTypeNetwork: + switch proto { + case ProtocolTCP, ProtocolUDP, ProtocolTLS, ProtocolTCPUDP: + default: + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + default: + switch proto { + case ProtocolHTTP, ProtocolHTTPS: + default: + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + updated.Protocol = proto + } + + if input.Port != nil { + newPort := *input.Port + if newPort != updated.Port { + existingListeners, listErr := s.store.ListListenersByLB(lb.LoadBalancerArn) + if listErr != nil { + slog.Error("ModifyListener: failed to list existing listeners", "lbArn", lb.LoadBalancerArn, "err", listErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + for _, l := range existingListeners { + if l.ListenerID == updated.ListenerID { + continue + } + if l.Port == newPort { + return nil, errors.New(awserrors.ErrorELBv2DuplicateListener) + } + } + updated.Port = newPort + } + } + + if len(input.DefaultActions) > 0 { + var actions []ListenerAction + for _, a := range input.DefaultActions { + action := ListenerAction{} + if a.Type != nil { + action.Type = *a.Type + } + if a.TargetGroupArn != nil { + action.TargetGroupArn = *a.TargetGroupArn + } + if action.Type == ActionTypeForward && action.TargetGroupArn != "" { + tg, tgErr := s.store.GetTargetGroupByArn(action.TargetGroupArn) + if tgErr != nil { + slog.Error("ModifyListener: failed to get target group", "arn", action.TargetGroupArn, "err", tgErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if tg == nil { + return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) + } + if !isCompatibleProtocol(updated.Protocol, tg.Protocol) { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + actions = append(actions, action) + } + updated.DefaultActions = actions + } else if input.Protocol != nil && updated.Protocol != listener.Protocol { + for _, a := range updated.DefaultActions { + if a.Type != ActionTypeForward || a.TargetGroupArn == "" { + continue + } + tg, tgErr := s.store.GetTargetGroupByArn(a.TargetGroupArn) + if tgErr != nil { + slog.Error("ModifyListener: failed to get target group", "arn", a.TargetGroupArn, "err", tgErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if tg == nil { + return nil, errors.New(awserrors.ErrorELBv2TargetGroupNotFound) + } + if !isCompatibleProtocol(updated.Protocol, tg.Protocol) { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + } + + if err := s.store.PutListener(&updated); err != nil { + slog.Error("ModifyListener: failed to persist record", "listenerId", updated.ListenerID, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + if err := s.updateStoredConfig(lb); err != nil { + slog.Error("ModifyListener: failed to update config", "listenerArn", updated.ListenerArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + slog.Info("ModifyListener completed", "listenerArn", updated.ListenerArn, "lbArn", lb.LoadBalancerArn, "port", updated.Port, "protocol", updated.Protocol, "accountID", accountID) + + return &elbv2.ModifyListenerOutput{ + Listeners: []*elbv2.Listener{s.listenerRecordToSDK(&updated)}, + }, nil +} + func (s *ELBv2ServiceImpl) DescribeListeners(input *elbv2.DescribeListenersInput, accountID string) (*elbv2.DescribeListenersOutput, error) { var listeners []*ListenerRecord var err error diff --git a/spinifex/handlers/elbv2/service_impl_test.go b/spinifex/handlers/elbv2/service_impl_test.go index bea368ac6..1fa41ad63 100644 --- a/spinifex/handlers/elbv2/service_impl_test.go +++ b/spinifex/handlers/elbv2/service_impl_test.go @@ -794,6 +794,375 @@ func TestDeleteListener_NotFound(t *testing.T) { assert.Contains(t, err.Error(), "ListenerNotFound") } +func TestModifyListener_Port(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-tg")}, testAccountID) + + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + 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, + Port: aws.Int64(8080), + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Listeners, 1) + assert.Equal(t, int64(8080), *out.Listeners[0].Port) + assert.Equal(t, "HTTP", *out.Listeners[0].Protocol) + require.Len(t, out.Listeners[0].DefaultActions, 1) + assert.Equal(t, *tgOut.TargetGroups[0].TargetGroupArn, *out.Listeners[0].DefaultActions[0].TargetGroupArn) + + desc, _ := svc.DescribeListeners(&elbv2.DescribeListenersInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + }, testAccountID) + require.Len(t, desc.Listeners, 1) + assert.Equal(t, int64(8080), *desc.Listeners[0].Port) +} + +func TestModifyListener_Protocol(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-proto-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-proto-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, + Protocol: aws.String("HTTPS"), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, "HTTPS", *out.Listeners[0].Protocol) +} + +func TestModifyListener_DefaultActions(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-act-lb")}, testAccountID) + tg1, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-act-tg1")}, testAccountID) + tg2, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-act-tg2")}, testAccountID) + + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tg1.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("forward"), TargetGroupArn: tg2.TargetGroups[0].TargetGroupArn}, + }, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Listeners[0].DefaultActions, 1) + assert.Equal(t, *tg2.TargetGroups[0].TargetGroupArn, *out.Listeners[0].DefaultActions[0].TargetGroupArn) +} + +func TestModifyListener_DuplicatePort(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-dup-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-dup-tg")}, testAccountID) + actions := []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}} + + _, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Port: aws.Int64(80), + DefaultActions: actions, + }, testAccountID) + require.NoError(t, err) + + lst443, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Port: aws.Int64(443), + DefaultActions: actions, + }, testAccountID) + require.NoError(t, err) + + _, err = svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lst443.Listeners[0].ListenerArn, + Port: aws.Int64(80), + }, testAccountID) + assert.Error(t, err) + assert.Contains(t, err.Error(), "DuplicateListener") +} + +func TestModifyListener_SamePortNoConflict(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-same-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-same-tg")}, testAccountID) + + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + require.NoError(t, err) + + // Setting same port should not trigger dup check against self. + out, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Port: aws.Int64(80), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, int64(80), *out.Listeners[0].Port) +} + +func TestModifyListener_NotFound(t *testing.T) { + svc := setupTestService(t) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: aws.String("arn:nonexistent"), + Port: aws.Int64(80), + }, testAccountID) + assert.Error(t, err) + assert.Contains(t, err.Error(), "ListenerNotFound") +} + +func TestModifyListener_WrongAccount(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-acct-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-acct-tg")}, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Port: aws.Int64(8080), + }, "999999999999") + assert.Error(t, err) + assert.Contains(t, err.Error(), "ListenerNotFound") +} + +func TestModifyListener_TargetGroupNotFound(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-tgnf-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-tgnf-tg")}, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + DefaultActions: []*elbv2.Action{ + {Type: aws.String("forward"), TargetGroupArn: aws.String("arn:nonexistent")}, + }, + }, testAccountID) + assert.Error(t, err) + assert.Contains(t, err.Error(), "TargetGroupNotFound") +} + +func TestModifyListener_NoOp(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-noop-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-noop-tg")}, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Port: aws.Int64(8080), + Protocol: aws.String("HTTP"), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + out, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Listeners, 1) + assert.Equal(t, int64(8080), *out.Listeners[0].Port) + assert.Equal(t, "HTTP", *out.Listeners[0].Protocol) +} + +func TestModifyListener_InvalidProtocolForLBType(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("mod-bad-proto-lb")}, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("mod-bad-proto-tg")}, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Protocol: aws.String("TCP"), + }, testAccountID) + assert.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + +func TestModifyListener_NilInput(t *testing.T) { + svc := setupTestService(t) + + _, err := svc.ModifyListener(nil, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "MissingParameter") +} + +func TestModifyListener_EmptyArn(t *testing.T) { + svc := setupTestService(t) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: aws.String(""), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "MissingParameter") + + _, err = svc.ModifyListener(&elbv2.ModifyListenerInput{}, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "MissingParameter") +} + +func TestModifyListener_NLB_AcceptsAllProtocols(t *testing.T) { + cases := []struct { + listenerProto string + tgProto string + }{ + {"TCP", "TCP"}, + {"UDP", "UDP"}, + {"TLS", "TCP"}, + {"TCP_UDP", "TCP_UDP"}, + } + for _, tc := range cases { + t.Run(tc.listenerProto, func(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("nlb-mod-" + tc.listenerProto), + Type: aws.String("network"), + }, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-nlb-mod-" + tc.listenerProto), + Protocol: aws.String(tc.tgProto), + Port: aws.Int64(8080), + }, testAccountID) + + lstOut, err := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String(tc.tgProto), + Port: aws.Int64(8080), + 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, + Protocol: aws.String(tc.listenerProto), + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, tc.listenerProto, *out.Listeners[0].Protocol) + }) + } +} + +func TestModifyListener_NLB_RejectsHTTP(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("nlb-mod-rej-http"), + Type: aws.String("network"), + }, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-nlb-rej"), + Protocol: aws.String("TCP"), + Port: aws.Int64(8080), + }, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("TCP"), + Port: aws.Int64(8080), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Protocol: aws.String("HTTP"), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + +func TestModifyListener_ProtocolOnlyChange_TGIncompatible(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("nlb-mod-incompat"), + Type: aws.String("network"), + }, testAccountID) + tgOut, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-udp-incompat"), + Protocol: aws.String("UDP"), + Port: aws.Int64(8080), + }, testAccountID) + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("UDP"), + Port: aws.Int64(8080), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Protocol: aws.String("TCP"), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + +func TestModifyListener_DefaultActions_IncompatibleProtocol(t *testing.T) { + svc := setupTestService(t) + + lbOut, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("nlb-da-incompat"), + Type: aws.String("network"), + }, testAccountID) + tcpTG, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-tcp-da"), + Protocol: aws.String("TCP"), + Port: aws.Int64(8080), + }, testAccountID) + udpTG, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-udp-da"), + Protocol: aws.String("UDP"), + Port: aws.Int64(8080), + }, testAccountID) + + lstOut, _ := svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbOut.LoadBalancers[0].LoadBalancerArn, + Protocol: aws.String("TCP"), + Port: aws.Int64(8080), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tcpTG.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + + _, err := svc.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + DefaultActions: []*elbv2.Action{ + {Type: aws.String("forward"), TargetGroupArn: udpTG.TargetGroups[0].TargetGroupArn}, + }, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") +} + func TestDescribeListeners_FilterByLBArn(t *testing.T) { svc := setupTestService(t) diff --git a/spinifex/handlers/elbv2/service_nats.go b/spinifex/handlers/elbv2/service_nats.go index a01423675..524c7f465 100644 --- a/spinifex/handlers/elbv2/service_nats.go +++ b/spinifex/handlers/elbv2/service_nats.go @@ -75,6 +75,10 @@ func (s *NATSELBv2Service) DeleteListener(input *elbv2.DeleteListenerInput, acco return utils.NATSRequest[elbv2.DeleteListenerOutput](s.natsConn, "elbv2.DeleteListener", input, defaultTimeout, accountID) } +func (s *NATSELBv2Service) ModifyListener(input *elbv2.ModifyListenerInput, accountID string) (*elbv2.ModifyListenerOutput, error) { + return utils.NATSRequest[elbv2.ModifyListenerOutput](s.natsConn, "elbv2.ModifyListener", input, defaultTimeout, accountID) +} + func (s *NATSELBv2Service) DescribeListeners(input *elbv2.DescribeListenersInput, accountID string) (*elbv2.DescribeListenersOutput, error) { return utils.NATSRequest[elbv2.DescribeListenersOutput](s.natsConn, "elbv2.DescribeListeners", input, defaultTimeout, accountID) } diff --git a/spinifex/handlers/elbv2/service_nats_e2e_test.go b/spinifex/handlers/elbv2/service_nats_e2e_test.go index f50ae2285..243772e79 100644 --- a/spinifex/handlers/elbv2/service_nats_e2e_test.go +++ b/spinifex/handlers/elbv2/service_nats_e2e_test.go @@ -36,6 +36,7 @@ func setupNATSELBv2Test(t *testing.T) (ELBv2Service, *ELBv2ServiceImpl) { "elbv2.DescribeTargetHealth": func(msg *nats.Msg) { handleNATSMsg(msg, backend.DescribeTargetHealth) }, "elbv2.CreateListener": func(msg *nats.Msg) { handleNATSMsg(msg, backend.CreateListener) }, "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) }, } @@ -233,6 +234,36 @@ func TestNATSE2E_DeleteLoadBalancerCascadesListeners(t *testing.T) { require.NoError(t, err) } +func TestNATSE2E_ModifyListener(t *testing.T) { + client, _ := setupNATSELBv2Test(t) + + tgOut, _ := client.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("e2e-mod-tg")}, testAccountID) + tgArn := tgOut.TargetGroups[0].TargetGroupArn + + lbOut, _ := client.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("e2e-mod-lb")}, testAccountID) + lbArn := lbOut.LoadBalancers[0].LoadBalancerArn + + lstOut, err := client.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbArn, + Port: aws.Int64(80), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tgArn}}, + }, testAccountID) + require.NoError(t, err) + + modOut, err := client.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: lstOut.Listeners[0].ListenerArn, + Port: aws.Int64(8080), + }, testAccountID) + require.NoError(t, err) + require.Len(t, modOut.Listeners, 1) + assert.Equal(t, int64(8080), *modOut.Listeners[0].Port) + + desc, err := client.DescribeListeners(&elbv2.DescribeListenersInput{LoadBalancerArn: lbArn}, testAccountID) + require.NoError(t, err) + require.Len(t, desc.Listeners, 1) + assert.Equal(t, int64(8080), *desc.Listeners[0].Port) +} + func TestNATSE2E_TargetGroupInUseProtection(t *testing.T) { client, _ := setupNATSELBv2Test(t) diff --git a/spinifex/handlers/elbv2/system_instance_launcher_nats_test.go b/spinifex/handlers/elbv2/system_instance_launcher_nats_test.go index f43540387..5cb0b9f96 100644 --- a/spinifex/handlers/elbv2/system_instance_launcher_nats_test.go +++ b/spinifex/handlers/elbv2/system_instance_launcher_nats_test.go @@ -137,6 +137,107 @@ func TestNATSSystemInstanceLauncher_TerminateRoundTrip(t *testing.T) { } } +func TestNewNATSSystemInstanceLauncher_DefaultTimeout(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + launcher := NewNATSSystemInstanceLauncher(nc, 0) + concrete, ok := launcher.(*natsSystemInstanceLauncher) + require.True(t, ok) + assert.Equal(t, defaultSystemInstanceTimeout, concrete.timeout) + + negative, ok := NewNATSSystemInstanceLauncher(nc, -1*time.Second).(*natsSystemInstanceLauncher) + require.True(t, ok) + assert.Equal(t, defaultSystemInstanceTimeout, negative.timeout) +} + +func TestNATSSystemInstanceLauncher_LaunchNilInput(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + launcher := NewNATSSystemInstanceLauncher(nc, time.Second) + _, err := launcher.LaunchSystemInstance(nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "input is nil") +} + +func TestNATSSystemInstanceLauncher_LaunchMissingInstanceType(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + launcher := NewNATSSystemInstanceLauncher(nc, time.Second) + _, err := launcher.LaunchSystemInstance(&SystemInstanceInput{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing InstanceType") +} + +func TestNATSSystemInstanceLauncher_LaunchDecodeFailure(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + sub, err := nc.Subscribe("system.LaunchInstance.sys.micro", func(msg *nats.Msg) { + _ = msg.Respond([]byte("{not valid json")) + }) + require.NoError(t, err) + defer func() { _ = sub.Unsubscribe() }() + + launcher := NewNATSSystemInstanceLauncher(nc, 2*time.Second) + _, err = launcher.LaunchSystemInstance(&SystemInstanceInput{InstanceType: "sys.micro"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "decode launch reply") +} + +func TestNATSSystemInstanceLauncher_LaunchMissingOutput(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + sub, err := nc.Subscribe("system.LaunchInstance.sys.micro", func(msg *nats.Msg) { + _ = msg.Respond([]byte("{}")) + }) + require.NoError(t, err) + defer func() { _ = sub.Unsubscribe() }() + + launcher := NewNATSSystemInstanceLauncher(nc, 2*time.Second) + _, err = launcher.LaunchSystemInstance(&SystemInstanceInput{InstanceType: "sys.micro"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing output payload") +} + +func TestNATSSystemInstanceLauncher_TerminateEmptyID(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + launcher := NewNATSSystemInstanceLauncher(nc, time.Second) + err := launcher.TerminateSystemInstance("") + require.Error(t, err) + assert.Contains(t, err.Error(), "empty instance ID") +} + +func TestNATSSystemInstanceLauncher_TerminateNoResponder(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + launcher := NewNATSSystemInstanceLauncher(nc, 250*time.Millisecond) + err := launcher.TerminateSystemInstance("i-no-one-listens") + require.Error(t, err) +} + +func TestNATSSystemInstanceLauncher_TerminateDecodeFailure(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + defer nc.Close() + + sub, err := nc.Subscribe("system.TerminateInstance.i-decode", func(msg *nats.Msg) { + _ = msg.Respond([]byte("{not valid json")) + }) + require.NoError(t, err) + defer func() { _ = sub.Unsubscribe() }() + + launcher := NewNATSSystemInstanceLauncher(nc, 2*time.Second) + err = launcher.TerminateSystemInstance("i-decode") + require.Error(t, err) + assert.Contains(t, err.Error(), "decode terminate reply") +} + func TestNATSSystemInstanceLauncher_TerminatePropagatesRemoteError(t *testing.T) { _, nc, _ := testutil.StartTestJetStream(t) defer nc.Close() diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.test.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.test.tsx index a289fcdc2..aa0371d15 100644 --- a/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.test.tsx +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.test.tsx @@ -54,16 +54,11 @@ describe("ListenersTab", () => { mockSend.mockReset() }) - it("renders empty state and delete-note when no listeners", () => { + it("renders empty state when no listeners", () => { const queryClient = seedClient({}) render(, { wrapper: createWrapper(queryClient), }) - expect( - screen.getByText( - /listeners cannot be edited\. delete and re-add to change/i, - ), - ).toBeInTheDocument() expect(screen.getByText("No listeners configured.")).toBeInTheDocument() expect( screen.getByRole("button", { name: "Add listener" }), @@ -162,6 +157,58 @@ describe("ListenersTab", () => { expect(screen.queryByText(/tg-other-vpc/)).not.toBeInTheDocument() }) + it("opens edit dialog pre-filled and calls ModifyListenerCommand", async () => { + const queryClient = seedClient({ + listeners: { + Listeners: [ + { + ListenerArn: "arn:listener/1", + LoadBalancerArn: LB_ARN, + Protocol: "HTTP", + Port: 80, + DefaultActions: [{ Type: "forward", TargetGroupArn: TG_ARN_A }], + }, + ], + }, + targetGroups: { + TargetGroups: [ + { + TargetGroupArn: TG_ARN_A, + TargetGroupName: "tg-a", + Protocol: "HTTP", + Port: 80, + VpcId: "vpc-aaa", + }, + ], + }, + }) + mockSend.mockResolvedValue({}) + const user = userEvent.setup() + render(, { + wrapper: createWrapper(queryClient), + }) + await user.click(screen.getByLabelText("Edit listener HTTP:80")) + await expect( + screen.findByRole("alertdialog", { name: /edit listener/i }), + ).resolves.toBeInTheDocument() + const portInput = screen.getByLabelText(/^port$/i) as HTMLInputElement + expect(portInput.value).toBe("80") + await user.clear(portInput) + await user.type(portInput, "8080") + const before = mockSend.mock.calls.length + await user.click(screen.getByRole("button", { name: /save changes/i })) + await waitFor(() => + expect(mockSend.mock.calls.length).toBeGreaterThan(before), + ) + const call = mockSend.mock.calls[before]?.[0] + expect(call.input).toStrictEqual({ + ListenerArn: "arn:listener/1", + Protocol: "HTTP", + Port: 8080, + DefaultActions: [{ Type: "forward", TargetGroupArn: TG_ARN_A }], + }) + }) + it("submits add-listener form and calls CreateListenerCommand", async () => { const queryClient = seedClient({ targetGroups: { diff --git a/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.tsx b/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.tsx index 1202b635c..bc95b018f 100644 --- a/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.tsx +++ b/spinifex/services/spinifexui/frontend/src/components/elbv2/listeners-tab.tsx @@ -1,7 +1,7 @@ import type { Listener } from "@aws-sdk/client-elastic-load-balancing-v2" import { zodResolver } from "@hookform/resolvers/zod" import { useSuspenseQuery } from "@tanstack/react-query" -import { Trash2 } from "lucide-react" +import { Pencil, Trash2 } from "lucide-react" import { useEffect, useState } from "react" import { useForm } from "react-hook-form" @@ -19,7 +19,11 @@ import { AlertDialogTitle, } from "@/components/ui/alert-dialog" import { Button } from "@/components/ui/button" -import { useCreateListener, useDeleteListener } from "@/mutations/elbv2" +import { + useCreateListener, + useDeleteListener, + useModifyListener, +} from "@/mutations/elbv2" import { elbv2ListenersQueryOptions, elbv2TargetGroupsQueryOptions, @@ -42,8 +46,10 @@ export function ListenersTab({ loadBalancerArn, vpcId }: ListenersTabProps) { const createMutation = useCreateListener() const deleteMutation = useDeleteListener() + const modifyMutation = useModifyListener() const [addOpen, setAddOpen] = useState(false) + const [editTarget, setEditTarget] = useState() const [deleteTarget, setDeleteTarget] = useState() const listeners = listenersData.Listeners ?? [] @@ -83,6 +89,24 @@ export function ListenersTab({ loadBalancerArn, vpcId }: ListenersTabProps) { } } + const handleEdit = async (data: CreateListenerFormData) => { + if (!editTarget?.ListenerArn) { + return + } + try { + await modifyMutation.mutateAsync({ + listenerArn: editTarget.ListenerArn, + loadBalancerArn, + protocol: data.protocol, + port: data.port, + defaultTargetGroupArn: data.defaultTargetGroupArn, + }) + setEditTarget(undefined) + } catch { + // surfaced via mutation.error + } + } + const handleDelete = async () => { if (!deleteTarget?.ListenerArn) { return @@ -96,16 +120,18 @@ export function ListenersTab({ loadBalancerArn, vpcId }: ListenersTabProps) { return (
-

- Listeners cannot be edited. Delete and re-add to change. -

- {createMutation.error && ( )} + {modifyMutation.error && ( + + )} {deleteMutation.error && ( - +
+ + +
))} @@ -177,6 +213,14 @@ export function ListenersTab({ loadBalancerArn, vpcId }: ListenersTabProps) { targetGroups={vpcTgs} /> + !open && setEditTarget(undefined)} + onSubmit={handleEdit} + targetGroups={vpcTgs} + /> + - + Add listener @@ -240,7 +287,7 @@ function AddListenerDialog({
{ e.preventDefault() void handleConfirm() @@ -259,3 +306,77 @@ function AddListenerDialog({ ) } + +interface EditListenerDialogProps { + listener: Listener | undefined + onOpenChange: (open: boolean) => void + targetGroups: React.ComponentProps["targetGroups"] + isPending: boolean + onSubmit: (data: CreateListenerFormData) => void +} + +function EditListenerDialog({ + listener, + onOpenChange, + targetGroups, + isPending, + onSubmit, +}: EditListenerDialogProps) { + const open = listener !== undefined + + const form = useForm({ + resolver: zodResolver(createListenerSchema), + defaultValues: { + protocol: "HTTP", + port: 80, + defaultTargetGroupArn: "", + }, + }) + + useEffect(() => { + if (!listener) { + return + } + form.reset({ + protocol: "HTTP", + port: listener.Port ?? 80, + defaultTargetGroupArn: listener.DefaultActions?.[0]?.TargetGroupArn ?? "", + }) + }, [listener, form]) + + const handleConfirm = form.handleSubmit(onSubmit) + + return ( + + + + Edit listener + + Update port, protocol, or default target group. Changes apply + without dropping the listener. + + + + { + e.preventDefault() + void handleConfirm() + }} + > + + + + + Cancel + + {isPending ? "Saving\u2026" : "Save changes"} + + + + + ) +} diff --git a/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts b/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts index dbd106c43..46ecd2bce 100644 --- a/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts +++ b/spinifex/services/spinifexui/frontend/src/mutations/elbv2.ts @@ -9,6 +9,7 @@ import { DeleteLoadBalancerCommand, DeleteTargetGroupCommand, DeregisterTargetsCommand, + ModifyListenerCommand, ModifyLoadBalancerAttributesCommand, ModifyTargetGroupAttributesCommand, RegisterTargetsCommand, @@ -196,6 +197,40 @@ export function useDeleteListener() { }) } +export interface ModifyListenerParams { + listenerArn: string + loadBalancerArn: string + protocol: "HTTP" + port: number + defaultTargetGroupArn: string +} + +export function useModifyListener() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: async (params: ModifyListenerParams) => { + const defaultActions: Action[] = [ + { + Type: "forward", + TargetGroupArn: params.defaultTargetGroupArn, + }, + ] + const command = new ModifyListenerCommand({ + ListenerArn: params.listenerArn, + Protocol: params.protocol, + Port: params.port, + DefaultActions: defaultActions, + }) + return await getElbv2Client().send(command) + }, + onSuccess: (_data, variables) => { + void queryClient.invalidateQueries({ + queryKey: ["elbv2", "listeners", variables.loadBalancerArn], + }) + }, + }) +} + export interface LbWizardCreatedResource { type: string id: string | undefined diff --git a/tests/e2e/TEST_COVERAGE.md b/tests/e2e/TEST_COVERAGE.md index 54e2dc3e9..bbd0723cf 100644 --- a/tests/e2e/TEST_COVERAGE.md +++ b/tests/e2e/TEST_COVERAGE.md @@ -760,6 +760,19 @@ Requires pool mode with external IPAM (NOT dev_networking). - Terminate client VM - Cleanup: delete LB, verify ENI removal, delete TG +### Phase 3e: ALB Internal — ModifyListener +- Create two HTTP target groups (tgA, tgB), register the same 2 app instances in each +- `create-load-balancer` (type=application, scheme=internal) +- `create-listener` (HTTP:80) → tgA +- Wait for ALB active, both TGs healthy +- Baseline probe at port 80 → assert round-robin across both instances +- `modify-listener` Port=8090 (same tgA), verify DescribeListeners persisted +- Probe at port 8090 → assert round-robin (proves HAProxy reconciled without + dropping the listener) +- `modify-listener` DefaultActions → tgB (same port 8090) +- Probe at port 8090 → assert round-robin still flows through tgB +- Cleanup: delete listener, LB, deregister, delete both TGs + ### Phase 4: Cleanup (trap) - Terminate app instances, wait for terminated - Delete key pair, subnet, IGW, VPC diff --git a/tests/e2e/lb/lb_test.go b/tests/e2e/lb/lb_test.go index b99f74f21..6deca3f37 100644 --- a/tests/e2e/lb/lb_test.go +++ b/tests/e2e/lb/lb_test.go @@ -105,6 +105,10 @@ func TestLoadBalancer(t *testing.T) { time.Sleep(15 * time.Second) runLBSuite(t, client, fixture, kindNLB, "internal", nil, "") }) + t.Run("Internal_ALB_ModifyListener", func(t *testing.T) { + time.Sleep(15 * time.Second) + runModifyListenerSuite(t, client, fixture) + }) } // --- Fixture: shared VPC, subnet, IGW, SG, app instances ---------------- @@ -439,7 +443,7 @@ func runLBSuite(t *testing.T, c *harness.AWSClient, f *sharedFixture, kind lbKin harness.WaitForLBActive(t, c, lb.ARN, label, 5*time.Minute) harness.WaitForTargetsHealthy(t, c, tgArn, 2, label, 2*time.Minute) - eni := lbENI(t, c, scheme, eniDescPrefix, lb.ID) + eni := lbENI(t, c, eniDescPrefix, lb) if scheme == "internet-facing" { ip := publicIP(eni) @@ -557,15 +561,14 @@ func deleteLB(t *testing.T, c *harness.AWSClient, lb lbInfo) { if lb.ARN == "" { return } - prefix, lbName := "app", "alb" + prefix := "app" if lb.Type == "network" { - prefix, lbName = "net", "nlb" + prefix = "net" } - suffix := "int" - if lb.Scheme == "internet-facing" { - suffix = "inet" - } - filter := fmt.Sprintf("ELB %s/lb-e2e-%s-%s/%s", prefix, lbName, suffix, lb.ID) + parts := strings.Split(lb.ARN, "/") + require.GreaterOrEqualf(t, len(parts), 3, "deleteLB: malformed LB ARN %q", lb.ARN) + lbName := parts[len(parts)-2] + filter := fmt.Sprintf("ELB %s/%s/%s", prefix, lbName, lb.ID) // Capture the underlying sys.micro VM id before deleting so we can wait // for it to actually terminate. ELBv2.DeleteLoadBalancer returns once @@ -656,17 +659,193 @@ func deleteListener(t *testing.T, c *harness.AWSClient, arn string) { } } -func lbENI(t *testing.T, c *harness.AWSClient, scheme, prefix, lbID string) *ec2.NetworkInterface { +// modifyListenerPort changes the listener's port in place. Verifies that +// HAProxy is reconciled without the listener being deleted (no traffic outage). +func modifyListenerPort(t *testing.T, c *harness.AWSClient, listenerArn string, port int64) { t.Helper() - suffix := "int" - if scheme == "internet-facing" { - suffix = "inet" + _, err := c.ELBv2.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: aws.String(listenerArn), + Port: aws.Int64(port), + }) + require.NoErrorf(t, err, "modify-listener port=%d", port) + t.Logf("listener %s: port → %d", listenerArn, port) +} + +// modifyListenerDefaultTG swaps the listener's default target group. +func modifyListenerDefaultTG(t *testing.T, c *harness.AWSClient, listenerArn, tgArn string) { + t.Helper() + _, err := c.ELBv2.ModifyListener(&elbv2.ModifyListenerInput{ + ListenerArn: aws.String(listenerArn), + DefaultActions: []*elbv2.Action{{ + Type: aws.String("forward"), + TargetGroupArn: aws.String(tgArn), + }}, + }) + require.NoError(t, err, "modify-listener default actions") + t.Logf("listener %s: default TG → %s", listenerArn, tgArn) +} + +// describeListenerPort fetches the current port of a listener — used to +// assert that ModifyListener actually persisted before probing. +func describeListenerPort(t *testing.T, c *harness.AWSClient, listenerArn string) int64 { + t.Helper() + out, err := c.ELBv2.DescribeListeners(&elbv2.DescribeListenersInput{ + ListenerArns: []*string{aws.String(listenerArn)}, + }) + require.NoError(t, err, "describe-listeners") + require.NotEmpty(t, out.Listeners) + return aws.Int64Value(out.Listeners[0].Port) +} + +// runModifyListenerSuite verifies that an in-place listener edit reroutes +// traffic without dropping the listener — the regression mulga-944 fixes +// (frontend previously delete+created on every edit). +// +// - Start: listener HTTP:80 → tgA, probes round-robin across 2 app instances. +// - ModifyListener port=8090 → same tgA. Probe at 8090 must succeed +// immediately (port: 80 must not be active any more, port 8090 must serve). +// - ModifyListener defaultActions → tgB (same backends). Probe still +// round-robins; proves DefaultActions swap is honoured by HAProxy. +func runModifyListenerSuite(t *testing.T, c *harness.AWSClient, f *sharedFixture) { + t.Helper() + const label = "ALB internal ModifyListener" + const altPort int64 = 8090 + + tgA := createTargetGroup(t, c, f, "lb-e2e-mod-tg-a", "HTTP", httpPort, "/index.html") + t.Cleanup(func() { deleteTargetGroup(t, c, tgA) }) + + tgB := createTargetGroup(t, c, f, "lb-e2e-mod-tg-b", "HTTP", httpPort, "/index.html") + t.Cleanup(func() { deleteTargetGroup(t, c, tgB) }) + + registerTargets(t, c, tgA, f.AppInstanceIDs) + t.Cleanup(func() { deregisterTargets(t, c, tgA, f.AppInstanceIDs) }) + registerTargets(t, c, tgB, f.AppInstanceIDs) + t.Cleanup(func() { deregisterTargets(t, c, tgB, f.AppInstanceIDs) }) + + lb := createLB(t, c, f, "lb-e2e-mod", "application", "internal") + t.Cleanup(func() { deleteLB(t, c, lb) }) + + listener := createListener(t, c, lb.ARN, "HTTP", httpPort, tgA) + t.Cleanup(func() { deleteListener(t, c, listener) }) + + harness.WaitForLBActive(t, c, lb.ARN, label, 5*time.Minute) + harness.WaitForTargetsHealthy(t, c, tgA, 2, label+" tgA", 2*time.Minute) + + eni := lbENI(t, c, "app", lb) + priv := privateIP(eni) + require.NotEmpty(t, priv, label+" needs private IP") + + // Baseline: traffic on the original port. + probeAtPort(t, f, priv, httpPort, label+" before modify (port 80)") + + // In-place port change. + modifyListenerPort(t, c, listener, altPort) + assert.Equal(t, altPort, describeListenerPort(t, c, listener), "listener port persisted") + // lb-agent picks up the new HAProxy config on its next 5s tick, not + // synchronously with the ModifyListener API. Poll until the listener + // actually answers on altPort before driving the assertive probe. + waitForListenerServing(t, f, priv, altPort, label+" port 8090", 60*time.Second) + probeAtPort(t, f, priv, altPort, label+" after port-modify (port 8090)") + + // Defaults: in-place TG swap on same port. + modifyListenerDefaultTG(t, c, listener, tgB) + harness.WaitForTargetsHealthy(t, c, tgB, 2, label+" tgB (post-swap)", 2*time.Minute) + probeAtPort(t, f, priv, altPort, label+" after TG-swap (still port 8090, now tgB)") +} + +// waitForListenerServing polls the shared client with a tiny round of probes +// until at least one returns a successful instance_id payload. Used after +// ModifyListener calls so the assertive probeAtPort doesn't race the +// lb-agent's poll-driven HAProxy reload (5s tick + reload latency). +func waitForListenerServing(t *testing.T, f *sharedFixture, lbIP string, port int64, label string, timeout time.Duration) { + t.Helper() + harness.EventuallyErr(t, func() error { + r, err := probeOnce(f, lbIP, port, 2) + if err != nil { + return err + } + if r.Successful == 0 { + return fmt.Errorf("%s: listener not serving on :%d (0/%d successful)", label, port, r.Total) + } + return nil + }, timeout, 2*time.Second) +} + +// probeOnce drives the shared client to issue `count` HTTP probes at +// lbIP:port and returns the parsed TrafficResult. Errors are returned rather +// than failing the test so callers can poll. +func probeOnce(f *sharedFixture, lbIP string, port int64, count int) (harness.TrafficResult, error) { + resultsFile := fmt.Sprintf("mod-%d-%d.txt", port, time.Now().UnixNano()) + order := map[string]any{ + "proto": "http", + "ip": lbIP, + "count": count, + "outfile": resultsFile, + "http_port": port, + "tcp_port": tcpPort, } - lbName := "alb" - if prefix == "net" { - lbName = "nlb" + body, err := json.Marshal(order) + if err != nil { + return harness.TrafficResult{}, err } - desc := fmt.Sprintf("ELB %s/lb-e2e-%s-%s/%s", prefix, lbName, suffix, lbID) + triggerURL := fmt.Sprintf("http://%s:%d/", f.ClientPublicIP, triggerPort) + client := &http.Client{Timeout: 90 * time.Second} + resp, err := client.Post(triggerURL, "application/json", bytes.NewReader(body)) + if err != nil { + return harness.TrafficResult{}, fmt.Errorf("trigger: %w", err) + } + resp.Body.Close() + if resp.StatusCode != 200 { + return harness.TrafficResult{}, fmt.Errorf("trigger status %d", resp.StatusCode) + } + results, err := plainHTTPGet( + fmt.Sprintf("http://%s:%d/%s", f.ClientPublicIP, httpPort, resultsFile), + 10*time.Second) + if err != nil { + return harness.TrafficResult{}, fmt.Errorf("fetch %s: %w", resultsFile, err) + } + return harness.VerifyResultsLines(results, "http"), nil +} + +// probeAtPort runs round-robin probes from the shared client to lbIP:port and +// asserts both app instances respond. Mirrors runInternalTrafficViaClient but +// takes an explicit port so ModifyListener edits can be verified. +func probeAtPort(t *testing.T, f *sharedFixture, lbIP string, port int64, label string) { + t.Helper() + resultsFile := fmt.Sprintf("mod-%d-%d.txt", port, time.Now().UnixNano()) + order := map[string]any{ + "proto": "http", + "ip": lbIP, + "count": probesPerRun, + "outfile": resultsFile, + "http_port": port, + "tcp_port": tcpPort, + } + body, err := json.Marshal(order) + require.NoError(t, err) + + triggerURL := fmt.Sprintf("http://%s:%d/", f.ClientPublicIP, triggerPort) + client := &http.Client{Timeout: 90 * time.Second} + resp, err := client.Post(triggerURL, "application/json", bytes.NewReader(body)) + require.NoErrorf(t, err, "trigger probe %s", label) + resp.Body.Close() + require.Equalf(t, 200, resp.StatusCode, "trigger probe %s status", label) + + results, err := plainHTTPGet( + fmt.Sprintf("http://%s:%d/%s", f.ClientPublicIP, httpPort, resultsFile), + 10*time.Second) + require.NoErrorf(t, err, "fetch %s", resultsFile) + harness.AssertRoundRobin(t, + harness.VerifyResultsLines(results, "http"), + 1, probesPerRun/2, label) +} + +func lbENI(t *testing.T, c *harness.AWSClient, prefix string, lb lbInfo) *ec2.NetworkInterface { + t.Helper() + parts := strings.Split(lb.ARN, "/") + require.GreaterOrEqualf(t, len(parts), 3, "lbENI: malformed LB ARN %q", lb.ARN) + lbName := parts[len(parts)-2] + desc := fmt.Sprintf("ELB %s/%s/%s", prefix, lbName, lb.ID) var eni *ec2.NetworkInterface harness.EventuallyErr(t, func() error { out, err := c.EC2.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{