diff --git a/tests/e2e/single/baseline_test.go b/tests/e2e/single/baseline_test.go index a780146c4..ea97c49d1 100644 --- a/tests/e2e/single/baseline_test.go +++ b/tests/e2e/single/baseline_test.go @@ -303,37 +303,3 @@ func runNewVPCEgressBaseline(t *testing.T, fix *Fixture) { idOut := runSSH(t, tgt, "id") assert.Containsf(t, idOut, "ubuntu", "ssh id in fresh-VPC subnet\n%s", idOut) } - -// runSameSGComms verifies that two instances in the default SG can ICMP-ping -// each other (permitted by the default SG's self-reference rule) while a -// separate runner SG handles tcp/22 from outside. No default resource mutated. -func runSameSGComms(t *testing.T, fix *Fixture) { - harness.Phase(t, "Single — Baseline: same default-SG instances communicate") - - vpcID, defSGID, subnetID := harness.DiscoverDefaultVPC(t, fix.AWS) - instType, _ := needInstanceTypeArch(t, fix) - keyName, keyPath := needKeyPair(t, fix) - ami := needAMI(t, fix) - - runnerSG := harness.EnsureSG(t, fix.Harness, vpcID, "baseline-runnersg") - harness.AuthorizeSSHIngress(t, fix.AWS, runnerSG) - harness.Detail(t, "vpc", vpcID, "default_sg", defSGID, "runner_sg", runnerSG) - - srcID := launchBaselineInstance(t, fix, ami, instType, keyName, subnetID, []string{defSGID, runnerSG}) - dstID := launchBaselineInstance(t, fix, ami, instType, keyName, subnetID, []string{defSGID, runnerSG}) - - dstPriv := instancePrivateIP(t, fix, dstID) - - srcInst := harness.WaitForInstanceState(t, fix.AWS, srcID, "running") - host, port := harness.InstancePublicSSHHost(t, srcInst) - harness.Detail(t, "src", srcID, "dst", dstID, "dst_private_ip", dstPriv, "ssh_host", host) - waitForSSHHandshake(t, host, port, keyPath) - - tgt := harness.SSHTarget{User: "ubuntu", Host: host, Port: port, KeyPath: keyPath} - harness.Step(t, "ping %s (%s) from %s via default-SG self-ingress", dstID, dstPriv, srcID) - out, converged := pingConverged(tgt, dstPriv, 45*time.Second) - require.Truef(t, converged, - "intra-default-SG east-west %s -> %s never reached 0%% loss within 45s; "+ - "ARP/L2 datapath unreachable across the default-SG self-ingress\n%s", - srcID, dstID, out) -} diff --git a/tests/e2e/single/datapath_test.go b/tests/e2e/single/datapath_test.go index 861d9fbfb..140a5def1 100644 --- a/tests/e2e/single/datapath_test.go +++ b/tests/e2e/single/datapath_test.go @@ -26,24 +26,49 @@ systemd-run --unit=sge-http --description="Phase 8e HTTP server" \ /usr/bin/python3 -m http.server 8080 --bind 0.0.0.0 ` -// runSGToSGDatapath validates the closed loop from RunInstances -// (--security-group-ids) -> ENI -> vpcd -> OVN port group + ACL -> datapath -// drop. Creates two SGs (client / target) with an SG-to-SG ingress rule for -// tcp/8080, launches one VM in each, then exercises: +// sgDatapathRevokeRounds returns how many times the revoke/re-authorize +// ingress round-trip repeats against the same client/target pair, +// overridable via SPINIFEX_SGDATAPATH_REVOKE_ROUNDS. Defaults to 1 — a +// single cycle, matching the original standalone test's behaviour; bump it +// for a soak run that repeatedly flips the sg-to-sg ACL to catch propagation +// flakiness that a single cycle would not surface. +func sgDatapathRevokeRounds() int { + return envPositiveIntOr("SPINIFEX_SGDATAPATH_REVOKE_ROUNDS", 1) +} + +// runSGPolicyDatapath merges two SG-policy checks that used to boot their own +// instance pair apiece — SG-to-SG datapath enforcement and same-default-SG +// east-west connectivity — around one shared client/target pair, cutting +// four boots to two. // -// 1. ovn-nbctl port_group membership for both LSPs (catches Phase 4 -// membership regressions independently of packet timing). -// 2. ovn-sbctl address_set _ip4 contains the client's private IP -// (auto-derived by northd from port_group membership; lives in SB). -// 3. Allowed traffic — client -> target:8080 over private IP (must succeed). -// 4. Denied traffic — client -> target:22 (target SG has no SSH ingress). -// 5. Revoke target SG's tcp/8080 ingress -> traffic immediately blocked -// (sync vpc.update-sg RequestEvent contract, no propagation sleep). +// Stage order and gating: +// - Setup (create SGs, launch client+target) is an unconditional +// prerequisite for everything below it; any failure here is fatal in the +// ordinary Go-test sense (require), matching the original. +// - PortGroupMembership confirms OVN control-plane state (port_group + +// address_set) and waits for client SSH. Every later stage depends on a +// working SSH session, so its failure aborts the rest of the scenario +// rather than let four more stages time out for the same reason. +// - AllowedTraffic proves the sg-to-sg 8080 rule actually passes traffic. +// DeniedTraffic (client -> target:22) checks a different port and an +// unrelated rule, so it runs regardless of AllowedTraffic's outcome — a +// real, independent signal either way. But the revoke/restore round-trip +// below is only meaningful against a *proven* working baseline: if +// AllowedTraffic never actually worked, "traffic is now blocked after +// revoke" would trivially and misleadingly pass for the wrong reason. So +// AllowedTraffic gates the revoke rounds. +// - Each revoke round's restore half only depends on the revoke API call +// having actually removed the rule (tracked separately from the +// ICMP-style "verify blocked" assertion) — a flaky drop-detection +// shouldn't stop the restore half from being independently verified. +// - SameSGComms runs last and only *adds* the default SG to both ENIs via +// ModifyNetworkInterfaceAttribute; see its own comment for why the +// ordering there is load-bearing. // // OVN gated — skipped on dev laptops without ovn-nbctl/sudo. Maps to -// run-e2e.sh ~3188-3442. -func runSGToSGDatapath(t *testing.T, fix *Fixture) { - harness.Phase(t, "Single — Security Group SG-to-SG Datapath (OVN)") +// run-e2e.sh ~3188-3442 plus the former standalone same-default-SG check. +func runSGPolicyDatapath(t *testing.T, fix *Fixture) { + harness.Phase(t, "Single — Security Group Policy on a Real Datapath (OVN)") harness.SkipIfNoOVN(t) requireSSHHealthy(t) @@ -57,8 +82,10 @@ func runSGToSGDatapath(t *testing.T, fix *Fixture) { def := harness.EnsureDefaultVPC(t, fix.Harness) require.NotEmpty(t, def.VPCID, "default VPC ID required") require.NotEmpty(t, def.SubnetID, "default subnet ID required") + require.NotEmpty(t, def.SGID, "default SG ID required") + + // --- Setup: create client-sg + target-sg, launch client-vm + target-vm --- - // Step 1: Create client-sg and target-sg. harness.Step(t, "8e-1 create sge-client + sge-target security groups") clientSG := createSG(t, fix, def.VPCID, "sge-client", "Phase 8e client SG (SSH ingress from anywhere)") targetSG := createSG(t, fix, def.VPCID, "sge-target", "Phase 8e target SG (TCP/8080 ingress from client-sg only)") @@ -84,7 +111,7 @@ func runSGToSGDatapath(t *testing.T, fix *Fixture) { harness.Detail(t, "client_sg", clientSG, "target_sg", targetSG) // SSH ingress on client SG so the test runner can reach it. Target SG has - // no SSH ingress — verified later by Step 5. + // no SSH ingress — verified later by the denied-traffic stage. harness.AuthorizeSSHIngress(t, fix.AWS, clientSG) // SG-to-SG ingress via UserIdGroupPair (VPC-form; --source-group shorthand @@ -103,10 +130,10 @@ func runSGToSGDatapath(t *testing.T, fix *Fixture) { }) require.NoError(t, err, "authorize-security-group-ingress tcp/8080 from %s", clientSG) - // Step 2: Launch client-vm and target-vm in the default subnet so both get - // public IPs (MapPublicIpOnLaunch=true). target's HTTP server is started - // via cloud-init user-data — target-sg has no SSH ingress, so the test - // runner cannot ssh into it directly, only nested-ssh from client. + // Launch client-vm and target-vm in the default subnet so both get public + // IPs (MapPublicIpOnLaunch=true). target's HTTP server is started via + // cloud-init user-data — target-sg has no SSH ingress, so the test runner + // cannot ssh into it directly, only nested-ssh from client. harness.Step(t, "8e-2 launch client-vm + target-vm") clientID := runSGEInstance(t, fix, def.SubnetID, clientSG, "" /* no user-data */) @@ -145,138 +172,221 @@ func runSGToSGDatapath(t *testing.T, fix *Fixture) { "target_priv", targetPriv, "target_eni", targetENI, ) - // Step 3: ovn-nbctl port_group membership — confirm each LSP joined the - // expected port group. Spinifex maps `sg-XXXX` -> `sg_XXXX` because OVN - // port group names use [_a-zA-Z0-9] only. LSP name format is - // `port-` (run-e2e.sh:3299-3300). - harness.Step(t, "8e-3 ovn-nbctl port_group membership") - clientPG := strings.ReplaceAll(clientSG, "-", "_") - targetPG := strings.ReplaceAll(targetSG, "-", "_") - clientLSP := "port-" + clientENI - targetLSP := "port-" + targetENI - - // WaitForPortGroupMember polls NB until the LSP UUID appears in the - // port_group's ports column. Bounds the race between RunInstances - // returning and OVN flow install — northd propagation is normally - // sub-second but tests run on busy nodes. - harness.WaitForPortGroupMember(t, clientPG, clientLSP) - harness.WaitForPortGroupMember(t, targetPG, targetLSP) - harness.Detail(t, "client_pg", clientPG, "target_pg", targetPG) - - // Confirm client's private IP made it into the `_ip4` address set so - // target-sg's SG-to-SG match expression resolves. The _ip4 / _ip6 - // sets are auto-derived by ovn-northd from port_group membership and live - // in SB, not NB. northd is async so poll briefly to ride out the - // post-join propagation window. Mirrors run-e2e.sh:3334-3349. - harness.Step(t, "8e-3 ovn-sbctl address_set %s_ip4 contains %s", clientPG, clientPriv) - addrSetName := clientPG + "_ip4" - harness.EventuallyErr(t, func() error { - addrs := harness.OvnSbctl(t, "--no-leader-only", "--bare", "--columns=addresses", - "find", "address_set", "name="+addrSetName) - if strings.Contains(addrs, clientPriv) { - return nil - } - return fmt.Errorf("client private IP %s missing from address_set %s (addresses=%s)", - clientPriv, addrSetName, addrs) - }, 10*time.Second, 1*time.Second) - - // Wait for client SSH to become reachable. The test runner uses the - // public IP / hostfwd to reach client-vm; the nested SSH below is just - // `ssh 'curl :8080'`. Target-vm has no SSH ingress on - // its own SG so we never connect to it directly. clientHost, clientPort := harness.InstancePublicSSHHost(t, clientInst) clientTgt := harness.SSHTarget{User: "ubuntu", Host: clientHost, Port: clientPort, KeyPath: keyPath} - harness.Step(t, "8e-3 wait for client-vm SSH at %s:%d", clientHost, clientPort) - // Non-fatal probe so a timeout dumps the guest console + OVN/datapath - // state before Fatal. A full 2min unreachable window on a fresh public-IP - // VM is the flake signature; capture it from CI artifacts alone. - if !trySSHReady(clientHost, clientPort, keyPath, 2*time.Minute) { - harness.DumpVPCFlowDiagnostics(t, fix.AWS, clientID, - fmt.Sprintf("8e-3 client-vm SSH timeout — vpc=%s sg=%s pub=%s", def.VPCID, clientSG, clientHost), - harness.VPCDiagnosticsOpts{ - ExternalIP: clientHost, - LogicalIP: clientPriv, - ArtifactDir: fix.ArtifactDir(t), - }) - t.Fatalf("client-vm SSH %s:%d never became reachable within 2min (see diagnostics above)", clientHost, clientPort) - } - - // Step 4: Allowed traffic — client -> target:8080 must succeed. - // Retry to give target's cloud-init time to start python3 -m http.server. - // Bash uses up to 30 attempts at 2s — keep the same outer budget. - harness.Step(t, "8e-4 allowed traffic client -> target:%s:8080", targetPriv) curlCmd := fmt.Sprintf("curl -sS -o /dev/null -m 5 http://%s:8080/", targetPriv) - harness.EventuallyErr(t, func() error { - out, err := runSSHCombined(clientTgt, curlCmd) - if err != nil { - return fmt.Errorf("client -> target:8080 failed: %w (out=%q)", err, out) + + // --- PortGroupMembership: OVN control-plane state + client SSH --- + + portGroupOK := t.Run("PortGroupMembership", func(t *testing.T) { + // ovn-nbctl port_group membership — confirm each LSP joined the + // expected port group. Spinifex maps `sg-XXXX` -> `sg_XXXX` because OVN + // port group names use [_a-zA-Z0-9] only. LSP name format is + // `port-` (run-e2e.sh:3299-3300). + harness.Step(t, "8e-3 ovn-nbctl port_group membership") + clientPG := strings.ReplaceAll(clientSG, "-", "_") + targetPG := strings.ReplaceAll(targetSG, "-", "_") + clientLSP := "port-" + clientENI + targetLSP := "port-" + targetENI + + // WaitForPortGroupMember polls NB until the LSP UUID appears in the + // port_group's ports column. Bounds the race between RunInstances + // returning and OVN flow install — northd propagation is normally + // sub-second but tests run on busy nodes. + harness.WaitForPortGroupMember(t, clientPG, clientLSP) + harness.WaitForPortGroupMember(t, targetPG, targetLSP) + harness.Detail(t, "client_pg", clientPG, "target_pg", targetPG) + + // Confirm client's private IP made it into the `_ip4` address set so + // target-sg's SG-to-SG match expression resolves. The _ip4 / _ip6 + // sets are auto-derived by ovn-northd from port_group membership and live + // in SB, not NB. northd is async so poll briefly to ride out the + // post-join propagation window. Mirrors run-e2e.sh:3334-3349. + harness.Step(t, "8e-3 ovn-sbctl address_set %s_ip4 contains %s", clientPG, clientPriv) + addrSetName := clientPG + "_ip4" + harness.EventuallyErr(t, func() error { + addrs := harness.OvnSbctl(t, "--no-leader-only", "--bare", "--columns=addresses", + "find", "address_set", "name="+addrSetName) + if strings.Contains(addrs, clientPriv) { + return nil + } + return fmt.Errorf("client private IP %s missing from address_set %s (addresses=%s)", + clientPriv, addrSetName, addrs) + }, 10*time.Second, 1*time.Second) + + // Wait for client SSH to become reachable. The test runner uses the + // public IP / hostfwd to reach client-vm; the nested SSH below is just + // `ssh 'curl :8080'`. Target-vm has no SSH ingress on + // its own SG so we never connect to it directly. + harness.Step(t, "8e-3 wait for client-vm SSH at %s:%d", clientHost, clientPort) + // Non-fatal probe so a timeout dumps the guest console + OVN/datapath + // state before Fatal. A full 2min unreachable window on a fresh public-IP + // VM is the flake signature; capture it from CI artifacts alone. + if !trySSHReady(clientHost, clientPort, keyPath, 2*time.Minute) { + harness.DumpVPCFlowDiagnostics(t, fix.AWS, clientID, + fmt.Sprintf("8e-3 client-vm SSH timeout — vpc=%s sg=%s pub=%s", def.VPCID, clientSG, clientHost), + harness.VPCDiagnosticsOpts{ + ExternalIP: clientHost, + LogicalIP: clientPriv, + ArtifactDir: fix.ArtifactDir(t), + }) + t.Fatalf("client-vm SSH %s:%d never became reachable within 2min (see diagnostics above)", clientHost, clientPort) } - return nil - }, 60*time.Second, 2*time.Second) - harness.Detail(t, "step4", "allowed_traffic_ok") - - // Step 5: Denied traffic — client -> target:22 must fail. target-sg has - // no SSH ingress; default-deny ACL must drop. `nc -z -w 5` exits 0 on - // connect, non-zero on timeout/refuse — we expect non-zero. - harness.Step(t, "8e-5 denied traffic client -> target:22 (no SSH ingress)") - if _, err := runSSHCombined(clientTgt, fmt.Sprintf("nc -z -w 5 %s 22", targetPriv)); err == nil { - t.Fatalf("FAIL: client reached target:22 — default-deny ACL not enforced") - } - harness.Detail(t, "step5", "denied_traffic_ok") - - // Step 6: Revoke target-sg's 8080 rule and retest immediately. The - // synchronous vpc.update-sg RequestEvent contract makes propagation to - // OVN immediate — no propagation sleep needed. Mirrors run-e2e.sh:3408. - harness.Step(t, "8e-6 revoke target-sg ingress, retest (sync RequestEvent contract)") - _, err = fix.AWS.EC2.RevokeSecurityGroupIngress(&ec2.RevokeSecurityGroupIngressInput{ - GroupId: aws.String(targetSG), - IpPermissions: []*ec2.IpPermission{{ - IpProtocol: aws.String("tcp"), - FromPort: aws.Int64(8080), - ToPort: aws.Int64(8080), - UserIdGroupPairs: []*ec2.UserIdGroupPair{{ - GroupId: aws.String(clientSG), - }}, - }}, }) - require.NoError(t, err, "revoke-security-group-ingress tcp/8080 from %s", clientSG) - - // Fresh TCP connection — conntrack does not affect new connections. - // Bash treats a single curl success as failure; allow a tight poll window - // to detect lingering reachability without flaking on the first attempt - // catching an in-flight flow flush. - harness.Step(t, "8e-6 verify client -> target:8080 now blocked") - if _, err := runSSHCombined(clientTgt, curlCmd); err == nil { - t.Fatalf("FAIL: client still reached target:8080 after revoke — propagation not immediate") + if !portGroupOK { + t.Fatalf("PortGroupMembership stage failed; skipping every later stage that depends on client SSH") } - harness.Detail(t, "step6", "revoke_blocked_ok") - // Step 7: Re-authorize, confirm traffic restored. Goes beyond the bash - // Phase 8e (bash stops at revoke) to round-trip the ACL state machine - // and prove the rule lifecycle is symmetric. Without this step a one-way - // regression (revoke works, re-add silently no-ops) would slip past. - harness.Step(t, "8e-7 re-authorize target-sg ingress, verify traffic restored") - _, err = fix.AWS.EC2.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ - GroupId: aws.String(targetSG), - IpPermissions: []*ec2.IpPermission{{ - IpProtocol: aws.String("tcp"), - FromPort: aws.Int64(8080), - ToPort: aws.Int64(8080), - UserIdGroupPairs: []*ec2.UserIdGroupPair{{ - GroupId: aws.String(clientSG), - }}, - }}, + // --- AllowedTraffic: client -> target:8080 must succeed --- + + allowedOK := t.Run("AllowedTraffic", func(t *testing.T) { + // Retry to give target's cloud-init time to start python3 -m http.server. + // Bash uses up to 30 attempts at 2s — keep the same outer budget. + harness.Step(t, "8e-4 allowed traffic client -> target:%s:8080", targetPriv) + harness.EventuallyErr(t, func() error { + out, err := runSSHCombined(clientTgt, curlCmd) + if err != nil { + return fmt.Errorf("client -> target:8080 failed: %w (out=%q)", err, out) + } + return nil + }, 60*time.Second, 2*time.Second) + harness.Detail(t, "step4", "allowed_traffic_ok") }) - require.NoError(t, err, "re-authorize tcp/8080 from %s", clientSG) - harness.EventuallyErr(t, func() error { - out, err := runSSHCombined(clientTgt, curlCmd) - if err != nil { - return fmt.Errorf("client -> target:8080 still blocked after re-add: %w (out=%q)", err, out) + // --- DeniedTraffic: client -> target:22 must fail --- + // + // Independent of AllowedTraffic: a different port and an unrelated rule + // (target-sg has no SSH ingress at all), so it runs and reports on its own + // merits even if the 8080 path above failed. + t.Run("DeniedTraffic", func(t *testing.T) { + harness.Step(t, "8e-5 denied traffic client -> target:22 (no SSH ingress)") + if _, err := runSSHCombined(clientTgt, fmt.Sprintf("nc -z -w 5 %s 22", targetPriv)); err == nil { + t.Fatalf("FAIL: client reached target:22 — default-deny ACL not enforced") } - return nil - }, 30*time.Second, 1*time.Second) - harness.Detail(t, "step7", "restore_ok") + harness.Detail(t, "step5", "denied_traffic_ok") + }) + + // --- Revoke / re-authorize round-trip --- + // + // Only meaningful against a proven-working baseline: if AllowedTraffic + // never actually got through, "blocked after revoke" would trivially and + // misleadingly pass for the wrong reason. + if !allowedOK { + t.Fatalf("AllowedTraffic never succeeded; skipping the revoke/restore round-trip since it would have nothing to detect a change from") + } + + rounds := sgDatapathRevokeRounds() + for round := 1; round <= rounds; round++ { + t.Run(fmt.Sprintf("Round%d", round), func(t *testing.T) { + // The synchronous vpc.update-sg RequestEvent contract makes + // propagation to OVN immediate — no propagation sleep needed. + // Mirrors run-e2e.sh:3408. + revokeMutationOK := true + revokeOK := t.Run("RevokeAndVerifyBlocked", func(t *testing.T) { + harness.Step(t, "8e-6 revoke target-sg ingress, retest (sync RequestEvent contract)") + _, err := fix.AWS.EC2.RevokeSecurityGroupIngress(&ec2.RevokeSecurityGroupIngressInput{ + GroupId: aws.String(targetSG), + IpPermissions: []*ec2.IpPermission{{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int64(8080), + ToPort: aws.Int64(8080), + UserIdGroupPairs: []*ec2.UserIdGroupPair{{ + GroupId: aws.String(clientSG), + }}, + }}, + }) + if err != nil { + revokeMutationOK = false + } + require.NoError(t, err, "revoke-security-group-ingress tcp/8080 from %s", clientSG) + + // Fresh TCP connection — conntrack does not affect new + // connections. Bash treats a single curl success as failure. + harness.Step(t, "8e-6 verify client -> target:8080 now blocked") + if _, err := runSSHCombined(clientTgt, curlCmd); err == nil { + t.Fatalf("FAIL: client still reached target:8080 after revoke — propagation not immediate") + } + harness.Detail(t, "step6", "revoke_blocked_ok") + }) + _ = revokeOK + + if !revokeMutationOK { + t.Fatalf("revoke-security-group-ingress mutation failed; skipping re-authorize since the rule was never removed") + } + + // Goes beyond the original bash Phase 8e (which stops at revoke) + // to round-trip the ACL state machine and prove the rule lifecycle + // is symmetric. Without this a one-way regression (revoke works, + // re-add silently no-ops) would slip past. + t.Run("ReauthorizeAndVerifyRestored", func(t *testing.T) { + harness.Step(t, "8e-7 re-authorize target-sg ingress, verify traffic restored") + _, err := fix.AWS.EC2.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(targetSG), + IpPermissions: []*ec2.IpPermission{{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int64(8080), + ToPort: aws.Int64(8080), + UserIdGroupPairs: []*ec2.UserIdGroupPair{{ + GroupId: aws.String(clientSG), + }}, + }}, + }) + require.NoError(t, err, "re-authorize tcp/8080 from %s", clientSG) + + harness.EventuallyErr(t, func() error { + out, err := runSSHCombined(clientTgt, curlCmd) + if err != nil { + return fmt.Errorf("client -> target:8080 still blocked after re-add: %w (out=%q)", err, out) + } + return nil + }, 30*time.Second, 1*time.Second) + harness.Detail(t, "step7", "restore_ok") + }) + }) + } + + // --- SameSGComms: east-west over the default SG's self-reference rule --- + + t.Run("SameSGComms", func(t *testing.T) { + // This stage only ever ADDS the default SG to each ENI, and runs last, + // after every SG-to-SG allow/deny assertion above has already + // completed and recorded its own result. The default SG's ingress + // rule is `-1/-1` from same-group members (see + // createDefaultSecurityGroupInternal) — i.e. it admits ALL traffic + // between co-members, not just ICMP. Joining it any earlier would + // silently defeat the DeniedTraffic assertion above (client and + // target would both gain a blanket allow via default-SG + // co-membership, independent of target-sg's rules), so the ordering + // here is load-bearing, not incidental. + harness.Step(t, "join client-vm + target-vm to the default SG for same-SG comms") + addSecurityGroup(t, fix, clientENI, []string{clientSG}, def.SGID) + addSecurityGroup(t, fix, targetENI, []string{targetSG}, def.SGID) + + harness.Step(t, "ping %s (%s) from %s via default-SG self-ingress", targetID, targetPriv, clientID) + out, converged := pingConverged(clientTgt, targetPriv, 45*time.Second) + require.Truef(t, converged, + "intra-default-SG east-west %s -> %s never reached 0%% loss within 45s; "+ + "ARP/L2 datapath unreachable across the default-SG self-ingress\n%s", + clientID, targetID, out) + }) +} + +// addSecurityGroup replaces eniID's security-group list with currentSGIDs +// plus addSGID. ModifyNetworkInterfaceAttribute's Groups field is a full +// replacement, not an additive patch (matches the real EC2 API), so the +// caller's existing membership must be restated alongside the new one. +func addSecurityGroup(t *testing.T, fix *Fixture, eniID string, currentSGIDs []string, addSGID string) { + t.Helper() + groups := make([]*string, 0, len(currentSGIDs)+1) + for _, id := range currentSGIDs { + groups = append(groups, aws.String(id)) + } + groups = append(groups, aws.String(addSGID)) + _, err := fix.AWS.EC2.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String(eniID), + Groups: groups, + }) + require.NoError(t, err, "modify-network-interface-attribute %s: add SG %s", eniID, addSGID) } // createSG creates a security group in the default VPC and returns its ID. diff --git a/tests/e2e/single/helpers_test.go b/tests/e2e/single/helpers_test.go index e4d86333f..71666bf1e 100644 --- a/tests/e2e/single/helpers_test.go +++ b/tests/e2e/single/helpers_test.go @@ -5,6 +5,9 @@ package single import ( "crypto/rand" "crypto/rsa" + "fmt" + "os" + "strconv" "testing" "github.com/aws/aws-sdk-go/aws" @@ -115,6 +118,23 @@ func needInstance(t *testing.T, fix *Fixture) (inst *ec2.Instance, rootVolumeID return inst, rootVolumeID } +// envPositiveIntOr returns the named environment variable parsed as a +// positive int, or def when unset. Mirrors storagegrowth's helper of the +// same shape: a malformed or non-positive value is a configuration error and +// panics rather than silently falling back, so a mistyped round count can't +// quietly run the wrong number of rounds and get attributed to the default. +func envPositiveIntOr(key string, def int) int { + v := os.Getenv(key) + if v == "" { + return def + } + n, err := strconv.Atoi(v) + if err != nil || n < 1 { + panic(fmt.Sprintf("%s=%q is not a positive integer", key, v)) + } + return n +} + // generateImportPubKey returns an OpenSSH-formatted public key (matching the // `ssh-keygen -t rsa` output the bash script feeds into import-key-pair). // 2048-bit to match the bash key length. diff --git a/tests/e2e/single/tests_test.go b/tests/e2e/single/tests_test.go index ea13fce83..ea615a59a 100644 --- a/tests/e2e/single/tests_test.go +++ b/tests/e2e/single/tests_test.go @@ -43,8 +43,9 @@ func TestClusterStatsCLI(t *testing.T) { runClusterStatsCLI(t, requireSingleNodeFixture(t)) } -// TestDefaultSGReachabilityBaseline, TestNewVPCEgressBaseline, and TestSameSGComms -// own all mutable resources and run before the singleton launch. +// TestDefaultSGReachabilityBaseline, TestNewVPCEgressBaseline, and +// TestGuestDNSResolution own all mutable resources and run before the +// singleton launch. func TestDefaultSGReachabilityBaseline(t *testing.T) { runDefaultSGReachabilityBaseline(t, requireSingleNodeFixture(t)) } @@ -53,10 +54,6 @@ func TestNewVPCEgressBaseline(t *testing.T) { runNewVPCEgressBaseline(t, requireSingleNodeFixture(t)) } -func TestSameSGComms(t *testing.T) { - runSameSGComms(t, requireSingleNodeFixture(t)) -} - func TestGuestDNSResolution(t *testing.T) { runGuestDNSResolution(t, requireSingleNodeFixture(t)) } @@ -154,6 +151,9 @@ func TestInstanceEIP(t *testing.T) { runInstanceEIP(t, requireSingleNodeFixture(t)) } -func TestSGToSGDatapath(t *testing.T) { - runSGToSGDatapath(t, requireSingleNodeFixture(t)) +// TestSGPolicyDatapath merges the former TestSameSGComms and +// TestSGToSGDatapath around one shared client/target pair (see +// runSGPolicyDatapath for the stage breakdown and gating rationale). +func TestSGPolicyDatapath(t *testing.T) { + runSGPolicyDatapath(t, requireSingleNodeFixture(t)) }