-
-
Notifications
You must be signed in to change notification settings - Fork 128
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 undef variable check puppet6 #480
Conversation
templates/_auth_ldap.erb
Outdated
@@ -14,13 +14,13 @@ provider_url = | |||
providerUrl="<%= provider_url %>" | |||
authenticationMethod="simple" | |||
forceBindingLogin="<%= @auth_config['ldap']['force_binding'] %>" | |||
<%- if @auth_config['ldap']['force_binding_use_root'] != :undef -%> | |||
<%- if !@auth_config['ldap']['force_binding_use_root'].nil? && @auth_config['ldap']['force_binding_use_root'] != :undef -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering force_binding_use_root
defaults to false
and not undef
, I'm not convinced this conditional is needed at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fe80 I've taken the liberty of pushing a commit onto your branch (something you can turn off when you open a PR) that removes this conditional and adds a test for this ldap property. Hoping you could follow the pattern and add tests for the other properties you've fixed? Feel free to discard/squash it if you want to add more tests or write them in a different way. :)
Thanks for the PR! I think I've come across a similar fix in another module before. So we can confirm everything now works correctly (and won't be broken in future), would you be able to write any tests? |
Since _this_ parameter actually defaults to `false` and not `undef` the template doesn't need the conditional.
22f507f
to
60dd95f
Compare
I've had more test and fix a bug (I've miss a |
Obsoleted by #520. |
Hello,
The variable check of some template not work with puppet6