Skip to content

Add more checks to the --mount flag parsing logic#5925

Merged
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
nalind:mount-flags-parsing
Jan 21, 2025
Merged

Add more checks to the --mount flag parsing logic#5925
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
nalind:mount-flags-parsing

Conversation

@nalind
Copy link
Member

@nalind nalind commented Jan 20, 2025

What type of PR is this?

/kind other

What this PR does / why we need it:

  • Make volumes.GetBindMount(), volumes.GetCacheMount(), and volumes.GetTmpfsMount() return errors when flags which expect arguments are given empty arguments, when flags which don't expect arguments are given arguments, and when the "relabel" flag, which expects an argument, doesn't get one.
  • Make volumes.GetCacheMount() not treat the "U" flag as affecting bind propagation.
  • Drop the special-case error message when a caller attempts to use "src" or "source" options in volumes.GetTmpfsMount(), which would already be covered by the general-purpose "unrecognized option" default.

How to verify it

New unit tests!

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

* Make volumes.GetBindMount(), volumes.GetCacheMount(), and
  volumes.GetTmpfsMount() return errors when flags which expect
  arguments are given empty arguments, when flags which don't expect
  arguments are given arguments, and when the "relabel" flag, which
  expects an argument, doesn't get one.
* Make volumes.GetCacheMount() not treat the "U" flag as affecting bind
  propagation.
* Drop the special-case error message when a caller attempts to use
  "src" or "source" options in volumes.GetTmpfsMount(), which would
  already be covered by the general-purpose "unrecognized option"
  default.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the mount-flags-parsing branch from 9324657 to 9b9c161 Compare January 20, 2025 22:51
@rhatdan
Copy link
Member

rhatdan commented Jan 21, 2025

LGTM
@flouthoc @giuseppe PTAL

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, nalind

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit eac9331 into containers:main Jan 21, 2025
@nalind nalind deleted the mount-flags-parsing branch January 21, 2025 16:14
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Apr 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants