kubernetes-sigs / windows-gmsa

External components to support Windows GMSA in Kubernetes
Apache License 2.0
30 stars 57 forks source link

Add credential.hostAccountConfig for CCG scenario #125

Closed aiyengar2 closed 1 year ago

aiyengar2 commented 1 year ago

https://github.com/kubernetes-sigs/windows-gmsa/issues/126

This commit adds a new field .Values.credential.hostAccountConfig for situations where the default credential deployed by this chart utilizes CCG.

Here are the tests I ran with my changes:

# No output expected when credential.enabled is false (default)
$ helm template gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig'

# Null output expected when credential.enabled is true but hostAccountConfig is not provided
$ helm template --set="credential.enabled=true" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig'
null

# If credential.enabled and hostAccountConfig is not empty, fields must be provided correctly
$ helm template --set="credential.enabled=true" --set="credential.hostAccountConfig.badkey=hi" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig'
Error: execution error at (gmsa/templates/credentialspec.yaml:16:29): credential.hostAccountConfig.portableCCGVersion must be provided if credential.hostAccountConfig is set

Use --debug flag to render out invalid YAML

$ helm template --set="credential.enabled=true" --set="credential.hostAccountConfig.portableCcgVersion=1" --set="credential.hostAccountConfig.pluginGUID=myguid" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig'
Error: execution error at (gmsa/templates/credentialspec.yaml:18:22): credential.hostAccountConfig.pluginInput must be provided if credential.hostAccountConfig is set

Use --debug flag to render out invalid YAML

$ helm template --set="credential.enabled=true" --set="credential.hostAccountConfig.pluginGUID=myguid" --set="credential.hostAccountConfig.pluginInput=myinput" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig' 
Error: execution error at (gmsa/templates/credentialspec.yaml:16:29): credential.hostAccountConfig.portableCCGVersion must be provided if credential.hostAccountConfig is set

Use --debug flag to render out invalid YAML

$ helm template --set="credential.enabled=true" --set="credential.hostAccountConfig.portableCcgVersion=1" --set="credential.hostAccountConfig.pluginInput=myinput" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig' 
Error: execution error at (gmsa/templates/credentialspec.yaml:17:38): credential.hostAccountConfig.pluginGUID must be provided if credential.hostAccountConfig is set

Use --debug flag to render out invalid YAML

# If credential.enabled and hostAccountConfig is properly set, HostAccountConfig is properly set
$ helm template --set="credential.enabled=true" --set="credential.hostAccountConfig.portableCcgVersion=1" --set="credential.hostAccountConfig.pluginGUID=myguid" --set="credential.hostAccountConfig.pluginInput=myinput" gmsa charts/gmsa | yq e 'select(.kind == "GMSACredentialSpec") | .credspec.ActiveDirectoryConfig.HostAccountConfig'
PortableCcgVersion: "1"
PluginGUID: "{myguid}"
PluginInput: "myinput"
aiyengar2 commented 1 year ago

@jsturtevant @marosset @jayunit100 anything I can do to assist in the review of this PR?

marosset commented 1 year ago

@jsturtevant @marosset @jayunit100 anything I can do to assist in the review of this PR?

I just needed this reminded - thanks!

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aiyengar2, marosset

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/windows-gmsa/blob/master/OWNERS)~~ [marosset] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
aiyengar2 commented 1 year ago

@marosset @jsturtevant @jayunit100 can I get an lgtm on this PR to merge it (unless there are any other concerns)?

jsturtevant commented 1 year ago

/lgtm