test: refactor import model tests (#1826)

* test: refactor import model tests

* docs: regenerate plug doc for import model

* refactor: remove jimm errors pkg

* refactor: use correct verb for error

* test: remove flag setting

Author: ale8k

cmd/jaas/cmd/export_test.go

@@ -88,15 +88,6 @@ func NewSetControllerDeprecatedCommandForTesting(store jujuclient.ClientStore, l
 	return modelcmd.WrapBase(cmd)
 }
 
-func NewImportModelCommandForTesting(store jujuclient.ClientStore, lp jujuapi.LoginProvider) cmd.Command {
-	cmd := &importModelCommand{
-		dialOpts: cmdtest.TestDialOpts(lp),
-	}
-	cmd.SetClientStore(store)
-
-	return modelcmd.WrapBase(cmd)
-}
-
 func NewUpdateMigratedModelCommandForTesting(store jujuclient.ClientStore, lp jujuapi.LoginProvider) cmd.Command {
 	cmd := &updateMigratedModelCommand{
 		dialOpts: cmdtest.TestDialOpts(lp),

cmd/jaas/cmd/importmodel.go

@@ -3,11 +3,11 @@
 package cmd
 
 import (
+	"errors"
 	"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"
@@ -37,24 +37,26 @@ local user and it will switch the model owner to the desired external user.
 func NewImportModelCommand() cmd.Command {
 	cmd := &importModelCommand{}
 	cmd.SetClientStore(jujuclient.NewFileClientStore())
+	cmd.jimmAPIFunc = cmd.newClient
 
 	return modelcmd.WrapBase(cmd)
 }
 
 // importModelCommand imports a model.
 type importModelCommand struct {
 	modelcmd.ControllerCommandBase
-	dialOpts *jujuapi.DialOpts
 
 	req apiparams.ImportModelRequest
+
+	jimmAPIFunc func() (JIMMAPI, error)
 }
 
 // Info implements the cmd.Command interface.
 func (c *importModelCommand) Info() *cmd.Info {
 	return jujucmd.Info(&cmd.Info{
 		Name:     "import-model",
 		Args:     "<controller name> <model uuid>",
-		Purpose:  "Import a model to jimm",
+		Purpose:  "Import a model to jimm.",
 		Doc:      importModelCommandDoc,
 		Examples: importModelCommandExample,
 		Aliases:  []string{"register-model"},
@@ -81,27 +83,40 @@ func (c *importModelCommand) Init(args []string) error {
 
 	c.req.Controller = args[0]
 	if !names.IsValidModel(args[1]) {
-		return fmt.Errorf("invalid model uuid")
+		return errors.New("invalid model uuid")
 	}
 	c.req.ModelTag = names.NewModelTag(args[1]).String()
 	return nil
 }
 
 // Run implements Command.Run.
 func (c *importModelCommand) Run(ctxt *cmd.Context) error {
-	currentController, err := c.ClientStore().CurrentController()
-	if err != nil {
-		return fmt.Errorf("could not determine controller: %w", err)
+	if c.jimmAPIFunc == nil {
+		c.jimmAPIFunc = c.newClient
 	}
 
-	apiCaller, err := c.NewAPIRootWithDialOpts(c.ClientStore(), currentController, "", c.dialOpts)
+	jimmAPI, err := c.jimmAPIFunc()
 	if err != nil {
-		return err
+		return fmt.Errorf("could not create JIMM API client: %w", err)
 	}
+	defer jimmAPI.Close()
 
-	client := api.NewClient(apiCaller)
-	if err := client.ImportModel(&c.req); err != nil {
-		return err
+	if err := jimmAPI.ImportModel(&c.req); err != nil {
+		return fmt.Errorf("could not import model: %w", err)
 	}
 	return nil
 }
+
+func (c *importModelCommand) newClient() (JIMMAPI, error) {
+	currentController, err := c.ClientStore().CurrentController()
+	if err != nil {
+		return nil, fmt.Errorf("could not determine controller: %w", err)
+	}
+
+	apiCaller, err := c.NewAPIRootWithDialOpts(c.ClientStore(), currentController, "", nil)
+	if err != nil {
+		return nil, err
+	}
+
+	return api.NewClient(apiCaller), nil
+}

cmd/jaas/cmd/importmodel_test.go

@@ -1,134 +1,113 @@
 // Copyright 2025 Canonical.
 
-package cmd_test
+package cmd
 
 import (
-	"context"
+	"testing"
 
+	qt "github.com/frankban/quicktest"
 	"github.com/juju/cmd/v3/cmdtesting"
-	jjcloud "github.com/juju/juju/cloud"
-	jujuparams "github.com/juju/juju/rpc/params"
-	"github.com/juju/juju/testing/factory"
-	"github.com/juju/names/v5"
-	gc "gopkg.in/check.v1"
-
-	"github.com/canonical/jimm/v3/cmd/jaas/cmd"
-	"github.com/canonical/jimm/v3/internal/dbmodel"
-	"github.com/canonical/jimm/v3/internal/testutils/cmdtest"
-	"github.com/canonical/jimm/v3/internal/testutils/jimmtest"
+	"github.com/juju/gnuflag"
+	"go.uber.org/mock/gomock"
+
+	apiparams "github.com/canonical/jimm/v3/pkg/api/params"
 )
 
-type importModelSuite struct {
-	cmdtest.JimmCmdSuite
+func TestImportModelInit_ArgValidation(t *testing.T) {
+	c := qt.New(t)
+
+	validModelUUID := "ac30d6ae-0bed-4398-bba7-75d49e39f189"
+
+	cases := []struct {
+		about    string
+		args     []string
+		errMatch string
+	}{
+		{about: "0 args", args: nil, errMatch: "controller not specified"},
+		{about: "1 arg", args: []string{"controller-1"}, errMatch: "model uuid not specified"},
+		{about: "too many args", args: []string{"controller-1", validModelUUID, "extra"}, errMatch: "too many args"},
+		{about: "invalid uuid", args: []string{"controller-1", "not-a-uuid"}, errMatch: "invalid model uuid"},
+		{about: "valid uuid", args: []string{"controller-1", validModelUUID}, errMatch: ""},
+	}
+
+	for _, tc := range cases {
+		c.Run(tc.about, func(c *qt.C) {
+			cmd := &importModelCommand{}
+			err := cmd.Init(tc.args)
+			if tc.errMatch != "" {
+				c.Assert(err, qt.ErrorMatches, tc.errMatch)
+				return
+			}
+			c.Assert(err, qt.IsNil)
+			c.Assert(cmd.req.Controller, qt.Equals, "controller-1")
+			c.Assert(cmd.req.ModelTag, qt.Equals, "model-"+validModelUUID)
+		})
+	}
 }
 
-var _ = gc.Suite(&importModelSuite{})
+func TestImportModelRun_PassesRequestToAPI(t *testing.T) {
+	c := qt.New(t)
 
-func (s *importModelSuite) TestImportModelSuperuser(c *gc.C) {
-	s.AddController(c, "controller-1", s.APIInfo(c))
+	cmdMocks := setupCmdMocks(c)
 
-	cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/charlie@canonical.com/cred")
-	s.UpdateCloudCredential(c, cct, jujuparams.CloudCredential{AuthType: "empty", Attributes: map[string]string{"key": "value"}})
+	modelUUID := "ac30d6ae-0bed-4398-bba7-75d49e39f189"
 
-	err := s.BackingState.UpdateCloudCredential(cct, jjcloud.NewCredential(jjcloud.EmptyAuthType, map[string]string{"key": "value"}))
-	c.Assert(err, gc.Equals, nil)
+	cmdMocks.client.EXPECT().
+		ImportModel(gomock.Any()).
+		DoAndReturn(func(req *apiparams.ImportModelRequest) error {
+			c.Assert(req.Controller, qt.Equals, "controller-1")
+			c.Assert(req.ModelTag, qt.Equals, "model-"+modelUUID)
+			c.Assert(req.Owner, qt.Equals, "alice@canonical.com")
+			return nil
+		}).
+		Times(1)
 
-	m := s.Factory.MakeModel(c, &factory.ModelParams{
-		Name:            "model-2",
-		Owner:           names.NewUserTag("charlie@canonical.com"),
-		CloudName:       jimmtest.TestCloudName,
-		CloudRegion:     jimmtest.TestCloudRegionName,
-		CloudCredential: cct,
-	})
-	defer m.Close()
+	cmdMocks.client.EXPECT().Close().Times(1)
 
-	// alice is superuser
-	bClient := s.SetupCLIAccess(c, "alice")
-	_, err = cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-1", m.ModelUUID())
-	c.Assert(err, gc.IsNil)
+	command := &importModelCommand{
+		jimmAPIFunc: func() (JIMMAPI, error) {
+			return cmdMocks.client, nil
+		},
+	}
 
-	var model2 dbmodel.Model
-	model2.SetTag(names.NewModelTag(m.ModelUUID()))
-	err = s.JIMM.Database.GetModel(context.Background(), &model2)
-	c.Assert(err, gc.Equals, nil)
-	c.Check(model2.OwnerIdentityName, gc.Equals, "charlie@canonical.com")
-}
+	err := cmdtesting.InitCommand(command, []string{"controller-1", modelUUID, "--owner", "alice@canonical.com"})
+	c.Assert(err, qt.IsNil)
 
-func (s *importModelSuite) TestImportModelFromLocalUser(c *gc.C) {
-	s.AddController(c, "controller-1", s.APIInfo(c))
-	s.AddAdminUser(c, "charlie@canonical.com")
-	cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/charlie@canonical.com/cred")
-	s.UpdateCloudCredential(c, cct, jujuparams.CloudCredential{AuthType: "empty"})
-	// Add credentials for Alice on the test cloud, they are needed for the Alice user to become the new model owner
-	cctAlice := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/alice@canonical.com/cred")
-	s.UpdateCloudCredential(c, cctAlice, jujuparams.CloudCredential{AuthType: "empty"})
-	mt := s.AddModel(c, names.NewUserTag("charlie@canonical.com"), "model-2", names.NewCloudTag(jimmtest.TestCloudName), jimmtest.TestCloudRegionName, cct)
-	var model dbmodel.Model
-	model.SetTag(mt)
-	err := s.JIMM.Database.GetModel(context.Background(), &model)
-	c.Assert(err, gc.Equals, nil)
-	err = s.JIMM.OpenFGAClient.RemoveControllerModel(context.Background(), model.Controller.ResourceTag(), model.ResourceTag())
-	c.Assert(err, gc.Equals, nil)
-	err = s.JIMM.Database.DeleteModel(context.Background(), &model)
-	c.Assert(err, gc.Equals, nil)
-
-	// alice is superuser
-	bClient := s.SetupCLIAccess(c, "alice")
-	_, err = cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-1", mt.Id(), "--owner", "alice@canonical.com")
-	c.Assert(err, gc.IsNil)
-
-	var model2 dbmodel.Model
-	model2.SetTag(mt)
-	err = s.JIMM.Database.GetModel(context.Background(), &model2)
-	c.Assert(err, gc.Equals, nil)
-	c.Check(model2.CreatedAt.After(model.CreatedAt), gc.Equals, true)
-	c.Check(model2.OwnerIdentityName, gc.Equals, "alice@canonical.com")
+	err = command.Run(newTestContext(c))
+	c.Assert(err, qt.IsNil)
 }
 
-func (s *importModelSuite) TestImportModelUnauthorized(c *gc.C) {
-	s.AddController(c, "controller-1", s.APIInfo(c))
+func TestImportModelRun_WithoutOwnerFlag(t *testing.T) {
+	c := qt.New(t)
 
-	cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/charlie@canonical.com/cred")
-	s.UpdateCloudCredential(c, cct, jujuparams.CloudCredential{AuthType: "empty"})
+	cmdMocks := setupCmdMocks(c)
 
-	err := s.BackingState.UpdateCloudCredential(cct, jjcloud.NewCredential(jjcloud.EmptyAuthType, map[string]string{"key": "value"}))
-	c.Assert(err, gc.Equals, nil)
+	modelUUID := "ac30d6ae-0bed-4398-bba7-75d49e39f189"
 
-	m := s.Factory.MakeModel(c, &factory.ModelParams{
-		Name:            "model-2",
-		Owner:           names.NewUserTag("charlie@canonical.com"),
-		CloudName:       jimmtest.TestCloudName,
-		CloudRegion:     jimmtest.TestCloudRegionName,
-		CloudCredential: cct,
-	})
-	defer m.Close()
+	cmdMocks.client.EXPECT().
+		ImportModel(gomock.Any()).
+		DoAndReturn(func(req *apiparams.ImportModelRequest) error {
+			c.Assert(req.Controller, qt.Equals, "controller-1")
+			c.Assert(req.ModelTag, qt.Equals, "model-"+modelUUID)
+			c.Assert(req.Owner, qt.Equals, "")
+			return nil
+		}).
+		Times(1)
 
-	// bob is not superuser
-	bClient := s.SetupCLIAccess(c, "bob")
-	_, err = cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-1", m.ModelUUID())
-	c.Assert(err, gc.ErrorMatches, `unauthorized \(unauthorized access\)`)
-}
+	cmdMocks.client.EXPECT().Close().Times(1)
 
-func (s *importModelSuite) TestImportModelNoController(c *gc.C) {
-	bClient := s.SetupCLIAccess(c, "bob")
-	_, err := cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient))
-	c.Assert(err, gc.ErrorMatches, `controller not specified`)
-}
+	command := &importModelCommand{
+		jimmAPIFunc: func() (JIMMAPI, error) {
+			return cmdMocks.client, nil
+		},
+	}
 
-func (s *importModelSuite) TestImportModelNoModelUUID(c *gc.C) {
-	bClient := s.SetupCLIAccess(c, "bob")
-	_, err := cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-id")
-	c.Assert(err, gc.ErrorMatches, `model uuid not specified`)
-}
+	fs := gnuflag.NewFlagSet("test", gnuflag.ContinueOnError)
+	command.SetFlags(fs)
 
-func (s *importModelSuite) TestImportModelInvalidModelUUID(c *gc.C) {
-	bClient := s.SetupCLIAccess(c, "bob")
-	_, err := cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-id", "not-a-uuid")
-	c.Assert(err, gc.ErrorMatches, `invalid model uuid`)
-}
+	err := cmdtesting.InitCommand(command, []string{"controller-1", modelUUID})
+	c.Assert(err, qt.IsNil)
 
-func (s *importModelSuite) TestImportModelTooManyArgs(c *gc.C) {
-	bClient := s.SetupCLIAccess(c, "bob")
-	_, err := cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-id", "not-a-uuid", "spare-argument")
-	c.Assert(err, gc.ErrorMatches, `too many args`)
+	err = command.Run(newTestContext(c))
+	c.Assert(err, qt.IsNil)
 }

docs/reference/list-of-jaas-plugin-commands/index.md

@@ -498,7 +498,7 @@ See also: topics
 **Aliases:** register-model
 
 ## Summary
-Import a model to jimm
+Import a model to jimm.
 
 ## Usage
 ```jaas import-model [options] <controller name> <model uuid>```