diff --git a/spinifex/daemon/eks_adapter.go b/spinifex/daemon/eks_adapter.go index 38e5a19ba..c68d3e28f 100644 --- a/spinifex/daemon/eks_adapter.go +++ b/spinifex/daemon/eks_adapter.go @@ -2,7 +2,10 @@ package daemon import ( "errors" + "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" ) @@ -28,4 +31,22 @@ func (a *daemonEKSSubnetResolver) GetSubnetVPC(accountID, subnetID string) (stri return rec.VpcId, nil } +func (a *daemonEKSSubnetResolver) GetVPCCIDR(accountID, vpcID string) (string, error) { + if a.d == nil || a.d.vpcService == nil { + return "", errors.New("eks: VPC service not initialized") + } + out, err := a.d.vpcService.DescribeVpcs(&ec2.DescribeVpcsInput{ + VpcIds: aws.StringSlice([]string{vpcID}), + }, accountID) + if err != nil { + return "", err + } + for _, v := range out.Vpcs { + if v != nil && aws.StringValue(v.VpcId) == vpcID { + return aws.StringValue(v.CidrBlock), nil + } + } + return "", fmt.Errorf("eks: VPC %s not found", vpcID) +} + var _ handlers_eks.SubnetVPCResolver = (*daemonEKSSubnetResolver)(nil) diff --git a/spinifex/handlers/eks/cluster_state.go b/spinifex/handlers/eks/cluster_state.go index c406757b3..dcea217a6 100644 --- a/spinifex/handlers/eks/cluster_state.go +++ b/spinifex/handlers/eks/cluster_state.go @@ -28,19 +28,32 @@ type ClusterVpcConfig struct { SubnetIds []string `json:"subnetIds"` SecurityGroupIds []string `json:"securityGroupIds,omitempty"` VpcId string `json:"vpcId,omitempty"` + // EndpointPublicAccess / EndpointPrivateAccess control apiserver endpoint + // exposure (AWS resourcesVpcConfig parity). Public ⇒ internet-facing cluster + // NLB (external-pool front-end IP, LAN-reachable); private ⇒ internal NLB + // (VPC-only). PublicAccessCidrs records the allowed source ranges for the + // public endpoint (stored for parity + Describe; ingress enforcement is a + // follow-up). + EndpointPublicAccess bool `json:"endpointPublicAccess"` + EndpointPrivateAccess bool `json:"endpointPrivateAccess"` + PublicAccessCidrs []string `json:"publicAccessCidrs,omitempty"` } // ClusterMeta is the persisted control-plane record for one cluster. The // blob lives at ClusterMetaKey(name) inside the per-account KV bucket and is // the source of truth for DescribeCluster. type ClusterMeta struct { - Name string `json:"name"` - Arn string `json:"arn"` - Status ClusterStatus `json:"status"` - StatusReason string `json:"statusReason,omitempty"` - Version string `json:"version"` - RoleArn string `json:"roleArn"` - Endpoint string `json:"endpoint,omitempty"` + Name string `json:"name"` + Arn string `json:"arn"` + Status ClusterStatus `json:"status"` + StatusReason string `json:"statusReason,omitempty"` + Version string `json:"version"` + RoleArn string `json:"roleArn"` + Endpoint string `json:"endpoint,omitempty"` + // EndpointIP is the host/LAN-reachable front-end IP the cluster NLB exposes + // (public-access ⇒ external-pool IP; private ⇒ VPC IP). The apiserver serving + // cert SANs this IP, so kubectl reaches Endpoint with TLS verification on. + EndpointIP string `json:"endpointIp,omitempty"` OIDCIssuer string `json:"oidcIssuer,omitempty"` CertificateAuthorityB64 string `json:"certificateAuthorityB64,omitempty"` ResourcesVpcConfig *ClusterVpcConfig `json:"resourcesVpcConfig,omitempty"` diff --git a/spinifex/handlers/eks/delete_cluster_test.go b/spinifex/handlers/eks/delete_cluster_test.go index 5808ab174..66b18593f 100644 --- a/spinifex/handlers/eks/delete_cluster_test.go +++ b/spinifex/handlers/eks/delete_cluster_test.go @@ -18,6 +18,7 @@ type fakeSubnetResolver struct{} var _ SubnetVPCResolver = (*fakeSubnetResolver)(nil) func (fakeSubnetResolver) GetSubnetVPC(_, _ string) (string, error) { return "vpc-aaa", nil } +func (fakeSubnetResolver) GetVPCCIDR(_, _ string) (string, error) { return "10.0.0.0/16", nil } func deleteInput(name string) *eks.DeleteClusterInput { return &eks.DeleteClusterInput{Name: aws.String(name)} diff --git a/spinifex/handlers/eks/k3s_server_vm.go b/spinifex/handlers/eks/k3s_server_vm.go index 37bae5555..ee74019d3 100644 --- a/spinifex/handlers/eks/k3s_server_vm.go +++ b/spinifex/handlers/eks/k3s_server_vm.go @@ -112,12 +112,17 @@ const ( // deferred behind cross-account-ENI work). Region is carried for future // region-aware AMI lookups but not consumed today. type K3sServerInput struct { - AccountID string - ClusterName string - Region string - SubnetID string - ControlPlaneSGID string - NLBDNS string + AccountID string + ClusterName string + Region string + SubnetID string + ControlPlaneSGID string + NLBDNS string + // EndpointIP is the reachable cluster NLB front-end IP. When set it is added + // to the apiserver serving-cert SANs (tls-san) so kubectl reaching the + // cluster on this IP validates TLS. Empty for an internal endpoint with no + // front-end IP read back. + EndpointIP string OIDCIssuer string OIDCPrivateKeyPEM string OIDCPublicKeyPEM string @@ -422,9 +427,14 @@ func buildK3sUserData(in K3sServerInput) string { if !in.BuiltinIngress { configLines = append(configLines, "disable:", " - traefik", " - servicelb") } + configLines = append(configLines, "tls-san:", " - "+in.NLBDNS) + // The reachable front-end IP must be a cert SAN so kubectl reaching the + // cluster on https://:443 validates TLS. k3s accepts IP literals + // in tls-san and classifies them as IP SANs. + if in.EndpointIP != "" { + configLines = append(configLines, " - "+in.EndpointIP) + } configLines = append(configLines, - "tls-san:", - " - "+in.NLBDNS, "kube-apiserver-arg:", " - service-account-key-file="+k3sOIDCPublicKeyPath, " - service-account-signing-key-file="+k3sOIDCSigningKeyPath, diff --git a/spinifex/handlers/eks/nlb.go b/spinifex/handlers/eks/nlb.go index dc7fc8b38..1aa6b04d2 100644 --- a/spinifex/handlers/eks/nlb.go +++ b/spinifex/handlers/eks/nlb.go @@ -27,6 +27,8 @@ type nlbProvisioner interface { RegisterTargets(input *elbv2.RegisterTargetsInput, accountID string) (*elbv2.RegisterTargetsOutput, error) DeregisterTargets(input *elbv2.DeregisterTargetsInput, accountID string) (*elbv2.DeregisterTargetsOutput, error) + + SetLoadBalancerIngressCIDRs(lbArn string, cidrs []string, accountID string) error } // AWS load-balancer + target-group names are bounded to 32 alphanumeric or @@ -51,6 +53,11 @@ type ClusterNLB struct { TargetGroupArn string ListenerArn string DNSName string + // FrontendIP is the NLB's reachable front-end address: the external-pool + // public IP for an internet-facing cluster NLB, or the VPC private IP for an + // internal one. Used as the kubeconfig endpoint host + apiserver cert SAN. + // Empty if the backing LB record has no address yet. + FrontendIP string } // ClusterNLBName returns the deterministic NLB name for a cluster. Stable so @@ -73,7 +80,18 @@ func ClusterTargetGroupName(clusterName string) string { // // The K3s server VM ENI IP is NOT registered here — Stage 4 // (k3s_server_vm.go) calls RegisterClusterTarget once the VM has its ENI IP. -func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnetIDs []string) (*ClusterNLB, error) { +// +// internetFacing selects the NLB scheme: true ⇒ internet-facing (external-pool +// front-end IP, LAN-reachable, for a public cluster endpoint); false ⇒ internal +// (VPC-only). After ensuring the LB, the reachable FrontendIP is read back from +// the backing LB record so the caller can use it as the endpoint host + cert SAN. +// +// publicAccessCidrs narrows who may reach an internet-facing front-end below the +// scheme default (0.0.0.0/0). It maps the cluster's publicAccessCidrs onto the +// NLB managed-SG ingress; ignored for an internal NLB (its ingress already +// tracks the VPC CIDR) and for the wide-open default, which the LB carries out +// of the box. +func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnetIDs []string, internetFacing bool, publicAccessCidrs []string) (*ClusterNLB, error) { if clusterName == "" { return nil, errors.New("eks: EnsureClusterNLB empty cluster name") } @@ -91,7 +109,7 @@ func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnet out := &ClusterNLB{} - if err := ensureClusterLB(nlbp, accountID, clusterName, lbName, subnetIDs, out); err != nil { + if err := ensureClusterLB(nlbp, accountID, clusterName, lbName, subnetIDs, internetFacing, out); err != nil { return nil, err } if err := ensureClusterTG(nlbp, accountID, clusterName, tgName, out); err != nil { @@ -100,9 +118,62 @@ func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnet if err := ensureClusterListener(nlbp, accountID, lbName, out); err != nil { return nil, err } + // Narrow the internet-facing front-end to the cluster's publicAccessCidrs + // when they restrict below the wide-open scheme default the LB already + // carries. The override is idempotent, so re-running on retry converges. + if internetFacing && narrowsPublicAccess(publicAccessCidrs) { + if err := nlbp.SetLoadBalancerIngressCIDRs(out.LoadBalancerArn, publicAccessCidrs, accountID); err != nil { + return nil, fmt.Errorf("eks: set NLB ingress CIDRs for %s: %w", lbName, err) + } + } + // Read the front-end IP back from the LB record (DescribeLoadBalancers → + // populated LoadBalancerAddresses). Best-effort: an empty IP is not fatal + // here — the caller falls back to the DNS-name endpoint. + if lb, err := lookupLBByName(nlbp, accountID, lbName); err == nil && lb != nil { + out.FrontendIP = frontendIPFromLB(lb, internetFacing) + } else if err != nil { + slog.Warn("EnsureClusterNLB: front-end IP read-back failed", "name", lbName, "err", err) + } return out, nil } +// narrowsPublicAccess reports whether publicAccessCidrs restrict the front-end +// below the wide-open default an internet-facing NLB already carries. Empty (no +// override) or the lone 0.0.0.0/0 default are no-ops and skip the extra ELBv2 +// round-trip. +func narrowsPublicAccess(cidrs []string) bool { + if len(cidrs) == 0 { + return false + } + if len(cidrs) == 1 && cidrs[0] == defaultPublicAccessCidr { + return false + } + return true +} + +// frontendIPFromLB extracts the reachable front-end address from a described +// load balancer: the public IpAddress for an internet-facing LB, else the VPC +// PrivateIPv4Address. Returns the first non-empty match across AZs. +func frontendIPFromLB(lb *elbv2.LoadBalancer, internetFacing bool) string { + for _, az := range lb.AvailabilityZones { + for _, addr := range az.LoadBalancerAddresses { + if addr == nil { + continue + } + if internetFacing { + if ip := aws.StringValue(addr.IpAddress); ip != "" { + return ip + } + continue + } + if ip := aws.StringValue(addr.PrivateIPv4Address); ip != "" { + return ip + } + } + } + return "" +} + // RegisterClusterTarget attaches the K3s server ENI IP to the cluster TG so // the NLB starts forwarding :443 traffic to :6443 on the VM. The ELBv2 // RegisterTargets impl already dedupes on (id, port), so re-invocation by @@ -197,7 +268,7 @@ func deleteClusterTG(nlbp nlbProvisioner, accountID, tgName string) error { return nil } -func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, subnetIDs []string, out *ClusterNLB) error { +func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, subnetIDs []string, internetFacing bool, out *ClusterNLB) error { if lb, err := lookupLBByName(nlbp, accountID, lbName); err != nil { return err } else if lb != nil { @@ -209,10 +280,14 @@ func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, return nil } + scheme := elbv2.LoadBalancerSchemeEnumInternal + if internetFacing { + scheme = elbv2.LoadBalancerSchemeEnumInternetFacing + } created, err := nlbp.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ Name: aws.String(lbName), Type: aws.String(elbv2.LoadBalancerTypeEnumNetwork), - Scheme: aws.String(elbv2.LoadBalancerSchemeEnumInternal), + Scheme: aws.String(scheme), Subnets: aws.StringSlice(subnetIDs), Tags: []*elbv2.Tag{ {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByEKS)}, diff --git a/spinifex/handlers/eks/nlb_test.go b/spinifex/handlers/eks/nlb_test.go index 0b60d9471..632c4a4e2 100644 --- a/spinifex/handlers/eks/nlb_test.go +++ b/spinifex/handlers/eks/nlb_test.go @@ -23,6 +23,7 @@ type fakeNLBProvisioner struct { describeListeners []*elbv2.DescribeListenersInput registerCalls []*elbv2.RegisterTargetsInput deregisterCalls []*elbv2.DeregisterTargetsInput + setIngressCalls []setIngressCIDRsCall lbByName map[string]*elbv2.LoadBalancer tgByName map[string]*elbv2.TargetGroup @@ -38,6 +39,12 @@ type fakeNLBProvisioner struct { deleteTGErr error registerErr error deregisterErr error + setIngressErr error +} + +type setIngressCIDRsCall struct { + lbArn string + cidrs []string } var _ nlbProvisioner = (*fakeNLBProvisioner)(nil) @@ -214,13 +221,18 @@ func (f *fakeNLBProvisioner) DeregisterTargets(input *elbv2.DeregisterTargetsInp return &elbv2.DeregisterTargetsOutput{}, nil } +func (f *fakeNLBProvisioner) SetLoadBalancerIngressCIDRs(lbArn string, cidrs []string, _ string) error { + f.setIngressCalls = append(f.setIngressCalls, setIngressCIDRsCall{lbArn: lbArn, cidrs: cidrs}) + return f.setIngressErr +} + func TestEnsureClusterNLB_EmptyInputsRejected(t *testing.T) { nlbp := newFakeNLBProvisioner() - _, err := EnsureClusterNLB(nlbp, "111122223333", "", []string{"subnet-aaa"}) + _, err := EnsureClusterNLB(nlbp, "111122223333", "", []string{"subnet-aaa"}, false, nil) require.Error(t, err) - _, err = EnsureClusterNLB(nlbp, "111122223333", "alpha", nil) + _, err = EnsureClusterNLB(nlbp, "111122223333", "alpha", nil, false, nil) require.Error(t, err) assert.Empty(t, nlbp.createLBCalls) @@ -230,7 +242,7 @@ func TestEnsureClusterNLB_NameTooLongRejected(t *testing.T) { nlbp := newFakeNLBProvisioner() longName := strings.Repeat("x", maxELBv2NameLen) // "eks-" + 32x = 36 chars - _, err := EnsureClusterNLB(nlbp, "111122223333", longName, []string{"subnet-aaa"}) + _, err := EnsureClusterNLB(nlbp, "111122223333", longName, []string{"subnet-aaa"}, false, nil) require.Error(t, err) assert.Contains(t, err.Error(), "exceeds") assert.Empty(t, nlbp.createLBCalls) @@ -239,7 +251,7 @@ func TestEnsureClusterNLB_NameTooLongRejected(t *testing.T) { func TestEnsureClusterNLB_FreshCreatesAllThree(t *testing.T) { nlbp := newFakeNLBProvisioner() - out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa", "subnet-bbb"}) + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa", "subnet-bbb"}, false, nil) require.NoError(t, err) require.NotNil(t, out) assert.NotEmpty(t, out.LoadBalancerArn) @@ -276,14 +288,14 @@ func TestEnsureClusterNLB_FreshCreatesAllThree(t *testing.T) { func TestEnsureClusterNLB_IdempotentReusesExisting(t *testing.T) { nlbp := newFakeNLBProvisioner() - first, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}) + first, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) require.NoError(t, err) createLBCount := len(nlbp.createLBCalls) createTGCount := len(nlbp.createTGCalls) createListenerCount := len(nlbp.createListenerCalls) - second, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}) + second, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) require.NoError(t, err) assert.Equal(t, first.LoadBalancerArn, second.LoadBalancerArn) @@ -300,12 +312,113 @@ func TestEnsureClusterNLB_LBCreateErrorSurfaced(t *testing.T) { nlbp := newFakeNLBProvisioner() nlbp.createLBErr = errors.New("InsufficientCapacity") - _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}) + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) require.Error(t, err) assert.Contains(t, err.Error(), "create NLB eks-alpha") assert.Empty(t, nlbp.createTGCalls, "TG create should not run when LB create fails") } +func TestEnsureClusterNLB_InternetFacingSchemeAndPublicFrontendIP(t *testing.T) { + nlbp := newFakeNLBProvisioner() + // Internet-facing LB record exposes a public IpAddress in its AZ addresses. + arn := "arn:aws:elasticloadbalancing:us-east-1:111122223333:loadbalancer/net/eks-alpha/lb-001" + nlbp.createLBOut = &elbv2.CreateLoadBalancerOutput{ + LoadBalancers: []*elbv2.LoadBalancer{{ + LoadBalancerArn: aws.String(arn), + LoadBalancerName: aws.String("eks-alpha"), + DNSName: aws.String("eks-alpha-lb-001.us-east-1.elb.spinifex.local"), + Type: aws.String(elbv2.LoadBalancerTypeEnumNetwork), + Scheme: aws.String(elbv2.LoadBalancerSchemeEnumInternetFacing), + AvailabilityZones: []*elbv2.AvailabilityZone{{ + LoadBalancerAddresses: []*elbv2.LoadBalancerAddress{ + {PrivateIPv4Address: aws.String("10.0.1.5")}, + {IpAddress: aws.String("203.0.113.10")}, + }, + }}, + }}, + } + + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, nil) + require.NoError(t, err) + require.Len(t, nlbp.createLBCalls, 1) + assert.Equal(t, elbv2.LoadBalancerSchemeEnumInternetFacing, aws.StringValue(nlbp.createLBCalls[0].Scheme)) + assert.Equal(t, "203.0.113.10", out.FrontendIP, "internet-facing front-end uses the public IpAddress") +} + +func TestEnsureClusterNLB_InternalSchemeUsesPrivateFrontendIP(t *testing.T) { + nlbp := newFakeNLBProvisioner() + arn := "arn:aws:elasticloadbalancing:us-east-1:111122223333:loadbalancer/net/eks-alpha/lb-001" + nlbp.createLBOut = &elbv2.CreateLoadBalancerOutput{ + LoadBalancers: []*elbv2.LoadBalancer{{ + LoadBalancerArn: aws.String(arn), + LoadBalancerName: aws.String("eks-alpha"), + DNSName: aws.String("eks-alpha-lb-001.us-east-1.elb.spinifex.local"), + Type: aws.String(elbv2.LoadBalancerTypeEnumNetwork), + Scheme: aws.String(elbv2.LoadBalancerSchemeEnumInternal), + AvailabilityZones: []*elbv2.AvailabilityZone{{ + LoadBalancerAddresses: []*elbv2.LoadBalancerAddress{ + {PrivateIPv4Address: aws.String("10.0.1.5")}, + }, + }}, + }}, + } + + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) + require.NoError(t, err) + require.Len(t, nlbp.createLBCalls, 1) + assert.Equal(t, elbv2.LoadBalancerSchemeEnumInternal, aws.StringValue(nlbp.createLBCalls[0].Scheme)) + assert.Equal(t, "10.0.1.5", out.FrontendIP, "internal front-end uses the private IPv4 address") +} + +func TestEnsureClusterNLB_NarrowedPublicAccessSetsIngressCIDRs(t *testing.T) { + nlbp := newFakeNLBProvisioner() + + cidrs := []string{"203.0.113.0/24", "198.51.100.7/32"} + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, cidrs) + require.NoError(t, err) + + require.Len(t, nlbp.setIngressCalls, 1, "narrowed public access should drive SetLoadBalancerIngressCIDRs") + assert.Equal(t, out.LoadBalancerArn, nlbp.setIngressCalls[0].lbArn) + assert.Equal(t, cidrs, nlbp.setIngressCalls[0].cidrs) +} + +func TestEnsureClusterNLB_DefaultPublicAccessSkipsIngressCIDRs(t *testing.T) { + for _, tc := range []struct { + name string + cidrs []string + }{ + {"nil", nil}, + {"empty", []string{}}, + {"wide-open default", []string{defaultPublicAccessCidr}}, + } { + t.Run(tc.name, func(t *testing.T) { + nlbp := newFakeNLBProvisioner() + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, tc.cidrs) + require.NoError(t, err) + assert.Empty(t, nlbp.setIngressCalls, "wide-open front-end needs no ingress override") + }) + } +} + +func TestEnsureClusterNLB_InternalSkipsIngressCIDRs(t *testing.T) { + nlbp := newFakeNLBProvisioner() + + // Even with narrowed CIDRs, an internal NLB ignores them — its ingress + // already tracks the VPC CIDR, not a public front-end. + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, []string{"203.0.113.0/24"}) + require.NoError(t, err) + assert.Empty(t, nlbp.setIngressCalls) +} + +func TestEnsureClusterNLB_SetIngressErrorSurfaced(t *testing.T) { + nlbp := newFakeNLBProvisioner() + nlbp.setIngressErr = errors.New("InvalidLoadBalancer") + + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, []string{"203.0.113.0/24"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "set NLB ingress CIDRs for eks-alpha") +} + func TestRegisterClusterTarget_PostsENIIPAndAPIPort(t *testing.T) { nlbp := newFakeNLBProvisioner() @@ -343,7 +456,7 @@ func TestDeregisterClusterTarget_PostsENIIPAndAPIPort(t *testing.T) { func TestDeleteClusterNLB_DeletesBoth(t *testing.T) { nlbp := newFakeNLBProvisioner() - out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}) + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) require.NoError(t, err) require.NoError(t, DeleteClusterNLB(nlbp, "111122223333", "alpha")) @@ -363,7 +476,7 @@ func TestDeleteClusterNLB_MissingResourcesNoOp(t *testing.T) { func TestDeleteClusterNLB_FirstErrorSurfacedSweepContinues(t *testing.T) { nlbp := newFakeNLBProvisioner() - _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}) + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, false, nil) require.NoError(t, err) nlbp.deleteLBErr = errors.New("LoadBalancerInUse") diff --git a/spinifex/handlers/eks/security_groups.go b/spinifex/handlers/eks/security_groups.go index 126f00416..fd0f964a9 100644 --- a/spinifex/handlers/eks/security_groups.go +++ b/spinifex/handlers/eks/security_groups.go @@ -208,6 +208,40 @@ func EnsureNodegroupSGRules(sgp sgProvisioner, accountID, clusterName, cpSGID, n return nil } +// EnsureControlPlaneIngress authorizes the ingress the cluster NLB needs to +// reach the K3s apiserver. The NLB's backing LB VM forwards the front-end +// listener to the apiserver on k3sAPIServerPort from inside the VPC, so the +// control-plane SG must admit that port from the VPC CIDR. Public exposure is +// gated separately at the NLB front-end by publicAccessCidrs; this rule only +// governs the in-VPC LB→apiserver hop. Idempotent: the +// InvalidPermission.Duplicate error a re-run triggers is treated as success. +func EnsureControlPlaneIngress(sgp sgProvisioner, accountID, cpSGID, vpcCIDR string) error { + if cpSGID == "" { + return errors.New("eks: EnsureControlPlaneIngress empty control-plane SG id") + } + if vpcCIDR == "" { + return errors.New("eks: EnsureControlPlaneIngress empty vpc cidr") + } + + perm := &ec2.IpPermission{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int64(k3sAPIServerPort), + ToPort: aws.Int64(k3sAPIServerPort), + IpRanges: []*ec2.IpRange{{ + CidrIp: aws.String(vpcCIDR), + Description: aws.String("EKS NLB to apiserver"), + }}, + } + _, err := sgp.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(cpSGID), + IpPermissions: []*ec2.IpPermission{perm}, + }, accountID) + if err != nil && !awserrors.IsErrorCode(err, awserrors.ErrorInvalidPermissionDuplicate) { + return fmt.Errorf("authorize control-plane apiserver ingress on %s: %w", cpSGID, err) + } + return nil +} + func lookupSGByName(sgp sgProvisioner, accountID, vpcID, name string) (string, error) { out, err := sgp.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{ diff --git a/spinifex/handlers/eks/security_groups_test.go b/spinifex/handlers/eks/security_groups_test.go index 75b002f42..41ff851f2 100644 --- a/spinifex/handlers/eks/security_groups_test.go +++ b/spinifex/handlers/eks/security_groups_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/tags" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -190,6 +191,49 @@ func TestDeleteClusterSGs_FirstErrorSurfacedSweepContinues(t *testing.T) { assert.Len(t, sgp.deleteCalls, 2, "both delete calls should be attempted despite first error") } +func TestEnsureControlPlaneIngress_AuthorizesAPIServerFromVPCCIDR(t *testing.T) { + sgp := newFakeSGProvisioner() + + err := EnsureControlPlaneIngress(sgp, "111122223333", "sg-cp-001", "10.0.0.0/16") + require.NoError(t, err) + + require.Len(t, sgp.authorizeCalls, 1) + in := sgp.authorizeCalls[0] + assert.Equal(t, "sg-cp-001", aws.StringValue(in.GroupId)) + require.Len(t, in.IpPermissions, 1) + perm := in.IpPermissions[0] + assert.Equal(t, "tcp", aws.StringValue(perm.IpProtocol)) + assert.Equal(t, k3sAPIServerPort, aws.Int64Value(perm.FromPort)) + assert.Equal(t, k3sAPIServerPort, aws.Int64Value(perm.ToPort)) + require.Len(t, perm.IpRanges, 1) + assert.Equal(t, "10.0.0.0/16", aws.StringValue(perm.IpRanges[0].CidrIp)) +} + +func TestEnsureControlPlaneIngress_EmptyInputsRejected(t *testing.T) { + sgp := newFakeSGProvisioner() + + require.Error(t, EnsureControlPlaneIngress(sgp, "111122223333", "", "10.0.0.0/16")) + require.Error(t, EnsureControlPlaneIngress(sgp, "111122223333", "sg-cp-001", "")) + assert.Empty(t, sgp.authorizeCalls) +} + +func TestEnsureControlPlaneIngress_DuplicateTolerated(t *testing.T) { + sgp := newFakeSGProvisioner() + sgp.authorizeErr = errors.New(awserrors.ErrorInvalidPermissionDuplicate) + + err := EnsureControlPlaneIngress(sgp, "111122223333", "sg-cp-001", "10.0.0.0/16") + require.NoError(t, err, "a duplicate rule on re-run must be treated as success") +} + +func TestEnsureControlPlaneIngress_AuthorizeErrorSurfaced(t *testing.T) { + sgp := newFakeSGProvisioner() + sgp.authorizeErr = errors.New("vpcd unavailable") + + err := EnsureControlPlaneIngress(sgp, "111122223333", "sg-cp-001", "10.0.0.0/16") + require.Error(t, err) + assert.Contains(t, err.Error(), "sg-cp-001") +} + func assertSGCreateTagged(t *testing.T, in *ec2.CreateSecurityGroupInput, name, vpcID, clusterName, role string) { t.Helper() require.NotNil(t, in) diff --git a/spinifex/handlers/eks/service_impl.go b/spinifex/handlers/eks/service_impl.go index 44f90f86f..c36a62851 100644 --- a/spinifex/handlers/eks/service_impl.go +++ b/spinifex/handlers/eks/service_impl.go @@ -22,11 +22,13 @@ import ( "github.com/nats-io/nats.go" ) -// SubnetVPCResolver resolves a subnet ID to its parent VPC ID. Narrow so -// EKSServiceImpl can stay free of the wider handlers/ec2/vpc surface; the -// daemon adapts VPCServiceImpl.GetSubnet onto this contract. +// SubnetVPCResolver resolves a subnet ID to its parent VPC ID and the VPC's +// CIDR block. Narrow so EKSServiceImpl can stay free of the wider +// handlers/ec2/vpc surface; the daemon adapts VPCServiceImpl onto this +// contract. type SubnetVPCResolver interface { GetSubnetVPC(accountID, subnetID string) (vpcID string, err error) + GetVPCCIDR(accountID, vpcID string) (cidr string, err error) } // EKSServiceDeps wires every external collaborator EKSServiceImpl needs. @@ -339,6 +341,9 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, region := s.deps.Region arn := fmt.Sprintf("arn:aws:eks:%s:%s:cluster/%s", region, accountID, name) + publicAccess, privateAccess := endpointAccess(input.ResourcesVpcConfig) + publicCidrs := publicAccessCidrs(input.ResourcesVpcConfig, publicAccess) + meta := &ClusterMeta{ Name: name, Arn: arn, @@ -346,8 +351,11 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, Version: deref(input.Version, defaultK8sVersion), RoleArn: aws.StringValue(input.RoleArn), ResourcesVpcConfig: &ClusterVpcConfig{ - SubnetIds: subnetIDs, - VpcId: vpcID, + SubnetIds: subnetIDs, + VpcId: vpcID, + EndpointPublicAccess: publicAccess, + EndpointPrivateAccess: privateAccess, + PublicAccessCidrs: publicCidrs, }, BuiltinIngress: builtinIngressEnabled(input.Tags), CreatedAt: time.Now().UTC(), @@ -363,12 +371,35 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, } meta.ResourcesVpcConfig.SecurityGroupIds = []string{cpSG, ngSG} - nlb, err := EnsureClusterNLB(s.deps.NLB, accountID, name, subnetIDs) + // The NLB's backing LB VM forwards the published endpoint to the apiserver + // from inside the VPC, so the control-plane SG must admit that hop or the + // NLB target stays unhealthy and the endpoint never serves. + vpcCIDR, err := s.deps.VPCSubnet.GetVPCCIDR(accountID, vpcID) + if err != nil { + s.markFailed(acctKV, name) + return nil, logCreateErr(name, accountID, "resolve vpc cidr", err) + } + if err := EnsureControlPlaneIngress(s.deps.VPCSG, accountID, cpSG, vpcCIDR); err != nil { + s.markFailed(acctKV, name) + return nil, logCreateErr(name, accountID, "ensure control-plane ingress", err) + } + + // Public access ⇒ internet-facing NLB (external-pool front-end IP, reachable + // on the LAN/edge network); private-only ⇒ internal NLB (VPC-only). + nlb, err := EnsureClusterNLB(s.deps.NLB, accountID, name, subnetIDs, publicAccess, publicCidrs) if err != nil { s.markFailed(acctKV, name) return nil, logCreateErr(name, accountID, "ensure cluster NLB", err) } - meta.Endpoint = "https://" + net.JoinHostPort(nlb.DNSName, strconv.FormatInt(clusterNLBListenPort, 10)) + // Prefer the reachable front-end IP as the kubeconfig endpoint host so the + // apiserver serving cert (which SANs this IP) validates with TLS verification + // on. Fall back to the synthetic NLB DNS name when no IP was read back. + endpointHost := nlb.FrontendIP + if endpointHost == "" { + endpointHost = nlb.DNSName + } + meta.Endpoint = "https://" + net.JoinHostPort(endpointHost, strconv.FormatInt(clusterNLBListenPort, 10)) + meta.EndpointIP = nlb.FrontendIP meta.NLBArn = nlb.LoadBalancerArn meta.NLBTargetGroupArn = nlb.TargetGroupArn @@ -407,6 +438,7 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, SubnetID: subnetIDs[0], ControlPlaneSGID: cpSG, NLBDNS: nlb.DNSName, + EndpointIP: nlb.FrontendIP, OIDCIssuer: oidcIssuer, OIDCPrivateKeyPEM: privPEM, OIDCPublicKeyPEM: pubPEM, @@ -1156,9 +1188,49 @@ func validateCreateClusterInput(input *eks.CreateClusterInput) error { return errors.New(awserrors.ErrorInvalidParameter) } } + // AWS rejects disabling both public and private endpoint access — the + // control plane would be unreachable. + if v := input.ResourcesVpcConfig; v != nil && + v.EndpointPublicAccess != nil && !*v.EndpointPublicAccess && + v.EndpointPrivateAccess != nil && !*v.EndpointPrivateAccess { + return errors.New(awserrors.ErrorInvalidParameterValue) + } return nil } +// defaultPublicAccessCidr is the AWS-default allowed source range for a public +// cluster endpoint when the caller specifies none. +const defaultPublicAccessCidr = "0.0.0.0/0" + +// endpointAccess resolves apiserver endpoint exposure from the request, +// applying AWS defaults (public on, private off) to omitted fields. +func endpointAccess(vpc *eks.VpcConfigRequest) (public, private bool) { + public, private = true, false + if vpc == nil { + return public, private + } + if vpc.EndpointPublicAccess != nil { + public = *vpc.EndpointPublicAccess + } + if vpc.EndpointPrivateAccess != nil { + private = *vpc.EndpointPrivateAccess + } + return public, private +} + +// publicAccessCidrs returns the allowed source ranges for a public endpoint, +// defaulting to 0.0.0.0/0 (AWS parity) when public access is on and none given. +// Nil for a private-only endpoint. +func publicAccessCidrs(vpc *eks.VpcConfigRequest, public bool) []string { + if !public { + return nil + } + if vpc != nil && len(vpc.PublicAccessCidrs) > 0 { + return aws.StringValueSlice(vpc.PublicAccessCidrs) + } + return []string{defaultPublicAccessCidr} +} + func (s *EKSServiceImpl) markFailed(kv nats.KeyValue, name string) { if err := SetClusterStatus(kv, name, ClusterStatusFailed); err != nil { slog.Warn("CreateCluster: SetClusterStatus(FAILED) failed", "cluster", name, "err", err) @@ -1238,9 +1310,12 @@ func clusterMetaToAWS(meta *ClusterMeta) *eks.Cluster { } if meta.ResourcesVpcConfig != nil { out.ResourcesVpcConfig = &eks.VpcConfigResponse{ - SubnetIds: aws.StringSlice(meta.ResourcesVpcConfig.SubnetIds), - SecurityGroupIds: aws.StringSlice(meta.ResourcesVpcConfig.SecurityGroupIds), - VpcId: aws.String(meta.ResourcesVpcConfig.VpcId), + SubnetIds: aws.StringSlice(meta.ResourcesVpcConfig.SubnetIds), + SecurityGroupIds: aws.StringSlice(meta.ResourcesVpcConfig.SecurityGroupIds), + VpcId: aws.String(meta.ResourcesVpcConfig.VpcId), + EndpointPublicAccess: aws.Bool(meta.ResourcesVpcConfig.EndpointPublicAccess), + EndpointPrivateAccess: aws.Bool(meta.ResourcesVpcConfig.EndpointPrivateAccess), + PublicAccessCidrs: aws.StringSlice(meta.ResourcesVpcConfig.PublicAccessCidrs), } } if meta.HealthIssue != "" { diff --git a/tests/e2e/eks/eks_test.go b/tests/e2e/eks/eks_test.go index 375d53233..38e19d73a 100644 --- a/tests/e2e/eks/eks_test.go +++ b/tests/e2e/eks/eks_test.go @@ -122,13 +122,13 @@ func TestEKS(t *testing.T) { t.Run("KubectlGetNodes", func(t *testing.T) { requireClusterReady(t, fx) - // Reachability shim: the kubeconfig endpoint (NLB DNS) is unresolvable - // from here, so point kubectl at the control-plane VM's mgmt IP:6443 - // directly. The serving cert SANs the NLB DNS + node IPs, not the mgmt - // IP, so skip TLS verification — auth still flows through the get-token - // webhook, which is what this asserts. - mgmtIP := harness.ControlPlaneMgmtIP(t, env, fx.AccountID, fx.ClusterName) - kcPath := writeKubectlKubeconfig(t, artifacts, fx.Cluster, mgmtIP) + // Reach the apiserver through the published cluster endpoint (the + // internet-facing NLB front-end IP:443) with TLS verification ON — the + // serving cert SANs this IP, so no insecure-skip-tls-verify and no + // mgmt-IP shim. A 401 here means the get-token webhook chain regressed; + // a TLS error means the endpoint/cert-SAN wiring regressed. + require.NotEmpty(t, aws.StringValue(fx.Cluster.Endpoint), "cluster must publish a reachable endpoint") + kcPath := writeKubeconfig(t, artifacts, fx.Cluster) kc := harness.NewKubectl(t, kcPath, getTokenEnv(t, env)) // Auth + reachability: poll until the apiserver answers and reports a @@ -307,46 +307,6 @@ users: return path } -// writeKubectlKubeconfig assembles a kubeconfig pointed at the control-plane -// VM's reachable mgmt IP (Phase-2 shim) with TLS verification disabled, and an -// exec block that mints a bearer token via `aws eks get-token`. Distinct from -// writeKubeconfig (the Phase-1 artifact built from the real NLB endpoint). -func writeKubectlKubeconfig(t *testing.T, artifacts string, cl *eks.Cluster, mgmtIP string) string { - t.Helper() - name := aws.StringValue(cl.Name) - server := fmt.Sprintf("https://%s:6443", mgmtIP) - kc := fmt.Sprintf(`apiVersion: v1 -kind: Config -clusters: -- name: %[1]s - cluster: - server: %[2]s - insecure-skip-tls-verify: true -contexts: -- name: %[1]s - context: - cluster: %[1]s - user: %[1]s -current-context: %[1]s -users: -- name: %[1]s - user: - exec: - apiVersion: client.authentication.k8s.io/v1beta1 - command: aws - args: - - eks - - get-token - - --cluster-name - - %[1]s -`, name, server) - - path := filepath.Join(artifacts, "kubectl-"+name+".yaml") - require.NoError(t, os.WriteFile(path, []byte(kc), 0o600), "write kubectl kubeconfig") - t.Logf("kubectl kubeconfig: %s (server=%s)", path, server) - return path -} - // --- get-token ------------------------------------------------------------ // awsEKSGetToken shells the stock AWS CLI `aws eks get-token`, which presigns an