From 62358b0be258d82049ae77b0b5c7bc5cd78cbfc0 Mon Sep 17 00:00:00 2001 From: Anastasia Macmood Date: Thu, 5 Sep 2019 11:48:19 +1000 Subject: [PATCH 1/5] add-credential ask-or-tell --- cmd/juju/cloud/add.go | 18 ++- cmd/juju/cloud/addcredential.go | 145 +++++++++++++---------- cmd/juju/cloud/addcredential_test.go | 49 ++++++-- cmd/juju/cloud/detectcredentials.go | 4 +- cmd/juju/cloud/detectcredentials_test.go | 2 +- cmd/juju/cloud/updatecredential.go | 22 ++-- cmd/juju/cloud/updatecredential_test.go | 4 +- 7 files changed, 148 insertions(+), 96 deletions(-) diff --git a/cmd/juju/cloud/add.go b/cmd/juju/cloud/add.go index 55996f5de04..8fa777eae36 100644 --- a/cmd/juju/cloud/add.go +++ b/cmd/juju/cloud/add.go @@ -71,10 +71,15 @@ positional argument: When is provided with , Juju stores that definition in the current controller (after validating the contents), or the specified controller if ---controller is used. To make use of this multi-cloud feature, -the controller needs to have the "multi-cloud" feature flag turned on. +--controller is used. -If --local is used, Juju stores that definition its internal cache directly. +If a current controller is detected, Juju will prompt the user to confirm +whether this new cloud also needs to be uploaded. +Use --no-prompt option when this prompt is undesirable, but the upload to +the current controller is wanted. +Use --controller option to upload a cloud to a different controller. + +Use --local option to add cloud to the current device only. DEPRECATED If already exists in Juju's cache, then the `[1:] + "`--replace`" + ` option is required. Use 'update-credential' instead. @@ -108,8 +113,9 @@ When a a running controller is updated, the credential for the cloud is also uploaded. As with the cloud, the credential needs to have been added to the local Juju cache; add-credential is used to do that. If there's only one credential for the cloud it will be -uploaded to the controller. If the cloud has multiple local credentials -you can specify which to upload with the --credential option. +uploaded to the controller automatically by add-clloud command. +However, if the cloud has multiple local credentials you can specify +which to upload with the --credential option. When adding clouds to a controller, some clouds are whitelisted and can be easily added: %v @@ -166,7 +172,6 @@ type AddCloudCommand struct { cloudMetadataStore CloudMetadataStore // These attributes are used when adding a cloud to a controller. - controllerName string credentialName string addCloudAPIFunc func() (AddCloudAPI, error) @@ -221,6 +226,7 @@ func (c *AddCloudCommand) SetFlags(f *gnuflag.FlagSet) { f.BoolVar(&c.Replace, "replace", false, "DEPRECATED: Overwrite any existing cloud information for ") f.BoolVar(&c.Force, "force", false, "Force add cloud to the controller") f.StringVar(&c.CloudFile, "f", "", "The path to a cloud definition file") + f.StringVar(&c.CloudFile, "file", "", "The path to a cloud definition file") f.StringVar(&c.credentialName, "credential", "", "Credential to use for new cloud") } diff --git a/cmd/juju/cloud/addcredential.go b/cmd/juju/cloud/addcredential.go index bb67f8b69d3..e62abb60234 100644 --- a/cmd/juju/cloud/addcredential.go +++ b/cmd/juju/cloud/addcredential.go @@ -25,7 +25,8 @@ import ( ) var usageAddCredentialSummary = ` -Adds or replaces credentials for a cloud, stored locally on this client.`[1:] +Adds a credential for a cloud, stored locally on this client, or +uploads a credential for a cloud remotely, stored on the controller.`[1:] var usageAddCredentialDetails = ` The juju add-credential command operates in two modes. @@ -36,16 +37,9 @@ the cloud provider. Providing the ` + "`-f ` " + `option switches to the non-interactive mode. must be a path to a correctly -formatted YAML-formatted file. Details of the format are provided at -"About credentials.yaml" below. +formatted YAML-formatted file. -The ` + "`--replace`" + ` option is required if credential information -for the named cloud already exists locally. All such information will be -overwritten. This option is DEPRECATED, use 'juju update-credential' instead. - -About credentials.yaml: -Here is a sample credentials.yaml showing four credentials being stored -against three clouds: +Sample yaml file shows four credentials being stored against three clouds: credentials: aws: @@ -67,22 +61,19 @@ against three clouds: auth-type: interactive trust-password: -More generally, here is a loosely defined grammar for credentials.yaml: - - credentials: - : - : - auth-type: - : - [: ] - -Every requires its own and -pairs. - The parameter of each credential is arbitrary, but must be unique within each . This allows allow each cloud to store multiple credentials. +The format for a credential is cloud-specific. Thus, it's best to use +'add-credential' command in an interactive mode. This will result in +adding this new credential locally and / or uploading it to a controller +in a correct format for the desired cloud. + +The ` + "`--replace`" + ` option is required if credential information +for the named cloud already exists locally. All such information will be +overwritten. This option is DEPRECATED, use 'juju update-credential' instead. + Examples: juju add-credential google juju add-credential google --local @@ -90,6 +81,7 @@ Examples: juju add-credential aws -f ~/credentials.yaml -c mycontroller juju add-credential aws -f ~/credentials.yaml juju add-credential aws -f ~/credentials.yaml --local + juju add-credential aws -f ~/credentials.yaml --no-prompt Notes: If you are setting up Juju for the first time, consider running @@ -100,9 +92,15 @@ This command does not set default regions nor default credentials for the cloud. The commands ` + "`juju default-region`" + ` and ` + "`juju default-credential`" + ` provide that functionality. -By default, after validating the contents, credentials are added both -to the current controller and the current client device. -Use --controller option to add credentials to a different controller. +By default, after validating the contents, Juju will add a credential locally, +to the current client device, and will upload it remotely, to a controller. + +If a current controller is detected, Juju will prompt the user to confirm +whether this new credential also needs to be uploaded. +Use --no-prompt option when this prompt is undesirable, but the upload to +the current controller is wanted. +Use --controller option to upload a credential to a different controller. + Use --local option to add credentials to the current device only. Further help: @@ -112,6 +110,7 @@ instructions. See also: credentials remove-credential + update-credential default-credential default-region autoload-credentials @@ -136,9 +135,11 @@ type addCredentialCommand struct { Region string // These attributes are used when adding credentials to a controller. - controllerName string remoteCloudFound bool credentialAPIFunc func() (CredentialAPI, error) + + // existsLocally whether this credential already exists locally. + existsLocally bool } // NewAddCredentialCommand returns a command to add credential information. @@ -165,8 +166,9 @@ func (c *addCredentialCommand) Info() *cmd.Info { func (c *addCredentialCommand) SetFlags(f *gnuflag.FlagSet) { c.OptionalControllerCommand.SetFlags(f) - f.BoolVar(&c.Replace, "replace", false, "Overwrite existing credential information") + f.BoolVar(&c.Replace, "replace", false, "DEPRECATED: Overwrite existing credential information") f.StringVar(&c.CredentialsFile, "f", "", "The YAML file containing credentials to add") + f.StringVar(&c.CredentialsFile, "file", "", "The YAML file containing credentials to add") f.StringVar(&c.Region, "region", "", "Cloud region that credential is valid for") } @@ -175,15 +177,6 @@ func (c *addCredentialCommand) Init(args []string) (err error) { return errors.New("Usage: juju add-credential [-f ]") } c.CloudName = args[0] - c.ControllerName, err = c.ControllerNameFromArg() - if err != nil && errors.Cause(err) != modelcmd.ErrNoControllersDefined { - return errors.Trace(err) - } - if c.ControllerName == "" { - // No controller was specified explicitly and we did not detect a current controller, - // this operation should be local only. - c.Local = true - } return cmd.CheckEmpty(args[1:]) } @@ -193,9 +186,19 @@ func (c *addCredentialCommand) Run(ctxt *cmd.Context) error { // https://bugs.launchpad.net/juju/+bug/1821279 ctxt.Warningf("--replace is DEPRECATED. Use 'juju update-credential' to update credentials.") } - // Check that the supplied cloud is valid. + var err error - if !c.Local { + if !c.Local && c.ControllerName == "" { + // The user may have specified the controller via a --controller option. + // If not, let's see if there is a current controller that can be detected. + c.ControllerName, err = c.MaybePromptCurrentController(ctxt, "add a credential to") + if err != nil { + return errors.Trace(err) + } + } + + // Check that the supplied cloud is valid. + if !c.Local && c.ControllerName != "" { if err := c.maybeRemoteCloud(ctxt); err != nil { if !errors.IsNotFound(err) { logger.Errorf("%v", err) @@ -278,11 +281,6 @@ func (c *addCredentialCommand) Run(ctxt *cmd.Context) error { if !names.IsValidCloudCredentialName(name) { return errors.Errorf("%q is not a valid credential name", name) } - - if _, ok := existingCredentials.AuthCredentials[name]; ok { - ctxt.Warningf("credential %q for cloud %q already exists locally, use 'juju update-credential %v %v' to update it", name, c.CloudName, c.CloudName, name) - continue - } if !validAuthType(cred.AuthType()) { return errors.Errorf("credential %q contains invalid auth type %q, valid auth types for cloud %q are %v", name, cred.AuthType(), c.CloudName, c.cloud.AuthTypes) } @@ -300,23 +298,48 @@ func (c *addCredentialCommand) Run(ctxt *cmd.Context) error { cred = *newCredential } + added[name] = cred + if _, ok := existingCredentials.AuthCredentials[name]; ok { + ctxt.Warningf("credential %q for cloud %q already exists locally, use 'juju update-credential %v %v' to update it", name, c.CloudName, c.CloudName, name) + continue + } + existingCredentials.AuthCredentials[name] = cred allNames = append(allNames, name) - added[name] = cred } + return c.internalAddCredential(ctxt, "added", *existingCredentials, added, allNames) +} + +func (c *addCredentialCommand) internalAddCredential(ctxt *cmd.Context, verb string, existingCredentials jujucloud.CloudCredential, added map[string]jujucloud.Credential, allNames []string) error { + var err error + // Local processing. if len(allNames) == 0 { fmt.Fprintf(ctxt.Stdout, "No local credentials for cloud %q changed.\n", c.CloudName) - return nil - } - err = c.Store.UpdateCredential(c.CloudName, *existingCredentials) - if err != nil { - return err + } else { + var msg string + if len(allNames) == 1 { + msg = fmt.Sprintf(" %q", allNames[0]) + } else { + msg = fmt.Sprintf("s %q", strings.Join(allNames, ", ")) + } + err = c.Store.UpdateCredential(c.CloudName, existingCredentials) + if err == nil { + fmt.Fprintf(ctxt.Stdout, "Credential%s %s locally for cloud %q.\n\n", msg, verb, c.CloudName) + } else { + fmt.Fprintf(ctxt.Stdout, "Credential%s not %v locally for cloud %q: %v\n\n", msg, verb, c.CloudName, err) + err = cmd.ErrSilent + } } - fmt.Fprintf(ctxt.Stdout, "Credentials %q added locally for cloud %q.\n", strings.Join(allNames, ", "), c.CloudName) + // Remote processing. if !c.Local { - return c.addRemoteCredentials(ctxt, added) + if c.ControllerName != "" { + return c.addRemoteCredentials(ctxt, added, err) + } else { + ctxt.Infof("There are no controllers specified - not adding a credential to any controller.") + return err + } } - return nil + return err } func (c *addCredentialCommand) existingCredentialsForCloud() (*jujucloud.CloudCredential, error) { @@ -392,15 +415,7 @@ func (c *addCredentialCommand) interactiveAddCredential(ctxt *cmd.Context, schem } existingCredentials.AuthCredentials[credentialName] = *newCredential - err = c.Store.UpdateCredential(c.CloudName, *existingCredentials) - if err != nil { - return errors.Trace(err) - } - fmt.Fprintf(ctxt.Stdout, "Credential %q %v locally for cloud %q.\n\n", credentialName, verb, c.CloudName) - if !c.Local { - return c.addRemoteCredentials(ctxt, map[string]jujucloud.Credential{credentialName: *newCredential}) - } - return nil + return c.internalAddCredential(ctxt, verb, *existingCredentials, map[string]jujucloud.Credential{credentialName: *newCredential}, []string{credentialName}) } func finalizeProvider(ctxt *cmd.Context, cloud *jujucloud.Cloud, regionName, defaultRegion string, authType jujucloud.AuthType, attrs map[string]string) (*jujucloud.Credential, error) { @@ -612,17 +627,17 @@ func (c *addCredentialCommand) maybeRemoteCloud(ctxt *cmd.Context) error { return nil } -func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[string]jujucloud.Credential) error { +func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[string]jujucloud.Credential, localError error) error { if len(all) == 0 { fmt.Fprintf(ctxt.Stdout, "No remote credentials for cloud %q added.\n", c.CloudName) - return nil + return localError } if !c.remoteCloudFound { fmt.Fprintf(ctxt.Stdout, "No remote cloud %v found on the controller %v: credentials are not added remotely.\n"+ "Use 'juju clouds -c %v' to see what clouds are available remotely.\n"+ "User 'juju add-cloud %v -c %v' to add your cloud to the controller.\n", c.CloudName, c.ControllerName, c.ControllerName, c.CloudName, c.ControllerName) - return nil + return localError } accountDetails, err := c.Store.AccountDetails(c.ControllerName) @@ -643,7 +658,7 @@ func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[s logger.Errorf("%v", err) ctxt.Warningf("Could not add credentials remotely, on controller %q", c.ControllerName) } - return processUpdateCredentialResult(ctxt, accountDetails, "added", results) + return processUpdateCredentialResult(ctxt, accountDetails, "added", results, c.ControllerName, localError) } func enterFile(name, descr string, p *interact.Pollster, expanded, optional bool) (string, error) { diff --git a/cmd/juju/cloud/addcredential_test.go b/cmd/juju/cloud/addcredential_test.go index c02f65c0783..989f7b110eb 100644 --- a/cmd/juju/cloud/addcredential_test.go +++ b/cmd/juju/cloud/addcredential_test.go @@ -254,7 +254,7 @@ Enter password: Credential "fred" added locally for cloud "somecloud". `[1:]) - c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "") + c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "There are no controllers specified - not adding a credential to any controller.\n") } func (s *addCredentialSuite) TestAddInteractiveInvalidRegionEntered(c *gc.C) { @@ -278,7 +278,7 @@ Enter password: Credential "fred" added locally for cloud "somecloud". `[1:]) - c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "") + c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "There are no controllers specified - not adding a credential to any controller.\n") } func (s *addCredentialSuite) TestAddInteractiveRegionSpecified(c *gc.C) { @@ -297,7 +297,7 @@ Enter password: Credential "fred" added locally for cloud "somecloud". `[1:]) - c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "") + c.Assert(cmdtesting.Stderr(ctxt), gc.Equals, "There are no controllers specified - not adding a credential to any controller.\n") } func (s *addCredentialSuite) assertCredentialAdded(c *gc.C, input string, args []string, specifiedRegion, expectedRegion string) *cmd.Context { @@ -836,7 +836,7 @@ Credential "blah" added locally for cloud "somecloud". `[1:] stderr := ` Using remote cloud "somecloud" from the controller to verify credentials. -Controller credential "blah" for user "admin@local" on cloud "somecloud" added. +Controller credential "blah" for user "admin@local" for cloud "somecloud" on controller "controller" added. For more information, see ‘juju show-credential somecloud blah’. `[1:] @@ -844,6 +844,10 @@ For more information, see ‘juju show-credential somecloud blah’. } func (s *addCredentialSuite) assertAddedCredentialForCloud(c *gc.C, cloudName, expectedStdout, expectedStderr string, uploaded bool) { + s.assertAddedCredentialForCloudWithArgs(c, cloudName, expectedStdout, "", expectedStderr, uploaded, "--no-prompt") +} + +func (s *addCredentialSuite) assertAddedCredentialForCloudWithArgs(c *gc.C, cloudName, expectedStdout, expectedStdin, expectedStderr string, uploaded bool, args ...string) { s.setupStore(c) expectedContents := fmt.Sprintf(` credentials: @@ -867,12 +871,12 @@ credentials: return []params.UpdateCredentialResult{{CredentialTag: expectedTag}}, nil } - stdin := strings.NewReader(fmt.Sprintf("blah\n%s\n", sourceFile)) + stdin := strings.NewReader(fmt.Sprintf("%vblah\n%s\n", expectedStdin, sourceFile)) - ctx, err := s.run(c, stdin, cloudName) - c.Assert(err, jc.ErrorIsNil) + ctx, err := s.run(c, stdin, append(args, cloudName)...) c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expectedStdout) c.Assert(cmdtesting.Stderr(ctx), gc.Equals, expectedStderr) + c.Assert(err, jc.ErrorIsNil) c.Assert(s.store.Credentials[cloudName].AuthCredentials["blah"].Attributes()["file"], gc.Not(jc.Contains), expectedContents) c.Assert(s.store.Credentials[cloudName].AuthCredentials["blah"].Attributes()["file"], gc.Equals, sourceFile) @@ -901,7 +905,7 @@ Credential "blah" added locally for cloud "remote". `[1:] stderr := ` Using remote cloud "remote" from the controller to verify credentials. -Controller credential "blah" for user "admin@local" on cloud "remote" added. +Controller credential "blah" for user "admin@local" for cloud "remote" on controller "controller" added. For more information, see ‘juju show-credential remote blah’. `[1:] s.assertAddedCredentialForCloud(c, "remote", stdout, stderr, true) @@ -934,3 +938,32 @@ User 'juju add-cloud somecloud -c controller' to add your cloud to the controlle `[1:] s.assertAddedCredentialForCloud(c, "somecloud", stdout, "", false) } + +func (s *addCredentialSuite) TestAddRemoteCloudPromptForController(c *gc.C) { + s.api.clouds = func() (map[names.CloudTag]jujucloud.Cloud, error) { + return map[names.CloudTag]jujucloud.Cloud{ + names.NewCloudTag("remote"): { + Name: "remote", + Type: "gce", + AuthTypes: []jujucloud.AuthType{jujucloud.JSONFileAuthType}, + }, + }, nil + } + stdout := ` +Do you want to add a credential to current controller "controller"? (Y/n): +Enter credential name: +Using auth-type "jsonfile". + +Enter path to the .json file containing a service account key for your project +(detailed instructions available at https://discourse.jujucharms.com/t/1508). +Path: +Credential "blah" added locally for cloud "remote". + +`[1:] + stderr := ` +Using remote cloud "remote" from the controller to verify credentials. +Controller credential "blah" for user "admin@local" for cloud "remote" on controller "controller" added. +For more information, see ‘juju show-credential remote blah’. +`[1:] + s.assertAddedCredentialForCloudWithArgs(c, "remote", stdout, "\n", stderr, true) +} diff --git a/cmd/juju/cloud/detectcredentials.go b/cmd/juju/cloud/detectcredentials.go index ce3144b3947..7527a8982dc 100644 --- a/cmd/juju/cloud/detectcredentials.go +++ b/cmd/juju/cloud/detectcredentials.go @@ -498,7 +498,9 @@ func (c *detectCredentialsCommand) addRemoteCredentials(ctxt *cmd.Context, cloud if moreCloudInfoNeeded { ctxt.Infof("Use 'juju clouds' to view all available clouds and 'juju add-cloud' to add missing ones.") } - return processUpdateCredentialResult(ctxt, accountDetails, "loaded", results) + // TODO (anastasiamac 2019-09-04) Local error passd in will eventially be an error from the + // local detection on this client. + return processUpdateCredentialResult(ctxt, accountDetails, "loaded", results, c.ControllerName, nil) } func (c *detectCredentialsCommand) printCredentialOptions(ctxt *cmd.Context, discovered []discoveredCredential) { diff --git a/cmd/juju/cloud/detectcredentials_test.go b/cmd/juju/cloud/detectcredentials_test.go index 716407f8882..03047089323 100644 --- a/cmd/juju/cloud/detectcredentials_test.go +++ b/cmd/juju/cloud/detectcredentials_test.go @@ -405,7 +405,7 @@ Saved credential to cloud test-cloud locally 1. credential (existing, will overwrite) Select a credential to save by number, or type Q to quit: -Controller credential "blah" for user "admin@local" on cloud "test-cloud" loaded. +Controller credential "blah" for user "admin@local" for cloud "test-cloud" on controller "controller" loaded. For more information, see ‘juju show-credential test-cloud blah’. `[1:]) c.Assert(called, jc.IsTrue) diff --git a/cmd/juju/cloud/updatecredential.go b/cmd/juju/cloud/updatecredential.go index 98f922cea0c..e89d21d6f10 100644 --- a/cmd/juju/cloud/updatecredential.go +++ b/cmd/juju/cloud/updatecredential.go @@ -362,10 +362,7 @@ func (c *updateCredentialCommand) updateRemoteCredentials(ctx *cmd.Context, upda ctx.Warningf("Could not update credentials remotely, on controller %q", controllerName) erred = cmd.ErrSilent } - if err := processUpdateCredentialResult(ctx, accountDetails, "updated", results); err != nil { - return err - } - return erred + return processUpdateCredentialResult(ctx, accountDetails, "updated", results, controllerName, erred) } func verifyCredentialsForUpload(ctx *cmd.Context, accountDetails *jujuclient.AccountDetails, aCloud *jujucloud.Cloud, region string, all map[string]jujucloud.Credential) (map[string]jujucloud.Credential, error) { @@ -390,32 +387,31 @@ func verifyCredentialsForUpload(ctx *cmd.Context, accountDetails *jujuclient.Acc return verified, erred } -func processUpdateCredentialResult(ctx *cmd.Context, accountDetails *jujuclient.AccountDetails, op string, results []params.UpdateCredentialResult) error { - var erred error +func processUpdateCredentialResult(ctx *cmd.Context, accountDetails *jujuclient.AccountDetails, op string, results []params.UpdateCredentialResult, controllerName string, localError error) error { for _, result := range results { tag, err := names.ParseCloudCredentialTag(result.CredentialTag) if err != nil { logger.Errorf("%v", err) ctx.Warningf("Could not parse credential tag %q", result.CredentialTag) - erred = cmd.ErrSilent + localError = cmd.ErrSilent } // We always want to display models information if there is any. common.OutputUpdateCredentialModelResult(ctx, result.Models, true) if result.Error != nil { - ctx.Warningf("Controller credential %q for user %q on cloud %q not %v: %v.", tag.Name(), accountDetails.User, tag.Cloud().Id(), op, result.Error) + ctx.Warningf("Controller credential %q for user %q for cloud %q on controller %q not %v: %v.", tag.Name(), accountDetails.User, tag.Cloud().Id(), controllerName, op, result.Error) if len(result.Models) != 0 { ctx.Infof("Failed models may require a different credential.") ctx.Infof("Use ‘juju set-credential’ to change credential for these models before repeating this update.") } // We do not want to return err here as we have already displayed it on the console. - erred = cmd.ErrSilent + localError = cmd.ErrSilent continue } ctx.Infof(` -Controller credential %q for user %q on cloud %q %v. +Controller credential %q for user %q for cloud %q on controller %q %v. For more information, see ‘juju show-credential %v %v’.`[1:], - tag.Name(), accountDetails.User, tag.Cloud().Id(), op, - tag.Cloud().Id(), tag.Name()) + tag.Name(), accountDetails.User, tag.Cloud().Id(), controllerName, + op, tag.Cloud().Id(), tag.Name()) } - return erred + return localError } diff --git a/cmd/juju/cloud/updatecredential_test.go b/cmd/juju/cloud/updatecredential_test.go index 095ee021b24..c3e61912aad 100644 --- a/cmd/juju/cloud/updatecredential_test.go +++ b/cmd/juju/cloud/updatecredential_test.go @@ -404,7 +404,7 @@ func (s *updateCredentialSuite) TestUpdateRemote(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(cmdtesting.Stdout(ctx), jc.Contains, ``) c.Assert(cmdtesting.Stderr(ctx), jc.Contains, ` -Controller credential "my-credential" for user "admin@local" on cloud "aws" updated. +Controller credential "my-credential" for user "admin@local" for cloud "aws" on controller "controller" updated. For more information, see ‘juju show-credential aws my-credential’. `[1:]) } @@ -513,7 +513,7 @@ Credential invalid for: Failed models may require a different credential. Use ‘juju set-credential’ to change credential for these models before repeating this update. `[1:]) - c.Assert(c.GetTestLog(), jc.Contains, `Controller credential "my-credential" for user "admin@local" on cloud "aws" not updated: models issues`) + c.Assert(c.GetTestLog(), jc.Contains, `Controller credential "my-credential" for user "admin@local" for cloud "aws" on controller "controller" not updated: models issues`) } type fakeUpdateCredentialAPI struct { From 1d86a474688ac8ffc207d08a3e84f274e05d27cb Mon Sep 17 00:00:00 2001 From: Anastasia Macmood Date: Thu, 5 Sep 2019 12:42:51 +1000 Subject: [PATCH 2/5] Updated feature test with newer message. --- featuretests/cmd_juju_credential_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/featuretests/cmd_juju_credential_test.go b/featuretests/cmd_juju_credential_test.go index ba271733454..f7d6309def6 100644 --- a/featuretests/cmd_juju_credential_test.go +++ b/featuretests/cmd_juju_credential_test.go @@ -91,7 +91,7 @@ clouds: c.Assert(cmdtesting.Stderr(ctx), gc.Equals, ` Credential valid for: controller -Controller credential "cred" for user "admin" on cloud "dummy" updated. +Controller credential "cred" for user "admin" for cloud "dummy" on controller "kontroll" updated. For more information, see ‘juju show-credential dummy cred’. `[1:]) c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "") From e625e39c334cf4e8b68aa7cf3d230e538150738e Mon Sep 17 00:00:00 2001 From: Anastasia Macmood Date: Fri, 6 Sep 2019 09:17:22 +1000 Subject: [PATCH 3/5] Fixed typos in a TODO message. --- cmd/juju/cloud/detectcredentials.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/juju/cloud/detectcredentials.go b/cmd/juju/cloud/detectcredentials.go index 7527a8982dc..7d5b23368d5 100644 --- a/cmd/juju/cloud/detectcredentials.go +++ b/cmd/juju/cloud/detectcredentials.go @@ -498,7 +498,7 @@ func (c *detectCredentialsCommand) addRemoteCredentials(ctxt *cmd.Context, cloud if moreCloudInfoNeeded { ctxt.Infof("Use 'juju clouds' to view all available clouds and 'juju add-cloud' to add missing ones.") } - // TODO (anastasiamac 2019-09-04) Local error passd in will eventially be an error from the + // TODO (anastasiamac 2019-09-04) Local error passed in will eventually be an error from the // local detection on this client. return processUpdateCredentialResult(ctxt, accountDetails, "loaded", results, c.ControllerName, nil) } From 9881d67c676d079c4cba3b15cc7355bd0cde2b4d Mon Sep 17 00:00:00 2001 From: Anastasia Macmood Date: Fri, 6 Sep 2019 09:21:49 +1000 Subject: [PATCH 4/5] Re-phrase error message. --- cmd/juju/cloud/addcredential.go | 2 +- cmd/juju/cloud/addcredential_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/juju/cloud/addcredential.go b/cmd/juju/cloud/addcredential.go index e62abb60234..29920bdb96e 100644 --- a/cmd/juju/cloud/addcredential.go +++ b/cmd/juju/cloud/addcredential.go @@ -620,7 +620,7 @@ func (c *addCredentialCommand) maybeRemoteCloud(ctxt *cmd.Context) error { return err } if remoteCloud, ok := remoteUserClouds[names.NewCloudTag(c.CloudName)]; ok { - ctxt.Infof("Using remote cloud %q from the controller to verify credentials.", c.CloudName) + ctxt.Infof("Using cloud %q from the controller to verify credentials.", c.CloudName) c.cloud = &remoteCloud c.remoteCloudFound = true } diff --git a/cmd/juju/cloud/addcredential_test.go b/cmd/juju/cloud/addcredential_test.go index 989f7b110eb..0a454af694d 100644 --- a/cmd/juju/cloud/addcredential_test.go +++ b/cmd/juju/cloud/addcredential_test.go @@ -835,7 +835,7 @@ Credential "blah" added locally for cloud "somecloud". `[1:] stderr := ` -Using remote cloud "somecloud" from the controller to verify credentials. +Using cloud "somecloud" from the controller to verify credentials. Controller credential "blah" for user "admin@local" for cloud "somecloud" on controller "controller" added. For more information, see ‘juju show-credential somecloud blah’. `[1:] @@ -874,9 +874,9 @@ credentials: stdin := strings.NewReader(fmt.Sprintf("%vblah\n%s\n", expectedStdin, sourceFile)) ctx, err := s.run(c, stdin, append(args, cloudName)...) + c.Assert(err, jc.ErrorIsNil) c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expectedStdout) c.Assert(cmdtesting.Stderr(ctx), gc.Equals, expectedStderr) - c.Assert(err, jc.ErrorIsNil) c.Assert(s.store.Credentials[cloudName].AuthCredentials["blah"].Attributes()["file"], gc.Not(jc.Contains), expectedContents) c.Assert(s.store.Credentials[cloudName].AuthCredentials["blah"].Attributes()["file"], gc.Equals, sourceFile) @@ -904,7 +904,7 @@ Credential "blah" added locally for cloud "remote". `[1:] stderr := ` -Using remote cloud "remote" from the controller to verify credentials. +Using cloud "remote" from the controller to verify credentials. Controller credential "blah" for user "admin@local" for cloud "remote" on controller "controller" added. For more information, see ‘juju show-credential remote blah’. `[1:] @@ -961,7 +961,7 @@ Credential "blah" added locally for cloud "remote". `[1:] stderr := ` -Using remote cloud "remote" from the controller to verify credentials. +Using cloud "remote" from the controller to verify credentials. Controller credential "blah" for user "admin@local" for cloud "remote" on controller "controller" added. For more information, see ‘juju show-credential remote blah’. `[1:] From 5a8981c3c7a7a890445b735c59df443ad747b856 Mon Sep 17 00:00:00 2001 From: Anastasia Macmood Date: Fri, 6 Sep 2019 09:40:53 +1000 Subject: [PATCH 5/5] Re-phrase all user facing messages and errors to avoid saying 'remote' or 'remotely'. --- cmd/juju/cloud/addcredential.go | 17 ++++++++--------- cmd/juju/cloud/addcredential_test.go | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/juju/cloud/addcredential.go b/cmd/juju/cloud/addcredential.go index 29920bdb96e..a8e48e38e90 100644 --- a/cmd/juju/cloud/addcredential.go +++ b/cmd/juju/cloud/addcredential.go @@ -25,8 +25,7 @@ import ( ) var usageAddCredentialSummary = ` -Adds a credential for a cloud, stored locally on this client, or -uploads a credential for a cloud remotely, stored on the controller.`[1:] +Adds a credential for a cloud to a local client and uploads it to a controller.`[1:] var usageAddCredentialDetails = ` The juju add-credential command operates in two modes. @@ -93,7 +92,7 @@ cloud. The commands ` + "`juju default-region`" + ` and ` + "`juju default-crede provide that functionality. By default, after validating the contents, Juju will add a credential locally, -to the current client device, and will upload it remotely, to a controller. +to the current client device, and will upload it to a controller. If a current controller is detected, Juju will prompt the user to confirm whether this new credential also needs to be uploaded. @@ -203,7 +202,7 @@ func (c *addCredentialCommand) Run(ctxt *cmd.Context) error { if !errors.IsNotFound(err) { logger.Errorf("%v", err) } - ctxt.Infof("Cloud %q is not remotely found on the controller, looking for a locally stored cloud.", c.CloudName) + ctxt.Infof("Cloud %q is not found on the controller, looking for a locally stored cloud.", c.CloudName) } } if c.cloud == nil { @@ -300,7 +299,7 @@ func (c *addCredentialCommand) Run(ctxt *cmd.Context) error { added[name] = cred if _, ok := existingCredentials.AuthCredentials[name]; ok { - ctxt.Warningf("credential %q for cloud %q already exists locally, use 'juju update-credential %v %v' to update it", name, c.CloudName, c.CloudName, name) + ctxt.Warningf("credential %q for cloud %q already exists locally, use 'juju update-credential %v %v -f %v' to update this local client copy", name, c.CloudName, c.CloudName, name, c.CredentialsFile) continue } @@ -629,12 +628,12 @@ func (c *addCredentialCommand) maybeRemoteCloud(ctxt *cmd.Context) error { func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[string]jujucloud.Credential, localError error) error { if len(all) == 0 { - fmt.Fprintf(ctxt.Stdout, "No remote credentials for cloud %q added.\n", c.CloudName) + fmt.Fprintf(ctxt.Stdout, "No credentials for cloud %q uploaded to controller %q.\n", c.CloudName, c.ControllerName) return localError } if !c.remoteCloudFound { - fmt.Fprintf(ctxt.Stdout, "No remote cloud %v found on the controller %v: credentials are not added remotely.\n"+ - "Use 'juju clouds -c %v' to see what clouds are available remotely.\n"+ + fmt.Fprintf(ctxt.Stdout, "No cloud %q found on the controller %q: credentials are not uploaded.\n"+ + "Use 'juju clouds -c %v' to see what clouds are available on the controller.\n"+ "User 'juju add-cloud %v -c %v' to add your cloud to the controller.\n", c.CloudName, c.ControllerName, c.ControllerName, c.CloudName, c.ControllerName) return localError @@ -656,7 +655,7 @@ func (c *addCredentialCommand) addRemoteCredentials(ctxt *cmd.Context, all map[s results, err := client.UpdateCloudsCredentials(verified) if err != nil { logger.Errorf("%v", err) - ctxt.Warningf("Could not add credentials remotely, on controller %q", c.ControllerName) + ctxt.Warningf("Could not upload credentials to controller %q", c.ControllerName) } return processUpdateCredentialResult(ctxt, accountDetails, "added", results, c.ControllerName, localError) } diff --git a/cmd/juju/cloud/addcredential_test.go b/cmd/juju/cloud/addcredential_test.go index 0a454af694d..ccf4af60d74 100644 --- a/cmd/juju/cloud/addcredential_test.go +++ b/cmd/juju/cloud/addcredential_test.go @@ -932,8 +932,8 @@ Using auth-type "jsonfile". Enter path to the credential file: Credential "blah" added locally for cloud "somecloud". -No remote cloud somecloud found on the controller controller: credentials are not added remotely. -Use 'juju clouds -c controller' to see what clouds are available remotely. +No cloud "somecloud" found on the controller "controller": credentials are not uploaded. +Use 'juju clouds -c controller' to see what clouds are available on the controller. User 'juju add-cloud somecloud -c controller' to add your cloud to the controller. `[1:] s.assertAddedCredentialForCloud(c, "somecloud", stdout, "", false)