-
Notifications
You must be signed in to change notification settings - Fork 699
Plain mode: support port forwarding #3699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ac63182
to
df5213b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear what is the suggested behavior. Can we start with the documentation instead of the code, so we have clear understanding of wanted behavior?
9dfcee3
to
d907638
Compare
9971c2f
to
330a2d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for user-defined static port forwarding in plain mode by extending the CLI, YAML schema, host agent logic, and validation.
- Introduce a
--port-forward
flag with parsing and YQ expression generation - Extend
PortForward
withStatic
, filter non-static rules in plain mode, and warn on large port ranges - Update host agent to separate/apply static forwards and add end-to-end tests + CI jobs
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
pkg/portfwd/forward.go | Skip dynamic forwarding for statically handled ports |
pkg/limayaml/validate.go | Warn for port ranges >10 in plain mode |
pkg/limayaml/limayaml.go | Add Static field to PortForward struct |
pkg/limayaml/defaults.go | Remove non-static forwards when plain mode is enabled |
pkg/limayaml/defaults_test.go | Tests for filtering static vs. non-static forwards |
pkg/hostagent/hostagent.go | Separate and apply static forwards; adjust event loop logic |
cmd/limactl/editflags/editflags.go | Register port-forward flag and implement parsing & expression |
cmd/limactl/editflags/editflags_test.go | Unit tests for ParsePortForward and BuildPortForwardExpression |
hack/test-templates/static-port-forward.yaml | Template covering static/dynamic port examples |
hack/test-plain-static-port-forward.sh | Script to verify static-only forwarding in plain mode |
hack/test-nonplain-static-port-forward.sh | Script to verify full forwarding in normal mode |
.github/workflows/test.yml | CI jobs to run the new static port forwarding tests |
Comments suppressed due to low confidence (2)
pkg/hostagent/hostagent.go:619
- [nitpick] Consider adding unit or integration tests for
separateStaticPortForwards
andaddStaticPortForwardsFromList
to verify correct separation and application of static port-forwarding rules.
func (a *HostAgent) addStaticPortForwardsFromList(ctx context.Context, staticPortForwards []limayaml.PortForward) {
pkg/hostagent/hostagent.go:642
- The loop uses
for i := range len(a.instConfig.PortForwards)
, which attempts to range over an int and will not compile. Change it tofor i := range a.instConfig.PortForwards
or use a classic indexed loop likefor i := 0; i < len(...); i++
.
for i := range len(a.instConfig.PortForwards) {
c4cea4c
to
5f856c3
Compare
0599df3
to
9764a70
Compare
Needs rebase |
@Horiodino Could you rebase? |
9764a70
to
f35fb07
Compare
f35fb07
to
ffaebf2
Compare
This kind of commit can be squashed: |
Also, GPG signature seems invalid
|
ffaebf2
to
2416181
Compare
Signed-off-by: Praful Khanduri <[email protected]>
2416181
to
fc32f37
Compare
Signed-off-by: Praful Khanduri <[email protected]>
fc32f37
to
b5cebb0
Compare
Signed-off-by: Praful Khanduri <[email protected]>
b5cebb0
to
c5b1478
Compare
Signed-off-by: Praful Khanduri <[email protected]>
c5b1478
to
5bd8ff7
Compare
Signed-off-by: Praful Khanduri <[email protected]>
5bd8ff7
to
acb8885
Compare
Signed-off-by: Praful Khanduri <[email protected]>
acb8885
to
95dbdd8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Are there any reasons not to support |
|
@@ -405,6 +412,13 @@ if [[ -n ${CHECKS["port-forwards"]} ]]; then | |||
set +x | |||
fi | |||
|
|||
if [[ -n ${CHECKS["static-port-forwards"]} ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this test performed in CI?
Fixes #2962
Changes
--port-forward
CLI flag tolimactl create
, allowing users to specify static port forwards even in plain mode.portForwards
from CLI.