Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Remove cloud references from file if last credential is removed. #7025
Conversation
| + c.Assert(err, jc.ErrorIsNil) | ||
| + | ||
| + _, exists := creds[s.cloudName] | ||
| + c.Assert(creds[s.cloudName].AuthCredentials, gc.HasLen, 0) |
axw
Feb 23, 2017
Member
this line is redundant. creds[s.cloudName] will have the zero value, as proven by c.Assert(exists, jc.IsFalse)
| + | ||
| + // If cloud has no credentials now, delete cloud references from | ||
| + // credentials file as well. | ||
| + if len(all[cloudName].AuthCredentials) == 0 { |
axw
Feb 23, 2017
Member
rather than assigning to all (above on line 631), then checking it and maybe deleting from it:
if len(details.AuthCredentials) > 0 {
all[cloudName] = details
} else {
delete(all, cloudName)
}| @@ -301,6 +301,11 @@ func (c *MemStore) RemoveAccount(controllerName string) error { | ||
| // UpdateCredential implements CredentialsUpdater. | ||
| func (c *MemStore) UpdateCredential(cloudName string, details cloud.CloudCredential) error { | ||
| c.Credentials[cloudName] = details | ||
| + // If cloud has no credentials now, delete cloud references from | ||
| + // credentials file as well. | ||
| + if len(c.Credentials[cloudName]) == 0 { |
|
$$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 |
jujubot
merged commit d7f0644
into
juju:develop
Feb 23, 2017
anastasiamac
deleted the
anastasiamac:credential-delete-lp1590237
branch
Feb 23, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
anastasiamac commentedFeb 23, 2017
Please provide the following details to expedite Pull Request review:
Description of change
When remove credential deletes last credential for a cloud, there is no need to keep reference for a cloud in credentials.yaml file.
In fact, it causes confusion as per the bug.
QA steps
Success criteria:
a. listing credentials does not show message to re-run with --show-secrets flag
b. listing credential with --show-secrets does not list the cloud
c. credentials.yaml does not reference delete cloud
Documentation changes
No documentation changes are needed.
Bug reference
https://bugs.launchpad.net/juju/+bug/1590237