Skip to content
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(quadlet): Don't crash on invalid mounts #20108

Closed
wants to merge 1 commit into from
Closed

fix(quadlet): Don't crash on invalid mounts #20108

wants to merge 1 commit into from

Conversation

mhutter
Copy link

@mhutter mhutter commented Sep 22, 2023

If Mount directives are not in the form Key=Value, quadlet crashes due to an index out of range error.

Check this & return an error message instead.

Fixes: #20104

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 22, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhutter
Once this PR has been reviewed and has the lgtm label, please assign mtrmac for approval. For more information see the Kubernetes Code Review Process.

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

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

@packit-as-a-service
Copy link

Cockpit tests failed for commit 9f4c7b380c59c3b1592d772f59d6cfbb33d10bba. @martinpitt, @jelly, @mvollmer please check.

@martinpitt
Copy link
Contributor

Cockpit tests failed for commit 9f4c7b3. @martinpitt, @jelly, @mvollmer please check.

Transient DNS failure on COPR setup. This is avoidable in the testing farm, I filed https://issues.redhat.com/browse/TFT-2256 for it.

If `Mount` directives are not in the form `Key=Value`, quadlet crashes
due to an index out of range error.

Check this & return an error message instead.

Fixes: #20104
Signed-off-by: Manuel Hutter <[email protected]>
@ygalblum
Copy link
Contributor

/hold
I've made a more elaborate fix in #20149

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2023
@vrothberg
Copy link
Member

Since we've settled on going forward with #20149, I am closing the PR here.

Thanks for reporting and contributing, @mhutter !

@vrothberg vrothberg closed this Sep 27, 2023
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Dec 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quadlet: Invalid Mount directive leads to core dump
6 participants