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

browseable share parameter should only be set if specified to be "no" #127

Closed
tdltdc opened this issue Dec 19, 2023 · 5 comments
Closed

Comments

@tdltdc
Copy link
Contributor

tdltdc commented Dec 19, 2023

Currently, the browseable and browsable share parameters are only set in smb.conf if their value is yes and thus it cannot be set to no.

This is undesirable, because the value of browseable defaults to yes, so setting yes is never useful. The only value that is of use is no.

@tdltdc tdltdc changed the title browseable share parameter should always be set if specified browseable share parameter should only be set if specified to be "no" Dec 19, 2023
@tdltdc
Copy link
Contributor Author

tdltdc commented Dec 19, 2023

Perhaps this simple change would already fix this?

#128

@vladgh
Copy link
Owner

vladgh commented Dec 20, 2023

Hmm, you bring up an excellent point!

However, it's more of a documentation issue. I know it's confusing but in this case, the yes/no values need to be quoted so that they are parsed as a String and not as a Boolean. That's why the ternary filter is also there to ensure just those two String values.

So, given this template:

{% if share.browseable is defined and share.browseable %}
  browseable = {{ share.browseable | ternary('yes', 'no') }}
{% endif %}

If browseable equals no or false, it will be skipped. But if the value is quoted in the variables (ex: browseable: 'no'), then the conditional will see that it is defined and that it is not an empty string, and will move on.

That being said, I am now very confused about what would be better: to change everything to a Boolean, and have that value of true/false/yes/no converted to a string in the template, or to keep it as is, and just update the documentation to specify it NEEDS to be quoted.
Look at the molecule/default/converge.yml for how the variable is now defined.

Copy link

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

@github-actions github-actions bot added the stale This issue or pull request has been marked 'stale' due to lack of recent activity label Mar 20, 2024
@vladgh vladgh removed the stale This issue or pull request has been marked 'stale' due to lack of recent activity label Mar 20, 2024
@DevSecNinja
Copy link

Hey @vladgh - thanks for this! I initially filled in false since that was my intuition based on using the other variables. Then I noticed it didn't work and used "no" which also didn't work. I also tried 'no' and no and in all these cases I can see browseable = yes in the smb.conf on the server. When I use false it removes the browseable = yes entry.

My 2 cents: I would convert false/no and true/yes to the respective yes/no string and write it to the browseable entry. Thanks!

@vladgh
Copy link
Owner

vladgh commented May 9, 2024

I think I figured out a way to fix this true/false/yes/no in the playbooks and templates. As of now, if the variable is a quoted 'no', it will act as a true statement because it is a valid string. So, the clean and simple way for this to work is to sanitize the inputs and defaults to always output just a yes or no.

browseable = {{ share.browseable | bool | ternary('yes', 'no') }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants