add resource gmsa #92
Conversation
ghost
added
the
| if len(sp) > 0 { | ||
| spnlist := strings.Join(sp, ",") | ||
| log.Printf("[DEBUG] SPN list: %s", spnlist) | ||
| cmds = append(cmds, fmt.Sprintf("; Set-ADServiceAccount -Identity %q -ServicePrincipalNames @{Add=%q}", g.GUID, spnlist)) |
There was a problem hiding this comment.
This is definitely a simpler way to handle additions and deletetions without having to compare lists
etc. My only concern here is that if this command fails for any reasons then the ServicePrincipalNames
attribute will be left empty, which could potentially cause operational issues.
What do you think?
There was a problem hiding this comment.
Indeed it would be better to update only what is needed for all parameters that have a list (SPN, principals etc) but the thing is that I'm not super comfortable (yet) to do this part.
I would need to look at what you did in the membership resource.
I'm afraid it's gonna take me a bit of time to achieve it but I fully agree it's the smoothest approach to avoid problems.
There was a problem hiding this comment.
In this example I call diffGroupMemberLists.
In that function I am passing two lists as arguments, the first one is the list of members as expressed in the configuration and the second is a list of members as retrieved from AD. It then compares one to the other and returns two new lists, a list of members to add and a list of members to remove.
It's not exactly performant, but given that the lists are relatively small it will do what we need.
simplify-double-quoted-comma-separated-lists
|
It would also be nice if you could add some acceptance tests, I've pushed a PR yesterday that removes all the hardcoded bits from the tests and moves them to env vars so anyone can run tests against their own test AD controller. |
|
Thanks for doing this - it would be really useful for me! Is it just the acceptance tests that are left to get it working? If I have time, I'd be happy to pitch in to get it over the line |
|
The way to handle the update of certain parameters (SPN,PrincipalsAllowedToDelegateToAccount,PrincipalsAllowedToRetrieveManagedPassword) isn't optimal. I updated the code to perform the update in one command (SPN parameters allows to use "replace" argument and both other parameters are natively replacing the value by the new input). To be honest I haven't digged into how manage/update arrays/list in golang yet. I still need to :
|
|
|
|
Closing this PR as preparing a fresh new one |
Description
Allow provider to manage gmsa (aka Group Managed Services Account(s)).
References
#54
Community Note