fix: reap orphan LBC ALBs and security groups on EKS cluster teardown (#444)

The in-cluster AWS LoadBalancer Controller provisions its own ALBs
(k8s-toc-*) and security groups (k8s-traffic-toc-*) in the customer VPC
via the awsgw ELBv2/EC2 APIs. Spinifex never tracks these, so cluster
teardown left them orphaned: DeleteSecurityGroup / DeleteVpc then failed
with DependencyViolation and the cluster wedged in DELETING.

purgeClusterInfra now reaps them by the AWS-standard ownership tag
elbv2.k8s.aws/cluster=<name>, scoped to the customer VPC, after the
cluster NLB teardown and before the SG / VPC block:

- ReapLBCLoadBalancers lists account LBs, filters to the customer VPC,
  confirms the ownership tag via DescribeTags, and deletes by ARN.
- DeleteClusterSGs deletes any VPC SG carrying the ownership tag (reusing
  the DependencyViolation retry budget), in addition to the named cluster
  SGs, after rules are revoked.
- DescribeTags is added to the narrow nlbProvisioner interface.

Errors join teardownErrs so a failed reap keeps the cluster in DELETING
for a retry rather than completing with leaked, VPC-pinning infra.

spinifex/handlers/eks/nlb.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/elbv2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
 	"github.com/mulgadc/spinifex/spinifex/handlers/sysinstance"
 	"github.com/mulgadc/spinifex/spinifex/tags"
 )
@@ -20,6 +21,7 @@ type nlbProvisioner interface {
 	CreateClusterNLBSync(input *elbv2.CreateLoadBalancerInput, accountID string, crossAccountENIs []sysinstance.ExtraENIInput) (*elbv2.CreateLoadBalancerOutput, error)
 	DescribeLoadBalancers(input *elbv2.DescribeLoadBalancersInput, accountID string) (*elbv2.DescribeLoadBalancersOutput, error)
 	DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInput, accountID string) (*elbv2.DeleteLoadBalancerOutput, error)
+	DescribeTags(input *elbv2.DescribeTagsInput, accountID string) (*elbv2.DescribeTagsOutput, error)
 
 	CreateTargetGroup(input *elbv2.CreateTargetGroupInput, accountID string) (*elbv2.CreateTargetGroupOutput, error)
 	DescribeTargetGroups(input *elbv2.DescribeTargetGroupsInput, accountID string) (*elbv2.DescribeTargetGroupsOutput, error)
@@ -44,6 +46,12 @@ const k3sAPIServerPort int64 = 6443
 // kubectl + the AWS SDK both expect TLS on 443.
 const clusterNLBListenPort int64 = 443
 
