-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix podman network IDs handling #9455
Fix podman network IDs handling #9455
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM |
b55756e
to
fa876f7
Compare
Note: It looks like previously you could do podman create with a non existing network. This is no longer possible with this commit. I think it is required to validate the network at create time as we should never leak network IDs into the state. |
That restriction seems fine to me. LGTM.
…On Sun, Feb 21, 2021 at 11:52 Paul Holzinger ***@***.***> wrote:
Note: It looks like previously you could do podman create with a non
existing network. This is no longer possible with this commit. I think it
is required to validate the network at create time as we should never leak
network IDs into the state.
I am not sure if we should backport this because it could break user
workflows.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#9455 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCFXJTESUKF6HNJERCLTAE24DANCNFSM4X7DAIVA>
.
|
fa876f7
to
c22657c
Compare
I added a SkipIfRemote for the tests. It just flaked to much. I have another PR to fix this. see cri-o/ocicni#85 and #9449. |
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.
LGTM
@baude PTAL
The libpod network logic knows about networks IDs but OCICNI does not. We cannot pass the network ID to OCICNI. Instead we need to make sure we only use network names internally. This is also important for libpod since we also only store the network names in the state. If we would add a ID there the same networks could accidentally be added twice. Fixes containers#9451 Signed-off-by: Paul Holzinger <[email protected]>
c22657c
to
9d818be
Compare
/lgtm |
The libpod network logic knows about networks IDs but OCICNI
does not. We cannot pass the network ID to OCICNI. Instead we
need to make sure we only use network names internally. This
is also important for libpod since we also only store the
network names in the state. If we would add a ID there the
same networks could accidentally be added twice.
Fixes #9451