From 3b0376c3a67da353ef3780154841e5239019aac9 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 23 Apr 2026 10:04:11 +1000 Subject: [PATCH 1/5] fix: Makefile --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 9e1d77763..7447a17d2 100644 --- a/Makefile +++ b/Makefile @@ -34,6 +34,9 @@ else endif # Quiet-mode filters (active when QUIET=1, set by preflight via recursive make) +# Note: grep pipelines use PIPESTATUS[0] so the exit status of `go test` +# propagates through the filter — otherwise a test failure is swallowed by +# grep's own (success) exit code and preflight prints "passed" on red. ifdef QUIET _Q = @ _COVQ = 2>&1 | { grep -Ev '^\s*(ok|PASS|\?|=== RUN|--- PASS:)\s' | grep -v 'coverage: 0\.0%' || true; }; exit $${PIPESTATUS[0]} From 0f89db1893fc754f582b40118d601815cfd15210 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 23 Apr 2026 10:46:16 +1000 Subject: [PATCH 2/5] chore: extract validation logic from gateway function --- .../ec2/account/DescribeAccountAttributes.go | 2 -- spinifex/gateway/ec2/eip/AssociateAddress.go | 18 ++++++++++---- .../gateway/ec2/eip/DisassociateAddress.go | 18 ++++++++++---- spinifex/gateway/ec2/eip/ReleaseAddress.go | 18 ++++++++++---- .../gateway/ec2/igw/AttachInternetGateway.go | 24 ++++++++++++------- .../gateway/ec2/igw/DeleteInternetGateway.go | 18 ++++++++++---- .../gateway/ec2/igw/DetachInternetGateway.go | 24 ++++++++++++------- .../gateway/ec2/instance/GetConsoleOutput.go | 16 +++++++++++-- .../gateway/ec2/instance/RebootInstances.go | 15 ++++++++++-- .../ec2/instance/RebootInstances_test.go | 2 +- .../gateway/ec2/instance/StartInstances.go | 17 ++++++++++--- .../ec2/instance/StartInstances_test.go | 3 ++- .../gateway/ec2/instance/StopInstances.go | 16 +++++++++++-- .../ec2/instance/StopInstances_test.go | 3 ++- .../ec2/instance/TerminateInstances.go | 16 +++++++++++-- .../ec2/instance/TerminateInstances_test.go | 3 ++- .../placementgroup/CreatePlacementGroup.go | 24 ++++++++++++------- .../placementgroup/DeletePlacementGroup.go | 18 ++++++++++---- .../gateway/ec2/vpc/CreateNetworkInterface.go | 18 ++++++++++---- spinifex/gateway/ec2/vpc/CreateSubnet.go | 24 ++++++++++++------- spinifex/gateway/ec2/vpc/CreateVpc.go | 18 ++++++++++---- .../gateway/ec2/vpc/DeleteNetworkInterface.go | 18 ++++++++++---- spinifex/gateway/ec2/vpc/DeleteSubnet.go | 18 ++++++++++---- spinifex/gateway/ec2/vpc/DeleteVpc.go | 18 ++++++++++---- .../gateway/ec2/vpc/DescribeVpcAttribute.go | 24 ++++++++++++------- .../vpc/ModifyNetworkInterfaceAttribute.go | 18 ++++++++++---- .../gateway/ec2/vpc/ModifySubnetAttribute.go | 18 ++++++++++---- .../gateway/ec2/vpc/ModifyVpcAttribute.go | 18 ++++++++++---- spinifex/gateway/elbv2/CreateListener.go | 24 ++++++++++++------- spinifex/gateway/elbv2/CreateLoadBalancer.go | 18 ++++++++++---- spinifex/gateway/elbv2/CreateTargetGroup.go | 18 ++++++++++---- spinifex/gateway/elbv2/DeleteListener.go | 18 ++++++++++---- spinifex/gateway/elbv2/DeleteLoadBalancer.go | 18 ++++++++++---- spinifex/gateway/elbv2/DeleteTargetGroup.go | 18 ++++++++++---- spinifex/gateway/elbv2/DeregisterTargets.go | 24 ++++++++++++------- .../elbv2/DescribeLoadBalancerAttributes.go | 18 ++++++++++---- .../elbv2/DescribeTargetGroupAttributes.go | 18 ++++++++++---- .../gateway/elbv2/DescribeTargetHealth.go | 18 ++++++++++---- spinifex/gateway/elbv2/GetLBConfig.go | 18 ++++++++++---- spinifex/gateway/elbv2/LBAgentHeartbeat.go | 18 ++++++++++---- .../elbv2/ModifyLoadBalancerAttributes.go | 24 ++++++++++++------- .../elbv2/ModifyTargetGroupAttributes.go | 24 ++++++++++++------- spinifex/gateway/elbv2/RegisterTargets.go | 24 ++++++++++++------- 43 files changed, 535 insertions(+), 212 deletions(-) diff --git a/spinifex/gateway/ec2/account/DescribeAccountAttributes.go b/spinifex/gateway/ec2/account/DescribeAccountAttributes.go index ac7358ca4..9f80c985a 100644 --- a/spinifex/gateway/ec2/account/DescribeAccountAttributes.go +++ b/spinifex/gateway/ec2/account/DescribeAccountAttributes.go @@ -9,8 +9,6 @@ import ( // DescribeAccountAttributes returns static account attributes for the Spinifex platform func DescribeAccountAttributes(input *ec2.DescribeAccountAttributesInput) (*ec2.DescribeAccountAttributesOutput, error) { - slog.Debug("DescribeAccountAttributes called") - // Build set of requested attribute names for filtering requestedAttrs := make(map[string]bool) for _, name := range input.AttributeNames { diff --git a/spinifex/gateway/ec2/eip/AssociateAddress.go b/spinifex/gateway/ec2/eip/AssociateAddress.go index 1b3ba4940..6e35b82be 100644 --- a/spinifex/gateway/ec2/eip/AssociateAddress.go +++ b/spinifex/gateway/ec2/eip/AssociateAddress.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateAssociateAddressInput validates the input parameters +func ValidateAssociateAddressInput(input *ec2.AssociateAddressInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.AllocationId == nil || *input.AllocationId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // AssociateAddress handles the EC2 AssociateAddress API call. func AssociateAddress(input *ec2.AssociateAddressInput, natsConn *nats.Conn, accountID string) (ec2.AssociateAddressOutput, error) { var output ec2.AssociateAddressOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.AllocationId == nil || *input.AllocationId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateAssociateAddressInput(input); err != nil { + return output, err } svc := handlers_ec2_eip.NewNATSEIPService(natsConn) diff --git a/spinifex/gateway/ec2/eip/DisassociateAddress.go b/spinifex/gateway/ec2/eip/DisassociateAddress.go index e9fcbc333..d444fe0e4 100644 --- a/spinifex/gateway/ec2/eip/DisassociateAddress.go +++ b/spinifex/gateway/ec2/eip/DisassociateAddress.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDisassociateAddressInput validates the input parameters +func ValidateDisassociateAddressInput(input *ec2.DisassociateAddressInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.AssociationId == nil || *input.AssociationId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DisassociateAddress handles the EC2 DisassociateAddress API call. func DisassociateAddress(input *ec2.DisassociateAddressInput, natsConn *nats.Conn, accountID string) (ec2.DisassociateAddressOutput, error) { var output ec2.DisassociateAddressOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.AssociationId == nil || *input.AssociationId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDisassociateAddressInput(input); err != nil { + return output, err } svc := handlers_ec2_eip.NewNATSEIPService(natsConn) diff --git a/spinifex/gateway/ec2/eip/ReleaseAddress.go b/spinifex/gateway/ec2/eip/ReleaseAddress.go index e0f2362ba..cdd963731 100644 --- a/spinifex/gateway/ec2/eip/ReleaseAddress.go +++ b/spinifex/gateway/ec2/eip/ReleaseAddress.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateReleaseAddressInput validates the input parameters +func ValidateReleaseAddressInput(input *ec2.ReleaseAddressInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.AllocationId == nil || *input.AllocationId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // ReleaseAddress handles the EC2 ReleaseAddress API call. func ReleaseAddress(input *ec2.ReleaseAddressInput, natsConn *nats.Conn, accountID string) (ec2.ReleaseAddressOutput, error) { var output ec2.ReleaseAddressOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.AllocationId == nil || *input.AllocationId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateReleaseAddressInput(input); err != nil { + return output, err } svc := handlers_ec2_eip.NewNATSEIPService(natsConn) diff --git a/spinifex/gateway/ec2/igw/AttachInternetGateway.go b/spinifex/gateway/ec2/igw/AttachInternetGateway.go index 1bf21179b..e6f42c655 100644 --- a/spinifex/gateway/ec2/igw/AttachInternetGateway.go +++ b/spinifex/gateway/ec2/igw/AttachInternetGateway.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateAttachInternetGatewayInput validates the input parameters +func ValidateAttachInternetGatewayInput(input *ec2.AttachInternetGatewayInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // AttachInternetGateway handles the EC2 AttachInternetGateway API call func AttachInternetGateway(input *ec2.AttachInternetGatewayInput, natsConn *nats.Conn, accountID string) (ec2.AttachInternetGatewayOutput, error) { var output ec2.AttachInternetGatewayOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateAttachInternetGatewayInput(input); err != nil { + return output, err } svc := handlers_ec2_igw.NewNATSIGWService(natsConn) diff --git a/spinifex/gateway/ec2/igw/DeleteInternetGateway.go b/spinifex/gateway/ec2/igw/DeleteInternetGateway.go index 721defa43..1c8e04092 100644 --- a/spinifex/gateway/ec2/igw/DeleteInternetGateway.go +++ b/spinifex/gateway/ec2/igw/DeleteInternetGateway.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteInternetGatewayInput validates the input parameters +func ValidateDeleteInternetGatewayInput(input *ec2.DeleteInternetGatewayInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteInternetGateway handles the EC2 DeleteInternetGateway API call func DeleteInternetGateway(input *ec2.DeleteInternetGatewayInput, natsConn *nats.Conn, accountID string) (ec2.DeleteInternetGatewayOutput, error) { var output ec2.DeleteInternetGatewayOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteInternetGatewayInput(input); err != nil { + return output, err } svc := handlers_ec2_igw.NewNATSIGWService(natsConn) diff --git a/spinifex/gateway/ec2/igw/DetachInternetGateway.go b/spinifex/gateway/ec2/igw/DetachInternetGateway.go index 5e97dab82..7603819cd 100644 --- a/spinifex/gateway/ec2/igw/DetachInternetGateway.go +++ b/spinifex/gateway/ec2/igw/DetachInternetGateway.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDetachInternetGatewayInput validates the input parameters +func ValidateDetachInternetGatewayInput(input *ec2.DetachInternetGatewayInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DetachInternetGateway handles the EC2 DetachInternetGateway API call func DetachInternetGateway(input *ec2.DetachInternetGatewayInput, natsConn *nats.Conn, accountID string) (ec2.DetachInternetGatewayOutput, error) { var output ec2.DetachInternetGatewayOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.InternetGatewayId == nil || *input.InternetGatewayId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDetachInternetGatewayInput(input); err != nil { + return output, err } svc := handlers_ec2_igw.NewNATSIGWService(natsConn) diff --git a/spinifex/gateway/ec2/instance/GetConsoleOutput.go b/spinifex/gateway/ec2/instance/GetConsoleOutput.go index ca6b1c9eb..857ae1e51 100644 --- a/spinifex/gateway/ec2/instance/GetConsoleOutput.go +++ b/spinifex/gateway/ec2/instance/GetConsoleOutput.go @@ -8,15 +8,27 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" ) +// ValidateGetConsoleOutputInput validates the input parameters +func ValidateGetConsoleOutputInput(input *ec2.GetConsoleOutputInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.InstanceId == nil || *input.InstanceId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // GetConsoleOutput retrieves console output for a specific instance via NATS. // Routes directly to the node running the instance via ec2.{instanceID}.GetConsoleOutput. func GetConsoleOutput(input *ec2.GetConsoleOutputInput, natsConn *nats.Conn, accountID string) (*ec2.GetConsoleOutputOutput, error) { - if input.InstanceId == nil || *input.InstanceId == "" { - return nil, fmt.Errorf("InstanceId is required") + if err := ValidateGetConsoleOutputInput(input); err != nil { + return nil, err } jsonData, err := json.Marshal(input) diff --git a/spinifex/gateway/ec2/instance/RebootInstances.go b/spinifex/gateway/ec2/instance/RebootInstances.go index cae94eeb1..ed7ffaf73 100644 --- a/spinifex/gateway/ec2/instance/RebootInstances.go +++ b/spinifex/gateway/ec2/instance/RebootInstances.go @@ -14,12 +14,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateRebootInstancesInput validates the input parameters +func ValidateRebootInstancesInput(input *ec2.RebootInstancesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if len(input.InstanceIds) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // RebootInstances sends reboot commands to specified instances via NATS. // Unlike stop+start, reboot keeps the instance running and sends a QMP system_reset. // Returns an empty response on success (AWS returns no state-change data). func RebootInstances(input *ec2.RebootInstancesInput, natsConn *nats.Conn, accountID string) (*ec2.RebootInstancesOutput, error) { - if len(input.InstanceIds) == 0 { - return nil, fmt.Errorf("no instance IDs provided") + if err := ValidateRebootInstancesInput(input); err != nil { + return nil, err } slog.Info("RebootInstances: Processing request", "instance_count", len(input.InstanceIds)) diff --git a/spinifex/gateway/ec2/instance/RebootInstances_test.go b/spinifex/gateway/ec2/instance/RebootInstances_test.go index 0079e8ee9..85631077a 100644 --- a/spinifex/gateway/ec2/instance/RebootInstances_test.go +++ b/spinifex/gateway/ec2/instance/RebootInstances_test.go @@ -70,7 +70,7 @@ func TestRebootInstances_EmptyInstanceIds(t *testing.T) { _, err := RebootInstances(input, nc, "123456789012") require.Error(t, err) - assert.Contains(t, err.Error(), "no instance IDs provided") + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) } func TestRebootInstances_NATSRequestFails(t *testing.T) { diff --git a/spinifex/gateway/ec2/instance/StartInstances.go b/spinifex/gateway/ec2/instance/StartInstances.go index 805267ba4..f4c8da133 100644 --- a/spinifex/gateway/ec2/instance/StartInstances.go +++ b/spinifex/gateway/ec2/instance/StartInstances.go @@ -3,11 +3,11 @@ package gateway_ec2_instance import ( "encoding/json" "errors" - "fmt" "log/slog" "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" ) @@ -17,11 +17,22 @@ type startStoppedInstanceRequest struct { InstanceID string `json:"instance_id"` } +// ValidateStartInstancesInput validates the input parameters +func ValidateStartInstancesInput(input *ec2.StartInstancesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if len(input.InstanceIds) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // StartInstances sends start requests to the ec2.start queue group topic. // Any available daemon can pick up the request and launch the stopped instance. func StartInstances(input *ec2.StartInstancesInput, natsConn *nats.Conn, accountID string) (*ec2.StartInstancesOutput, error) { - if len(input.InstanceIds) == 0 { - return nil, fmt.Errorf("no instance IDs provided") + if err := ValidateStartInstancesInput(input); err != nil { + return nil, err } slog.Info("StartInstances: Processing request", "instance_count", len(input.InstanceIds)) diff --git a/spinifex/gateway/ec2/instance/StartInstances_test.go b/spinifex/gateway/ec2/instance/StartInstances_test.go index babdfd4a5..aeea76531 100644 --- a/spinifex/gateway/ec2/instance/StartInstances_test.go +++ b/spinifex/gateway/ec2/instance/StartInstances_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -82,7 +83,7 @@ func TestStartInstances_EmptyInstanceIds(t *testing.T) { _, err := StartInstances(input, nc, "123456789012") require.Error(t, err) - assert.Contains(t, err.Error(), "no instance IDs provided") + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) } func TestStartInstances_NilInstanceIdSkipped(t *testing.T) { diff --git a/spinifex/gateway/ec2/instance/StopInstances.go b/spinifex/gateway/ec2/instance/StopInstances.go index 9a101380e..59427ba53 100644 --- a/spinifex/gateway/ec2/instance/StopInstances.go +++ b/spinifex/gateway/ec2/instance/StopInstances.go @@ -8,16 +8,28 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/types" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" ) +// ValidateStopInstancesInput validates the input parameters +func ValidateStopInstancesInput(input *ec2.StopInstancesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if len(input.InstanceIds) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // StopInstances sends stop commands to specified instances via NATS // Uses system_powerdown with stop_instance attribute to prevent auto-restart on daemon boot func StopInstances(input *ec2.StopInstancesInput, natsConn *nats.Conn, accountID string) (*ec2.StopInstancesOutput, error) { - if len(input.InstanceIds) == 0 { - return nil, fmt.Errorf("no instance IDs provided") + if err := ValidateStopInstancesInput(input); err != nil { + return nil, err } slog.Info("StopInstances: Processing request", "instance_count", len(input.InstanceIds)) diff --git a/spinifex/gateway/ec2/instance/StopInstances_test.go b/spinifex/gateway/ec2/instance/StopInstances_test.go index 104a857f1..d16111a57 100644 --- a/spinifex/gateway/ec2/instance/StopInstances_test.go +++ b/spinifex/gateway/ec2/instance/StopInstances_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/types" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" @@ -83,7 +84,7 @@ func TestStopInstances_EmptyInstanceIds(t *testing.T) { _, err := StopInstances(input, nc, "123456789012") require.Error(t, err) - assert.Contains(t, err.Error(), "no instance IDs provided") + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) } func TestStopInstances_NilInstanceIdSkipped(t *testing.T) { diff --git a/spinifex/gateway/ec2/instance/TerminateInstances.go b/spinifex/gateway/ec2/instance/TerminateInstances.go index 024081363..588fc5acc 100644 --- a/spinifex/gateway/ec2/instance/TerminateInstances.go +++ b/spinifex/gateway/ec2/instance/TerminateInstances.go @@ -8,6 +8,7 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/types" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" @@ -18,11 +19,22 @@ type terminateStoppedInstanceRequest struct { InstanceID string `json:"instance_id"` } +// ValidateTerminateInstancesInput validates the input parameters +func ValidateTerminateInstancesInput(input *ec2.TerminateInstancesInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if len(input.InstanceIds) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // TerminateInstances sends terminate commands to specified instances via NATS // Uses system_powerdown with stop_instance attribute to prevent restart func TerminateInstances(input *ec2.TerminateInstancesInput, natsConn *nats.Conn, accountID string) (*ec2.TerminateInstancesOutput, error) { - if len(input.InstanceIds) == 0 { - return nil, fmt.Errorf("no instance IDs provided") + if err := ValidateTerminateInstancesInput(input); err != nil { + return nil, err } slog.Info("TerminateInstances: Processing request", "instance_count", len(input.InstanceIds)) diff --git a/spinifex/gateway/ec2/instance/TerminateInstances_test.go b/spinifex/gateway/ec2/instance/TerminateInstances_test.go index 71b04b703..876d34391 100644 --- a/spinifex/gateway/ec2/instance/TerminateInstances_test.go +++ b/spinifex/gateway/ec2/instance/TerminateInstances_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/types" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" @@ -83,7 +84,7 @@ func TestTerminateInstances_EmptyInstanceIds(t *testing.T) { _, err := TerminateInstances(input, nc, "123456789012") require.Error(t, err) - assert.Contains(t, err.Error(), "no instance IDs provided") + assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) } func TestTerminateInstances_NilInstanceIdSkipped(t *testing.T) { diff --git a/spinifex/gateway/ec2/placementgroup/CreatePlacementGroup.go b/spinifex/gateway/ec2/placementgroup/CreatePlacementGroup.go index b3e8be0a4..36bd3887e 100644 --- a/spinifex/gateway/ec2/placementgroup/CreatePlacementGroup.go +++ b/spinifex/gateway/ec2/placementgroup/CreatePlacementGroup.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreatePlacementGroupInput validates the input parameters +func ValidateCreatePlacementGroupInput(input *ec2.CreatePlacementGroupInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.GroupName == nil || *input.GroupName == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if input.Strategy == nil || *input.Strategy == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreatePlacementGroup handles the EC2 CreatePlacementGroup API call. func CreatePlacementGroup(input *ec2.CreatePlacementGroupInput, natsConn *nats.Conn, accountID string) (ec2.CreatePlacementGroupOutput, error) { var output ec2.CreatePlacementGroupOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.GroupName == nil || *input.GroupName == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if input.Strategy == nil || *input.Strategy == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreatePlacementGroupInput(input); err != nil { + return output, err } svc := handlers_ec2_placementgroup.NewNATSPlacementGroupService(natsConn) diff --git a/spinifex/gateway/ec2/placementgroup/DeletePlacementGroup.go b/spinifex/gateway/ec2/placementgroup/DeletePlacementGroup.go index 33ca644be..36bb4c0f5 100644 --- a/spinifex/gateway/ec2/placementgroup/DeletePlacementGroup.go +++ b/spinifex/gateway/ec2/placementgroup/DeletePlacementGroup.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeletePlacementGroupInput validates the input parameters +func ValidateDeletePlacementGroupInput(input *ec2.DeletePlacementGroupInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.GroupName == nil || *input.GroupName == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeletePlacementGroup handles the EC2 DeletePlacementGroup API call. func DeletePlacementGroup(input *ec2.DeletePlacementGroupInput, natsConn *nats.Conn, accountID string) (ec2.DeletePlacementGroupOutput, error) { var output ec2.DeletePlacementGroupOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.GroupName == nil || *input.GroupName == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeletePlacementGroupInput(input); err != nil { + return output, err } svc := handlers_ec2_placementgroup.NewNATSPlacementGroupService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/CreateNetworkInterface.go b/spinifex/gateway/ec2/vpc/CreateNetworkInterface.go index 008c402e2..f7694f8ab 100644 --- a/spinifex/gateway/ec2/vpc/CreateNetworkInterface.go +++ b/spinifex/gateway/ec2/vpc/CreateNetworkInterface.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateNetworkInterfaceInput validates the input parameters +func ValidateCreateNetworkInterfaceInput(input *ec2.CreateNetworkInterfaceInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.SubnetId == nil || *input.SubnetId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateNetworkInterface handles the EC2 CreateNetworkInterface API call func CreateNetworkInterface(input *ec2.CreateNetworkInterfaceInput, natsConn *nats.Conn, accountID string) (ec2.CreateNetworkInterfaceOutput, error) { var output ec2.CreateNetworkInterfaceOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.SubnetId == nil || *input.SubnetId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateNetworkInterfaceInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/CreateSubnet.go b/spinifex/gateway/ec2/vpc/CreateSubnet.go index 76eeb597b..e3281afad 100644 --- a/spinifex/gateway/ec2/vpc/CreateSubnet.go +++ b/spinifex/gateway/ec2/vpc/CreateSubnet.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateSubnetInput validates the input parameters +func ValidateCreateSubnetInput(input *ec2.CreateSubnetInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if input.CidrBlock == nil || *input.CidrBlock == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateSubnet handles the EC2 CreateSubnet API call func CreateSubnet(input *ec2.CreateSubnetInput, natsConn *nats.Conn, accountID string) (ec2.CreateSubnetOutput, error) { var output ec2.CreateSubnetOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if input.CidrBlock == nil || *input.CidrBlock == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateSubnetInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/CreateVpc.go b/spinifex/gateway/ec2/vpc/CreateVpc.go index 98e5d37e7..162bfb55f 100644 --- a/spinifex/gateway/ec2/vpc/CreateVpc.go +++ b/spinifex/gateway/ec2/vpc/CreateVpc.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateVpcInput validates the input parameters +func ValidateCreateVpcInput(input *ec2.CreateVpcInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.CidrBlock == nil || *input.CidrBlock == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateVpc handles the EC2 CreateVpc API call func CreateVpc(input *ec2.CreateVpcInput, natsConn *nats.Conn, accountID string) (ec2.CreateVpcOutput, error) { var output ec2.CreateVpcOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.CidrBlock == nil || *input.CidrBlock == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateVpcInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/DeleteNetworkInterface.go b/spinifex/gateway/ec2/vpc/DeleteNetworkInterface.go index 1ae33a436..4c71867da 100644 --- a/spinifex/gateway/ec2/vpc/DeleteNetworkInterface.go +++ b/spinifex/gateway/ec2/vpc/DeleteNetworkInterface.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteNetworkInterfaceInput validates the input parameters +func ValidateDeleteNetworkInterfaceInput(input *ec2.DeleteNetworkInterfaceInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteNetworkInterface handles the EC2 DeleteNetworkInterface API call func DeleteNetworkInterface(input *ec2.DeleteNetworkInterfaceInput, natsConn *nats.Conn, accountID string) (ec2.DeleteNetworkInterfaceOutput, error) { var output ec2.DeleteNetworkInterfaceOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteNetworkInterfaceInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/DeleteSubnet.go b/spinifex/gateway/ec2/vpc/DeleteSubnet.go index 0934c65f1..9ad34a6eb 100644 --- a/spinifex/gateway/ec2/vpc/DeleteSubnet.go +++ b/spinifex/gateway/ec2/vpc/DeleteSubnet.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteSubnetInput validates the input parameters +func ValidateDeleteSubnetInput(input *ec2.DeleteSubnetInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.SubnetId == nil || *input.SubnetId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteSubnet handles the EC2 DeleteSubnet API call func DeleteSubnet(input *ec2.DeleteSubnetInput, natsConn *nats.Conn, accountID string) (ec2.DeleteSubnetOutput, error) { var output ec2.DeleteSubnetOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.SubnetId == nil || *input.SubnetId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteSubnetInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/DeleteVpc.go b/spinifex/gateway/ec2/vpc/DeleteVpc.go index eaf2c444b..8c2a832e2 100644 --- a/spinifex/gateway/ec2/vpc/DeleteVpc.go +++ b/spinifex/gateway/ec2/vpc/DeleteVpc.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteVpcInput validates the input parameters +func ValidateDeleteVpcInput(input *ec2.DeleteVpcInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteVpc handles the EC2 DeleteVpc API call func DeleteVpc(input *ec2.DeleteVpcInput, natsConn *nats.Conn, accountID string) (ec2.DeleteVpcOutput, error) { var output ec2.DeleteVpcOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteVpcInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/DescribeVpcAttribute.go b/spinifex/gateway/ec2/vpc/DescribeVpcAttribute.go index 781147682..bb0df28fc 100644 --- a/spinifex/gateway/ec2/vpc/DescribeVpcAttribute.go +++ b/spinifex/gateway/ec2/vpc/DescribeVpcAttribute.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDescribeVpcAttributeInput validates the input parameters +func ValidateDescribeVpcAttributeInput(input *ec2.DescribeVpcAttributeInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if input.Attribute == nil || *input.Attribute == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DescribeVpcAttribute handles the EC2 DescribeVpcAttribute API call func DescribeVpcAttribute(input *ec2.DescribeVpcAttributeInput, natsConn *nats.Conn, accountID string) (ec2.DescribeVpcAttributeOutput, error) { var output ec2.DescribeVpcAttributeOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if input.Attribute == nil || *input.Attribute == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDescribeVpcAttributeInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go index 0d24c49e0..3769e5e76 100644 --- a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go +++ b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateModifyNetworkInterfaceAttributeInput validates the input parameters +func ValidateModifyNetworkInterfaceAttributeInput(input *ec2.ModifyNetworkInterfaceAttributeInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // ModifyNetworkInterfaceAttribute handles the EC2 ModifyNetworkInterfaceAttribute API call func ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput, natsConn *nats.Conn, accountID string) (ec2.ModifyNetworkInterfaceAttributeOutput, error) { var output ec2.ModifyNetworkInterfaceAttributeOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateModifyNetworkInterfaceAttributeInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/ModifySubnetAttribute.go b/spinifex/gateway/ec2/vpc/ModifySubnetAttribute.go index df651749d..df3f0195a 100644 --- a/spinifex/gateway/ec2/vpc/ModifySubnetAttribute.go +++ b/spinifex/gateway/ec2/vpc/ModifySubnetAttribute.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateModifySubnetAttributeInput validates the input parameters +func ValidateModifySubnetAttributeInput(input *ec2.ModifySubnetAttributeInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.SubnetId == nil || *input.SubnetId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // ModifySubnetAttribute handles the EC2 ModifySubnetAttribute API call func ModifySubnetAttribute(input *ec2.ModifySubnetAttributeInput, natsConn *nats.Conn, accountID string) (ec2.ModifySubnetAttributeOutput, error) { var output ec2.ModifySubnetAttributeOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.SubnetId == nil || *input.SubnetId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateModifySubnetAttributeInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/ec2/vpc/ModifyVpcAttribute.go b/spinifex/gateway/ec2/vpc/ModifyVpcAttribute.go index 7420452ac..2e75f1cd5 100644 --- a/spinifex/gateway/ec2/vpc/ModifyVpcAttribute.go +++ b/spinifex/gateway/ec2/vpc/ModifyVpcAttribute.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateModifyVpcAttributeInput validates the input parameters +func ValidateModifyVpcAttributeInput(input *ec2.ModifyVpcAttributeInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.VpcId == nil || *input.VpcId == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // ModifyVpcAttribute handles the EC2 ModifyVpcAttribute API call func ModifyVpcAttribute(input *ec2.ModifyVpcAttributeInput, natsConn *nats.Conn, accountID string) (ec2.ModifyVpcAttributeOutput, error) { var output ec2.ModifyVpcAttributeOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.VpcId == nil || *input.VpcId == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateModifyVpcAttributeInput(input); err != nil { + return output, err } svc := handlers_ec2_vpc.NewNATSVPCService(natsConn) diff --git a/spinifex/gateway/elbv2/CreateListener.go b/spinifex/gateway/elbv2/CreateListener.go index 41e8ec099..876deed8a 100644 --- a/spinifex/gateway/elbv2/CreateListener.go +++ b/spinifex/gateway/elbv2/CreateListener.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateListenerInput validates the input parameters +func ValidateCreateListenerInput(input *elbv2.CreateListenerInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if len(input.DefaultActions) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateListener handles the ELBv2 CreateListener API call. func CreateListener(input *elbv2.CreateListenerInput, natsConn *nats.Conn, accountID string) (elbv2.CreateListenerOutput, error) { var output elbv2.CreateListenerOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if len(input.DefaultActions) == 0 { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateListenerInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/CreateLoadBalancer.go b/spinifex/gateway/elbv2/CreateLoadBalancer.go index 3e27ca263..aec75e01d 100644 --- a/spinifex/gateway/elbv2/CreateLoadBalancer.go +++ b/spinifex/gateway/elbv2/CreateLoadBalancer.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateLoadBalancerInput validates the input parameters +func ValidateCreateLoadBalancerInput(input *elbv2.CreateLoadBalancerInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.Name == nil || *input.Name == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateLoadBalancer handles the ELBv2 CreateLoadBalancer API call. func CreateLoadBalancer(input *elbv2.CreateLoadBalancerInput, natsConn *nats.Conn, accountID string) (elbv2.CreateLoadBalancerOutput, error) { var output elbv2.CreateLoadBalancerOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.Name == nil || *input.Name == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateLoadBalancerInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/CreateTargetGroup.go b/spinifex/gateway/elbv2/CreateTargetGroup.go index ef94b4fec..bb03ed371 100644 --- a/spinifex/gateway/elbv2/CreateTargetGroup.go +++ b/spinifex/gateway/elbv2/CreateTargetGroup.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateCreateTargetGroupInput validates the input parameters +func ValidateCreateTargetGroupInput(input *elbv2.CreateTargetGroupInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.Name == nil || *input.Name == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // CreateTargetGroup handles the ELBv2 CreateTargetGroup API call. func CreateTargetGroup(input *elbv2.CreateTargetGroupInput, natsConn *nats.Conn, accountID string) (elbv2.CreateTargetGroupOutput, error) { var output elbv2.CreateTargetGroupOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.Name == nil || *input.Name == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateCreateTargetGroupInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DeleteListener.go b/spinifex/gateway/elbv2/DeleteListener.go index 00462e17b..d233adb41 100644 --- a/spinifex/gateway/elbv2/DeleteListener.go +++ b/spinifex/gateway/elbv2/DeleteListener.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteListenerInput validates the input parameters +func ValidateDeleteListenerInput(input *elbv2.DeleteListenerInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.ListenerArn == nil || *input.ListenerArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteListener handles the ELBv2 DeleteListener API call. func DeleteListener(input *elbv2.DeleteListenerInput, natsConn *nats.Conn, accountID string) (elbv2.DeleteListenerOutput, error) { var output elbv2.DeleteListenerOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.ListenerArn == nil || *input.ListenerArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteListenerInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DeleteLoadBalancer.go b/spinifex/gateway/elbv2/DeleteLoadBalancer.go index dc930320e..3b423c1e1 100644 --- a/spinifex/gateway/elbv2/DeleteLoadBalancer.go +++ b/spinifex/gateway/elbv2/DeleteLoadBalancer.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteLoadBalancerInput validates the input parameters +func ValidateDeleteLoadBalancerInput(input *elbv2.DeleteLoadBalancerInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteLoadBalancer handles the ELBv2 DeleteLoadBalancer API call. func DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInput, natsConn *nats.Conn, accountID string) (elbv2.DeleteLoadBalancerOutput, error) { var output elbv2.DeleteLoadBalancerOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteLoadBalancerInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DeleteTargetGroup.go b/spinifex/gateway/elbv2/DeleteTargetGroup.go index 90d5553c0..673fc2d47 100644 --- a/spinifex/gateway/elbv2/DeleteTargetGroup.go +++ b/spinifex/gateway/elbv2/DeleteTargetGroup.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeleteTargetGroupInput validates the input parameters +func ValidateDeleteTargetGroupInput(input *elbv2.DeleteTargetGroupInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeleteTargetGroup handles the ELBv2 DeleteTargetGroup API call. func DeleteTargetGroup(input *elbv2.DeleteTargetGroupInput, natsConn *nats.Conn, accountID string) (elbv2.DeleteTargetGroupOutput, error) { var output elbv2.DeleteTargetGroupOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeleteTargetGroupInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DeregisterTargets.go b/spinifex/gateway/elbv2/DeregisterTargets.go index 065591935..df101604e 100644 --- a/spinifex/gateway/elbv2/DeregisterTargets.go +++ b/spinifex/gateway/elbv2/DeregisterTargets.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDeregisterTargetsInput validates the input parameters +func ValidateDeregisterTargetsInput(input *elbv2.DeregisterTargetsInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if len(input.Targets) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DeregisterTargets handles the ELBv2 DeregisterTargets API call. func DeregisterTargets(input *elbv2.DeregisterTargetsInput, natsConn *nats.Conn, accountID string) (elbv2.DeregisterTargetsOutput, error) { var output elbv2.DeregisterTargetsOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Targets) == 0 { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDeregisterTargetsInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go b/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go index 61add9f37..59f827d06 100644 --- a/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go +++ b/spinifex/gateway/elbv2/DescribeLoadBalancerAttributes.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDescribeLoadBalancerAttributesInput validates the input parameters +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 input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDescribeLoadBalancerAttributesInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go b/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go index 541e95f8d..5046931a1 100644 --- a/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go +++ b/spinifex/gateway/elbv2/DescribeTargetGroupAttributes.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDescribeTargetGroupAttributesInput validates the input parameters +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 input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDescribeTargetGroupAttributesInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/DescribeTargetHealth.go b/spinifex/gateway/elbv2/DescribeTargetHealth.go index d06640068..62704d0fa 100644 --- a/spinifex/gateway/elbv2/DescribeTargetHealth.go +++ b/spinifex/gateway/elbv2/DescribeTargetHealth.go @@ -9,15 +9,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateDescribeTargetHealthInput validates the input parameters +func ValidateDescribeTargetHealthInput(input *elbv2.DescribeTargetHealthInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // DescribeTargetHealth handles the ELBv2 DescribeTargetHealth API call. func DescribeTargetHealth(input *elbv2.DescribeTargetHealthInput, natsConn *nats.Conn, accountID string) (elbv2.DescribeTargetHealthOutput, error) { var output elbv2.DescribeTargetHealthOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateDescribeTargetHealthInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/GetLBConfig.go b/spinifex/gateway/elbv2/GetLBConfig.go index d1b0e4e87..bea59e4c7 100644 --- a/spinifex/gateway/elbv2/GetLBConfig.go +++ b/spinifex/gateway/elbv2/GetLBConfig.go @@ -8,15 +8,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateGetLBConfigInput validates the input parameters +func ValidateGetLBConfigInput(input *handlers_elbv2.GetLBConfigInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.LBID == nil || *input.LBID == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // GetLBConfig handles the ELBv2 GetLBConfig API call. func GetLBConfig(input *handlers_elbv2.GetLBConfigInput, natsConn *nats.Conn, accountID string) (handlers_elbv2.GetLBConfigOutput, error) { var output handlers_elbv2.GetLBConfigOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LBID == nil || *input.LBID == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateGetLBConfigInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/LBAgentHeartbeat.go b/spinifex/gateway/elbv2/LBAgentHeartbeat.go index f02a5dd6f..8ab430a69 100644 --- a/spinifex/gateway/elbv2/LBAgentHeartbeat.go +++ b/spinifex/gateway/elbv2/LBAgentHeartbeat.go @@ -8,15 +8,23 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateLBAgentHeartbeatInput validates the input parameters +func ValidateLBAgentHeartbeatInput(input *handlers_elbv2.LBAgentHeartbeatInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.LBID == nil || *input.LBID == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // LBAgentHeartbeat handles the ELBv2 LBAgentHeartbeat API call. func LBAgentHeartbeat(input *handlers_elbv2.LBAgentHeartbeatInput, natsConn *nats.Conn, accountID string) (handlers_elbv2.LBAgentHeartbeatOutput, error) { var output handlers_elbv2.LBAgentHeartbeatOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LBID == nil || *input.LBID == "" { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateLBAgentHeartbeatInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go b/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go index 6a806de5a..39fa656e3 100644 --- a/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go +++ b/spinifex/gateway/elbv2/ModifyLoadBalancerAttributes.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateModifyLoadBalancerAttributesInput validates the input parameters +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 input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.LoadBalancerArn == nil || *input.LoadBalancerArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Attributes) == 0 { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateModifyLoadBalancerAttributesInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go b/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go index 1e0845d0f..8f826e28c 100644 --- a/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go +++ b/spinifex/gateway/elbv2/ModifyTargetGroupAttributes.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateModifyTargetGroupAttributesInput validates the input parameters +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 input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Attributes) == 0 { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateModifyTargetGroupAttributesInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) diff --git a/spinifex/gateway/elbv2/RegisterTargets.go b/spinifex/gateway/elbv2/RegisterTargets.go index 68b4493a7..0f116953b 100644 --- a/spinifex/gateway/elbv2/RegisterTargets.go +++ b/spinifex/gateway/elbv2/RegisterTargets.go @@ -9,18 +9,26 @@ import ( "github.com/nats-io/nats.go" ) +// ValidateRegisterTargetsInput validates the input parameters +func ValidateRegisterTargetsInput(input *elbv2.RegisterTargetsInput) error { + if input == nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + if len(input.Targets) == 0 { + return errors.New(awserrors.ErrorMissingParameter) + } + return nil +} + // RegisterTargets handles the ELBv2 RegisterTargets API call. func RegisterTargets(input *elbv2.RegisterTargetsInput, natsConn *nats.Conn, accountID string) (elbv2.RegisterTargetsOutput, error) { var output elbv2.RegisterTargetsOutput - if input == nil { - return output, errors.New(awserrors.ErrorInvalidParameterValue) - } - if input.TargetGroupArn == nil || *input.TargetGroupArn == "" { - return output, errors.New(awserrors.ErrorMissingParameter) - } - if len(input.Targets) == 0 { - return output, errors.New(awserrors.ErrorMissingParameter) + if err := ValidateRegisterTargetsInput(input); err != nil { + return output, err } svc := handlers_elbv2.NewNATSELBv2Service(natsConn) From fd17c353307b7bab7638af4ca5e412b51eae8e7e Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 23 Apr 2026 12:23:55 +1000 Subject: [PATCH 3/5] chore: disable frontend lint rules --- spinifex/services/spinifexui/frontend/oxlint.config.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spinifex/services/spinifexui/frontend/oxlint.config.ts b/spinifex/services/spinifexui/frontend/oxlint.config.ts index e6cb7c82c..fabb9bcc8 100644 --- a/spinifex/services/spinifexui/frontend/oxlint.config.ts +++ b/spinifex/services/spinifexui/frontend/oxlint.config.ts @@ -39,9 +39,11 @@ export default defineConfig({ "typescript/no-floating-promises": "off", "typescript/no-misused-promises": "off", "typescript/only-throw-error": "off", + "typescript/prefer-readonly-parameter-types": "off", "typescript/promise-function-async": "off", "typescript/return-await": "off", "typescript/strict-boolean-expressions": "off", + "typescript/strict-void-return": "off", "unicorn/filename-case": [ "error", { cases: { kebabCase: true, camelCase: true } }, From 3c5e09a9371c2c7f3d019901361c5745765ae277 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 23 Apr 2026 12:43:53 +1000 Subject: [PATCH 4/5] chore: enable frontend lint rules --- spinifex/services/spinifexui/frontend/oxlint.config.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spinifex/services/spinifexui/frontend/oxlint.config.ts b/spinifex/services/spinifexui/frontend/oxlint.config.ts index fabb9bcc8..d5f7074db 100644 --- a/spinifex/services/spinifexui/frontend/oxlint.config.ts +++ b/spinifex/services/spinifexui/frontend/oxlint.config.ts @@ -24,7 +24,13 @@ export default defineConfig({ "@tanstack/router/create-route-property-order": "error", "@tanstack/router/route-param-names": "error", "eslint/complexity": "off", - "eslint/func-style": "off", + "eslint/func-style": [ + "error", + "declaration", + { + allowArrowFunctions: true, + }, + ], "eslint/no-plusplus": ["error", { allowForLoopAfterthoughts: true }], "eslint/no-use-before-define": "off", "eslint/prefer-destructuring": "off", @@ -33,8 +39,6 @@ export default defineConfig({ "eslint/sort-vars": "off", "import/consistent-type-specifier-style": "off", "promise/prefer-await-to-callbacks": "off", - "react/jsx-handler-names": "off", - "react-perf/jsx-no-new-function-as-prop": "off", "typescript/no-confusing-void-expression": "off", "typescript/no-floating-promises": "off", "typescript/no-misused-promises": "off", From 75fb15be2bd1d3f53fd715c2aaedf97d469f6879 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Thu, 23 Apr 2026 14:02:12 +1000 Subject: [PATCH 5/5] feat: spx admin init/join listen-vs-advertise split + br-mgmt OVS provisioning (#158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: provision br-mgmt as OVS bridge + loopback guard for LB gateway Convert br-mgmt from an externally-provisioned Linux bridge to an OVS bridge owned by scripts/setup-ovn.sh, giving every deployment type (tofu-cluster, production single-node, production multi-node) a single provisioning path. setup-ovn.sh grows --mgmt-bridge / --mgmt-cidr / --mgmt-iface / --no-mgmt-bridge flags. Default creates br-mgmt with fail-mode=standalone (excluded from ovn-bridge-mappings so ovn-controller ignores it) and assigns the mgmt IP via a systemd-networkd drop-in so it survives reboot. SetupMgmtTapDevice / CleanupMgmtTapDevice switch from `ip link master` to `ovs-vsctl add-port` / `del-port` to match the new bridge type. Add a loopback guard to wireLBAgentConfig's fallback branch: on a production single-node with awsgw bound to 127.0.0.1 and no mgmt bridge, the fallback previously injected 127.0.0.1 into LB VM cloud-init — silently unreachable because the VM's loopback is its own. The guard leaves GatewayURL empty and the existing "LB agent gateway URL not configured" error fires instead. Regression covered in daemon_wire_lb_test.go. * fix: auto-detect primary IP for admin init --bind default Bare `spx admin init` used a 127.0.0.1 default that kept awsgw on loopback only, so system VMs on br-mgmt could never reach it. Auto-detect the default-route source IPv4 instead, which keeps TLS cert SANs specific (still not a wildcard). Dev-install scripts now pass --bind 127.0.0.1 explicitly to preserve loopback-only behavior on laptops. Closes mulga-siv-7. * test: refresh wire lb loopback-bind comment for siv-7 * fix: dev-install scripts should let admin init auto-detect --bind Pinning --bind to 127.0.0.1 on dev installs left awsgw loopback-only while setup-ovn.sh still provisions br-mgmt, so LB VMs hit 10.15.8.1:9999 with no listener. Drop the explicit --bind so auto-detect picks the primary IP and the siv-6 mgmtRouteVia path routes LB traffic to a real listener. * fix: bind awsgw and predastore on 0.0.0.0 in spinifex.toml template PR #144 (nats TLS) made every service listen on --bind's specific IP. For awsgw and predastore this broke localhost-reachability that the UI proxy (spinifexui/spinifexui.go:193-194) and AWS CLI spinifex profile rely on — latent since #144, surfaced by siv-7's auto-detect default on dev installs (502 from the UI). Hardcode awsgw and predastore host fields to 0.0.0.0 so both listen wildcard; loopback clients (UI proxy, spinifex AWS CLI profile, same-host daemon → predastore) keep working, LAN clients still reach them via the LAN-IP SAN on the cert. daemon.host and nats.host stay as {{.BindIP}} because peer-advertised values must not resolve to 0.0.0.0/loopback. predastore.host normalization at config.go:207 rewrites 0.0.0.0 → 127.0.0.1 for local-node consumers, so existing callers see a usable connect address. * revert: siv-7 --bind auto-detect + siv-6 loopback guard (prep mulga-siv-8) Clears the deck before layering the listen-vs-advertise split (mulga-siv-8). The plan restores --bind=0.0.0.0 as the single-node default and introduces an explicit --advertise flag for off-host dial targets; the reverted approach (auto-detect a specific IP at init, hardcode 0.0.0.0 in the template, guard against loopback in wireLBAgentConfig) becomes redundant once that lands. Reverts: - bb0ad3a fix: bind awsgw and predastore on 0.0.0.0 in spinifex.toml template - f20a47b fix: dev-install scripts should let admin init auto-detect --bind - 49b0b4e test: refresh wire lb loopback-bind comment for siv-7 - b88dfdf fix: auto-detect primary IP for admin init --bind default Also drops the loopback guard from siv-6 (74b6e2d) wireLBAgentConfig fallback and its regression test — subsumed by the GatewayURL precedence rework in mulga-siv-8.5 (br-mgmt → AdvertiseIP → error, no silent empty-string fallthrough). The br-mgmt OVS provisioning from 74b6e2d is retained. Plan: docs/development/bugs/spx-admin-init-bind.md * feat: admin.DialTarget + AdvertiseHost helpers (mulga-siv-8.2) Introduce the two small address-resolution helpers the listen-vs- advertise split needs, with full unit coverage. No callers yet — dial-site migration lands in mulga-siv-8.5. - DialTarget(listenAddr): rewrites wildcard listen addresses (0.0.0.0:N, [::]:N, :N) to their loopback equivalents so in-process clients dial a target the daemon's cert already has in its SANs. Specific IPs, loopback, hostnames, and malformed input pass through. - AdvertiseHost(listenAddr, advertiseIP): picks the host off-host clients should dial. Wildcard listen → advertiseIP; specific host → that host. Port is dropped so callers append the protocol-appropriate port. Plan: docs/development/bugs/spx-admin-init-bind.md §Proposed Changes/1 * feat: plumb AdvertiseIP through config/formation (mulga-siv-8.3) Pure data-model change. Add AdvertiseIP to: - admin.ConfigSettings (local node, populated by init). - config.Config (per-node; mapstructure "advertise"). - formation.NodeInfo (json "advertise_ip,omitempty") so the join handshake carries it between peers. buildRemoteNodes now prefers n.AdvertiseIP and falls back to n.BindIP so joined nodes from older clusters / pre-siv-8 joiners keep working. No behaviour change yet — templates, CLI flag, and dial-site migration land in siv-8.4 and siv-8.5. * feat: --advertise flag + resolveAdvertiseIP + templates (mulga-siv-8.4) Introduce the listen-vs-advertise split at the CLI boundary. admin init/join: - --bind default for init is 0.0.0.0 (was 127.0.0.1 under siv-7 revert). - New --advertise flag on both init and join; optional, auto-detected from DetectNetwork().WAN.IP when --bind is wildcard or loopback. - resolveAdvertiseIP(bindIP, advertiseFlag, detected) implements precedence: flag > specific bindIP > WAN auto-detect > 127.0.0.1 + warning. - Join resolves advertise before building the JoinRequest so peers record a reachable dial target (not 0.0.0.0). - isMultiNode: trigger on --nodes >= 2 regardless of bind literal; operator can now run "init --nodes 3" without typing a specific --bind. - Init success message prints bind / advertise / loopback three-way split. Templates: - spinifex.toml: [nodes.X] gains advertise = "{{.AdvertiseIP}}" rendered only when AdvertiseIP is set. - predastore-multinode.toml: iam.nats_url host wrapped in the same {{if eq .BindIP "0.0.0.0"}}localhost{{...}}{{end}} shim that predastore.toml already uses, so co-resident predastore dials its local NATS via loopback instead of 0.0.0.0. Tests cover the resolveAdvertiseIP matrix, the template advertise field (present + omitted), and the predastore nats_url loopback shim. Dial-site migration and GatewayURL rework land in siv-8.5. * feat: dial-site migration + GatewayURL AdvertiseIP fallback (mulga-siv-8.5) Switch every in-process dial site to admin.DialTarget so the wildcard listen address (0.0.0.0) is rewritten to loopback before Dial(): daemon → NATS and → predastore (boot, reconnect, status probe, S3 store, service manifest write), awsgw → NATS. The TLS cert already SANs 127.0.0.1, so loopback-dial a wildcard-listen service is always valid; the old 0.0.0.0 literal was tripping TLS SAN verification. Rework wireLBAgentConfig GatewayURL precedence (spinifex/daemon/daemon.go): 1. AWSGW on specific WAN IP + br-mgmt present → WAN IP (unchanged) 2. br-mgmt present, AWSGW on 0.0.0.0 → br-mgmt IP (unchanged) 3. d.config.AdvertiseIP (single-node default install) → AdvertiseIP (NEW) 4. DevNetworking shim → 10.0.2.2 (unchanged) 5. AWSGW on specific IP, no bridge → that IP (unchanged) 6. Else → log error, skip assignment Kills the pre-siv-8 silent empty-URL failure mode that made CreateLoadBalancer fail with 'missing system credentials: gatewayURL=' on every fresh single-node install. Supersedes siv-6's loopback guard. Extend SetupAWSCredentials with a wanIP parameter (reserved for a future --operator-endpoint flag) and thread AdvertiseIP through finalizeNodeSetup's three call sites (single-node init, multi-node init, join). Print 'Bind / Advertise / Loopback' on multi-node init and join completion to match the single-node summary. Plan: docs/development/bugs/spx-admin-init-bind.md §2, §3 --- cmd/spinifex/cmd/admin.go | 135 ++++++++++++++---- cmd/spinifex/cmd/admin_test.go | 131 +++++++++++++++++ .../cmd/templates/predastore-multinode.toml | 2 +- cmd/spinifex/cmd/templates/spinifex.toml | 3 + scripts/setup-ovn.sh | 78 +++++++++- spinifex/admin/admin.go | 12 +- spinifex/admin/admin_test.go | 8 +- spinifex/admin/dial.go | 42 ++++++ spinifex/admin/dial_test.go | 59 ++++++++ spinifex/config/config.go | 15 +- spinifex/daemon/daemon.go | 54 ++++--- spinifex/daemon/daemon_wire_lb_test.go | 30 ++++ spinifex/daemon/network.go | 19 +-- spinifex/formation/formation.go | 18 ++- spinifex/services/awsgw/awsgw.go | 3 +- 15 files changed, 530 insertions(+), 79 deletions(-) create mode 100644 spinifex/admin/dial.go create mode 100644 spinifex/admin/dial_test.go diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index ac77f25e1..66dbe2299 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -239,7 +239,8 @@ func init() { adminInitCmd.Flags().Int("nodes", 3, "Number of nodes to expect for cluster") adminInitCmd.Flags().String("host", "", "Leader node to join (if not specified, tries multicast discovery)") adminInitCmd.Flags().Int("port", 4432, "Port to bind cluster services on") - adminInitCmd.Flags().String("bind", "127.0.0.1", "IP address to bind services to (e.g., 10.11.12.1 for multi-node)") + adminInitCmd.Flags().String("bind", "0.0.0.0", "IP address to bind services to (e.g., 10.11.12.1 for multi-node). Default 0.0.0.0 listens on all interfaces.") + adminInitCmd.Flags().String("advertise", "", "External IP that off-host clients (ALB VMs, remote operators) should dial. Auto-detected from default route when empty.") adminInitCmd.Flags().String("cluster-bind", "", "IP address to bind NATS cluster services to (e.g., 10.11.12.1 for multi-node)") adminInitCmd.Flags().String("cluster-routes", "", "NATS cluster hosts for routing specify multiple with comma (e.g., 10.11.12.1:4248,10.11.12.2:4248 for multi-node)") adminInitCmd.Flags().String("predastore-nodes", "", "Comma-separated IPs for multi-node Predastore cluster (e.g., 10.11.12.1,10.11.12.2,10.11.12.3). Requires >= 3 nodes.") @@ -267,6 +268,7 @@ func init() { adminJoinCmd.Flags().String("data-dir", "", "Data directory for this node (default: ~/spinifex)") adminJoinCmd.Flags().Int("port", 4432, "Port to bind cluster services on") adminJoinCmd.Flags().String("bind", "0.0.0.0", "IP address to bind services to (e.g., 10.11.12.2 for multi-node on single host)") + adminJoinCmd.Flags().String("advertise", "", "External IP this node advertises to other cluster members. Defaults to --bind, or auto-detected WAN IP when --bind is 0.0.0.0.") adminJoinCmd.Flags().String("cluster-bind", "", "IP address to bind NATS cluster services to (e.g., 10.11.12.1 for multi-node)") adminJoinCmd.Flags().String("cluster-routes", "", "NATS cluster hosts for routing specify multiple with comma (e.g., 10.11.12.1:4248,10.11.12.2:4248 for multi-node)") adminJoinCmd.Flags().String("token", "", "Join token from the init node (required)") @@ -607,6 +609,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { nodes, _ := cmd.Flags().GetInt("nodes") port, _ := cmd.Flags().GetInt("port") bindIP, _ := cmd.Flags().GetString("bind") + advertiseFlag, _ := cmd.Flags().GetString("advertise") clusterBind, _ := cmd.Flags().GetString("cluster-bind") clusterRoutesStr, _ := cmd.Flags().GetString("cluster-routes") var clusterRoutes []string @@ -776,6 +779,19 @@ func runAdminInit(cmd *cobra.Command, args []string) { os.Exit(1) } + // Resolve the off-host advertise IP. DetectNetwork may have been skipped + // when --no-external is set; detect lazily if we need the WAN IP. + if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") && detectedNet == nil { + if d, derr := admin.DetectNetwork(); derr == nil { + detectedNet = d + } + } + advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error: %v\n", err) + os.Exit(1) + } + // Validate port range if port < 1 || port > 65535 { fmt.Fprintf(os.Stderr, "❌ Error: Port must be between 1 and 65535, got: %d\n", port) @@ -787,7 +803,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { clusterBind = bindIP } - fmt.Printf("Initializing Spinifex with bind IP: %s, port: %d\n", bindIP, port) + fmt.Printf("Initializing Spinifex with bind IP: %s, advertise IP: %s, port: %d\n", bindIP, advertiseIP, port) // Default config directory if configDir == "" { @@ -873,8 +889,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { } spxRoot = filepath.Clean(spxRoot) - // Determine if this is a multi-node formation - isMultiNode := nodes >= 2 && bindIP != "0.0.0.0" && bindIP != "127.0.0.1" + // Determine if this is a multi-node formation. Operator intent comes from + // --nodes, not from whether --bind was left at the 0.0.0.0 default. + isMultiNode := nodes >= 2 if isMultiNode { // Build cluster-wide network config for propagation to joining nodes @@ -904,7 +921,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { } runAdminInitMultiNode(cmd, accessKey, secretKey, accountID, natsToken, clusterName, - configDir, spxRoot, certPath, region, az, node, bindIP, clusterBind, + configDir, spxRoot, certPath, region, az, node, bindIP, advertiseIP, clusterBind, port, nodes, formationTimeoutStr, tokenTTLStr, services, networkConfig) return } @@ -987,6 +1004,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: clusterName, @@ -1040,7 +1058,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { os.Exit(1) } - finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP) + finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP, advertiseIP) // Print success message fmt.Println("\n🎉 Spinifex initialization complete!") @@ -1048,6 +1066,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { fmt.Println("🔗 Configuration:") fmt.Printf(" Config file: %s\n", spinifexTomlPath) fmt.Printf(" Data directory: %s\n", spxRoot) + fmt.Printf(" Bind IP: %s (listen)\n", bindIP) + fmt.Printf(" Advertise IP: %s (off-host dial target)\n", advertiseIP) + fmt.Printf(" Loopback: 127.0.0.1 (in-process dial target)\n") fmt.Println() } @@ -1055,7 +1076,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { // It starts a formation server, registers this node, waits for all nodes to join, // then generates configs with complete cluster topology. func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, natsToken, clusterName, - configDir, spxRoot, certPath, region, az, node, bindIP, clusterBind string, + configDir, spxRoot, certPath, region, az, node, bindIP, advertiseIP, clusterBind string, port, expectedNodes int, formationTimeoutStr, tokenTTLStr string, services []string, networkConfig *formation.NetworkConfig) { formationTimeout, err := time.ParseDuration(formationTimeoutStr) if err != nil { @@ -1124,12 +1145,13 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, // Register self (init node) as the first node selfNode := formation.NodeInfo{ - Name: node, - BindIP: bindIP, - ClusterIP: clusterBind, - Region: region, - AZ: az, - Port: port, + Name: node, + BindIP: bindIP, + AdvertiseIP: advertiseIP, + ClusterIP: clusterBind, + Region: region, + AZ: az, + Port: port, } if err := fs.RegisterNode(selfNode); err != nil { fmt.Fprintf(os.Stderr, "❌ Error registering self: %v\n", err) @@ -1217,6 +1239,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: clusterName, @@ -1239,7 +1262,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, os.Exit(1) } - finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP) + finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP, advertiseIP) // Keep formation server running briefly so joining nodes can fetch complete status fmt.Println("\n⏳ Waiting for joining nodes to fetch cluster data...") @@ -1255,9 +1278,14 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, fmt.Println("\n🎉 Cluster formation complete!") fmt.Printf(" Cluster: %s (%d nodes)\n", clusterName, expectedNodes) fmt.Printf(" Region: %s\n", region) + fmt.Printf(" Bind: %s Advertise: %s Loopback: 127.0.0.1\n", bindIP, advertiseIP) fmt.Println(" Nodes:") for name, n := range allNodes { - fmt.Printf(" - %s (%s)\n", name, n.BindIP) + adv := n.AdvertiseIP + if adv == "" { + adv = n.BindIP + } + fmt.Printf(" - %s (bind=%s advertise=%s)\n", name, n.BindIP, adv) } fmt.Println("\n📋 Next steps:") fmt.Println(" 1. Start services on ALL nodes:") @@ -1280,6 +1308,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { dataDir, _ := cmd.Flags().GetString("data-dir") port, _ := cmd.Flags().GetInt("port") bindIP, _ := cmd.Flags().GetString("bind") + advertiseFlag, _ := cmd.Flags().GetString("advertise") configDir, _ := cmd.Flags().GetString("config-dir") clusterBind, _ := cmd.Flags().GetString("cluster-bind") services, _ := cmd.Flags().GetStringSlice("services") @@ -1307,6 +1336,20 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } + // Resolve the off-host advertise IP before the JoinRequest is built so + // peers record a reachable dial target instead of 0.0.0.0. + var detectedNet *admin.DetectedNetwork + if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") { + if d, derr := admin.DetectNetwork(); derr == nil { + detectedNet = d + } + } + advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) + if err != nil { + fmt.Fprintf(os.Stderr, "❌ Error: %v\n", err) + os.Exit(1) + } + // Validate port range if port < 1 || port > 65535 { fmt.Fprintf(os.Stderr, "❌ Error: Port must be between 1 and 65535, got: %d\n", port) @@ -1329,17 +1372,19 @@ func runAdminJoin(cmd *cobra.Command, args []string) { fmt.Printf("Region: %s\n", region) fmt.Printf("AZ: %s\n", az) fmt.Printf("Bind IP: %s\n", bindIP) + fmt.Printf("Advertise IP: %s\n", advertiseIP) fmt.Printf("Port: %d\n\n", port) // POST join request to formation server joinReq := formation.JoinRequest{ NodeInfo: formation.NodeInfo{ - Name: node, - BindIP: bindIP, - ClusterIP: clusterBind, - Region: region, - AZ: az, - Port: port, + Name: node, + BindIP: bindIP, + AdvertiseIP: advertiseIP, + ClusterIP: clusterBind, + Region: region, + AZ: az, + Port: port, }, } @@ -1588,6 +1633,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: creds.ClusterName, @@ -1610,17 +1656,44 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } - finalizeNodeSetup(dataDir, caCertPath, creds.AdminAccessKey, creds.AdminSecretKey, creds.Region, bindIP) + finalizeNodeSetup(dataDir, caCertPath, creds.AdminAccessKey, creds.AdminSecretKey, creds.Region, bindIP, advertiseIP) // Print cluster summary fmt.Println("\n🎉 Node successfully joined cluster!") fmt.Printf(" Cluster: %s (%d nodes)\n", creds.ClusterName, len(statusResp.Nodes)) + fmt.Printf(" Bind: %s Advertise: %s Loopback: 127.0.0.1\n", bindIP, advertiseIP) fmt.Println(" Nodes:") for name, n := range statusResp.Nodes { - fmt.Printf(" - %s (%s)\n", name, n.BindIP) + adv := n.AdvertiseIP + if adv == "" { + adv = n.BindIP + } + fmt.Printf(" - %s (bind=%s advertise=%s)\n", name, n.BindIP, adv) } } +// resolveAdvertiseIP picks the off-host dial target for this node. +// Precedence: explicit --advertise flag > specific --bind IP > auto-detected +// WAN IP > loopback (with warning that off-host clients cannot reach this node). +func resolveAdvertiseIP(bindIP, advertiseFlag string, detected *admin.DetectedNetwork) (string, error) { + if advertiseFlag != "" { + if net.ParseIP(advertiseFlag) == nil { + return "", fmt.Errorf("--advertise: invalid IP %q", advertiseFlag) + } + return advertiseFlag, nil + } + if bindIP != "" && bindIP != "0.0.0.0" && bindIP != "127.0.0.1" { + return bindIP, nil + } + if detected != nil && detected.WAN != nil && detected.WAN.IP != "" { + return detected.WAN.IP, nil + } + fmt.Fprintln(os.Stderr, + "⚠️ Could not auto-detect a WAN IP. Off-host clients (ALB VMs, remote operators) "+ + "will not be able to reach this node. Re-run with --advertise to fix.") + return "127.0.0.1", nil +} + // buildRemoteNodes converts formation NodeInfo into RemoteNode entries, // excluding the local node. This puts all cluster members into spinifex.toml // so config is the source of truth for expected cluster membership. @@ -1630,9 +1703,15 @@ func buildRemoteNodes(allNodes map[string]formation.NodeInfo, localNode string) if name == localNode { continue } + // Prefer the peer's advertise IP (off-host dial target); fall back + // to BindIP for pre-siv-8 joiners that didn't send AdvertiseIP. + host := n.AdvertiseIP + if host == "" { + host = n.BindIP + } remote = append(remote, admin.RemoteNode{ Name: name, - Host: n.BindIP, + Host: host, Region: n.Region, AZ: n.AZ, Services: n.Services, @@ -1885,10 +1964,12 @@ func generateAndWriteConfigs(dirs configDirs, spinifexTomlPath string, settings } // finalizeNodeSetup configures AWS credentials, creates service directories, -// and sets ownership when running as root. -func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region, bindIP string) { +// and sets ownership when running as root. advertiseIP is the off-host dial +// target for this node; it is threaded through SetupAWSCredentials's wanIP +// param for a future --operator-endpoint flag (see SetupAWSCredentials doc). +func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region, bindIP, advertiseIP string) { fmt.Println("\n🔧 Configuring AWS credentials...") - if err := admin.SetupAWSCredentials(adminAccessKey, adminSecretKey, region, certPath, bindIP); err != nil { + if err := admin.SetupAWSCredentials(adminAccessKey, adminSecretKey, region, certPath, bindIP, advertiseIP); err != nil { fmt.Fprintf(os.Stderr, "Warning: Could not update AWS credentials: %v\n", err) } else { fmt.Println("✅ AWS credentials configured") diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index 4fdb92cac..5ce6871a2 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -4,10 +4,15 @@ import ( "bytes" "errors" "fmt" + "os" + "path/filepath" "testing" + "github.com/mulgadc/spinifex/spinifex/admin" + "github.com/mulgadc/spinifex/spinifex/formation" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Regression guard: source URL must print for every error kind, not just @@ -32,3 +37,129 @@ func TestPrintChecksumError(t *testing.T) { assert.Contains(t, out, "spx admin images import --name "+imageName+" --force") } } + +// buildRemoteNodes must prefer AdvertiseIP (off-host dial target) and fall +// back to BindIP when the peer pre-dates siv-8 and didn't send AdvertiseIP. +func TestBuildRemoteNodes_AdvertiseFallback(t *testing.T) { + nodes := map[string]formation.NodeInfo{ + "node1": {Name: "node1", BindIP: "10.0.0.1", AdvertiseIP: "203.0.113.1"}, + "node2": {Name: "node2", BindIP: "10.0.0.2"}, // legacy joiner + "node3": {Name: "node3", BindIP: "10.0.0.3", AdvertiseIP: "203.0.113.3"}, + } + got := buildRemoteNodes(nodes, "node3") + if assert.Len(t, got, 2) { + assert.Equal(t, "node1", got[0].Name) + assert.Equal(t, "203.0.113.1", got[0].Host, "advertise wins when set") + assert.Equal(t, "node2", got[1].Name) + assert.Equal(t, "10.0.0.2", got[1].Host, "bind fallback when advertise empty") + } +} + +func TestResolveAdvertiseIP(t *testing.T) { + wan := &admin.DetectedNetwork{WAN: &admin.DetectedInterface{IP: "192.168.1.21"}} + noWAN := &admin.DetectedNetwork{} + + tests := []struct { + name string + bindIP string + advertise string + detected *admin.DetectedNetwork + want string + wantErr bool + }{ + {"single-node default, WAN detected", "0.0.0.0", "", wan, "192.168.1.21", false}, + {"single-node default, no WAN", "0.0.0.0", "", noWAN, "127.0.0.1", false}, + {"single-node explicit advertise", "0.0.0.0", "203.0.113.5", wan, "203.0.113.5", false}, + {"multi-node init, specific bind", "10.11.12.1", "", nil, "10.11.12.1", false}, + {"multi-node dual-homed override", "10.11.12.1", "203.0.113.5", nil, "203.0.113.5", false}, + {"loopback-only test", "127.0.0.1", "", noWAN, "127.0.0.1", false}, + {"invalid advertise flag", "0.0.0.0", "not-an-ip", wan, "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := resolveAdvertiseIP(tt.bindIP, tt.advertise, tt.detected) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +// Generated spinifex.toml must include both host (listen, BindIP) and +// advertise (off-host dial target, AdvertiseIP) for the local node. +func TestSpinifexTomlTemplate_AdvertiseField(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "spinifex.toml") + settings := admin.ConfigSettings{ + Node: "node1", + Az: "ap-southeast-2a", + Port: "4432", + Region: "ap-southeast-2", + BindIP: "0.0.0.0", + AdvertiseIP: "192.168.1.21", + AccessKey: "AKIATEST", + SecretKey: "SECRET", + AccountID: "123456789012", + NatsToken: "token", + ConfigDir: dir, + OVNNBAddr: "tcp:127.0.0.1:6641", + OVNSBAddr: "tcp:127.0.0.1:6642", + } + require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, settings)) + + data, err := os.ReadFile(path) + require.NoError(t, err) + content := string(data) + assert.Contains(t, content, `host = "0.0.0.0"`, "listen address") + assert.Contains(t, content, `advertise = "192.168.1.21"`, "off-host dial target") +} + +// Empty AdvertiseIP (e.g. loading an existing cluster pre-siv-8) must NOT +// render an empty advertise = "" line — downstream fallback to Host kicks in. +func TestSpinifexTomlTemplate_AdvertiseOmittedWhenEmpty(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "spinifex.toml") + settings := admin.ConfigSettings{ + Node: "node1", + Az: "ap-southeast-2a", + Port: "4432", + Region: "ap-southeast-2", + BindIP: "10.11.12.1", + AccessKey: "AKIATEST", + SecretKey: "SECRET", + AccountID: "123456789012", + NatsToken: "token", + ConfigDir: dir, + OVNNBAddr: "tcp:127.0.0.1:6641", + OVNSBAddr: "tcp:127.0.0.1:6642", + } + require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, settings)) + data, _ := os.ReadFile(path) + assert.NotContains(t, string(data), "advertise =") +} + +// iam.nats_url must not embed a bare 0.0.0.0 — the predastore process dials +// its own NATS via loopback when listening on wildcard. +func TestPredastoreMultinodeTemplate_NatsURLLoopbackShim(t *testing.T) { + nodes := []admin.PredastoreNodeConfig{ + {ID: 1, Host: "10.0.0.1"}, + {ID: 2, Host: "10.0.0.2"}, + {ID: 3, Host: "10.0.0.3"}, + } + content, err := admin.GenerateMultiNodePredastoreConfig( + predastoreMultiNodeTemplate, nodes, "AK", "SK", "ap-southeast-2", "token", "/tmp", "0.0.0.0", + ) + require.NoError(t, err) + assert.Contains(t, content, `nats_url = "nats://localhost:4222"`) + assert.NotContains(t, content, `nats_url = "nats://0.0.0.0:4222"`) + + // Specific bind IP → stays as-is. + content2, err := admin.GenerateMultiNodePredastoreConfig( + predastoreMultiNodeTemplate, nodes, "AK", "SK", "ap-southeast-2", "token", "/tmp", "10.11.12.1", + ) + require.NoError(t, err) + assert.Contains(t, content2, `nats_url = "nats://10.11.12.1:4222"`) +} diff --git a/cmd/spinifex/cmd/templates/predastore-multinode.toml b/cmd/spinifex/cmd/templates/predastore-multinode.toml index a0b180ba7..7b99141e4 100644 --- a/cmd/spinifex/cmd/templates/predastore-multinode.toml +++ b/cmd/spinifex/cmd/templates/predastore-multinode.toml @@ -63,7 +63,7 @@ burst = 200 # IAM authentication via NATS KV (enables multi-account S3 access) [iam] -nats_url = "nats://{{.BindIP}}:4222" +nats_url = "nats://{{if eq .BindIP "0.0.0.0"}}localhost{{else}}{{.BindIP}}{{end}}:4222" nats_token = "{{.NatsToken}}" nats_ca_cert = "{{.ConfigDir}}/ca.pem" master_key_path = "{{.ConfigDir}}/master.key" diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/cmd/spinifex/cmd/templates/spinifex.toml index 432dd70f0..7ecc41e63 100644 --- a/cmd/spinifex/cmd/templates/spinifex.toml +++ b/cmd/spinifex/cmd/templates/spinifex.toml @@ -42,6 +42,9 @@ subnet_cidr = "{{.BootstrapSubnetCidr}}" [nodes.{{.Node}}] node = "{{.Node}}" host = "{{.BindIP}}" +{{- if .AdvertiseIP}} +advertise = "{{.AdvertiseIP}}" +{{- end}} region = "{{.Region}}" az = "{{.Az}}" {{- if .Services}} diff --git a/scripts/setup-ovn.sh b/scripts/setup-ovn.sh index cb1fbbb4d..b12fe39ed 100755 --- a/scripts/setup-ovn.sh +++ b/scripts/setup-ovn.sh @@ -20,6 +20,10 @@ # --macvlan Create macvlan off --wan-iface instead of moving NIC directly. # SSH-safe for single-NIC hosts where WAN NIC carries SSH. # --dhcp Obtain gateway IP via DHCP on the WAN bridge interface +# --mgmt-bridge=NAME OVS bridge for system-instance control plane (default: br-mgmt) +# --mgmt-cidr=CIDR IPv4 CIDR to assign on the mgmt bridge (default: 10.15.8.1/24) +# --mgmt-iface=NAME Physical/virtual NIC to enslave to the mgmt bridge (multi-node only) +# --no-mgmt-bridge Skip mgmt bridge provisioning (for dev-networking hosts) # --ovn-remote=ADDR OVN SB DB address (default: tcp:127.0.0.1:6642) # --encap-ip=IP Geneve tunnel endpoint IP (default: auto-detect) # --chassis-id=ID OVN chassis identifier (default: chassis-$(hostname -s)) @@ -55,6 +59,10 @@ WAN_BRIDGE="" WAN_IFACE="" MACVLAN_MODE=false EXTERNAL_DHCP=false +MGMT_BRIDGE_ENABLED=true +MGMT_BRIDGE="br-mgmt" +MGMT_CIDR="10.15.8.1/24" +MGMT_IFACE="" OVN_REMOTE="tcp:127.0.0.1:6642" ENCAP_IP="" CHASSIS_ID="" @@ -67,11 +75,15 @@ for arg in "$@"; do --dhcp) EXTERNAL_DHCP=true ;; --wan-bridge=*) WAN_BRIDGE="${arg#*=}" ;; --wan-iface=*) WAN_IFACE="${arg#*=}" ;; + --mgmt-bridge=*) MGMT_BRIDGE="${arg#*=}" ;; + --mgmt-cidr=*) MGMT_CIDR="${arg#*=}" ;; + --mgmt-iface=*) MGMT_IFACE="${arg#*=}" ;; + --no-mgmt-bridge) MGMT_BRIDGE_ENABLED=false ;; --ovn-remote=*) OVN_REMOTE="${arg#*=}" ;; --encap-ip=*) ENCAP_IP="${arg#*=}" ;; --chassis-id=*) CHASSIS_ID="${arg#*=}" ;; --help|-h) - head -44 "$0" | tail -42 + head -50 "$0" | tail -48 exit 0 ;; *) @@ -451,6 +463,59 @@ if [ -n "$WAN_BRIDGE" ]; then fi fi +# --- Step 3d: Management bridge for system-instance control plane --- +# br-mgmt is an OVS bridge (not L2-learning Linux bridge, not part of OVN +# overlay). fail-mode=standalone makes it behave like a plain L2 switch +# without OVN flows; it is excluded from ovn-bridge-mappings so +# ovn-controller ignores it. System instance TAPs attach via ovs-vsctl +# add-port (see SetupMgmtTapDevice in daemon/network.go). +if [ "$MGMT_BRIDGE_ENABLED" = true ]; then + echo "" + echo "Step 3d: Configuring management bridge ($MGMT_BRIDGE)..." + + sudo ovs-vsctl --may-exist add-br "$MGMT_BRIDGE" + sudo ovs-vsctl set Bridge "$MGMT_BRIDGE" \ + fail-mode=standalone \ + other-config:disable-in-band=true + sudo ip link set "$MGMT_BRIDGE" up + echo " $MGMT_BRIDGE: OVS bridge, fail-mode=standalone, up" + + # Enslave physical/virtual mgmt NIC when provided (multi-node deployments + # where the node's mgmt subnet spans multiple hosts). + if [ -n "$MGMT_IFACE" ]; then + if ! ip link show "$MGMT_IFACE" >/dev/null 2>&1; then + echo " ERROR: mgmt interface $MGMT_IFACE does not exist" + ip -o link show | awk -F': ' '{print " " $2}' + exit 1 + fi + + # Drop any existing IP on the NIC — IP belongs on the bridge. + sudo ip addr flush dev "$MGMT_IFACE" || true + sudo ovs-vsctl --may-exist add-port "$MGMT_BRIDGE" "$MGMT_IFACE" + sudo ip link set "$MGMT_IFACE" up + echo " $MGMT_IFACE: port on $MGMT_BRIDGE" + fi + + if [ -n "$MGMT_CIDR" ]; then + sudo ip addr replace "$MGMT_CIDR" dev "$MGMT_BRIDGE" + echo " $MGMT_BRIDGE: address $MGMT_CIDR" + + # Persist the IP across reboots via a systemd-networkd drop-in. The + # OVS bridge definition itself survives in ovsdb; only the L3 IP + # needs re-applying on boot. + MGMT_NETD_UNIT="/etc/systemd/network/10-spinifex-mgmt.network" + sudo tee "$MGMT_NETD_UNIT" >/dev/null </dev/null | awk '{print $4}' | head -1) + echo " $MGMT_BRIDGE:$(printf '%*s' $((15 - ${#MGMT_BRIDGE})) '') OK (${MGMT_IP_ACTUAL:-no IP})" + else + echo " $MGMT_BRIDGE:$(printf '%*s' $((15 - ${#MGMT_BRIDGE})) '') FAILED" + OK=false + fi +fi + # Check ovn-controller if sudo ovs-appctl -t ovn-controller version >/dev/null 2>&1 || systemctl is-active --quiet ovn-controller 2>/dev/null; then echo " ovn-controller: OK" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 679c831a8..b2448a046 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -48,6 +48,9 @@ type ConfigSettings struct { Az string Port string BindIP string + // AdvertiseIP is the off-host dial target rendered into the local node's + // [nodes.X].advertise field. Empty → callers fall back to BindIP. + AdvertiseIP string // Cluster settings ClusterBindIP string @@ -783,9 +786,14 @@ func GenerateSelfSignedCert(certPath, keyPath string) error { } // SetupAWSCredentials updates ~/.aws/credentials and ~/.aws/config. -// bindIP is the IP the AWS gateway listens on. If empty or "0.0.0.0", defaults to "localhost". +// bindIP is the IP the AWS gateway listens on. If empty or "0.0.0.0", the +// operator's local ~/.aws/config endpoint_url falls back to "localhost" (this +// runs on the same box as the gateway). wanIP is reserved for a future +// --operator-endpoint flag that will let remote operators point their CLI at +// the host's WAN IP; today it is accepted but unused. // When running under sudo, writes to SUDO_USER's home instead of root's. -func SetupAWSCredentials(accessKey, secretKey, region, certPath, bindIP string) error { +func SetupAWSCredentials(accessKey, secretKey, region, certPath, bindIP, wanIP string) error { + _ = wanIP homeDir, err := os.UserHomeDir() if err != nil { return err diff --git a/spinifex/admin/admin_test.go b/spinifex/admin/admin_test.go index b4aa57253..a8a2b0434 100644 --- a/spinifex/admin/admin_test.go +++ b/spinifex/admin/admin_test.go @@ -222,7 +222,7 @@ func TestSetupAWSCredentials_CreatesFiles(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/path/to/ca.pem", "") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/path/to/ca.pem", "", "") require.NoError(t, err) credData, _ := os.ReadFile(filepath.Join(dir, ".aws", "credentials")) @@ -245,7 +245,7 @@ func TestSetupAWSCredentials_PreservesExistingProfiles(t *testing.T) { "aws_access_key_id": "EXISTING_KEY", })) - err := SetupAWSCredentials("NEWAKEY", "NEWSECRET", "us-west-2", "/ca.pem", "") + err := SetupAWSCredentials("NEWAKEY", "NEWSECRET", "us-west-2", "/ca.pem", "", "") require.NoError(t, err) data, _ := os.ReadFile(filepath.Join(awsDir, "credentials")) @@ -258,7 +258,7 @@ func TestSetupAWSCredentials_UsesBindIP(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "10.11.12.1") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "10.11.12.1", "") require.NoError(t, err) configData, _ := os.ReadFile(filepath.Join(dir, ".aws", "config")) @@ -270,7 +270,7 @@ func TestSetupAWSCredentials_FallsBackToLocalhostForWildcard(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "0.0.0.0") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "0.0.0.0", "") require.NoError(t, err) configData, _ := os.ReadFile(filepath.Join(dir, ".aws", "config")) diff --git a/spinifex/admin/dial.go b/spinifex/admin/dial.go new file mode 100644 index 000000000..ccaa96d9a --- /dev/null +++ b/spinifex/admin/dial.go @@ -0,0 +1,42 @@ +package admin + +import "net" + +// DialTarget returns a host:port suitable for an in-process client to dial, +// given a configured listen address. Inputs of the form "0.0.0.0:N" or "[::]:N" +// are rewritten to "127.0.0.1:N" or "[::1]:N" — a service listening on the +// wildcard is reachable on loopback, and loopback is the TLS SAN that the +// daemon's own cert always covers. Loopback, specific IPs, and bare hostnames +// pass through unchanged. Malformed input (no host:port) passes through so +// callers still see the original value in error messages. +func DialTarget(listenAddr string) string { + host, port, err := net.SplitHostPort(listenAddr) + if err != nil { + return listenAddr + } + switch host { + case "0.0.0.0", "": + return net.JoinHostPort("127.0.0.1", port) + case "::": + return net.JoinHostPort("::1", port) + } + return listenAddr +} + +// AdvertiseHost picks the host that off-host clients should dial for a service +// listening on listenAddr. If the listen host is a wildcard (0.0.0.0, ::, or +// empty), the caller-supplied advertiseIP is returned; otherwise the listen +// host itself is returned. Port is intentionally dropped — callers append the +// port appropriate for the protocol (some services listen and advertise on +// different ports). +func AdvertiseHost(listenAddr, advertiseIP string) string { + host, _, err := net.SplitHostPort(listenAddr) + if err != nil { + host = listenAddr + } + switch host { + case "0.0.0.0", "::", "": + return advertiseIP + } + return host +} diff --git a/spinifex/admin/dial_test.go b/spinifex/admin/dial_test.go new file mode 100644 index 000000000..910d17bfb --- /dev/null +++ b/spinifex/admin/dial_test.go @@ -0,0 +1,59 @@ +package admin + +import "testing" + +func TestDialTarget(t *testing.T) { + t.Parallel() + cases := []struct { + name string + in string + want string + }{ + {"ipv4 wildcard", "0.0.0.0:4222", "127.0.0.1:4222"}, + {"ipv6 wildcard", "[::]:4222", "[::1]:4222"}, + {"ipv4 loopback passthrough", "127.0.0.1:4222", "127.0.0.1:4222"}, + {"ipv6 loopback passthrough", "[::1]:4222", "[::1]:4222"}, + {"specific ipv4 passthrough", "192.168.1.5:4222", "192.168.1.5:4222"}, + {"hostname passthrough", "nats.cluster.local:4222", "nats.cluster.local:4222"}, + {"empty host rewrites to loopback", ":4222", "127.0.0.1:4222"}, + {"malformed passthrough", "malformed", "malformed"}, + {"ipv4 wildcard tls port", "0.0.0.0:8443", "127.0.0.1:8443"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := DialTarget(tc.in) + if got != tc.want { + t.Fatalf("DialTarget(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestAdvertiseHost(t *testing.T) { + t.Parallel() + cases := []struct { + name string + listen string + advertiseIP string + want string + }{ + {"ipv4 wildcard resolves", "0.0.0.0:9999", "192.168.1.5", "192.168.1.5"}, + {"ipv6 wildcard resolves", "[::]:9999", "192.168.1.5", "192.168.1.5"}, + {"specific ip passthrough", "10.11.12.1:9999", "192.168.1.5", "10.11.12.1"}, + {"loopback passthrough", "127.0.0.1:9999", "192.168.1.5", "127.0.0.1"}, + {"hostname passthrough", "nats.cluster.local:9999", "192.168.1.5", "nats.cluster.local"}, + {"no-port input treated as host", "0.0.0.0", "192.168.1.5", "192.168.1.5"}, + {"empty listen falls back to advertise", "", "192.168.1.5", "192.168.1.5"}, + {"empty advertise with wildcard returns empty", "0.0.0.0:9999", "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := AdvertiseHost(tc.listen, tc.advertiseIP) + if got != tc.want { + t.Fatalf("AdvertiseHost(%q, %q) = %q, want %q", tc.listen, tc.advertiseIP, got, tc.want) + } + }) + } +} diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 92ef94ee6..d480038e5 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -55,12 +55,15 @@ type BootstrapConfig struct { // Config holds all configuration for the application type Config struct { // Node config - Node string `json:"Node" mapstructure:"node"` - Host string `json:"Host" mapstructure:"host"` // Unique hostname or IP of this node - Region string `json:"Region" mapstructure:"region"` - AZ string `json:"AZ" mapstructure:"az"` - DataDir string `json:"DataDir" mapstructure:"data_dir"` - Services []string `json:"Services" mapstructure:"services"` // Which services this node runs locally + Node string `json:"Node" mapstructure:"node"` + Host string `json:"Host" mapstructure:"host"` // Unique hostname or IP of this node + // AdvertiseIP is the off-host dial target for this node. Empty → callers + // fall back to Host for backward compat with pre-siv-8 cluster configs. + AdvertiseIP string `json:"AdvertiseIP" mapstructure:"advertise"` + Region string `json:"Region" mapstructure:"region"` + AZ string `json:"AZ" mapstructure:"az"` + DataDir string `json:"DataDir" mapstructure:"data_dir"` + Services []string `json:"Services" mapstructure:"services"` // Which services this node runs locally Daemon DaemonConfig `json:"Daemon" mapstructure:"daemon"` NATS NATSConfig `json:"NATS" mapstructure:"nats"` diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index ea7a79759..08b01bdc2 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -569,15 +569,15 @@ func (d *Daemon) Start() error { if err := d.jsManager.WriteServiceManifest( d.node, d.config.GetServices(), - d.config.NATS.Host, - d.config.Predastore.Host, + admin.DialTarget(d.config.NATS.Host), + admin.DialTarget(d.config.Predastore.Host), ); err != nil { slog.Warn("Failed to write service manifest", "error", err) } } // Create services before loading/launching instances, since LaunchInstance depends on them - store := objectstore.NewS3ObjectStoreFromConfig(d.config.Predastore.Host, d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey) + store := objectstore.NewS3ObjectStoreFromConfig(admin.DialTarget(d.config.Predastore.Host), d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey) d.instanceService = handlers_ec2_instance.NewInstanceServiceImpl(d.config, d.resourceMgr.instanceTypes, d.natsConn, &d.Instances, store) d.keyService = handlers_ec2_key.NewKeyServiceImpl(d.config) d.imageService = handlers_ec2_image.NewImageServiceImpl(d.config, d.natsConn) @@ -844,7 +844,7 @@ func (d *Daemon) Start() error { // be ready immediately after daemon start (e.g. if start-dev.sh is still // launching services). This retries for up to 5 minutes before giving up. func (d *Daemon) connectNATS() error { - nc, err := utils.ConnectNATSWithRetry(d.config.NATS.Host, d.config.NATS.ACL.Token, d.config.NATS.CACert, d.natsRetryOpts...) + nc, err := utils.ConnectNATSWithRetry(admin.DialTarget(d.config.NATS.Host), d.config.NATS.ACL.Token, d.config.NATS.CACert, d.natsRetryOpts...) if err != nil { return err } @@ -993,7 +993,7 @@ func (d *Daemon) checkViperblockReady() bool { // checkPredastoreReady checks if predastore is reachable via TCP. func (d *Daemon) checkPredastoreReady() bool { - host := d.config.Predastore.Host + host := admin.DialTarget(d.config.Predastore.Host) if host == "" { return true // no predastore configured, skip check } @@ -2782,9 +2782,15 @@ func (d *Daemon) wireLBAgentConfig() { } // Resolve gateway URL — the address LB VMs use to reach the AWS gateway. - // Internet-facing LB VMs reach it via VPC networking; internal LB VMs - // reach it via the management NIC (br-mgmt) with a host route added - // to their cloud-init when AWSGW binds to a specific WAN IP. + // Precedence: + // 1. AWSGW listens on a specific WAN IP + br-mgmt present → use WAN IP + // and add host route via br-mgmt for internal LBs. + // 2. br-mgmt present + AWSGW on 0.0.0.0 → br-mgmt IP (both LB flavours + // reach the daemon via mgmt). + // 3. This node's AdvertiseIP (single-node default install) → AdvertiseIP. + // 4. DevNetworking shim → 10.0.2.2. + // 5. AWSGW bound to specific IP (no br-mgmt, no advertise) → that IP. + // 6. Else: error and skip assignment — no silent empty URL. var gatewayHost string awsgwBindIP := "" if d.config.AWSGW.Host != "" { @@ -2793,20 +2799,23 @@ func (d *Daemon) wireLBAgentConfig() { } } - if d.mgmtBridgeIP != "" { - if awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" && !net.ParseIP(awsgwBindIP).IsLoopback() { - // Multi-node: AWSGW listens on a specific WAN IP. Use that IP - // as the gateway URL. Internal LBs will get a host route via - // br-mgmt to reach it (see LaunchSystemInstance). - gatewayHost = awsgwBindIP - d.mgmtRouteVia = awsgwBindIP - } else { - // Single-node: AWSGW listens on all interfaces — br-mgmt IP works. - gatewayHost = d.mgmtBridgeIP - } - } else if d.config.Daemon.DevNetworking { + advertiseIP := d.config.AdvertiseIP + + switch { + case d.mgmtBridgeIP != "" && awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" && !net.ParseIP(awsgwBindIP).IsLoopback(): + // Multi-node: AWSGW listens on a specific WAN IP. Use that IP as the + // gateway URL; internal LBs get a host route via br-mgmt. + gatewayHost = awsgwBindIP + d.mgmtRouteVia = awsgwBindIP + case d.mgmtBridgeIP != "": + // Multi-node with AWSGW on 0.0.0.0 — br-mgmt reaches all LBs. + gatewayHost = d.mgmtBridgeIP + case advertiseIP != "" && advertiseIP != "0.0.0.0": + // Single-node default install — off-host clients dial the advertised IP. + gatewayHost = advertiseIP + case d.config.Daemon.DevNetworking: gatewayHost = "10.0.2.2" - } else if awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" { + case awsgwBindIP != "" && awsgwBindIP != "0.0.0.0": gatewayHost = awsgwBindIP } @@ -2823,7 +2832,8 @@ func (d *Daemon) wireLBAgentConfig() { d.elbv2Service.GatewayURL = gatewayURL slog.Info("LB agent gateway URL configured", "url", gatewayURL) } else { - slog.Error("LB agent gateway URL not configured: no reachable host found — all CreateLoadBalancer calls will fail") + slog.Error("LB agent gateway URL not configured: no reachable host found — CreateLoadBalancer will fail until --advertise or br-mgmt is configured", + "awsgwBindIP", awsgwBindIP, "mgmtBridgeIP", d.mgmtBridgeIP, "advertiseIP", advertiseIP) } // Pass mgmt route info so lbVMUserData can add a bootcmd route for diff --git a/spinifex/daemon/daemon_wire_lb_test.go b/spinifex/daemon/daemon_wire_lb_test.go index 52e8620de..21fdab452 100644 --- a/spinifex/daemon/daemon_wire_lb_test.go +++ b/spinifex/daemon/daemon_wire_lb_test.go @@ -172,3 +172,33 @@ func TestWireLBAgentConfig_NoMgmtRoute(t *testing.T) { assert.Empty(t, d.mgmtRouteVia) } + +// AdvertiseIP covers the single-node default install (no br-mgmt, AWSGW on +// wildcard): the gateway URL falls back to the node's advertised off-host IP. +// Replaces the siv-6 "empty gateway URL" silent failure. +func TestWireLBAgentConfig_GatewayURL_AdvertiseFallback(t *testing.T) { + cfg := &config.Config{ + AdvertiseIP: "192.168.1.21", + AWSGW: config.AWSGWConfig{Host: "0.0.0.0:9999"}, + } + d := newWireLBTestDaemon(t, cfg) + + d.wireLBAgentConfig() + + assert.Equal(t, "https://192.168.1.21:9999", d.elbv2Service.GatewayURL) + assert.Empty(t, d.mgmtRouteVia) +} + +// When there's no br-mgmt, no AdvertiseIP, no DevNetworking, and AWSGW is on +// the wildcard, the gateway URL must stay empty and the daemon must log an +// error (rather than silently assigning a broken URL like pre-siv-8). +func TestWireLBAgentConfig_GatewayURL_NoAdvertiseNoBridge(t *testing.T) { + cfg := &config.Config{ + AWSGW: config.AWSGWConfig{Host: "0.0.0.0:9999"}, + } + d := newWireLBTestDaemon(t, cfg) + + d.wireLBAgentConfig() + + assert.Empty(t, d.elbv2Service.GatewayURL) +} diff --git a/spinifex/daemon/network.go b/spinifex/daemon/network.go index 2aca7b2e1..57e76bc76 100644 --- a/spinifex/daemon/network.go +++ b/spinifex/daemon/network.go @@ -203,15 +203,17 @@ func GetBridgeIPv4(bridgeName string) (string, error) { return "", fmt.Errorf("no IPv4 address on %s", bridgeName) } -// SetupMgmtTapDevice creates a TAP device and adds it to the management Linux bridge. -// Unlike VPC TAPs (OVS/OVN), this is a plain Linux bridge TAP. +// SetupMgmtTapDevice creates a TAP device and attaches it to the management +// OVS bridge (provisioned by scripts/setup-ovn.sh). br-mgmt uses +// fail-mode=standalone and is excluded from ovn-bridge-mappings, so it +// behaves as a plain L2 switch outside the OVN overlay. func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { tapName := MgmtTapName(instanceID) // Clean up stale TAP if present if _, err := os.Stat("/sys/class/net/" + tapName); err == nil { slog.Warn("Stale mgmt tap found, cleaning up", "tap", tapName) - if err := sudoCommand("ip", "link", "set", tapName, "nomaster").Run(); err != nil { + if err := sudoCommand("ovs-vsctl", "--if-exists", "del-port", bridge, tapName).Run(); err != nil { slog.Warn("Failed to remove stale mgmt tap from bridge", "tap", tapName, "err", err) } if err := sudoCommand("ip", "tuntap", "del", "dev", tapName, "mode", "tap").Run(); err != nil { @@ -230,8 +232,8 @@ func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { return "", fmt.Errorf("bring up mgmt tap %s: %s: %w", tapName, strings.TrimSpace(string(out)), err) } - // Add to Linux bridge - if out, err := sudoCommand("ip", "link", "set", tapName, "master", bridge).CombinedOutput(); err != nil { + // Attach to OVS mgmt bridge + if out, err := sudoCommand("ovs-vsctl", "--may-exist", "add-port", bridge, tapName).CombinedOutput(); err != nil { _ = sudoCommand("ip", "tuntap", "del", "dev", tapName, "mode", "tap").Run() return "", fmt.Errorf("add mgmt tap to %s: %s: %w", bridge, strings.TrimSpace(string(out)), err) } @@ -240,10 +242,11 @@ func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { return tapName, nil } -// CleanupMgmtTapDevice removes a management TAP device from its bridge and deletes it. +// CleanupMgmtTapDevice removes a management TAP device from the mgmt OVS +// bridge and deletes it. func CleanupMgmtTapDevice(tapName string) error { - // Remove from bridge - if out, err := sudoCommand("ip", "link", "set", tapName, "nomaster").CombinedOutput(); err != nil { + // Detach from whichever OVS bridge holds this tap (usually br-mgmt) + if out, err := sudoCommand("ovs-vsctl", "--if-exists", "del-port", tapName).CombinedOutput(); err != nil { slog.Warn("Failed to remove mgmt tap from bridge", "tap", tapName, "err", err, "out", strings.TrimSpace(string(out))) } diff --git a/spinifex/formation/formation.go b/spinifex/formation/formation.go index e0a29acc8..811cdc48e 100644 --- a/spinifex/formation/formation.go +++ b/spinifex/formation/formation.go @@ -17,13 +17,17 @@ import ( // NodeInfo describes a node participating in cluster formation. type NodeInfo struct { - Name string `json:"name"` - BindIP string `json:"bind_ip"` - ClusterIP string `json:"cluster_ip"` - Region string `json:"region"` - AZ string `json:"az"` - Port int `json:"port"` - Services []string `json:"services,omitempty"` + Name string `json:"name"` + // BindIP is the listen address on the joining node. + BindIP string `json:"bind_ip"` + // AdvertiseIP is the off-host dial target for this node. Empty → + // peers fall back to BindIP for backward compat with older joiners. + AdvertiseIP string `json:"advertise_ip,omitempty"` + ClusterIP string `json:"cluster_ip"` + Region string `json:"region"` + AZ string `json:"az"` + Port int `json:"port"` + Services []string `json:"services,omitempty"` } // JoinRequest is the payload POSTed by joining nodes. diff --git a/spinifex/services/awsgw/awsgw.go b/spinifex/services/awsgw/awsgw.go index add0370b3..3850a375c 100644 --- a/spinifex/services/awsgw/awsgw.go +++ b/spinifex/services/awsgw/awsgw.go @@ -11,6 +11,7 @@ import ( "time" "github.com/mulgadc/predastore/ratelimit" + "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/gateway" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" @@ -102,7 +103,7 @@ func launchService(config *config.ClusterConfig) error { // Connect to NATS for service communication. On concurrent startup the // local NATS server may not be listening yet, so retry with backoff. - natsConn, err := utils.ConnectNATSWithRetry(nodeConfig.NATS.Host, nodeConfig.NATS.ACL.Token, nodeConfig.NATS.CACert) + natsConn, err := utils.ConnectNATSWithRetry(admin.DialTarget(nodeConfig.NATS.Host), nodeConfig.NATS.ACL.Token, nodeConfig.NATS.CACert) if err != nil { return err }