+// lbcClusterOwnershipTagKey is the AWS-standard ownership tag the in-cluster AWS
+// LoadBalancer Controller stamps on every ALB and SG it provisions. Spinifex
+// passes it through verbatim, so it reliably discriminates LBC-orphaned
+// resources from spinifex-owned ones (which use spinifex:eks-cluster).
+const lbcClusterOwnershipTagKey = "elbv2.k8s.aws/cluster"
+
 // ClusterNLB is the NLB resource tuple produced by EnsureClusterNLB; ARNs are persisted to ClusterMeta.
 type ClusterNLB struct {
 	LoadBalancerArn string
@@ -439,6 +447,69 @@ func lookupTGByName(nlbp nlbProvisioner, accountID, name string) (*elbv2.TargetG
 	return nil, nil
 }
 
+// ReapLBCLoadBalancers deletes ALBs the in-cluster AWS LoadBalancer Controller
+// provisioned in the customer VPC (k8s-toc-*) but spinifex never tracked, so the
+// cluster's own teardown would otherwise leave them pinning the VPC undeletable
+// on DependencyViolation. Match is by AWS-standard ownership tag scoped to this
+// cluster + VPC; the cluster's own NLB is already torn down before this runs, so
+// it is not re-matched. A raced-away LB (NotFound) is skipped. Errors are joined
+// so a failed reap keeps the cluster in DELETING for a retry rather than
+// completing with a leaked ALB.
+func ReapLBCLoadBalancers(nlbp nlbProvisioner, accountID, clusterName, vpcID string) error {
+	if clusterName == "" || vpcID == "" {
+		return nil
+	}
+	out, err := nlbp.DescribeLoadBalancers(&elbv2.DescribeLoadBalancersInput{}, accountID)
+	if err != nil {
+		return fmt.Errorf("describe LBs for LBC reap: %w", err)
+	}
+	var errs []error
+	for _, lb := range out.LoadBalancers {
+		if lb == nil || lb.LoadBalancerArn == nil || aws.StringValue(lb.VpcId) != vpcID {
+			continue
+		}
+		arn := aws.StringValue(lb.LoadBalancerArn)
+		owned, ownErr := lbOwnedByCluster(nlbp, accountID, arn, clusterName)
+		if ownErr != nil {
+			errs = append(errs, ownErr)
+			continue
+		}
+		if !owned {
+			continue
+		}
+		if _, err := nlbp.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{LoadBalancerArn: lb.LoadBalancerArn}, accountID); err != nil && !awserrors.IsNotFound(err) {
+			slog.Warn("ReapLBCLoadBalancers: delete LBC ALB failed", "arn", arn, "err", err)
+			errs = append(errs, fmt.Errorf("delete LBC ALB %s: %w", arn, err))
+			continue
+		}
+		slog.Info("ReapLBCLoadBalancers: reaped orphan LBC ALB", "arn", arn, "cluster", clusterName)
+	}
+	return errors.Join(errs...)
+}
+
+// lbOwnedByCluster reports whether the LB carries the LBC ownership tag for this
+// cluster. A NotFound (raced-away LB) is treated as not-owned so the reap skips it.
+func lbOwnedByCluster(nlbp nlbProvisioner, accountID, arn, clusterName string) (bool, error) {
+	out, err := nlbp.DescribeTags(&elbv2.DescribeTagsInput{ResourceArns: aws.StringSlice([]string{arn})}, accountID)
+	if err != nil {
+		if awserrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, fmt.Errorf("describe tags for %s: %w", arn, err)
+	}
+	for _, desc := range out.TagDescriptions {
+		if desc == nil {
+			continue
+		}
+		for _, tag := range desc.Tags {
+			if tag != nil && aws.StringValue(tag.Key) == lbcClusterOwnershipTagKey && aws.StringValue(tag.Value) == clusterName {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 func lookupListenerByPort(nlbp nlbProvisioner, accountID, lbArn string, port int64) (*elbv2.Listener, error) {
 	if lbArn == "" {
 		return nil, errors.New("eks: lookupListenerByPort empty LB arn")

spinifex/handlers/eks/nlb_test.go

@@ -37,6 +37,10 @@ type fakeNLBProvisioner struct {
 	lbByName       map[string]*elbv2.LoadBalancer
 	tgByName       map[string]*elbv2.TargetGroup
 	listenerByPort map[string]map[int64]*elbv2.Listener // lbArn → port → listener
+	tagsByArn      map[string][]*elbv2.Tag              // lb arn → tags (LBC ownership reap)
+
+	describeTagsCalls []*elbv2.DescribeTagsInput
+	describeTagsErr   error
 
 	createLBOut       *elbv2.CreateLoadBalancerOutput
 	createTGOut       *elbv2.CreateTargetGroupOutput
@@ -63,6 +67,7 @@ func newFakeNLBProvisioner() *fakeNLBProvisioner {
 		lbByName:       map[string]*elbv2.LoadBalancer{},
 		tgByName:       map[string]*elbv2.TargetGroup{},
 		listenerByPort: map[string]map[int64]*elbv2.Listener{},
+		tagsByArn:      map[string][]*elbv2.Tag{},
 	}
 }
 
@@ -124,6 +129,14 @@ func (f *fakeNLBProvisioner) CreateLoadBalancer(input *elbv2.CreateLoadBalancerI
 func (f *fakeNLBProvisioner) DescribeLoadBalancers(input *elbv2.DescribeLoadBalancersInput, _ string) (*elbv2.DescribeLoadBalancersOutput, error) {
 	f.describeLBCalls = append(f.describeLBCalls, input)
 	out := &elbv2.DescribeLoadBalancersOutput{}
+	// No name/arn filter: list every LB (account-scoped in the real impl), as the
+	// LBC ALB reap relies on to enumerate untracked load balancers.
+	if len(input.Names) == 0 && len(input.LoadBalancerArns) == 0 {
+		for _, lb := range f.lbByName {
+			out.LoadBalancers = append(out.LoadBalancers, lb)
+		}
+		return out, nil
+	}
 	for _, n := range input.Names {
 		if n == nil {
 			continue
@@ -135,6 +148,24 @@ func (f *fakeNLBProvisioner) DescribeLoadBalancers(input *elbv2.DescribeLoadBala
 	return out, nil
 }
 
+func (f *fakeNLBProvisioner) DescribeTags(input *elbv2.DescribeTagsInput, _ string) (*elbv2.DescribeTagsOutput, error) {
+	f.describeTagsCalls = append(f.describeTagsCalls, input)
+	if f.describeTagsErr != nil {
+		return nil, f.describeTagsErr
+	}
+	out := &elbv2.DescribeTagsOutput{}
+	for _, arn := range input.ResourceArns {
+		if arn == nil {
+			continue
+		}
+		out.TagDescriptions = append(out.TagDescriptions, &elbv2.TagDescription{
+			ResourceArn: arn,
+			Tags:        f.tagsByArn[*arn],
+		})
+	}
+	return out, nil
+}
+
 func (f *fakeNLBProvisioner) DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInput, _ string) (*elbv2.DeleteLoadBalancerOutput, error) {
 	f.deleteLBCalls = append(f.deleteLBCalls, input)
 	if f.deleteLBErr != nil {
@@ -608,3 +639,58 @@ func assertELBv2TaggedAsEKS(t *testing.T, tgs []*elbv2.Tag, clusterName string)
 	assert.Equal(t, tags.ManagedByEKS, got[tags.ManagedByKey])
 	assert.Equal(t, clusterName, got[clusterEKSClusterTagKey])
 }
+
+// seedLBCALB registers an LBC-style ALB in the fake with the given VPC and an
+// optional ownership-tag cluster value (empty = untagged); returns its ARN.
+func (f *fakeNLBProvisioner) seedLBCALB(name, vpcID, ownerCluster string) string {
+	arn := "arn:aws:elasticloadbalancing:us-east-1:111122223333:loadbalancer/app/" + name + "/lbc-" + name
+	f.lbByName[name] = &elbv2.LoadBalancer{
+		LoadBalancerArn:  aws.String(arn),
+		LoadBalancerName: aws.String(name),
+		VpcId:            aws.String(vpcID),
+	}
+	if ownerCluster != "" {
+		f.tagsByArn[arn] = []*elbv2.Tag{{
+			Key:   aws.String(lbcClusterOwnershipTagKey),
+			Value: aws.String(ownerCluster),
+		}}
+	}
+	return arn
+}
+
+// Only an LBC ALB in the customer VPC carrying this cluster's ownership tag is
+// reaped — a different VPC, a different cluster, or no ownership tag is left alone.
+func TestReapLBCLoadBalancers_DeletesOnlyOwnedInVPC(t *testing.T) {
+	nlbp := newFakeNLBProvisioner()
+	owned := nlbp.seedLBCALB("k8s-toc-owned", "vpc-cust", "alpha")
+	nlbp.seedLBCALB("k8s-toc-othervpc", "vpc-other", "alpha")   // wrong VPC
+	nlbp.seedLBCALB("k8s-toc-othercluster", "vpc-cust", "beta") // wrong cluster
+	nlbp.seedLBCALB("k8s-toc-untagged", "vpc-cust", "")         // no ownership tag
+
+	require.NoError(t, ReapLBCLoadBalancers(nlbp, "111122223333", "alpha", "vpc-cust"))
+
+	require.Len(t, nlbp.deleteLBCalls, 1)
+	assert.Equal(t, owned, aws.StringValue(nlbp.deleteLBCalls[0].LoadBalancerArn))
+}
+
+// Empty cluster name or VPC id is a no-op (nothing to scope the reap to).
+func TestReapLBCLoadBalancers_EmptyArgsNoop(t *testing.T) {
+	nlbp := newFakeNLBProvisioner()
+	nlbp.seedLBCALB("k8s-toc-owned", "vpc-cust", "alpha")
+
+	require.NoError(t, ReapLBCLoadBalancers(nlbp, "111122223333", "", "vpc-cust"))
+	require.NoError(t, ReapLBCLoadBalancers(nlbp, "111122223333", "alpha", ""))
+	assert.Empty(t, nlbp.deleteLBCalls)
+}
+
+// A delete failure surfaces so the teardown backstop retries rather than leaking
+// the ALB (which would pin the VPC undeletable).
+func TestReapLBCLoadBalancers_DeleteErrorSurfaces(t *testing.T) {
+	nlbp := newFakeNLBProvisioner()
+	nlbp.seedLBCALB("k8s-toc-owned", "vpc-cust", "alpha")
+	nlbp.deleteLBErr = errors.New("boom")
+
+	err := ReapLBCLoadBalancers(nlbp, "111122223333", "alpha", "vpc-cust")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "boom")
+}

spinifex/handlers/eks/security_groups.go

@@ -140,6 +140,24 @@ func DeleteClusterSGs(sgp sgProvisioner, accountID, clusterName, vpcID string) e
 		}
 	}
 
+	// Reap LBC-orphaned SGs (k8s-traffic-toc-*): the in-cluster AWS LoadBalancer
+	// Controller creates these in the customer VPC and spinifex never tracks them,
+	// so nothing else deletes them and they pin the VPC on DependencyViolation.
+	// Match by the LBC ownership tag; rules were already revoked above. Runs after
+	// the matching ALB reap, so the LB no longer references the SG.
+	for _, g := range out.SecurityGroups {
+		if g == nil || g.GroupId == nil || aws.StringValue(g.GroupName) == "default" {
+			continue
+		}
+		if !sgHasLBCClusterTag(g, clusterName) {
+			continue
+		}
+		if delErr := deleteSGAwaitingDetach(sgp, accountID, aws.StringValue(g.GroupId)); delErr != nil {
+			slog.Warn("DeleteClusterSGs: LBC SG delete failed", "sg", aws.StringValue(g.GroupId), "err", delErr)
+			record(fmt.Errorf("delete LBC SG %s: %w", aws.StringValue(g.GroupId), delErr))
+		}
+	}
+
 	// Delete only the cluster-owned SGs.
 	for _, name := range []string{ClusterControlPlaneSGName(clusterName), ClusterNodegroupSGName(clusterName)} {
 		id, lookupErr := lookupSGByName(sgp, accountID, vpcID, name)
@@ -439,6 +457,17 @@ func EnsureControlPlaneHAIngress(sgp sgProvisioner, accountID, cpSGID string) er
 	return nil
 }
 
+// sgHasLBCClusterTag reports whether the SG carries the AWS LoadBalancer
+// Controller ownership tag for this cluster.
+func sgHasLBCClusterTag(g *ec2.SecurityGroup, clusterName string) bool {
+	for _, tag := range g.Tags {
+		if tag != nil && aws.StringValue(tag.Key) == lbcClusterOwnershipTagKey && aws.StringValue(tag.Value) == clusterName {
+			return true
+		}
+	}
+	return false
+}
+
 func lookupSGByName(sgp sgProvisioner, accountID, vpcID, name string) (string, error) {
 	out, err := sgp.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{
 		Filters: []*ec2.Filter{

spinifex/handlers/eks/security_groups_test.go

@@ -32,6 +32,10 @@ type fakeSGProvisioner struct {
 	perms       map[string][]*ec2.IpPermission
 	permsEgress map[string][]*ec2.IpPermission
 
+	// tagsByID maps groupId → its tags, served on the VPC sweep so the LBC-SG reap
+	// can match the ownership tag.
+	tagsByID map[string][]*ec2.Tag
+
 	// enforceDepViolation models AWS refusing to delete an SG that still has
 	// ingress rules (e.g. a sibling cross-reference), so a test proves the
 	// revoke-before-delete ordering.
@@ -124,6 +128,7 @@ func (f *fakeSGProvisioner) DescribeSecurityGroups(input *ec2.DescribeSecurityGr
 				VpcId:               aws.String(vpc),
 				IpPermissions:       f.perms[id],
 				IpPermissionsEgress: f.permsEgress[id],
+				Tags:                f.tagsByID[id],
 			})
 		}
 		return out, nil
@@ -263,6 +268,35 @@ func TestDeleteClusterSGs_DeletesBothExisting(t *testing.T) {
 	assert.Equal(t, "sg-existing-ng", aws.StringValue(sgp.deleteCalls[1].GroupId))
 }
 
+// An LBC-orphaned SG (k8s-traffic-toc-*) carrying this cluster's ownership tag is
+// deleted alongside the named cluster SGs, while an untagged third-party SG in the
+// same VPC has its rules revoked but is left intact.
+func TestDeleteClusterSGs_ReapsLBCTaggedSGs(t *testing.T) {
+	sgp := newFakeSGProvisioner()
+	sgp.existing["eks-cluster-alpha-control-plane-sg|vpc-aaa"] = "sg-cp"
+	sgp.existing["eks-cluster-alpha-nodegroup-sg|vpc-aaa"] = "sg-ng"
+	sgp.existing["k8s-traffic-toc-abc|vpc-aaa"] = "sg-lbc"
+	sgp.existing["k8s-traffic-toc-other|vpc-aaa"] = "sg-lbc-other"
+	sgp.existing["user-app-sg|vpc-aaa"] = "sg-user"
+	sgp.tagsByID = map[string][]*ec2.Tag{
+		"sg-lbc":       {{Key: aws.String(lbcClusterOwnershipTagKey), Value: aws.String("alpha")}},
+		"sg-lbc-other": {{Key: aws.String(lbcClusterOwnershipTagKey), Value: aws.String("beta")}},
+	}
+
+	err := DeleteClusterSGs(sgp, "111122223333", "alpha", "vpc-aaa")
+	require.NoError(t, err)
+
+	deleted := map[string]bool{}
+	for _, c := range sgp.deleteCalls {
+		deleted[aws.StringValue(c.GroupId)] = true
+	}
+	assert.True(t, deleted["sg-cp"], "control-plane SG deleted")
+	assert.True(t, deleted["sg-ng"], "nodegroup SG deleted")
+	assert.True(t, deleted["sg-lbc"], "LBC SG owned by alpha deleted")
+	assert.False(t, deleted["sg-lbc-other"], "LBC SG owned by beta left intact")
+	assert.False(t, deleted["sg-user"], "untagged third-party SG left intact")
+}
+
 func TestDeleteClusterSGs_MissingSGsNoOp(t *testing.T) {
 	sgp := newFakeSGProvisioner()
 

spinifex/handlers/eks/service_impl.go

@@ -1013,6 +1013,17 @@ func (s *EKSServiceImpl) purgeClusterInfra(accountID, name string, meta *Cluster
 		}
 	}
 
+	// Reap LBC-orphaned ALBs (k8s-toc-*) in the customer VPC before the SG sweep:
+	// the in-cluster AWS LoadBalancer Controller runs under the customer account and
+	// spinifex never tracks its ALBs, so nothing else deletes them and they pin the
+	// VPC (and their k8s-traffic-toc-* SGs) undeletable. Must precede DeleteClusterSGs
+	// so the ALB no longer references the SG it reaps.
+	if meta.ResourcesVpcConfig != nil && meta.ResourcesVpcConfig.VpcId != "" {
+		if err := ReapLBCLoadBalancers(s.deps.NLB, accountID, name, meta.ResourcesVpcConfig.VpcId); err != nil {
+			teardownErrs = append(teardownErrs, fmt.Errorf("reap LBC ALBs: %w", err))
+		}
+	}
+
 	// SG / IGW / CP-VPC cleanup runs even when an earlier step failed: the cluster
 	// SGs sit in the customer VPC, so skipping them would pin the VPC. Failures are
 	// joined into teardownErrs (not swallowed) so the meta survives for a retry.