diff --git a/spinifex/handlers/elbv2/service_impl.go b/spinifex/handlers/elbv2/service_impl.go index 65dcb20c9..ed4bb1e5a 100644 --- a/spinifex/handlers/elbv2/service_impl.go +++ b/spinifex/handlers/elbv2/service_impl.go @@ -1161,7 +1161,22 @@ func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInp var eniIDs []string var availZones []AvailZoneInfo vpcID := "" + var nlbManagedSGID string if s.VPCService != nil && len(subnets) > 0 { + // NLBs reject customer SGs (rejected above), so the ENIs would fall back + // to the VPC default SG whose intra-SG-only ingress drops inbound + // listener traffic. Mint a dedicated managed SG up front and attach it to + // every LB ENI; CreateListener opens the listener ports on it. + eniGroups := securityGroups + if lbType == LoadBalancerTypeNetwork { + sgID, sgErr := s.createNLBManagedSG(lbID, lbArn, subnets[0], accountID) + if sgErr != nil { + slog.Error("CreateLoadBalancer: failed to create managed NLB SG", "lbId", lbID, "err", sgErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + nlbManagedSGID = sgID + eniGroups = []string{sgID} + } for _, subnetID := range subnets { eniIn := &ec2.CreateNetworkInterfaceInput{ SubnetId: aws.String(subnetID), @@ -1177,11 +1192,12 @@ func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInp }, } // SG enforcement gates inbound traffic by the ENI's port-group - // membership, which is set at ENI creation. Empty Groups - // intentionally falls back to the VPC default SG inside + // membership, which is set at ENI creation. For NLBs this is the + // managed SG minted above; for ALBs it is the customer SGs. Empty + // Groups intentionally falls back to the VPC default SG inside // eni.CreateNetworkInterface. - if len(securityGroups) > 0 { - eniIn.Groups = aws.StringSlice(securityGroups) + if len(eniGroups) > 0 { + eniIn.Groups = aws.StringSlice(eniGroups) } eniOut, eniErr := s.VPCService.CreateNetworkInterface(eniIn, accountID) if eniErr != nil { @@ -1193,6 +1209,7 @@ func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInp slog.Error("CreateLoadBalancer: rollback failed to delete ENI", "eni", rollbackENI, "err", delErr) } } + s.deleteNLBManagedSG(nlbManagedSGID, accountID) slog.Error("CreateLoadBalancer: failed to create ENI", "subnet", subnetID, "err", eniErr) return nil, errors.New(awserrors.ErrorELBv2SubnetNotFound) } @@ -1264,6 +1281,7 @@ func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInp State: state, VpcId: vpcID, SecurityGroups: securityGroups, + NLBManagedSGID: nlbManagedSGID, Subnets: subnets, AvailZones: availZones, ENIs: eniIDs, @@ -1349,6 +1367,8 @@ func (s *ELBv2ServiceImpl) DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInp slog.Warn("Failed to delete ALB ENI during cleanup", "eniId", eniID, "err", eniErr) } } + // The managed NLB SG can only be removed once its ENIs are gone. + s.deleteNLBManagedSG(lb.NLBManagedSGID, accountID) } if err := s.store.DeleteLoadBalancer(lb.LoadBalancerID); err != nil { @@ -2141,6 +2161,21 @@ func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, acco return nil, errors.New(awserrors.ErrorServerInternal) } + // Open the listener port on the NLB's managed front-end SG so inbound + // traffic from the configured client CIDRs is admitted by the OVN ACL + // (NLB ENIs would otherwise sit in the intra-SG-only default SG). + if lb.Type == LoadBalancerTypeNetwork && lb.NLBManagedSGID != "" && s.VPCService != nil { + cidrs, cidrErr := s.resolveNLBIngressCIDRs(lb) + if cidrErr != nil { + slog.Error("CreateListener: resolve ingress CIDRs failed", "lbArn", lb.LoadBalancerArn, "err", cidrErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if authErr := s.authorizeNLBListenerPort(lb, protocol, port, cidrs, accountID); authErr != nil { + slog.Error("CreateListener: authorize listener port failed", "lbArn", lb.LoadBalancerArn, "port", port, "err", authErr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + } + // Start or reload HAProxy now that a listener exists if err := s.updateStoredConfig(lb); err != nil { slog.Error("CreateListener: failed to update config", "listenerId", listenerID, "err", err) @@ -2198,6 +2233,16 @@ func (s *ELBv2ServiceImpl) DeleteListener(input *elbv2.DeleteListenerInput, acco // Reload or stop HAProxy after listener removal lb, lbErr := s.store.GetLoadBalancerByArn(listener.LoadBalancerArn) if lbErr == nil && lb != nil { + // Close the listener port on the NLB's managed front-end SG. + if lb.Type == LoadBalancerTypeNetwork && lb.NLBManagedSGID != "" && s.VPCService != nil { + if cidrs, cidrErr := s.resolveNLBIngressCIDRs(lb); cidrErr == nil { + if revokeErr := s.revokeNLBListenerPort(lb, listener.Protocol, listener.Port, cidrs, accountID); revokeErr != nil { + slog.Warn("DeleteListener: revoke listener port failed", "lbArn", lb.LoadBalancerArn, "port", listener.Port, "err", revokeErr) + } + } else { + slog.Warn("DeleteListener: resolve ingress CIDRs failed", "lbArn", lb.LoadBalancerArn, "err", cidrErr) + } + } if err := s.updateStoredConfig(lb); err != nil { slog.Error("DeleteListener: failed to update config", "listenerArn", *input.ListenerArn, "err", err) return nil, errors.New(awserrors.ErrorServerInternal) diff --git a/spinifex/handlers/elbv2/service_impl_lb_network.go b/spinifex/handlers/elbv2/service_impl_lb_network.go index 9194249a1..215d8e40d 100644 --- a/spinifex/handlers/elbv2/service_impl_lb_network.go +++ b/spinifex/handlers/elbv2/service_impl_lb_network.go @@ -310,8 +310,8 @@ func (s *ELBv2ServiceImpl) createLBENI(subnetID string, lb *LoadBalancerRecord, }, }, } - if len(lb.SecurityGroups) > 0 { - eniIn.Groups = aws.StringSlice(lb.SecurityGroups) + if groups := lbENIGroups(lb); len(groups) > 0 { + eniIn.Groups = aws.StringSlice(groups) } out, err := s.VPCService.CreateNetworkInterface(eniIn, accountID) if err != nil { diff --git a/spinifex/handlers/elbv2/service_impl_nlb_sg.go b/spinifex/handlers/elbv2/service_impl_nlb_sg.go new file mode 100644 index 000000000..a1a2f92b5 --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_nlb_sg.go @@ -0,0 +1,252 @@ +package handlers_elbv2 + +import ( + "errors" + "fmt" + "log/slog" + "net" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// nlbManagedSGName returns the deterministic name of the internal security +// group minted for an NLB. Stable so recovery paths can re-resolve it. +func nlbManagedSGName(lbID string) string { + return "spinifex-nlb-" + lbID +} + +// lbENIGroups returns the security groups to attach to an LB's data-plane ENIs. +// NLBs use the internally-minted managed SG (customer SGs are rejected at +// create time); ALBs use the customer-supplied SGs. Shared by the create-time +// ENI loop and the SetSubnets relaunch path so both attach the same SG. +func lbENIGroups(lb *LoadBalancerRecord) []string { + if lb.Type == LoadBalancerTypeNetwork { + if lb.NLBManagedSGID != "" { + return []string{lb.NLBManagedSGID} + } + return nil + } + return lb.SecurityGroups +} + +// createNLBManagedSG mints the internal front-end SG for an NLB in the LB's VPC +// and returns its ID. The VPC is resolved from the LB's first subnet. The SG is +// tagged like the LB's other managed resources so the ownership sweep can find +// it. +func (s *ELBv2ServiceImpl) createNLBManagedSG(lbID, lbArn, subnetID, accountID string) (string, error) { + subnet, err := s.VPCService.GetSubnet(accountID, subnetID) + if err != nil { + return "", fmt.Errorf("resolve subnet %s vpc: %w", subnetID, err) + } + if subnet == nil || subnet.VpcId == "" { + return "", fmt.Errorf("subnet %s has no vpc", subnetID) + } + + out, err := s.VPCService.CreateSecurityGroup(&ec2.CreateSecurityGroupInput{ + GroupName: aws.String(nlbManagedSGName(lbID)), + Description: aws.String(fmt.Sprintf("Managed front-end SG for NLB %s", lbID)), + VpcId: aws.String(subnet.VpcId), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("security-group"), + Tags: []*ec2.Tag{ + {Key: aws.String(elbv2ManagedByTag), Value: aws.String(elbv2ManagedByValue)}, + {Key: aws.String(elbv2LBTag), Value: aws.String(lbArn)}, + }, + }}, + }, accountID) + if err != nil { + return "", fmt.Errorf("create NLB SG: %w", err) + } + if out == nil || out.GroupId == nil || *out.GroupId == "" { + return "", errors.New("CreateSecurityGroup returned empty GroupId") + } + return *out.GroupId, nil +} + +// deleteNLBManagedSG best-effort deletes an NLB's managed SG. Callable on the +// create-rollback and delete paths; a nil VPC service or empty ID is a no-op, +// and a delete failure is logged but not surfaced (the SG can only be deleted +// once its ENIs are gone, which the delete path guarantees). +func (s *ELBv2ServiceImpl) deleteNLBManagedSG(sgID, accountID string) { + if s.VPCService == nil || sgID == "" { + return + } + if _, err := s.VPCService.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{ + GroupId: aws.String(sgID), + }, accountID); err != nil { + slog.Warn("deleteNLBManagedSG: delete failed", "sg", sgID, "err", err) + } +} + +// resolveNLBIngressCIDRs returns the client CIDRs a listener port is opened to. +// An explicit override wins; otherwise the default is scheme-based: +// internet-facing → 0.0.0.0/0, internal → the LB's VPC CIDR. +func (s *ELBv2ServiceImpl) resolveNLBIngressCIDRs(lb *LoadBalancerRecord) ([]string, error) { + if len(lb.NLBIngressCIDRs) > 0 { + return lb.NLBIngressCIDRs, nil + } + if lb.Scheme == SchemeInternal { + cidr, err := s.vpcCIDR(lb.VpcId, lb.AccountID) + if err != nil { + return nil, err + } + return []string{cidr}, nil + } + return []string{"0.0.0.0/0"}, nil +} + +// vpcCIDR returns the primary CIDR block of a VPC. +func (s *ELBv2ServiceImpl) vpcCIDR(vpcID, accountID string) (string, error) { + if s.VPCService == nil { + return "", errors.New("vpc service unavailable") + } + if vpcID == "" { + return "", errors.New("empty vpc id") + } + out, err := s.VPCService.DescribeVpcs(&ec2.DescribeVpcsInput{ + VpcIds: aws.StringSlice([]string{vpcID}), + }, accountID) + if err != nil { + return "", fmt.Errorf("describe vpc %s: %w", vpcID, err) + } + for _, v := range out.Vpcs { + if v != nil && aws.StringValue(v.CidrBlock) != "" { + return *v.CidrBlock, nil + } + } + return "", fmt.Errorf("vpc %s has no cidr", vpcID) +} + +// listenerIPProtocols maps an ELBv2 listener protocol to the IP protocols its +// SG ingress rule(s) cover. TCP/TLS ride tcp; UDP rides udp; TCP_UDP opens both. +func listenerIPProtocols(protocol string) []string { + switch protocol { + case ProtocolUDP: + return []string{"udp"} + case ProtocolTCPUDP: + return []string{"tcp", "udp"} + default: // TCP, TLS + return []string{"tcp"} + } +} + +// authorizeNLBListenerPort opens a listener port on the NLB's managed SG for +// each (protocol, CIDR) pair. Each rule is authorized in its own call so an +// already-present rule (Duplicate, swallowed) never blocks a sibling rule — +// keeping re-create / reconcile idempotent. No-op when there is no managed SG. +func (s *ELBv2ServiceImpl) authorizeNLBListenerPort(lb *LoadBalancerRecord, protocol string, port int64, cidrs []string, accountID string) error { + if s.VPCService == nil || lb.NLBManagedSGID == "" { + return nil + } + for _, proto := range listenerIPProtocols(protocol) { + for _, cidr := range cidrs { + _, err := s.VPCService.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(lb.NLBManagedSGID), + IpPermissions: []*ec2.IpPermission{{ + IpProtocol: aws.String(proto), + FromPort: aws.Int64(port), + ToPort: aws.Int64(port), + IpRanges: []*ec2.IpRange{{ + CidrIp: aws.String(cidr), + Description: aws.String(fmt.Sprintf("NLB %s listener :%d", lb.LoadBalancerID, port)), + }}, + }}, + }, accountID) + if err != nil && !awserrors.IsErrorCode(err, awserrors.ErrorInvalidPermissionDuplicate) { + return fmt.Errorf("authorize :%d/%s from %s: %w", port, proto, cidr, err) + } + } + } + return nil +} + +// revokeNLBListenerPort removes the listener-port rules authorizeNLBListenerPort +// added. An absent rule (NotFound, swallowed) is fine so delete / re-CIDR paths +// stay idempotent. No-op when there is no managed SG. +func (s *ELBv2ServiceImpl) revokeNLBListenerPort(lb *LoadBalancerRecord, protocol string, port int64, cidrs []string, accountID string) error { + if s.VPCService == nil || lb.NLBManagedSGID == "" { + return nil + } + for _, proto := range listenerIPProtocols(protocol) { + for _, cidr := range cidrs { + _, err := s.VPCService.RevokeSecurityGroupIngress(&ec2.RevokeSecurityGroupIngressInput{ + GroupId: aws.String(lb.NLBManagedSGID), + IpPermissions: []*ec2.IpPermission{{ + IpProtocol: aws.String(proto), + FromPort: aws.Int64(port), + ToPort: aws.Int64(port), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String(cidr)}}, + }}, + }, accountID) + if err != nil && !awserrors.IsErrorCode(err, awserrors.ErrorInvalidPermissionNotFound) { + return fmt.Errorf("revoke :%d/%s from %s: %w", port, proto, cidr, err) + } + } + } + return nil +} + +// SetLoadBalancerIngressCIDRs overrides the client CIDRs the NLB's listener +// ports are opened to (e.g. an EKS public endpoint's publicAccessCidrs) and +// re-authorizes every existing listener accordingly. Passing nil/empty reverts +// to the scheme-based default. Idempotent: the old rules are revoked and the +// new ones authorized, both swallowing the benign duplicate / not-found errors. +func (s *ELBv2ServiceImpl) SetLoadBalancerIngressCIDRs(lbArn string, cidrs []string, accountID string) error { + if lbArn == "" { + return errors.New(awserrors.ErrorMissingParameter) + } + lb, err := s.store.GetLoadBalancerByArn(lbArn) + if err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: failed to get LB", "arn", lbArn, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + if lb == nil { + return errors.New(awserrors.ErrorELBv2LoadBalancerNotFound) + } + if lb.Type != LoadBalancerTypeNetwork { + return errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest) + } + for _, c := range cidrs { + if _, _, perr := net.ParseCIDR(c); perr != nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + } + + oldCIDRs, err := s.resolveNLBIngressCIDRs(lb) + if err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: resolve old CIDRs failed", "arn", lbArn, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + listeners, err := s.store.ListListenersByLB(lb.LoadBalancerArn) + if err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: list listeners failed", "arn", lbArn, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + lb.NLBIngressCIDRs = cidrs + newCIDRs, err := s.resolveNLBIngressCIDRs(lb) + if err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: resolve new CIDRs failed", "arn", lbArn, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + for _, l := range listeners { + if err := s.revokeNLBListenerPort(lb, l.Protocol, l.Port, oldCIDRs, accountID); err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: revoke failed", "arn", lbArn, "port", l.Port, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + if err := s.authorizeNLBListenerPort(lb, l.Protocol, l.Port, newCIDRs, accountID); err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: authorize failed", "arn", lbArn, "port", l.Port, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + } + + if err := s.store.PutLoadBalancer(lb); err != nil { + slog.Error("SetLoadBalancerIngressCIDRs: persist failed", "arn", lbArn, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + slog.Info("SetLoadBalancerIngressCIDRs completed", "arn", lbArn, "cidrs", cidrs, "listeners", len(listeners), "accountID", accountID) + return nil +} diff --git a/spinifex/handlers/elbv2/service_impl_nlb_sg_test.go b/spinifex/handlers/elbv2/service_impl_nlb_sg_test.go new file mode 100644 index 000000000..b63e6618d --- /dev/null +++ b/spinifex/handlers/elbv2/service_impl_nlb_sg_test.go @@ -0,0 +1,298 @@ +package handlers_elbv2 + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/elbv2" + handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// firstSubnet returns the ID + VPC ID of the subnet setupTestServiceWithVPC +// pre-creates. +func firstSubnet(t *testing.T, vpcSvc *handlers_ec2_vpc.VPCServiceImpl) (subnetID, vpcID string) { + t.Helper() + subnets, err := vpcSvc.DescribeSubnets(&ec2.DescribeSubnetsInput{}, testAccountID) + require.NoError(t, err) + require.NotEmpty(t, subnets.Subnets) + return *subnets.Subnets[0].SubnetId, *subnets.Subnets[0].VpcId +} + +// describeSG returns the security group record by ID. +func describeSG(t *testing.T, vpcSvc *handlers_ec2_vpc.VPCServiceImpl, sgID string) *ec2.SecurityGroup { + t.Helper() + out, err := vpcSvc.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{ + GroupIds: aws.StringSlice([]string{sgID}), + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.SecurityGroups, 1) + return out.SecurityGroups[0] +} + +// sgHasRule reports whether the SG has an ingress rule for the proto/port/cidr. +func sgHasRule(sg *ec2.SecurityGroup, proto string, port int64, cidr string) bool { + for _, p := range sg.IpPermissions { + if aws.StringValue(p.IpProtocol) != proto { + continue + } + if aws.Int64Value(p.FromPort) != port || aws.Int64Value(p.ToPort) != port { + continue + } + for _, r := range p.IpRanges { + if aws.StringValue(r.CidrIp) == cidr { + return true + } + } + } + return false +} + +func managedENI(t *testing.T, vpcSvc *handlers_ec2_vpc.VPCServiceImpl) *ec2.NetworkInterface { + t.Helper() + out, err := vpcSvc.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{}, testAccountID) + require.NoError(t, err) + for _, eni := range out.NetworkInterfaces { + if eni.RequesterManaged != nil && *eni.RequesterManaged { + return eni + } + } + return nil +} + +func createNLB(t *testing.T, svc *ELBv2ServiceImpl, name, scheme, subnetID string) *elbv2.LoadBalancer { + t.Helper() + in := &elbv2.CreateLoadBalancerInput{ + Name: aws.String(name), + Type: aws.String("network"), + Subnets: []*string{aws.String(subnetID)}, + } + if scheme != "" { + in.Scheme = aws.String(scheme) + } + out, err := svc.CreateLoadBalancer(in, testAccountID) + require.NoError(t, err) + require.Len(t, out.LoadBalancers, 1) + return out.LoadBalancers[0] +} + +func createTCPListener(t *testing.T, svc *ELBv2ServiceImpl, lbArn *string, port int64) { + t.Helper() + tg, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-nlb-sg"), + Protocol: aws.String(elbv2.ProtocolEnumTcp), + Port: aws.Int64(port), + }, testAccountID) + require.NoError(t, err) + _, err = svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lbArn, + Protocol: aws.String(elbv2.ProtocolEnumTcp), + Port: aws.Int64(port), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tg.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + require.NoError(t, err) +} + +// TestCreateNLB_MintsManagedSGAttachedToENI verifies a network LB gets a +// dedicated internal SG (not the VPC default) attached to its ENI, recorded in +// NLBManagedSGID, while DescribeLoadBalancers still reports no customer SGs. +func TestCreateNLB_MintsManagedSGAttachedToENI(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, vpcID := firstSubnet(t, vpcSvc) + + defaultSGID, err := vpcSvc.FindDefaultSGForVPC(testAccountID, vpcID) + require.NoError(t, err) + + lb := createNLB(t, svc, "nlb-sg", "internal", subnetID) + assert.Empty(t, lb.SecurityGroups, "NLB must not surface the managed SG as a customer SG") + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + require.NotEmpty(t, rec.NLBManagedSGID, "NLB record must carry the managed SG id") + assert.NotEqual(t, defaultSGID, rec.NLBManagedSGID, "managed SG must be distinct from the VPC default SG") + + eni := managedENI(t, vpcSvc) + require.NotNil(t, eni) + require.Len(t, eni.Groups, 1, "NLB ENI must join exactly the managed SG, not the default SG") + assert.Equal(t, rec.NLBManagedSGID, *eni.Groups[0].GroupId) +} + +// TestCreateALB_NoManagedSG verifies the managed-SG behavior is NLB-only: an ALB +// keeps the existing customer-SG / default-SG semantics. +func TestCreateALB_NoManagedSG(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + out, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("alb-no-sg"), + Subnets: []*string{aws.String(subnetID)}, + }, testAccountID) + require.NoError(t, err) + + rec, err := svc.store.GetLoadBalancerByArn(*out.LoadBalancers[0].LoadBalancerArn) + require.NoError(t, err) + assert.Empty(t, rec.NLBManagedSGID, "ALB must not mint a managed NLB SG") +} + +// TestCreateListener_Internal_OpensPortFromVPCCIDR verifies an internal NLB's +// listener port is authorized on the managed SG from the VPC CIDR. +func TestCreateListener_Internal_OpensPortFromVPCCIDR(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-int", "internal", subnetID) + createTCPListener(t, svc, lb.LoadBalancerArn, 443) + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + sg := describeSG(t, vpcSvc, rec.NLBManagedSGID) + assert.True(t, sgHasRule(sg, "tcp", 443, "10.0.0.0/16"), + "internal NLB listener :443 must be open from the VPC CIDR") + assert.False(t, sgHasRule(sg, "tcp", 443, "0.0.0.0/0")) +} + +// TestCreateListener_InternetFacing_OpensPortFromAnywhere verifies an +// internet-facing NLB's listener port is authorized from 0.0.0.0/0. +func TestCreateListener_InternetFacing_OpensPortFromAnywhere(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-pub", "internet-facing", subnetID) + createTCPListener(t, svc, lb.LoadBalancerArn, 443) + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + sg := describeSG(t, vpcSvc, rec.NLBManagedSGID) + assert.True(t, sgHasRule(sg, "tcp", 443, "0.0.0.0/0"), + "internet-facing NLB listener :443 must be open from 0.0.0.0/0") +} + +// TestCreateListener_TCPUDP_OpensBothProtocols verifies a TCP_UDP listener opens +// both tcp and udp on the listener port. +func TestCreateListener_TCPUDP_OpensBothProtocols(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-tcpudp", "internet-facing", subnetID) + tg, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("tg-tcpudp"), + Protocol: aws.String(elbv2.ProtocolEnumTcpUdp), + Port: aws.Int64(53), + }, testAccountID) + require.NoError(t, err) + _, err = svc.CreateListener(&elbv2.CreateListenerInput{ + LoadBalancerArn: lb.LoadBalancerArn, + Protocol: aws.String(elbv2.ProtocolEnumTcpUdp), + Port: aws.Int64(53), + DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tg.TargetGroups[0].TargetGroupArn}}, + }, testAccountID) + require.NoError(t, err) + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + sg := describeSG(t, vpcSvc, rec.NLBManagedSGID) + assert.True(t, sgHasRule(sg, "tcp", 53, "0.0.0.0/0")) + assert.True(t, sgHasRule(sg, "udp", 53, "0.0.0.0/0")) +} + +// TestSetLoadBalancerIngressCIDRs_RewritesListenerRules verifies the override API +// swaps the scheme-default rule for the supplied CIDRs across existing listeners +// and is idempotent on re-invocation. +func TestSetLoadBalancerIngressCIDRs_RewritesListenerRules(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-cidr", "internet-facing", subnetID) + createTCPListener(t, svc, lb.LoadBalancerArn, 443) + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + sgID := rec.NLBManagedSGID + + // Default opened 0.0.0.0/0. + assert.True(t, sgHasRule(describeSG(t, vpcSvc, sgID), "tcp", 443, "0.0.0.0/0")) + + require.NoError(t, svc.SetLoadBalancerIngressCIDRs(*lb.LoadBalancerArn, []string{"203.0.113.0/24"}, testAccountID)) + + sg := describeSG(t, vpcSvc, sgID) + assert.True(t, sgHasRule(sg, "tcp", 443, "203.0.113.0/24"), "new CIDR must be authorized") + assert.False(t, sgHasRule(sg, "tcp", 443, "0.0.0.0/0"), "old default rule must be revoked") + + // Idempotent re-apply. + require.NoError(t, svc.SetLoadBalancerIngressCIDRs(*lb.LoadBalancerArn, []string{"203.0.113.0/24"}, testAccountID)) + sg = describeSG(t, vpcSvc, sgID) + assert.True(t, sgHasRule(sg, "tcp", 443, "203.0.113.0/24")) + + // Persisted override drives the resolved CIDRs. + rec, err = svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + assert.Equal(t, []string{"203.0.113.0/24"}, rec.NLBIngressCIDRs) +} + +func TestSetLoadBalancerIngressCIDRs_RejectsBadInput(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-badcidr", "internet-facing", subnetID) + err := svc.SetLoadBalancerIngressCIDRs(*lb.LoadBalancerArn, []string{"not-a-cidr"}, testAccountID) + require.Error(t, err) + + // ALB is rejected — managed SG is NLB-only. + albOut, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("alb-reject"), + Subnets: []*string{aws.String(subnetID)}, + }, testAccountID) + require.NoError(t, err) + err = svc.SetLoadBalancerIngressCIDRs(*albOut.LoadBalancers[0].LoadBalancerArn, []string{"10.0.0.0/8"}, testAccountID) + require.Error(t, err) +} + +// TestDeleteListener_RevokesPort verifies removing a listener closes its port on +// the managed SG. +func TestDeleteListener_RevokesPort(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-del-lst", "internet-facing", subnetID) + createTCPListener(t, svc, lb.LoadBalancerArn, 443) + + lst, err := svc.DescribeListeners(&elbv2.DescribeListenersInput{LoadBalancerArn: lb.LoadBalancerArn}, testAccountID) + require.NoError(t, err) + require.Len(t, lst.Listeners, 1) + + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + require.True(t, sgHasRule(describeSG(t, vpcSvc, rec.NLBManagedSGID), "tcp", 443, "0.0.0.0/0")) + + _, err = svc.DeleteListener(&elbv2.DeleteListenerInput{ListenerArn: lst.Listeners[0].ListenerArn}, testAccountID) + require.NoError(t, err) + + assert.False(t, sgHasRule(describeSG(t, vpcSvc, rec.NLBManagedSGID), "tcp", 443, "0.0.0.0/0"), + "deleting the listener must close its port") +} + +// TestDeleteNLB_DeletesManagedSG verifies the managed SG is removed on LB +// teardown. +func TestDeleteNLB_DeletesManagedSG(t *testing.T) { + svc, vpcSvc := setupTestServiceWithVPC(t) + subnetID, _ := firstSubnet(t, vpcSvc) + + lb := createNLB(t, svc, "nlb-del", "internal", subnetID) + rec, err := svc.store.GetLoadBalancerByArn(*lb.LoadBalancerArn) + require.NoError(t, err) + sgID := rec.NLBManagedSGID + require.NotEmpty(t, sgID) + + _, err = svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{LoadBalancerArn: lb.LoadBalancerArn}, testAccountID) + require.NoError(t, err) + + out, err := vpcSvc.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{ + GroupIds: aws.StringSlice([]string{sgID}), + }, testAccountID) + // Deleted SG: either an empty result or a not-found error is acceptable. + if err == nil { + assert.Empty(t, out.SecurityGroups, "managed SG must be deleted with the NLB") + } +} diff --git a/spinifex/handlers/elbv2/types.go b/spinifex/handlers/elbv2/types.go index 57f2cb364..e11682691 100644 --- a/spinifex/handlers/elbv2/types.go +++ b/spinifex/handlers/elbv2/types.go @@ -100,15 +100,24 @@ const ( // LoadBalancerRecord represents a stored load balancer (ALB or NLB). type LoadBalancerRecord struct { - LoadBalancerArn string `json:"load_balancer_arn"` - LoadBalancerID string `json:"load_balancer_id"` // Short ID (hex suffix) - DNSName string `json:"dns_name"` - Name string `json:"name"` - Scheme string `json:"scheme"` // "internet-facing" or "internal" - Type string `json:"type"` // Always "application" - State string `json:"state"` // "provisioning", "active", "failed" - VpcId string `json:"vpc_id"` - SecurityGroups []string `json:"security_groups"` + LoadBalancerArn string `json:"load_balancer_arn"` + LoadBalancerID string `json:"load_balancer_id"` // Short ID (hex suffix) + DNSName string `json:"dns_name"` + Name string `json:"name"` + Scheme string `json:"scheme"` // "internet-facing" or "internal" + Type string `json:"type"` // Always "application" + State string `json:"state"` // "provisioning", "active", "failed" + VpcId string `json:"vpc_id"` + SecurityGroups []string `json:"security_groups"` + // NLBManagedSGID is the internal security group the ELBv2 service mints for + // a network LB (NLBs reject customer SGs, so this is created and owned here). + // It is attached to every LB ENI and is the SG listener-port ingress is + // authorized on. Not surfaced by DescribeLoadBalancers. + NLBManagedSGID string `json:"nlb_managed_sg_id,omitempty"` + // NLBIngressCIDRs overrides the scheme-based default client CIDRs that + // listener ports are opened to on NLBManagedSGID. Empty ⇒ default + // (internet-facing: 0.0.0.0/0; internal: the VPC CIDR). + NLBIngressCIDRs []string `json:"nlb_ingress_cidrs,omitempty"` Subnets []string `json:"subnets"` AvailZones []AvailZoneInfo `json:"availability_zones"` ENIs []string `json:"enis,omitempty"` // ENI IDs created for this ALB (internal)