feat(jimmctl migrate): enable users to specify model names (#1500)

* feat(jimmctl migrate): enable users to specify model names

* style(pr comments): update doc strings

* style(command example): pR comment suggestion

* pr comments

cmd/jimmctl/cmd/migratemodel.go

@@ -3,15 +3,12 @@
 package cmd
 
 import (
-	"fmt"
-
 	"github.com/juju/cmd/v3"
 	"github.com/juju/gnuflag"
 	jujuapi "github.com/juju/juju/api"
 	jujucmd "github.com/juju/juju/cmd"
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/jujuclient"
-	"github.com/juju/names/v5"
 
 	"github.com/canonical/jimm/v3/internal/errors"
 	"github.com/canonical/jimm/v3/pkg/api"
@@ -20,15 +17,17 @@ import (
 
 const (
 	migrateModelCommandDoc = `
-The migrate command migrates a model(s) to a new controller. Specify
-a model-uuid to migrate and the destination controller name.
+The migrate commands migrates a model, or many models between two controllers
+registered within JIMM. 
+
+You may specify a model name (of the form owner/name) or model UUID.
 
-Note that multiple models can be targeted for migration by supplying
-multiple model uuids.
 `
 	migrateModelCommandExample = `
-    jimmctl migrate mycontroller 2cb433a6-04eb-4ec4-9567-90426d20a004 
     jimmctl migrate mycontroller 2cb433a6-04eb-4ec4-9567-90426d20a004 fd469983-27c2-423b-bebf-84f616fb036b ...
+    jimmctl migrate mycontroller [email protected]/model-a [email protected]/model-b ...
+    jimmctl migrate mycontroller [email protected]/model-a fd469983-27c2-423b-bebf-84f616fb036b ...
+
 `
 )
 
@@ -49,7 +48,7 @@ type migrateModelCommand struct {
 	store            jujuclient.ClientStore
 	dialOpts         *jujuapi.DialOpts
 	targetController string
-	modelTags        []string
+	modelTargets     []string
 }
 
 func (c *migrateModelCommand) Info() *cmd.Info {
@@ -74,19 +73,14 @@ func (c *migrateModelCommand) SetFlags(f *gnuflag.FlagSet) {
 // Init implements the cmd.Command interface.
 func (c *migrateModelCommand) Init(args []string) error {
 	if len(args) < 2 {
-		return errors.E("Missing controller name and model uuid arguments")
+		return errors.E("Missing controller name and model target arguments")
 	}
 	for i, arg := range args {
 		if i == 0 {
 			c.targetController = arg
 			continue
 		}
-		mt := names.NewModelTag(arg)
-		_, err := names.ParseModelTag(mt.String())
-		if err != nil {
-			return errors.E(err, fmt.Sprintf("%s is not a valid model uuid", arg))
-		}
-		c.modelTags = append(c.modelTags, mt.String())
+		c.modelTargets = append(c.modelTargets, arg)
 	}
 	return nil
 }
@@ -105,8 +99,8 @@ func (c *migrateModelCommand) Run(ctxt *cmd.Context) error {
 
 	client := api.NewClient(apiCaller)
 	specs := []apiparams.MigrateModelInfo{}
-	for _, model := range c.modelTags {
-		specs = append(specs, apiparams.MigrateModelInfo{ModelTag: model, TargetController: c.targetController})
+	for _, model := range c.modelTargets {
+		specs = append(specs, apiparams.MigrateModelInfo{TargetModelNameOrUUID: model, TargetController: c.targetController})
 	}
 	req := apiparams.MigrateModelRequest{Specs: specs}
 	events, err := client.MigrateModel(&req)

cmd/jimmctl/cmd/migratemodel_test.go

@@ -3,10 +3,13 @@
 package cmd_test
 
 import (
+	"fmt"
+
 	"github.com/juju/cmd/v3/cmdtesting"
 	jujuparams "github.com/juju/juju/rpc/params"
 	"github.com/juju/names/v5"
 	gc "gopkg.in/check.v1"
+	"gopkg.in/yaml.v3"
 
 	"github.com/canonical/jimm/v3/cmd/jimmctl/cmd"
 	"github.com/canonical/jimm/v3/internal/testutils/cmdtest"
@@ -19,21 +22,6 @@ type migrateModelSuite struct {
 
 var _ = gc.Suite(&migrateModelSuite{})
 
-var migrationResultRegex = `results:
-- modeltag: model-.*
-  error:
-    message: 'target prechecks failed: model with same UUID already exists (.*)'
-    code: ""
-    info: {}
-  migrationid: ""
-- modeltag: model-.*
-  error:
-    message: 'target prechecks failed: model with same UUID already exists (.*)'
-    code: ""
-    info: {}
-  migrationid: ""
-`
-
 // TestMigrateModelCommandSuperuser tests that a migration request makes it through to the Juju controller.
 // Because our test suite only spins up 1 controller the furthest we can go is reaching Juju pre-checks which
 // detect that a model with the same UUID already exists on the target controller.
@@ -48,26 +36,26 @@ func (s *migrateModelSuite) TestMigrateModelCommandSuperuser(c *gc.C) {
 
 	// alice is superuser
 	bClient := s.SetupCLIAccess(c, "alice")
-	context, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", mt.Id(), mt2.Id())
+	context, err := cmdtesting.RunCommand(
+		c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient),
+		"controller-1",
+		mt.Id(),
+		"[email protected]/model-2",
+	)
 	c.Assert(err, gc.IsNil)
-	c.Assert(cmdtesting.Stdout(context), gc.Matches, migrationResultRegex)
-}
-
-func (s *migrateModelSuite) TestMigrateModelCommandFailsWithInvalidModelTag(c *gc.C) {
-	s.AddController(c, "controller-1", s.APIInfo(c))
 
-	cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/[email protected]/cred")
-	s.UpdateCloudCredential(c, cct, jujuparams.CloudCredential{AuthType: "empty"})
-	s.AddModel(c, names.NewUserTag("[email protected]"), "model-2", names.NewCloudTag(jimmtest.TestCloudName), jimmtest.TestCloudRegionName, cct)
+	res := &jujuparams.InitiateMigrationResults{}
+	out := cmdtesting.Stdout(context)
+	err = yaml.Unmarshal([]byte(out), res)
+	c.Assert(err, gc.IsNil)
 
-	// alice is superuser
-	bClient := s.SetupCLIAccess(c, "alice")
-	_, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", "001", "002")
-	c.Assert(err, gc.ErrorMatches, ".* is not a valid model uuid")
+	expected := "target prechecks failed: model with same UUID already exists (%s)"
+	c.Assert(res.Results[0].Error.Message, gc.Equals, fmt.Sprintf(expected, mt.Id()))
+	c.Assert(res.Results[1].Error.Message, gc.Equals, fmt.Sprintf(expected, mt2.Id()))
 }
 
 func (s *migrateModelSuite) TestMigrateModelCommandFailsWithMissingArgs(c *gc.C) {
 	bClient := s.SetupCLIAccess(c, "alice")
 	_, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "myController")
-	c.Assert(err, gc.ErrorMatches, "Missing controller name and model uuid arguments")
+	c.Assert(err, gc.ErrorMatches, "Missing controller name and model target arguments")
 }

internal/jimm/jimm.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery"
+	"github.com/google/uuid"
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/core/crossmodel"
 	jujuparams "github.com/juju/juju/rpc/params"
@@ -730,25 +731,45 @@ func fillMigrationTarget(db *db.Database, credStore credentials.CredentialStore,
 }
 
 // InitiateInternalMigration initiates a model migration between two controllers within JIMM.
-func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) {
+func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelNameOrUUID string, targetController string) (jujuparams.InitiateMigrationResult, error) {
 	const op = errors.Op("jimm.InitiateInternalMigration")
 
 	migrationTarget, _, err := fillMigrationTarget(j.Database, j.CredentialStore, targetController)
 	if err != nil {
 		return jujuparams.InitiateMigrationResult{}, errors.E(op, err)
 	}
-	// Check that the model exists
-	model := dbmodel.Model{
-		UUID: sql.NullString{
-			String: modelTag.Id(),
+
+	model := dbmodel.Model{}
+	// Check if the user is providing a model UUID or name
+	_, err = uuid.Parse(modelNameOrUUID)
+	if err != nil {
+		s := strings.Split(modelNameOrUUID, "/")
+		if len(s) != 2 {
+			return jujuparams.InitiateMigrationResult{}, errors.E(op, "invalid model target")
+		}
+
+		owner, name := s[0], s[1]
+		if !names.IsValidUser(owner) {
+			return jujuparams.InitiateMigrationResult{}, errors.E(op, "invalid user name")
+		}
+		if !names.IsValidModelName(name) {
+			return jujuparams.InitiateMigrationResult{}, errors.E(op, "invalid model name")
+		}
+
+		model.Name = name
+		model.OwnerIdentityName = owner
+	} else {
+		model.UUID = sql.NullString{
+			String: modelNameOrUUID,
 			Valid:  true,
-		},
+		}
 	}
+
 	err = j.Database.GetModel(ctx, &model)
 	if err != nil {
 		return jujuparams.InitiateMigrationResult{}, errors.E(op, err)
 	}
-	spec := jujuparams.MigrationSpec{ModelTag: modelTag.String(), TargetInfo: migrationTarget}
+	spec := jujuparams.MigrationSpec{ModelTag: model.ResourceTag().String(), TargetInfo: migrationTarget}
 	result, err := initiateMigration(ctx, j, user, spec)
 	if err != nil {
 		return result, errors.E(op, err)

internal/jimm/jimm_test.go

@@ -778,16 +778,39 @@ func TestInitiateInternalMigration(t *testing.T) {
 		migrateInfo   params.MigrateModelInfo
 		expectedError string
 	}{{
-		about:       "success",
+		about:       "success with uuid",
 		user:        "[email protected]",
-		migrateInfo: params.MigrateModelInfo{ModelTag: "model-00000002-0000-0000-0000-000000000001", TargetController: "myController"},
+		migrateInfo: params.MigrateModelInfo{TargetModelNameOrUUID: "00000002-0000-0000-0000-000000000001", TargetController: "myController"},
+	}, {
+		about:       "a success with name",
+		user:        "[email protected]",
+		migrateInfo: params.MigrateModelInfo{TargetModelNameOrUUID: "[email protected]/model-1", TargetController: "myController"},
 	}, {
 		about:         "model doesn't exist",
 		user:          "[email protected]",
-		migrateInfo:   params.MigrateModelInfo{ModelTag: "model-00000002-0000-0000-0000-000000000002", TargetController: "myController"},
+		migrateInfo:   params.MigrateModelInfo{TargetModelNameOrUUID: "00000002-0000-0000-0000-000000000002", TargetController: "myController"},
 		expectedError: "model not found",
-	},
-	}
+	}, {
+		about:         "model doesn't exist",
+		user:          "[email protected]",
+		migrateInfo:   params.MigrateModelInfo{TargetModelNameOrUUID: "00000002-0000-0000-0000-000000000002", TargetController: "myController"},
+		expectedError: "model not found",
+	}, {
+		about:         "a missing model target",
+		user:          "[email protected]",
+		migrateInfo:   params.MigrateModelInfo{TargetModelNameOrUUID: "[email protected]", TargetController: "myController"},
+		expectedError: "invalid model target",
+	}, {
+		about:         "using an invalid user name",
+		user:          "[email protected]",
+		migrateInfo:   params.MigrateModelInfo{TargetModelNameOrUUID: "*bad wolf*@canonical.com/model-1", TargetController: "myController"},
+		expectedError: "invalid user name",
+	}, {
+		about:         "using an invalid model name",
+		user:          "[email protected]",
+		migrateInfo:   params.MigrateModelInfo{TargetModelNameOrUUID: "[email protected]/*bad wolf*", TargetController: "myController"},
+		expectedError: "invalid model name",
+	}}
 	for _, test := range tests {
 		c.Run(test.about, func(c *qt.C) {
 			c.Patch(jimm.InitiateMigration, func(ctx context.Context, j *jimm.JIMM, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) {
@@ -806,9 +829,14 @@ func TestInitiateInternalMigration(t *testing.T) {
 
 			dbUser := env.User(test.user).DBObject(c, j.Database)
 			user := openfga.NewUser(&dbUser, nil)
-			mt, err := names.ParseModelTag(test.migrateInfo.ModelTag)
 			c.Assert(err, qt.IsNil)
-			res, err := j.InitiateInternalMigration(ctx, user, mt, test.migrateInfo.TargetController)
+
+			res, err := j.InitiateInternalMigration(
+				ctx,
+				user,
+				test.migrateInfo.TargetModelNameOrUUID,
+				test.migrateInfo.TargetController,
+			)
 			if test.expectedError != "" {
 				c.Assert(err, qt.ErrorMatches, test.expectedError)
 			} else {

internal/jujuapi/interface.go

@@ -57,7 +57,7 @@ type JIMM interface {
 	GrantModelAccess(ctx context.Context, user *openfga.User, mt names.ModelTag, ut names.UserTag, access jujuparams.UserAccessPermission) error
 	GrantOfferAccess(ctx context.Context, u *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error
 	GrantServiceAccountAccess(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []string) error
-	InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error)
+	InitiateInternalMigration(ctx context.Context, user *openfga.User, modelNameOrUUID string, targetController string) (jujuparams.InitiateMigrationResult, error)
 	InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error)
 	ListApplicationOffers(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error)
 	ListIdentities(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]openfga.User, error)

internal/jujuapi/jimm.go

@@ -501,13 +501,9 @@ func (r *controllerRoot) MigrateModel(ctx context.Context, args apiparams.Migrat
 	const op = errors.Op("jujuapi.MigrateModel")
 
 	results := make([]jujuparams.InitiateMigrationResult, len(args.Specs))
+
 	for i, arg := range args.Specs {
-		mt, err := names.ParseModelTag(arg.ModelTag)
-		if err != nil {
-			results[i].Error = mapError(errors.E(op, err))
-			continue
-		}
-		result, err := r.jimm.InitiateInternalMigration(ctx, r.user, mt, arg.TargetController)
+		result, err := r.jimm.InitiateInternalMigration(ctx, r.user, arg.TargetModelNameOrUUID, arg.TargetController)
 		if err != nil {
 			result.Error = mapError(errors.E(op, err))
 		}

internal/jujuapi/jimm_test.go

@@ -855,15 +855,22 @@ func (s *jimmSuite) TestJimmModelMigrationSuperuser(c *gc.C) {
 
 	res, err := client.MigrateModel(&apiparams.MigrateModelRequest{
 		Specs: []apiparams.MigrateModelInfo{
-			{ModelTag: mt.String(), TargetController: "controller-1"},
+			{TargetModelNameOrUUID: mt.Id(), TargetController: "controller-1"},
+			{TargetModelNameOrUUID: "[email protected]/model-20", TargetController: "controller-1"},
 		},
 	})
 	c.Assert(err, gc.IsNil)
-	c.Assert(res.Results, gc.HasLen, 1)
+	c.Assert(res.Results, gc.HasLen, 2)
+
 	item := res.Results[0]
 	c.Assert(item.ModelTag, gc.Equals, mt.String())
 	c.Assert(item.MigrationId, gc.Equals, "")
 	c.Assert(item.Error.Message, gc.Matches, "target prechecks failed: model with same UUID already exists .*")
+
+	item2 := res.Results[1]
+	c.Assert(item2.ModelTag, gc.Equals, mt.String())
+	c.Assert(item2.MigrationId, gc.Equals, "")
+	c.Assert(item2.Error.Message, gc.Matches, "target prechecks failed: model with same UUID already exists .*")
 }
 
 func (s *jimmSuite) TestJimmModelMigrationNonSuperuser(c *gc.C) {
@@ -882,7 +889,7 @@ func (s *jimmSuite) TestJimmModelMigrationNonSuperuser(c *gc.C) {
 
 	res, err := client.MigrateModel(&apiparams.MigrateModelRequest{
 		Specs: []apiparams.MigrateModelInfo{
-			{ModelTag: mt.String(), TargetController: "controller-1"},
+			{TargetModelNameOrUUID: mt.Id(), TargetController: "controller-1"},
 		},
 	})
 	c.Assert(err, gc.IsNil)

internal/testutils/jimmtest/jimm_mock.go

@@ -65,7 +65,7 @@ type JIMM struct {
 	GrantOfferAccess_                  func(ctx context.Context, u *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error
 	GrantServiceAccountAccess_         func(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, entities []string) error
 	GroupManager_                      func() jimm.GroupManager
-	InitiateInternalMigration_         func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error)
+	InitiateInternalMigration_         func(ctx context.Context, user *openfga.User, modelNameOrUUID string, targetController string) (jujuparams.InitiateMigrationResult, error)
 	InitiateMigration_                 func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error)
 	ListApplicationOffers_             func(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error)
 	ListIdentities_                    func(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]openfga.User, error)
@@ -307,11 +307,11 @@ func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec j
 	}
 	return j.InitiateMigration_(ctx, user, spec)
 }
-func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) {
+func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelNameOrUUID string, targetController string) (jujuparams.InitiateMigrationResult, error) {
 	if j.InitiateInternalMigration_ == nil {
 		return jujuparams.InitiateMigrationResult{}, errors.E(errors.CodeNotImplemented)
 	}
-	return j.InitiateInternalMigration_(ctx, user, modelTag, targetController)
+	return j.InitiateInternalMigration_(ctx, user, modelNameOrUUID, targetController)
 }
 func (j *JIMM) ListApplicationOffers(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) {
 	if j.ListApplicationOffers_ == nil {

pkg/api/params/params.go

@@ -471,8 +471,8 @@ type PurgeLogsResponse struct {
 // target controller must be specified with both the source model and
 // target controller residing within JIMM.
 type MigrateModelInfo struct {
-	// ModelTag is a tag of the form "model-<UIID>"
-	ModelTag string `json:"model-tag"`
+	// TargetModelNameOrUUID can be either the model name or model UUID.
+	TargetModelNameOrUUID string `json:"model-tag"`
 	// TargetController is the controller name of the form "<name>"
 	TargetController string `json:"target-controller"`
 }