Feat/global vpn#316
Conversation
- global VPN devices incremental IP generation - allows provisions for reclaiming freed cluster subnets, and device IPs - default allocation, allows for 8K Service IPs, per cluster, and reserves 40K IPs for kloudlite devices/vm/remote-IDEs etc.
for Vector GRPC Proxy server
- also ensures Devices as peers for GlobalVPNConnections, ensuring 2way communication
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder17 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 6 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -0,0 +1,383 @@ | |||
| package domain | |||
There was a problem hiding this comment.
suggestion (code_refinement): Consider adding package documentation
It's a good practice to provide a package-level comment explaining the purpose and scope of the package.
| package domain | |
| // Package domain defines the domain layer logic for global VPN cluster connections. | |
| // It encapsulates the core business logic and rules associated with VPN connections. | |
| package domain |
| @@ -0,0 +1,283 @@ | |||
| package domain | |||
There was a problem hiding this comment.
suggestion (code_refinement): Add package documentation for clarity
Providing a brief overview of the package's functionality can help maintainers and new developers understand its role within the project.
| package domain | |
| // Package domain defines the domain layer logic for global VPN devices. | |
| // It encapsulates the core business rules and entities involved in managing | |
| // VPN devices within the infrastructure. | |
| package domain |
|
|
||
| return ipAddr, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (code_refinement): Review error handling strategy
Ensure that errors from external libraries or systems are handled gracefully, possibly adding retries or more descriptive error messages.
| @@ -0,0 +1,147 @@ | |||
| package domain | |||
There was a problem hiding this comment.
suggestion (code_refinement): Add documentation for the 'domain' package
A top-level comment explaining the purpose of the package helps others understand its use and scope more quickly.
| package domain | |
| // Package domain defines the domain layer logic for global VPN configurations. | |
| // It encapsulates the core business rules and entities involved in the VPN management. | |
| package domain |
| package domain | ||
|
|
||
| import ( | ||
| "math" | ||
|
|
||
| iamT "github.com/kloudlite/api/apps/iam/types" | ||
| "github.com/kloudlite/api/apps/infra/internal/entities" | ||
| fc "github.com/kloudlite/api/apps/infra/internal/entities/field-constants" | ||
| "github.com/kloudlite/api/common" | ||
| "github.com/kloudlite/api/common/fields" |
There was a problem hiding this comment.
suggestion (testing): Missing integration tests for GlobalVPN domain logic
Integration tests are needed to validate the interactions between the GlobalVPN domain logic and the database, especially for the create, update, and delete operations to ensure data integrity and error handling.
| package wgutils | ||
|
|
||
| import ( | ||
| "github.com/kloudlite/api/pkg/errors" | ||
| "golang.zx2c4.com/wireguard/wgctrl/wgtypes" | ||
| ) | ||
|
|
||
| func GenerateKeyPair() (privateKey string, publicKey string, err error) { | ||
| key, err := wgtypes.GenerateKey() | ||
| if err != nil { |
There was a problem hiding this comment.
suggestion (testing): Missing unit tests for key generation utility
Add unit tests for GenerateKeyPair to ensure that the keys generated are valid and meet the expected format and security standards.
1 participant
GlobalVPN implementation
enabling service communication across clusters
GlobalVPNDevice
enabling wireguard device 2-way communication across all the clusters, and other devices in a globalVPN