fix: clearer error publishing a privileged port to a specific IP on macOS#28982
fix: clearer error publishing a privileged port to a specific IP on macOS#28982stefan8 wants to merge 1 commit into
Conversation
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
Please note https://github.com/podman-container-tools/podman/blob/main/CONTRIBUTING.md https://github.com/podman-container-tools/podman/blob/main/LLM_POLICY.md
It would be good if you can follow our contributing guidelines and follow the PR template instead of this AI summary format which is way to verbose.
Mentioning which file you changes is pointless as anyone can see that in the diff.
Also I see you opened many PRs in very short time, so I kindly would ask you to stop that as new contributor. That means we need to explain the some guidelines all these PRs many times and also practically exhaust our review capacity.
| err := annotateGvproxyResponseError(strings.NewReader(tt.body), machineExpose{Local: tt.local}) | ||
| if err == nil { | ||
| t.Fatal("expected an error, got nil") | ||
| } | ||
| if gotHint := strings.Contains(err.Error(), "gvproxy"); gotHint != tt.wantHint { | ||
| t.Fatalf("gvproxy hint = %v, want %v (err: %q)", gotHint, tt.wantHint, err.Error()) | ||
| } | ||
| // The raw gvproxy response body is always preserved for debugging. | ||
| if !strings.Contains(err.Error(), tt.body) { | ||
| t.Fatalf("error %q should contain the original response body %q", err.Error(), tt.body) | ||
| } |
There was a problem hiding this comment.
we are using we are using github.com/stretchr/testify/assert /github.com/stretchr/testify/require in the repo already which produces better errors and reduces the boilerplate so please use them instead
i.e. require.NoError() assert.Equal()/Contains()
There was a problem hiding this comment.
Thanks, and sorry for the noise. I've switched the test to require/assert and trimmed the PR descriptions down to the template.
stefan8
force-pushed
the
fix/gvproxy-privileged-port-error
branch
2 times, most recently
from
41ab92a to
241712b
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
With podman machine, gvproxy forwards published ports by binding them on the host (not inside the VM) and runs unprivileged. macOS refuses to bind a privileged port (< 1024) to a specific IP for a normal user, even though binding all interfaces is fine, so publishing e.g. -p 127.0.0.1:80:80 used to fail with an opaque "something went wrong with the request". Pass the published ip:port into the gvproxy error helper and, when the body says "permission denied" for a < 1024 port on a specific IP, return an error that explains gvproxy binds on the host and suggests dropping the host IP or using a port >= 1024. The raw body is kept for every other case. Add unit tests for the helper. Fixes: podman-container-tools#28009 Signed-off-by: Grzegorz Szczepanczyk <g.szczepanczyk@getprintbox.com>
stefan8
force-pushed
the
fix/gvproxy-privileged-port-error
branch
from
49533e9 to
697fa0c
Compare
2 participants
Publishing a privileged port to a specific IP with
podman machineon macOS failed with an opaque error. gvproxy binds the port on the host and runs unprivileged, and macOS rejects binding a< 1024port to a specific IP. This returns a clearer error for that case and adds unit tests.Checklist
git commit -s).Fixes: #28009in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?