From ec2bb542e3858c737d358a15af24ae1bfd4c9bf9 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Mon, 25 May 2026 11:18:56 +1000 Subject: [PATCH] fix(e2e): authorize default SG tcp/22 + icmp in single-node fixture admin init leaves the default SG closed (AWS parity); the Go single suite never ported the Phase-5 SG-ingress block that run-e2e.sh has had since day one, so every test runner SSH/ping probe hits the OVN port-group ACL drop. - ec2helpers: add AuthorizeICMPIngress + EnsureDefaultSGOpen helpers. - single/main_test: open default SG once per process in pkgFixOnce.Do. - witness: extend ensureDefaultSGSSHIngress to authorize ICMP too. - run-e2e.sh: append matching ICMP authorize next to tcp/22 block. mulga-siv-134 --- tests/e2e/harness/ec2helpers.go | 34 ++++++++++++++++++++++++++++++ tests/e2e/harness/witness.go | 37 ++++++++++++++++++++++----------- tests/e2e/run-e2e.sh | 15 +++++++++++++ tests/e2e/single/main_test.go | 5 +++++ 4 files changed, 79 insertions(+), 12 deletions(-) diff --git a/tests/e2e/harness/ec2helpers.go b/tests/e2e/harness/ec2helpers.go index a999f3d22..9490c5162 100644 --- a/tests/e2e/harness/ec2helpers.go +++ b/tests/e2e/harness/ec2helpers.go @@ -226,6 +226,40 @@ func AuthorizeSSHIngress(t *testing.T, c *AWSClient, sgID string) { t.Fatalf("authorize-security-group-ingress tcp/22 on %s: %v", sgID, err) } +// AuthorizeICMPIngress idempotently authorizes ICMP (all types) ingress from +// 0.0.0.0/0 on sgID. Same Duplicate-tolerant contract as AuthorizeSSHIngress. +func AuthorizeICMPIngress(t *testing.T, c *AWSClient, sgID string) { + t.Helper() + _, err := c.EC2.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(sgID), + IpPermissions: []*ec2.IpPermission{{ + IpProtocol: aws.String("icmp"), + FromPort: aws.Int64(-1), + ToPort: aws.Int64(-1), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, + }}, + }) + if err == nil { + return + } + var aerr awserr.Error + if asErr(err, &aerr) && aerr.Code() == "InvalidPermission.Duplicate" { + return + } + t.Fatalf("authorize-security-group-ingress icmp on %s: %v", sgID, err) +} + +// EnsureDefaultSGOpen authorizes tcp/22 + ICMP on the default VPC's default +// SG. AWS default SGs only admit same-SG members; e2e probes come from the +// test runner's external IP, so without this every SSH/ping hits the OVN +// port-group ACL drop. Mirrors the Phase-5 block in run-e2e.sh. +func EnsureDefaultSGOpen(t *testing.T, c *AWSClient) { + t.Helper() + _, sgID, _ := DiscoverDefaultVPC(t, c) + AuthorizeSSHIngress(t, c, sgID) + AuthorizeICMPIngress(t, c, sgID) +} + // asErr is a thin errors.As that lets the helper stay testify-free. func asErr(err error, target *awserr.Error) bool { if a, ok := err.(awserr.Error); ok { diff --git a/tests/e2e/harness/witness.go b/tests/e2e/harness/witness.go index 23e702bfd..3abc2c489 100644 --- a/tests/e2e/harness/witness.go +++ b/tests/e2e/harness/witness.go @@ -255,11 +255,12 @@ func AssertProgressed(ctx context.Context, t *testing.T, v *WitnessVM) { // --- internals ------------------------------------------------------------ -// ensureDefaultSGSSHIngress authorises tcp/22 from 0.0.0.0/0 on the default -// security group via the structured IpPermissions form. The daemon currently -// drops the top-level shortcut form silently (mulga-siv-79), so without this -// the witness EIP has no inbound :22 and awaitBaseline times out. Idempotent: -// duplicate-rule errors from a prior run are tolerated. +// ensureDefaultSGSSHIngress authorises tcp/22 + ICMP from 0.0.0.0/0 on the +// default security group via the structured IpPermissions form. The daemon +// currently drops the top-level shortcut form silently (mulga-siv-79), so +// without this the witness EIP has no inbound :22 and awaitBaseline times +// out. ICMP is required for downstream gateway-ping probes (mulga-siv-134). +// Idempotent: duplicate-rule errors from a prior run are tolerated. func (w *Witness) ensureDefaultSGSSHIngress(ctx context.Context) error { sgs, err := w.ec2.DescribeSecurityGroupsWithContext(ctx, &ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{{ @@ -275,17 +276,29 @@ func (w *Witness) ensureDefaultSGSSHIngress(ctx context.Context) error { } groupID := aws.StringValue(sgs.SecurityGroups[0].GroupId) - _, err = w.ec2.AuthorizeSecurityGroupIngressWithContext(ctx, &ec2.AuthorizeSecurityGroupIngressInput{ - GroupId: aws.String(groupID), - IpPermissions: []*ec2.IpPermission{{ + rules := []*ec2.IpPermission{ + { IpProtocol: aws.String("tcp"), FromPort: aws.Int64(22), ToPort: aws.Int64(22), IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, - }}, - }) - if err != nil && !strings.Contains(err.Error(), "InvalidPermission.Duplicate") { - return fmt.Errorf("AuthorizeSecurityGroupIngress :22 on %s: %w", groupID, err) + }, + { + IpProtocol: aws.String("icmp"), + FromPort: aws.Int64(-1), + ToPort: aws.Int64(-1), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, + }, + } + for _, rule := range rules { + _, err = w.ec2.AuthorizeSecurityGroupIngressWithContext(ctx, &ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(groupID), + IpPermissions: []*ec2.IpPermission{rule}, + }) + if err != nil && !strings.Contains(err.Error(), "InvalidPermission.Duplicate") { + return fmt.Errorf("AuthorizeSecurityGroupIngress %s on %s: %w", + aws.StringValue(rule.IpProtocol), groupID, err) + } } return nil } diff --git a/tests/e2e/run-e2e.sh b/tests/e2e/run-e2e.sh index 817bc3ff7..27b8cee61 100755 --- a/tests/e2e/run-e2e.sh +++ b/tests/e2e/run-e2e.sh @@ -285,6 +285,21 @@ if [ $AUTH_EXIT -ne 0 ] && ! echo "$AUTH_OUTPUT" | grep -q 'InvalidPermission.Du fi echo " Default SG ingress: tcp/22 from 0.0.0.0/0" +# Phase 5f exercises ICMP egress through OVN; the reply path needs explicit +# ICMP ingress on the default SG even though SGs are stateful in AWS — keeps +# parity with the Go single-suite fixture (mulga-siv-134). +set +e +ICMP_OUTPUT=$(aws ec2 authorize-security-group-ingress \ + --group-id "$DEFAULT_SG_PHASE5" \ + --protocol icmp --port -1 --cidr 0.0.0.0/0 2>&1) +ICMP_EXIT=$? +set -e +if [ $ICMP_EXIT -ne 0 ] && ! echo "$ICMP_OUTPUT" | grep -q 'InvalidPermission.Duplicate'; then + echo "Failed to authorize ICMP ingress on default SG: $ICMP_OUTPUT" + exit 1 +fi +echo " Default SG ingress: icmp from 0.0.0.0/0" + # Launch a VM (run-instances) echo "Running: aws ec2 run-instances --image-id $AMI_ID --instance-type $INSTANCE_TYPE --key-name test-key-1" # Capture full output for debugging diff --git a/tests/e2e/single/main_test.go b/tests/e2e/single/main_test.go index af3fb71f2..0f703c04f 100644 --- a/tests/e2e/single/main_test.go +++ b/tests/e2e/single/main_test.go @@ -79,6 +79,11 @@ func requireSingleNodeFixture(t *testing.T) *Fixture { TmpDir: tmpDir, } fix.PoolMode = detectPoolMode(env) + // admin init leaves the default SG closed (AWS parity); the e2e suite + // drives SSH + ICMP probes from the test runner's external IP, so open + // both on the default VPC's default SG once per process. Idempotent — + // mirrors run-e2e.sh Phase 5 (mulga-siv-134). + harness.EnsureDefaultSGOpen(t, awsCli) pkgFix = fix }) if pkgFixErr != nil {