From 283cb77b586f6752659d6079155d22bae11c2387 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 01:26:16 +0000 Subject: [PATCH 1/6] Add networking validation functions - Add ValidateInboundPortRestriction to ensure only SSH port is accessible - Add ValidateEastWestConnectivity to test inter-instance communication - Follow existing validation patterns from instance.go and image.go - Use SSH client for remote port scanning and connectivity testing - Refactor into helper functions to meet linter requirements Co-Authored-By: Alec Fong --- pkg/v1/networking.go | 184 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 183 insertions(+), 1 deletion(-) diff --git a/pkg/v1/networking.go b/pkg/v1/networking.go index 5b03e1e..41f1673 100644 --- a/pkg/v1/networking.go +++ b/pkg/v1/networking.go @@ -1,6 +1,12 @@ package v1 -import "context" +import ( + "context" + "fmt" + + "github.com/brevdev/cloud/pkg/ssh" + "github.com/google/uuid" +) type CloudModifyFirewall interface { AddFirewallRulesToInstance(ctx context.Context, args AddFirewallRulesToInstanceArgs) error @@ -33,3 +39,179 @@ type PortMapping struct { FromPort int ToPort int } + +func ValidateInboundPortRestriction(ctx context.Context, client CloudInstanceReader, instance *Instance, privateKey string) error { + var err error + instance, err = WaitForInstanceLifecycleStatus(ctx, client, instance, LifecycleStatusRunning, PendingToRunningTimeout) + if err != nil { + return err + } + + if instance.SSHUser == "" { + return fmt.Errorf("SSH user is not set for instance %s", instance.CloudID) + } + if instance.SSHPort == 0 { + return fmt.Errorf("SSH port is not set for instance %s", instance.CloudID) + } + if instance.PublicIP == "" { + return fmt.Errorf("public IP is not available for instance %s", instance.CloudID) + } + + sshClient, err := ssh.ConnectToHost(ctx, ssh.ConnectionConfig{ + User: instance.SSHUser, + HostPort: fmt.Sprintf("%s:%d", instance.PublicIP, instance.SSHPort), + PrivKey: privateKey, + }) + if err != nil { + return fmt.Errorf("failed to connect to instance via SSH: %w", err) + } + defer func() { + if closeErr := sshClient.Close(); closeErr != nil { + fmt.Printf("warning: failed to close SSH connection: %v\n", closeErr) + } + }() + + portsToCheck := []int{21, 23, 25, 53, 80, 443, 993, 995, 3389, 5432, 3306} + + for _, port := range portsToCheck { + cmd := fmt.Sprintf("timeout 5 nc -z %s %d", instance.PublicIP, port) + stdout, stderr, err := sshClient.RunCommand(ctx, cmd) + + if err == nil { + return fmt.Errorf("security violation: port %d is accessible from external sources, stdout: %s, stderr: %s", port, stdout, stderr) + } + + fmt.Printf("Port %d properly blocked (expected): %s\n", port, stderr) + } + + cmd := fmt.Sprintf("timeout 5 nc -z %s %d", instance.PublicIP, instance.SSHPort) + stdout, stderr, err := sshClient.RunCommand(ctx, cmd) + if err != nil { + return fmt.Errorf("SSH port %d should be accessible but is not: %w, stdout: %s, stderr: %s", instance.SSHPort, err, stdout, stderr) + } + + fmt.Printf("Inbound port validation passed: only SSH port %d is accessible\n", instance.SSHPort) + return nil +} + +func ValidateEastWestConnectivity(ctx context.Context, client CloudCreateTerminateInstance, attrs CreateInstanceAttrs, privateKey string) error { + instance1, instance2, err := createTestInstances(ctx, client, attrs) + if err != nil { + return err + } + + defer cleanupInstances(ctx, client, instance1, instance2) + + err = waitForInstancesReady(ctx, client, instance1, instance2, privateKey) + if err != nil { + return err + } + + return testConnectivity(ctx, instance1, instance2, privateKey) +} + +func createTestInstances(ctx context.Context, client CloudCreateTerminateInstance, attrs CreateInstanceAttrs) (*Instance, *Instance, error) { + attrs1 := attrs + attrs1.RefID = uuid.New().String() + attrs1.Name = fmt.Sprintf("%s-east", attrs.Name) + + instance1, err := client.CreateInstance(ctx, attrs1) + if err != nil { + return nil, nil, fmt.Errorf("failed to create first instance: %w", err) + } + + attrs2 := attrs + attrs2.RefID = uuid.New().String() + attrs2.Name = fmt.Sprintf("%s-west", attrs.Name) + + instance2, err := client.CreateInstance(ctx, attrs2) + if err != nil { + return instance1, nil, fmt.Errorf("failed to create second instance: %w", err) + } + + return instance1, instance2, nil +} + +func cleanupInstances(ctx context.Context, client CloudCreateTerminateInstance, instance1, instance2 *Instance) { + if instance1 != nil { + if termErr := client.TerminateInstance(ctx, instance1.CloudID); termErr != nil { + fmt.Printf("warning: failed to terminate first instance %s: %v\n", instance1.CloudID, termErr) + } + } + if instance2 != nil { + if termErr := client.TerminateInstance(ctx, instance2.CloudID); termErr != nil { + fmt.Printf("warning: failed to terminate second instance %s: %v\n", instance2.CloudID, termErr) + } + } +} + +func waitForInstancesReady(ctx context.Context, client CloudCreateTerminateInstance, instance1, instance2 *Instance, privateKey string) error { + var err error + instance1, err = WaitForInstanceLifecycleStatus(ctx, client, instance1, LifecycleStatusRunning, PendingToRunningTimeout) + if err != nil { + return fmt.Errorf("first instance failed to reach running state: %w", err) + } + + instance2, err = WaitForInstanceLifecycleStatus(ctx, client, instance2, LifecycleStatusRunning, PendingToRunningTimeout) + if err != nil { + return fmt.Errorf("second instance failed to reach running state: %w", err) + } + + err = ssh.WaitForSSH(ctx, ssh.ConnectionConfig{ + User: instance1.SSHUser, + HostPort: fmt.Sprintf("%s:%d", instance1.PublicIP, instance1.SSHPort), + PrivKey: privateKey, + }, ssh.WaitForSSHOptions{ + Timeout: RunningSSHTimeout, + }) + if err != nil { + return fmt.Errorf("SSH not available on first instance: %w", err) + } + + err = ssh.WaitForSSH(ctx, ssh.ConnectionConfig{ + User: instance2.SSHUser, + HostPort: fmt.Sprintf("%s:%d", instance2.PublicIP, instance2.SSHPort), + PrivKey: privateKey, + }, ssh.WaitForSSHOptions{ + Timeout: RunningSSHTimeout, + }) + if err != nil { + return fmt.Errorf("SSH not available on second instance: %w", err) + } + + return nil +} + +func testConnectivity(ctx context.Context, instance1, instance2 *Instance, privateKey string) error { + sshClient1, err := ssh.ConnectToHost(ctx, ssh.ConnectionConfig{ + User: instance1.SSHUser, + HostPort: fmt.Sprintf("%s:%d", instance1.PublicIP, instance1.SSHPort), + PrivKey: privateKey, + }) + if err != nil { + return fmt.Errorf("failed to connect to first instance via SSH: %w", err) + } + defer func() { + if closeErr := sshClient1.Close(); closeErr != nil { + fmt.Printf("warning: failed to close SSH connection to first instance: %v\n", closeErr) + } + }() + + pingCmd := fmt.Sprintf("ping -c 3 -W 5 %s", instance2.PrivateIP) + stdout, stderr, err := sshClient1.RunCommand(ctx, pingCmd) + if err != nil { + return fmt.Errorf("ping from instance1 to instance2 failed: %w, stdout: %s, stderr: %s", err, stdout, stderr) + } + + sshTestCmd := fmt.Sprintf("timeout 10 nc -z %s %d", instance2.PrivateIP, instance2.SSHPort) + stdout, stderr, err = sshClient1.RunCommand(ctx, sshTestCmd) + if err != nil { + fmt.Printf("SSH port connectivity test between instances failed (may be expected): %s, stderr: %s\n", stdout, stderr) + } else { + fmt.Printf("SSH port connectivity between instances successful: %s\n", stdout) + } + + fmt.Printf("East-west connectivity validation passed: instance1 (%s) can communicate with instance2 (%s)\n", + instance1.CloudID, instance2.CloudID) + return nil +} From db5a8c0d5ff603d759b8f18312b9e11762fcab22 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 01:36:18 +0000 Subject: [PATCH 2/6] Wire up networking validation functions in validation suite - Add ValidateInboundPortRestriction test after ValidateInstanceImage - Add ValidateEastWestConnectivity test before ValidateTerminateInstance - Follow existing validation suite patterns and error handling Co-Authored-By: Alec Fong --- internal/validation/suite.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/validation/suite.go b/internal/validation/suite.go index ee9b9da..33c7db1 100644 --- a/internal/validation/suite.go +++ b/internal/validation/suite.go @@ -113,6 +113,11 @@ func RunInstanceLifecycleValidation(t *testing.T, config ProviderConfig) { require.NoError(t, err, "ValidateInstanceImage should pass") }) + t.Run("ValidateInboundPortRestriction", func(t *testing.T) { + err := v1.ValidateInboundPortRestriction(ctx, client, instance, ssh.GetTestPrivateKey()) + require.NoError(t, err, "ValidateInboundPortRestriction should pass") + }) + if capabilities.IsCapable(v1.CapabilityStopStartInstance) && instance.Stoppable { t.Run("ValidateStopStartInstance", func(t *testing.T) { err := v1.ValidateStopStartInstance(ctx, client, instance) @@ -120,6 +125,17 @@ func RunInstanceLifecycleValidation(t *testing.T, config ProviderConfig) { }) } + t.Run("ValidateEastWestConnectivity", func(t *testing.T) { + attrs := v1.CreateInstanceAttrs{ + InstanceType: instance.InstanceType, + Location: instance.Location, + PublicKey: ssh.GetTestPublicKey(), + Name: "test-connectivity", + } + err := v1.ValidateEastWestConnectivity(ctx, client, attrs, ssh.GetTestPrivateKey()) + require.NoError(t, err, "ValidateEastWestConnectivity should pass") + }) + t.Run("ValidateTerminateInstance", func(t *testing.T) { err := v1.ValidateTerminateInstance(ctx, client, instance) require.NoError(t, err, "ValidateTerminateInstance should pass") From 4a7a340657d35f437c222361e9a6941a23b2ea3f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 02:00:52 +0000 Subject: [PATCH 3/6] Fix SSH authentication in ValidateEastWestConnectivity - Use makeDebuggableName() for test instance creation like ValidateCreateInstance - Follow exact same pattern as working instance creation functions - Ensures proper SSH key setup and unique naming for test instances Co-Authored-By: Alec Fong --- pkg/v1/networking.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/v1/networking.go b/pkg/v1/networking.go index 41f1673..1081956 100644 --- a/pkg/v1/networking.go +++ b/pkg/v1/networking.go @@ -113,7 +113,11 @@ func ValidateEastWestConnectivity(ctx context.Context, client CloudCreateTermina func createTestInstances(ctx context.Context, client CloudCreateTerminateInstance, attrs CreateInstanceAttrs) (*Instance, *Instance, error) { attrs1 := attrs attrs1.RefID = uuid.New().String() - attrs1.Name = fmt.Sprintf("%s-east", attrs.Name) + name1, err := makeDebuggableName(fmt.Sprintf("%s-east", attrs.Name)) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate debuggable name for first instance: %w", err) + } + attrs1.Name = name1 instance1, err := client.CreateInstance(ctx, attrs1) if err != nil { @@ -122,7 +126,11 @@ func createTestInstances(ctx context.Context, client CloudCreateTerminateInstanc attrs2 := attrs attrs2.RefID = uuid.New().String() - attrs2.Name = fmt.Sprintf("%s-west", attrs.Name) + name2, err := makeDebuggableName(fmt.Sprintf("%s-west", attrs.Name)) + if err != nil { + return instance1, nil, fmt.Errorf("failed to generate debuggable name for second instance: %w", err) + } + attrs2.Name = name2 instance2, err := client.CreateInstance(ctx, attrs2) if err != nil { From a205443da56c38f0796f71eea9a7ebb546d61455 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 02:03:25 +0000 Subject: [PATCH 4/6] Fix CreateInstanceAttrs setup in ValidateEastWestConnectivity test - Remove explicit Name field to match ValidateCreateInstance pattern - Let makeDebuggableName() handle naming internally during instance creation - Ensures proper SSH key setup for test instances Co-Authored-By: Alec Fong --- internal/validation/suite.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/validation/suite.go b/internal/validation/suite.go index 33c7db1..c936bd8 100644 --- a/internal/validation/suite.go +++ b/internal/validation/suite.go @@ -130,7 +130,6 @@ func RunInstanceLifecycleValidation(t *testing.T, config ProviderConfig) { InstanceType: instance.InstanceType, Location: instance.Location, PublicKey: ssh.GetTestPublicKey(), - Name: "test-connectivity", } err := v1.ValidateEastWestConnectivity(ctx, client, attrs, ssh.GetTestPrivateKey()) require.NoError(t, err, "ValidateEastWestConnectivity should pass") From f66e11f7a030a57dfa02cc4e902bd9fc468bd269 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 02:19:56 +0000 Subject: [PATCH 5/6] Fix SSH authentication by refreshing instance data before SSH connections - Add client.GetInstance calls in waitForInstancesReady to refresh instances from API - Follow the same pattern used in validation suite at line 108 - Ensures instances have complete SSH connection details before attempting connections - Update function signature to return refreshed instances Co-Authored-By: Alec Fong --- pkg/v1/networking.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/pkg/v1/networking.go b/pkg/v1/networking.go index 1081956..1b37998 100644 --- a/pkg/v1/networking.go +++ b/pkg/v1/networking.go @@ -102,7 +102,7 @@ func ValidateEastWestConnectivity(ctx context.Context, client CloudCreateTermina defer cleanupInstances(ctx, client, instance1, instance2) - err = waitForInstancesReady(ctx, client, instance1, instance2, privateKey) + instance1, instance2, err = waitForInstancesReady(ctx, client, instance1, instance2, privateKey) if err != nil { return err } @@ -153,16 +153,26 @@ func cleanupInstances(ctx context.Context, client CloudCreateTerminateInstance, } } -func waitForInstancesReady(ctx context.Context, client CloudCreateTerminateInstance, instance1, instance2 *Instance, privateKey string) error { +func waitForInstancesReady(ctx context.Context, client CloudCreateTerminateInstance, instance1, instance2 *Instance, privateKey string) (*Instance, *Instance, error) { var err error instance1, err = WaitForInstanceLifecycleStatus(ctx, client, instance1, LifecycleStatusRunning, PendingToRunningTimeout) if err != nil { - return fmt.Errorf("first instance failed to reach running state: %w", err) + return nil, nil, fmt.Errorf("first instance failed to reach running state: %w", err) } instance2, err = WaitForInstanceLifecycleStatus(ctx, client, instance2, LifecycleStatusRunning, PendingToRunningTimeout) if err != nil { - return fmt.Errorf("second instance failed to reach running state: %w", err) + return nil, nil, fmt.Errorf("second instance failed to reach running state: %w", err) + } + + instance1, err = client.GetInstance(ctx, instance1.CloudID) + if err != nil { + return nil, nil, fmt.Errorf("failed to refresh first instance: %w", err) + } + + instance2, err = client.GetInstance(ctx, instance2.CloudID) + if err != nil { + return nil, nil, fmt.Errorf("failed to refresh second instance: %w", err) } err = ssh.WaitForSSH(ctx, ssh.ConnectionConfig{ @@ -173,7 +183,7 @@ func waitForInstancesReady(ctx context.Context, client CloudCreateTerminateInsta Timeout: RunningSSHTimeout, }) if err != nil { - return fmt.Errorf("SSH not available on first instance: %w", err) + return nil, nil, fmt.Errorf("SSH not available on first instance: %w", err) } err = ssh.WaitForSSH(ctx, ssh.ConnectionConfig{ @@ -184,10 +194,10 @@ func waitForInstancesReady(ctx context.Context, client CloudCreateTerminateInsta Timeout: RunningSSHTimeout, }) if err != nil { - return fmt.Errorf("SSH not available on second instance: %w", err) + return nil, nil, fmt.Errorf("SSH not available on second instance: %w", err) } - return nil + return instance1, instance2, nil } func testConnectivity(ctx context.Context, instance1, instance2 *Instance, privateKey string) error { From 156de8a81e4692ca07b5e19bf0b082914feec2b6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Aug 2025 02:22:36 +0000 Subject: [PATCH 6/6] Fix empty Name field handling in ValidateEastWestConnectivity - Provide default 'test-connectivity' name when attrs.Name is empty - Prevents malformed instance names like '-east' and '-west' - Ensures proper SSH key association for test instances - Fixes SSH authentication failure in CI tests Co-Authored-By: Alec Fong --- pkg/v1/networking.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/v1/networking.go b/pkg/v1/networking.go index 1b37998..20eca83 100644 --- a/pkg/v1/networking.go +++ b/pkg/v1/networking.go @@ -113,7 +113,11 @@ func ValidateEastWestConnectivity(ctx context.Context, client CloudCreateTermina func createTestInstances(ctx context.Context, client CloudCreateTerminateInstance, attrs CreateInstanceAttrs) (*Instance, *Instance, error) { attrs1 := attrs attrs1.RefID = uuid.New().String() - name1, err := makeDebuggableName(fmt.Sprintf("%s-east", attrs.Name)) + baseName := attrs.Name + if baseName == "" { + baseName = "test-connectivity" + } + name1, err := makeDebuggableName(fmt.Sprintf("%s-east", baseName)) if err != nil { return nil, nil, fmt.Errorf("failed to generate debuggable name for first instance: %w", err) } @@ -126,7 +130,11 @@ func createTestInstances(ctx context.Context, client CloudCreateTerminateInstanc attrs2 := attrs attrs2.RefID = uuid.New().String() - name2, err := makeDebuggableName(fmt.Sprintf("%s-west", attrs.Name)) + baseName2 := attrs.Name + if baseName2 == "" { + baseName2 = "test-connectivity" + } + name2, err := makeDebuggableName(fmt.Sprintf("%s-west", baseName2)) if err != nil { return instance1, nil, fmt.Errorf("failed to generate debuggable name for second instance: %w", err) }