allow using public subnets #119
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good to me! Reviewed entire PR up to commit ecf1e2a
Reviewed 290 lines of code across 7 files in 1 minute(s) and 48 second(s).
See details
- Skipped files: 0
- Confidence threshold:
50% - Drafted
1additional comments. - Workflow ID:
wflow_RoYOAbDww8pe3Onh
View 1 draft comments
These comments were drafted by Ellipsis, but were filtered out of the final review. They're included here so you can see our internal thought process and help you configure your ellipsis.yaml.
Drafted 1 comments under confidence threshold
Filtered comment at provider/pkg/provider/aws/bastion.go:187
Notes: The PR is adding the ability to use public subnets for the bastion host. It adds a new 'public' argument to the BastionArgs class and uses it to conditionally open a UDP port in the security group and associate a public IP address with the bastion host. The changes look correct and follow best practices. However, the PR does not include any tests to verify the new functionality.
The changes look correct, but it would be good to add tests to verify the new functionality.
Confidence changes required: 50%
Something look wrong? Tag @ellipsis-dev in a comment, or customize the ellipsis.yaml for this repository.
Generated with ❤️ by ellipsis.dev
There was a problem hiding this comment.
Looks good to me! Incremental review on commit e0e7a02
Reviewed 289 lines of code across 7 files in 2 minute(s) and 28 second(s).
See details
- Skipped files: 0
- Confidence threshold:
50% - Drafted
1additional comments. - Workflow ID:
wflow_TnwcB307aUEACUXU
View 1 draft comments
These comments were drafted by Ellipsis, but were filtered out of the final review. They're included here so you can see our internal thought process and help you configure your ellipsis.yaml.
Drafted 1 comments under confidence threshold
Filtered comment at provider/pkg/provider/aws/bastion.go:33
Notes: The PR seems to be well implemented. The author has added a new argument 'public' to the BastionArgs struct in the bastion.go file. This argument is used to determine whether the bastion is going in public subnets. If it is, the UDP port is opened to ensure that DERP relays are not used. This change is reflected in all the language SDKs (Python, Node.js, .NET, and Go). The example has also been updated to use public subnets. I don't see any clear violations of best practices, logical bugs, performance bugs, or security bugs.
The implementation of the 'public' argument in the BastionArgs struct and its usage in the NewBastion function looks correct. Good job on updating the example and all the language SDKs to reflect this change.
Confidence changes required: 10%
Something look wrong? Tag @ellipsis-dev in a comment, or customize the ellipsis.yaml for this repository.
Generated with ❤️ by ellipsis.dev
There was a problem hiding this comment.
Looks good to me! Incremental review on commit e0e7a02
Reviewed 289 lines of code across 7 files in 2 minute(s) and 21 second(s).
See details
- Skipped files: 0
- Confidence threshold:
50% - Drafted
1additional comments. - Workflow ID:
wflow_EOIS00qbMy5qqRjK
View 1 draft comments
These comments were drafted by Ellipsis, but were filtered out of the final review. They're included here so you can see our internal thought process and help you configure your ellipsis.yaml.
Drafted 1 comments under confidence threshold
Filtered comment at provider/pkg/provider/aws/bastion.go:187
Notes: The PR seems to be well implemented. The author has added a new argument 'public' to the BastionArgs struct in multiple languages. This argument is then used in the NewBastion function to determine whether to open the UDP port or not. The author has also updated the example to use public subnets. I don't see any clear violations of best practices, logical bugs, performance bugs, or security bugs. The code is clean and well-structured.
The implementation of the 'public' argument in the NewBastion function looks good. It correctly opens the UDP port when 'public' is true.
Confidence changes required: 10%
Something look wrong? Tag @ellipsis-dev in a comment, or customize the ellipsis.yaml for this repository.
Generated with ❤️ by ellipsis.dev
Summary:
This PR adds support for using public subnets for the bastion by introducing a new
publicargument toBastionArgs, updating theNewBastionfunction, example,schema.yaml, and SDKs accordingly.Key points:
publicargument toBastionArgsin multiple languages.NewBastionfunction to open UDP port ifpublicistrue.schema.yamland SDKs to reflect changes.Generated with ❤️ by ellipsis.dev