Add "idmap config" parameter #45
Conversation
bschonec
force-pushed
the
merge_idmap_config
branch
2 times, most recently
from
ab296c3 to
2b4998f
Compare
manifests/option.pp
Outdated
| @@ -13,7 +13,7 @@ | |||
|
|
|||
| define samba::option ( | |||
| String $key = $title, | |||
| Variant[Boolean, Integer, String, Array[String], Undef] $value = undef, | |||
| Variant[Hash, Boolean, Integer, String, Array[String], Undef] $value = undef, | |||
There was a problem hiding this comment.
I think the samba:option is not the right place to add a hash type. It will be very complex to pass it on to the augeas. I think this needs to be handled in a different way. I am not fully sure yet but it looks very similar to the this module handles the shares.
There was a problem hiding this comment.
Indeed, I have been having trouble with Hash and augeas. I'll be working on the code more today and tomorrow; perhaps I can come up with a solution. The reason for the [new] hash here is to accommodate #37 which I have refactored.
Perhaps we could abandon augeas altogether? I've always thought of augeas as the last choice when managing configuration files. Yes, it would be significant effort in moving to template-based management of the smb.conf file but we'd have significantly more control over the creation and validation of the smb.conf parameters.
There was a problem hiding this comment.
The reason I prefer augeas over templates is the constant upkeep required to align the template with upstream changes. It is just not worth the effort across many different platforms. This hash can be solved similar to how shares work. They are simply hash values as well.
bschonec
force-pushed
the
merge_idmap_config
branch
from
e78ff55 to
1f00aea
Compare
|
This PR does look very promising! I think that looks to be the right approach without changing too many things. I could not spend too much time today to look at individual changes though I will try to do this in a few day. Thank you once again @bschonec for the heavy lift! |
|
@bschonec , please let me know if this PR is ready to be merged? I will create a release once this PR is complete. |
|
Yes, it's ready. |
|
@rehanone , when you merge this, I'll update the other requests that are pending. |
This pull request supersedes #37.