[v4.4.1-rhel] do not pass volume-opt as bind mounts options to runtime#28450
Conversation
Starting with runc 1.3.0 it errors when we pass unknown mount options to the runtime, the volume-opt options are specifc to the volume we create and should not be passed to the mount in the oci spec. Fixes: containers#26938 (originally) Follow up PR to: containers#28092 Just before merging it was realized that the commit in this PR were also needed to completely address CVE-2025-52881 Fixes: https://issues.redhat.com/browse/OCPBUGS-67036, https://issues.redhat.com/browse/OCPBUGS-67053, https://issues.redhat.com/browse/OCPBUGS-67070, https://issues.redhat.com/browse/OCPBUGS-67090, https://issues.redhat.com/browse/RHEL-134783, https://issues.redhat.com/browse/RHEL-134787 Signed-off-by: Paul Holzinger <pholzing@redhat.com> (cherry picked from commit 4e2a04d) Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
Starting with runc 1.3.0 it errors when we pass unknown mount options to the runtime, the copy/nocopy options are specific to podman when we mount the volume and are not valid mount options for the runtime. Fixes: containers#26938 Signed-off-by: Paul Holzinger <pholzing@redhat.com> (cherry picked from commit 46d7575) Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
lsm5
left a comment
lsm5
left a comment
There was a problem hiding this comment.
I'd like to run tests on testing-farm using this branch, but integration tests are failing on PodmanExitCleanly.
| copySession := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch"}) | ||
| copySession.WaitWithDefaultTimeout() | ||
| Expect(copySession).Should(Exit(0)) | ||
| podmanTest.PodmanExitCleanly("run", "--name", "testctr", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch") |
There was a problem hiding this comment.
PodmanExitCleanly doesn't exist on this branch yet, so we gotta revert to the old way or backport PodmanExitCleanly changes too.
There was a problem hiding this comment.
I'll take a look, thanks for the pointer.
Combined test for (exitcode == 0) && (nothing on stderr). Returns more useful diagnostic messages than the default: old: Expected N to equal 0 new: Command failed with exit status N new: Unexpected warnings seen on stderr: "...." Adding fro the ExitCleanOnly function that is present in some tests that were cherry picked for this PR. Signed-off-by: Ed Santiago <santiago@redhat.com> (cherry picked from commit 6cbd17c) Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
|
@lsm5 I decided to try backporting the ExitCleanly() and the associated functions rather than converting. It looks to be very contained, and they might be handy later. If the port goes badly, I'll convert the calls. |
ack, doing an integration test run. Btw, just finished doing other test runs. Failures don't seem related to the PR. Will report back on integration jobs once I have them. |
|
@TomSweeneyRedHat we need this for PodmanExitCleanly : 0c18beaea7 |
|
@lsm5 any chance to run this through the test grinder yet? |
will run the integration tests once more. The other tests are not impacted because of this PR. |
|
@lsm5 any updates? |
Doesn't look like this is done yet. We need this for the tests to compile. In a prior run, I got this added separately and tests seem to run well and issues were not related to the PR. So, once this is added and integration tests are verified to compile, we should be good to go. |
This significantly simplifies the ceromony of running a Podman command
in integration tests, from
> session := p.Podman([]string{"stop", id})
> session.WaitWithDefaultTimeout()
> Expect(session).Should(ExitCleanly())
to
> p.PodmanExitCleanly("stop", id)
There are >4650 instances of ExitCleanly() in the tests,
and many could be migrated; this does not do that.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
(cherry picked from commit 0c18bea)
Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
| // PodmanExitCleanly runs a podman command with args, and expects it to ExitCleanly within the default timeout. | ||
| // It returns the session (to allow consuming output if desired). | ||
| func (p *PodmanTestIntegration) PodmanExitCleanly(args ...string) *PodmanSessionIntegration { | ||
| GinkgoHelper() |
There was a problem hiding this comment.
@TomSweeneyRedHat remove this line too. This is not present on ginkgo v1. Sorry should've pointed that out earlier.
5 participants
Follow up PR to: #28092
Just before merging it was realized that the commit in this PR were also
needed to completely address GHSA-cgrx-mc8f-2prm
Fixes: https://issues.redhat.com/browse/OCPBUGS-67036, https://issues.redhat.com/browse/OCPBUGS-67053,
https://issues.redhat.com/browse/OCPBUGS-67070,
https://issues.redhat.com/browse/OCPBUGS-67090,
https://issues.redhat.com/browse/RHEL-134783,
https://issues.redhat.com/browse/RHEL-134787
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?