Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--force for update- and remove-credential CLI. #10968

Merged
merged 1 commit into from Dec 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 17 additions & 8 deletions api/cloud/cloud.go
Expand Up @@ -96,18 +96,27 @@ func (c *Client) UserCredentials(user names.UserTag, cloud names.CloudTag) ([]na

// UpdateCloudsCredentials updates clouds credentials content on the controller.
// Passed in credentials are keyed on the credential tag.
func (c *Client) UpdateCloudsCredentials(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
var tagged []params.TaggedCredential
// This operation can be forced to ignore validation checks.
func (c *Client) UpdateCloudsCredentials(cloudCredentials map[string]jujucloud.Credential, force bool) ([]params.UpdateCredentialResult, error) {
return c.internalUpdateCloudsCredentials(params.UpdateCredentialArgs{Force: force}, cloudCredentials)
}

// AddCloudsCredentials adds/uploads clouds credentials content to the controller.
// Passed in credentials are keyed on the credential tag.
func (c *Client) AddCloudsCredentials(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
return c.internalUpdateCloudsCredentials(params.UpdateCredentialArgs{}, cloudCredentials)
}

func (c *Client) internalUpdateCloudsCredentials(in params.UpdateCredentialArgs, cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
for tag, credential := range cloudCredentials {
tagged = append(tagged, params.TaggedCredential{
in.Credentials = append(in.Credentials, params.TaggedCredential{
Tag: tag,
Credential: params.CloudCredential{
AuthType: string(credential.AuthType()),
Attributes: credential.Attributes(),
},
})
}
in := params.UpdateCredentialArgs{Credentials: tagged}
count := len(cloudCredentials)

countErr := func(got int) error {
Expand All @@ -127,7 +136,7 @@ func (c *Client) UpdateCloudsCredentials(cloudCredentials map[string]jujucloud.C
}
converted := make([]params.UpdateCredentialResult, count)
for i, one := range out.Results {
converted[i] = params.UpdateCredentialResult{CredentialTag: tagged[i].Tag, Error: one.Error}
converted[i] = params.UpdateCredentialResult{CredentialTag: in.Credentials[i].Tag, Error: one.Error}
}
return converted, nil
}
Expand All @@ -146,7 +155,7 @@ func (c *Client) UpdateCloudsCredentials(cloudCredentials map[string]jujucloud.C
// stored on the controller. This call validates that the new content works
// for all models that are using this credential.
func (c *Client) UpdateCredentialsCheckModels(tag names.CloudCredentialTag, credential jujucloud.Credential) ([]params.UpdateCredentialModelResult, error) {
out, err := c.UpdateCloudsCredentials(map[string]jujucloud.Credential{tag.String(): credential})
out, err := c.UpdateCloudsCredentials(map[string]jujucloud.Credential{tag.String(): credential}, false)
if err != nil {
return nil, errors.Trace(err)
}
Expand All @@ -158,7 +167,7 @@ func (c *Client) UpdateCredentialsCheckModels(tag names.CloudCredentialTag, cred
}

// RevokeCredential revokes/deletes a cloud credential.
func (c *Client) RevokeCredential(tag names.CloudCredentialTag) error {
func (c *Client) RevokeCredential(tag names.CloudCredentialTag, force bool) error {
var results params.ErrorResults

if c.facade.BestAPIVersion() < 3 {
Expand All @@ -175,7 +184,7 @@ func (c *Client) RevokeCredential(tag names.CloudCredentialTag) error {

args := params.RevokeCredentialArgs{
Credentials: []params.RevokeCredentialArg{
{Tag: tag.String()},
{Tag: tag.String(), Force: force},
},
}
if err := c.facade.FacadeCall("RevokeCredentialsCheckModels", args, &results); err != nil {
Expand Down
66 changes: 53 additions & 13 deletions api/cloud/cloud_test.go
Expand Up @@ -455,7 +455,7 @@ func (s *cloudSuite) TestRevokeCredential(c *gc.C) {
c.Assert(result, gc.FitsTypeOf, &params.ErrorResults{})
c.Assert(a, jc.DeepEquals, params.RevokeCredentialArgs{
Credentials: []params.RevokeCredentialArg{
{Tag: "cloudcred-foo_bob_bar"},
{Tag: "cloudcred-foo_bob_bar", Force: true},
},
})
*result.(*params.ErrorResults) = params.ErrorResults{
Expand All @@ -472,7 +472,7 @@ func (s *cloudSuite) TestRevokeCredential(c *gc.C) {

client := cloudapi.NewClient(apiCaller)
tag := names.NewCloudCredentialTag("foo/bob/bar")
err := client.RevokeCredential(tag)
err := client.RevokeCredential(tag, true)
c.Assert(err, jc.ErrorIsNil)
c.Assert(s.called, jc.IsTrue)
}
Expand Down Expand Up @@ -505,7 +505,7 @@ func (s *cloudSuite) TestRevokeCredentialV2(c *gc.C) {

client := cloudapi.NewClient(apiCaller)
tag := names.NewCloudCredentialTag("foo/bob/bar")
err := client.RevokeCredential(tag)
err := client.RevokeCredential(tag, false)
c.Assert(err, jc.ErrorIsNil)
c.Assert(s.called, jc.IsTrue)
}
Expand Down Expand Up @@ -1087,6 +1087,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentials(c *gc.C) {
c.Check(request, gc.Equals, "UpdateCredentialsCheckModels")
c.Assert(result, gc.FitsTypeOf, &params.UpdateCredentialResults{})
c.Assert(a, jc.DeepEquals, params.UpdateCredentialArgs{
Force: true,
Credentials: []params.TaggedCredential{{
Tag: "cloudcred-foo_bob_bar0",
Credential: params.CloudCredential{
Expand All @@ -1107,7 +1108,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentials(c *gc.C) {
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.UpdateCloudsCredentials(createCredentials(1))
result, err := client.UpdateCloudsCredentials(createCredentials(1), true)
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.DeepEquals, []params.UpdateCredentialResult{{}})
c.Assert(s.called, jc.IsTrue)
Expand All @@ -1132,7 +1133,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsErrorV2(c *gc.C) {
BestVersion: 2,
}
client := cloudapi.NewClient(apiCaller)
errs, err := client.UpdateCloudsCredentials(createCredentials(1))
errs, err := client.UpdateCloudsCredentials(createCredentials(1), true)
c.Assert(err, jc.ErrorIsNil)
c.Assert(errs, gc.DeepEquals, []params.UpdateCredentialResult{
{CredentialTag: "cloudcred-foo_bob_bar0", Error: &params.Error{Message: "validation failure", Code: ""}},
Expand Down Expand Up @@ -1163,7 +1164,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsError(c *gc.C) {
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
errs, err := client.UpdateCloudsCredentials(createCredentials(1))
errs, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, jc.ErrorIsNil)
c.Assert(errs, gc.DeepEquals, []params.UpdateCredentialResult{
{CredentialTag: "cloudcred-foo_bob_bar0", Error: common.ServerError(errors.New("validation failure"))},
Expand All @@ -1187,7 +1188,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsCallErrorV2(c *gc.C) {
BestVersion: 2,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.UpdateCloudsCredentials(createCredentials(1))
result, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, gc.ErrorMatches, "scary but true")
c.Assert(result, gc.IsNil)
c.Assert(s.called, jc.IsTrue)
Expand All @@ -1209,7 +1210,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsCallError(c *gc.C) {
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.UpdateCloudsCredentials(createCredentials(1))
result, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, gc.ErrorMatches, "scary but true")
c.Assert(result, gc.IsNil)
c.Assert(s.called, jc.IsTrue)
Expand Down Expand Up @@ -1237,7 +1238,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsManyResultsV2(c *gc.C) {
BestVersion: 2,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.UpdateCloudsCredentials(createCredentials(1))
result, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, gc.ErrorMatches, `expected 1 result got 2 when updating credentials`)
c.Assert(result, gc.IsNil)
c.Assert(s.called, jc.IsTrue)
Expand Down Expand Up @@ -1266,7 +1267,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsManyMatchingResultsV2(c *gc.C) {
}
client := cloudapi.NewClient(apiCaller)
in := createCredentials(2)
result, err := client.UpdateCloudsCredentials(in)
result, err := client.UpdateCloudsCredentials(in, false)
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.HasLen, len(in))
for _, one := range result {
Expand Down Expand Up @@ -1297,7 +1298,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsManyResults(c *gc.C) {
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.UpdateCloudsCredentials(createCredentials(1))
result, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, gc.ErrorMatches, `expected 1 result got 2 when updating credentials`)
c.Assert(result, gc.IsNil)
c.Assert(s.called, jc.IsTrue)
Expand Down Expand Up @@ -1325,7 +1326,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsManyMatchingResults(c *gc.C) {
}
client := cloudapi.NewClient(apiCaller)
count := 2
result, err := client.UpdateCloudsCredentials(createCredentials(count))
result, err := client.UpdateCloudsCredentials(createCredentials(count), false)
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.HasLen, count)
c.Assert(s.called, jc.IsTrue)
Expand Down Expand Up @@ -1363,7 +1364,7 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsModelErrors(c *gc.C) {
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
errs, err := client.UpdateCloudsCredentials(createCredentials(1))
errs, err := client.UpdateCloudsCredentials(createCredentials(1), false)
c.Assert(err, jc.ErrorIsNil)
c.Assert(errs, gc.DeepEquals, []params.UpdateCredentialResult{
{CredentialTag: "cloudcred-foo_bob_bar",
Expand All @@ -1380,3 +1381,42 @@ func (s *cloudSuite) TestUpdateCloudsCredentialsModelErrors(c *gc.C) {
})
c.Assert(s.called, jc.IsTrue)
}

func (s *cloudSuite) TestAddCloudsCredentials(c *gc.C) {
apiCaller := basetesting.BestVersionCaller{
APICallerFunc: basetesting.APICallerFunc(
func(objType string,
version int,
id, request string,
a, result interface{},
) error {
c.Check(objType, gc.Equals, "Cloud")
c.Check(id, gc.Equals, "")
c.Check(request, gc.Equals, "UpdateCredentialsCheckModels")
c.Assert(result, gc.FitsTypeOf, &params.UpdateCredentialResults{})
c.Assert(a, jc.DeepEquals, params.UpdateCredentialArgs{
Credentials: []params.TaggedCredential{{
Tag: "cloudcred-foo_bob_bar0",
Credential: params.CloudCredential{
AuthType: "userpass",
Attributes: map[string]string{
"username": "admin",
"password": "adm1n",
},
},
}}})
*result.(*params.UpdateCredentialResults) = params.UpdateCredentialResults{
Results: []params.UpdateCredentialResult{{}},
}
s.called = true
return nil
},
),
BestVersion: 3,
}
client := cloudapi.NewClient(apiCaller)
result, err := client.AddCloudsCredentials(createCredentials(1))
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.DeepEquals, []params.UpdateCredentialResult{{}})
c.Assert(s.called, jc.IsTrue)
}
16 changes: 8 additions & 8 deletions apiserver/common/credentialcommon/modelcredential.go
Expand Up @@ -18,7 +18,7 @@ import (
)

// ValidateExistingModelCredential checks if the cloud credential that a given model uses is valid for it.
func ValidateExistingModelCredential(backend PersistentBackend, callCtx context.ProviderCallContext, migrating bool) (params.ErrorResults, error) {
func ValidateExistingModelCredential(backend PersistentBackend, callCtx context.ProviderCallContext, checkCloudInstances bool) (params.ErrorResults, error) {
model, err := backend.Model()
if err != nil {
return params.ErrorResults{}, errors.Trace(err)
Expand All @@ -38,11 +38,11 @@ func ValidateExistingModelCredential(backend PersistentBackend, callCtx context.
return params.ErrorResults{}, errors.NotValidf("credential %q", storedCredential.Name)
}
credential := cloud.NewCredential(cloud.AuthType(storedCredential.AuthType), storedCredential.Attributes)
return ValidateNewModelCredential(backend, callCtx, credentialTag, &credential, migrating)
return ValidateNewModelCredential(backend, callCtx, credentialTag, &credential, checkCloudInstances)
}

// ValidateNewModelCredential checks if a new cloud credential could be valid for a given model.
func ValidateNewModelCredential(backend PersistentBackend, callCtx context.ProviderCallContext, credentialTag names.CloudCredentialTag, credential *cloud.Credential, migrating bool) (params.ErrorResults, error) {
func ValidateNewModelCredential(backend PersistentBackend, callCtx context.ProviderCallContext, credentialTag names.CloudCredentialTag, credential *cloud.Credential, checkCloudInstances bool) (params.ErrorResults, error) {
openParams, err := buildOpenParams(backend, credentialTag, credential)
if err != nil {
return params.ErrorResults{}, errors.Trace(err)
Expand All @@ -55,7 +55,7 @@ func ValidateNewModelCredential(backend PersistentBackend, callCtx context.Provi
case state.ModelTypeCAAS:
return checkCAASModelCredential(openParams)
case state.ModelTypeIAAS:
return checkIAASModelCredential(openParams, backend, callCtx, migrating)
return checkIAASModelCredential(openParams, backend, callCtx, checkCloudInstances)
default:
return params.ErrorResults{}, errors.NotSupportedf("model type %q", model.Type())
}
Expand All @@ -74,7 +74,7 @@ func checkCAASModelCredential(brokerParams environs.OpenParams) (params.ErrorRes
return params.ErrorResults{}, nil
}

func checkIAASModelCredential(openParams environs.OpenParams, backend PersistentBackend, callCtx context.ProviderCallContext, migrating bool) (params.ErrorResults, error) {
func checkIAASModelCredential(openParams environs.OpenParams, backend PersistentBackend, callCtx context.ProviderCallContext, checkCloudInstances bool) (params.ErrorResults, error) {
env, err := newEnv(openParams)
if err != nil {
return params.ErrorResults{}, errors.Trace(err)
Expand All @@ -83,13 +83,13 @@ func checkIAASModelCredential(openParams environs.OpenParams, backend Persistent
// In the future, this check may be extended to other cloud resources,
// entities and operation-level authorisations such as interfaces,
// ability to CRUD storage, etc.
return checkMachineInstances(backend, env, callCtx, migrating)
return checkMachineInstances(backend, env, callCtx, checkCloudInstances)
}

// checkMachineInstances compares model machines from state with
// the ones reported by the provider using supplied credential.
// This only makes sense for non-k8s providers.
func checkMachineInstances(backend PersistentBackend, provider CloudProvider, callCtx context.ProviderCallContext, migrating bool) (params.ErrorResults, error) {
func checkMachineInstances(backend PersistentBackend, provider CloudProvider, callCtx context.ProviderCallContext, checkCloudInstances bool) (params.ErrorResults, error) {
fail := func(original error) (params.ErrorResults, error) {
return params.ErrorResults{}, original
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func checkMachineInstances(backend PersistentBackend, provider CloudProvider, ca
for _, instance := range instances {
id := string(instance.Id())
instanceIds.Add(id)
if migrating {
if checkCloudInstances {
if _, found := machinesByInstance[id]; !found {
results = append(results, serverError(errors.Errorf("no machine with instance %q", id)))
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/cloud/addcredential.go
Expand Up @@ -642,7 +642,7 @@ func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[s
return err
}
defer client.Close()
results, err := client.UpdateCloudsCredentials(verified)
results, err := client.AddCloudsCredentials(verified)
if err != nil {
logger.Errorf("%v", err)
ctxt.Warningf("Could not upload credentials to controller %q", c.ControllerName)
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/cloud/addcredential_test.go
Expand Up @@ -851,7 +851,7 @@ credentials:
sourceFile := s.createTestCredentialFile(c, expectedContents)

called := false
s.api.updateCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
s.api.addCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
c.Assert(cloudCredentials, gc.HasLen, 1)
called = true
expectedTag := names.NewCloudCredentialTag(fmt.Sprintf("%v/admin@local/blah", cloudName)).String()
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/cloud/detectcredentials.go
Expand Up @@ -491,7 +491,7 @@ func (c *detectCredentialsCommand) addRemoteCredentials(ctxt *cmd.Context, cloud
if len(verified) == 0 {
return erred
}
result, err := client.UpdateCloudsCredentials(verified)
result, err := client.AddCloudsCredentials(verified)
if err != nil {
logger.Errorf("%v", err)
ctxt.Warningf("Could not upload credentials to controller %q", c.ControllerName)
Expand Down
4 changes: 2 additions & 2 deletions cmd/juju/cloud/detectcredentials_test.go
Expand Up @@ -372,7 +372,7 @@ func (s *detectCredentialsSuite) TestRemoteLoad(c *gc.C) {
s.setupStore(c)
cloudName := "test-cloud"
called := false
s.api.updateCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
s.api.addCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
c.Assert(cloudCredentials, gc.HasLen, 1)
called = true
expectedTag := names.NewCloudCredentialTag(fmt.Sprintf("%v/admin@local/blah", cloudName)).String()
Expand Down Expand Up @@ -446,7 +446,7 @@ func (s *detectCredentialsSuite) assertAutoloadCredentials(c *gc.C, expectedStde
s.setupStore(c)
cloudName := "test-cloud"
called := false
s.api.updateCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
s.api.addCloudsCredentials = func(cloudCredentials map[string]jujucloud.Credential) ([]params.UpdateCredentialResult, error) {
c.Assert(cloudCredentials, gc.HasLen, 1)
called = true
expectedTag := names.NewCloudCredentialTag(fmt.Sprintf("%v/admin@local/blah", cloudName)).String()
Expand Down