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 issue #27160 - escape whitespaces in fstab entry in module mount when persist=True #39593

Merged
merged 5 commits into from Feb 24, 2017

Conversation

Projects
None yet
7 participants
@zwo-bot
Copy link
Contributor

commented Feb 23, 2017

What does this PR do?

When using the state mount.mounted and set persist=True the /etc/fstab entry is written based on the 'device' key in the mount state. If the device contains whitespaces (Bad idea, but often seen in heterogeneous environments with windows based fileservers and cifs shares), there will be written an invalid fstab entry. Linux supports escaping the whitespaces in /etc/fstab with \040, but the mount command which is used to do the mount did not support this kind of escaping. That makes is impossible to use the escaped device in state definition -> either mount or fstab is working but not both. This patch replaces the escaped whitespaces with \040 before the fstab entry is written, and translates is back when /proc/self/mountinfo is read.

According to documentation, FreeBSD and Solaris did not support any kind of whitespace escaping in fstab/vfstab.

What issues does this PR fix or reference?

#27160

Previous Behavior

/mnt/dir
  mounts.mounted:
    - device: '//servername/this\ is\ a\ share/'
    - fstype: cifs
    - persist: True

results in fstab entry:
//servername/this\ is\ a\ dir'/mnt/share cifs defaults 0 0

New Behavior

/mnt/dir
  mounts.mounted:
    - device: '//servername/this\ is\ a\ share/'
    - fstype: cifs
    - persist: True

results in fstab entry:
//servername/this\040is\040a\040share/'/mnt/dir cifs defaults 0 0

Tests written?

No

@salt-jenkins

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

@zwo-bot, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sirkonst, @cachedout and @sjorge to be potential reviewers.

@sjorge

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

LGTM, I had a look, this does seem to properly escape spaces in device names.

zwo-bot added some commits Feb 23, 2017

@cachedout cachedout merged commit 89635b4 into saltstack:develop Feb 24, 2017

3 of 6 checks passed

default Build finished.
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #8925 — FAILURE
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #9071 — FAILURE
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #11557 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » salt-pr-docs-n #4395 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt Linode Ubuntu14.04 #8847 — SUCCESS
Details
@sirkonst
Copy link
Contributor

left a comment

Ok, but need unit tests.

@rasathus

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2017

Is it possible to get this fix merged into the 2016 branch, or should I create a separate PR ?

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2017

@rasathus If you could create a separate PR, that would be great! You should just be able to cherry-pick the commit.

garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Jan 17, 2019

garethgreenaway added a commit that referenced this pull request Jan 19, 2019

Merge pull request #51226 from garethgreenaway/51195_handle_spaces_in…
…_fstab_opts

[2018.3] Handle spaces in fstab opts, similar fix to #39593

Ch3LL added a commit to Ch3LL/salt that referenced this pull request Jan 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.