Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Update FinalizeCredential, use in add-credential #6265
Conversation
| + "somecloud": { | ||
| + AuthCredentials: map[string]jujucloud.Credential{ | ||
| + "fred": jujucloud.NewCredential(jujucloud.UserPassAuthType, map[string]string{ | ||
| + "username": "user", |
anastasiamac
Sep 16, 2016
Member
I know it's a test and this is test data but username = "user" caught me off-guard \o/
Could this be "username" too for clarity?
| @@ -35,7 +35,8 @@ func formatControllersTabular(writer io.Writer, set ControllerSet, promptRefresh | ||
| w := output.Wrapper{tw} | ||
| if promptRefresh && len(set.Controllers) > 0 { | ||
| - fmt.Fprintln(writer, "Use --refresh to see the latest information.\n") | ||
| + fmt.Fprintln(writer, "Use --refresh to see the latest information.") | ||
| + fmt.Fprintln(writer) |
| + CloudName string | ||
| + | ||
| + // CloudRegion is the name of the region that the user has specified. | ||
| + // If this is empty, then GetCredentials will determine the default |
anastasiamac
Sep 16, 2016
Member
Could we please be a bit more precise of what "default" means here? Maybe even say that it's the first region in a list that is sorted in alphabetical order? :D
| + | ||
| + cloudEndpoint := args.Cloud.Endpoint | ||
| + cloudIdentityEndpoint := args.Cloud.IdentityEndpoint | ||
| + if regionName != "" { |
anastasiamac
Sep 16, 2016
Member
From the reading of above code, the possibility that regionName == "" is slim. Either we do not need this check or we need to do something for the case if it is "". No?
axw
Sep 16, 2016
Member
regionName will be "" if there are no regions in the cloud. I've added more tests.
anastasiamac
approved these changes
Sep 16, 2016
Code looks awesome \o/
+1 from me
However, you might also get a stamp from someone with Azure account to verify with QA steps.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
axw commentedSep 16, 2016
This diff updates the FinalizeCredential provider
method to take cloud endpoints, so that providers
can communicate with the cloud in order to finalize
the credential. Specifically, Azure needs to connect
to Active Directory to perform interactive authentication,
and then configure the service principal.
QA
With a follow-up branch that adds interactive auth-type
for azure, I have done the following:
sets up the service principal
It is also possible, even if undesirable, to add an
interactive auth-type credential in credentials.yaml by
hand, and go through the above steps at bootstrap or
add-model time. The service principal password will not
be persisted to disk then.