Added additional OpenStack credential flags #38
Conversation
afritzler
changed the title
gardener-robot-ci-1
added
reviewed/ok-to-test
afritzler
changed the title
gardener-robot-ci-1
added
reviewed/ok-to-test
gardener-robot-ci-1
added
the
reviewed/ok-to-test
|
@MartinWeindel I now removed the validation logic leaving it to gophercloud/OpenStack backend to provider a proper error handling. |
| @@ -38,6 +39,7 @@ type Handler struct { | |||
|
|
|||
| var _ provider.DNSHandler = &Handler{} | |||
|
|
|||
| // NewHandler constructs a new DNSHandler object. | |||
| func NewHandler(config *provider.DNSHandlerConfig) (provider.DNSHandler, error) { | |||
| authConfig, err := readAuthConfig(config) | |||
There was a problem hiding this comment.
There is no need to use the readAuthConfig function, since clientconfig.AuthOptions already fetches env parameters. I also don't see a reason why do you need to keep the authConfig struct. Take a look on this commit: 02d50ec
There was a problem hiding this comment.
The properties are read from a Kubernetes secret. Although the code may look similar to reading environmental parameters, this is not the case here. We only use the same names in the secret data. And the secrets are associated with the DNS providers, which can be managed during the runtime of the controller-manager.
There was a problem hiding this comment.
Thanks, I missed this part. Nevertheless, I don't like the current situation with the auth options. It looks even worse than in https://github.com/kubernetes/cloud-provider-openstack. I propose to create a package, which will handle all the auth options in one place.
There was a problem hiding this comment.
That's true, it looks a bit complicated now. I suggest to apply these changes to the pull request:
88eeb96
There was a problem hiding this comment.
Can we keep that for now as is?
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
gardener-robot-ci-1
added
reviewed/ok-to-test
What this PR does / why we need it:
This PR adds some additional authentication flags to the designate client code. It now support the following values:
userDomanName,OS_USER_DOMAIN_NAMEuserDomainID,OS_USER_DOMAIN_IDtenantID,OS_PROJECT_IDdomainID,OS_DOMAIN_IDWhich issue(s) this PR fixes:
This PR is needed for the overall extension support of this flags: https://github.com/gardener/gardener-extensions/issues/154
Release note: