Network #480
Network #480
Conversation
|
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #480 +/- ##
=========================================
+ Coverage 8.23% 10.76% +2.52%
=========================================
Files 31 34 +3
Lines 3036 3856 +820
=========================================
+ Hits 250 415 +165
- Misses 2764 3411 +647
- Partials 22 30 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Adds comprehensive jumphost support and network management capabilities to the onctl cloud management tool. The PR enables connecting to VMs through jump hosts for secure access patterns and provides full network lifecycle management across cloud providers.
- Implements SSH ProxyJump functionality for secure VM access through jump hosts
- Adds network resource management with CRUD operations for AWS and Hetzner
- Enhances VM listing to only show running instances across all providers
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tools/ssh.go | Adds jumphost support to SSH connections using ProxyJump |
| internal/tools/scp.go | Implements jumphost functionality for SCP file transfers |
| internal/tools/remote-run.go | Adds jumphost support for remote SSH connections |
| internal/provideraws/common.go | Adds GetLatestUbuntu2204AMI function with dynamic AMI selection |
| internal/cloud/hetzner.go | Adds network management and filters VM list to running instances |
| internal/cloud/gcp.go | Filters VM list to running instances and adds stub network methods |
| internal/cloud/cloud.go | Adds network management interfaces and jumphost VM field |
| internal/cloud/azure.go | Filters VM list to running instances and adds stub network methods |
| internal/cloud/aws.go | Adds network management and filters VM list to running instances |
| cmd/vm.go | New command for VM network attach/detach operations |
| cmd/ssh.go | Adds jumphost resolution and IP selection logic |
| cmd/root.go | Initializes network managers for supported providers |
| cmd/network.go | New command for network CRUD operations |
| cmd/create.go | Adds jumphost support for VM creation and domain registration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func (r *Remote) downloadFileWithJumpHost(srcPath, dstPath string) error { | ||
| // Create a temporary file for the private key | ||
| tempKeyFile, err := os.CreateTemp("", "onctl_ssh_key_*") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create temp key file: %v", err) | ||
| } | ||
| defer func() { | ||
| if err := os.Remove(tempKeyFile.Name()); err != nil { | ||
| log.Printf("Failed to remove temp key file: %v", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
[nitpick] The code for creating and cleaning up temporary key files is duplicated in both downloadFileWithJumpHost and uploadFileWithJumpHost methods. Consider extracting this logic into a helper method to reduce duplication and improve maintainability.
| DeviceIndex: aws.Int64(0), | ||
| // SubnetId: aws.String(subnetIds[0]), | ||
| AssociatePublicIpAddress: aws.Bool(true), | ||
| AssociatePublicIpAddress: aws.Bool(server.JumpHost == ""), // Only associate public IP if no jumphost |
There was a problem hiding this comment.
[nitpick] The condition server.JumpHost == \"\" is used to determine whether to associate a public IP address. This logic should be more explicit and potentially configurable. Consider adding a dedicated boolean field or method to make this decision clearer.
| var targetIP string | ||
| if resolvedJumpHost != "" && (vm.IP == "" || vm.IP == "<nil>") { |
There was a problem hiding this comment.
[nitpick] The string comparison vm.IP == \"<nil>\" suggests inconsistent handling of nil/empty IP addresses. Consider standardizing how empty IP addresses are represented throughout the codebase (e.g., always use empty string or a dedicated constant).
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
configuration examples
- Updated golang.org/x/net to v0.43.0 from main - Kept golang.org/x/image dependency for go-term-markdown - Resolved go.mod conflicts
- Remove github.com/MichaelMure/go-term-markdown dependency - Add github.com/charmbracelet/glamour v0.10.0 dependency - Update templates.go to use glamour.NewTermRenderer with auto-style detection - Improve markdown rendering with better terminal compatibility - Tested with onctl templates describe azure command
No description provided.