From 878274360899b04db184ea9d7ab81e940d8f088c Mon Sep 17 00:00:00 2001
From: Kian Parvin <kian.parvin@canonical.com>
Date: Wed, 6 Sep 2023 10:16:46 +0200
Subject: [PATCH] PR suggestions

---
 cmd/jimmctl/cmd/importmodel.go |  7 +++++++
 internal/jimm/controller.go    | 13 ++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/cmd/jimmctl/cmd/importmodel.go b/cmd/jimmctl/cmd/importmodel.go
index 2563a939e..881593722 100644
--- a/cmd/jimmctl/cmd/importmodel.go
+++ b/cmd/jimmctl/cmd/importmodel.go
@@ -19,6 +19,13 @@ import (
 const importModelCommandDoc = `
 	import-model imports a model running on a controller to jimm.
 
+	When importing, it is necessary for JIMM to contain cloud credentials
+	for the user against the cloud the incoming model resides.
+
+	The --owner command is necessary when importing a model created by a 
+	local user and will switch the model owner to the desired external user.
+	E.g. --owner my-user@external
+
 	Example:
 		jimmctl import-model <controller name> <model-uuid>
 		jimmctl import-model <controller name> <model-uuid> --owner <username>
diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go
index 0caad17c6..3be35d32f 100644
--- a/internal/jimm/controller.go
+++ b/internal/jimm/controller.go
@@ -482,6 +482,9 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
 	if err != nil {
 		return errors.E(op, err)
 	}
+	if ownerTag.IsLocal() {
+		return errors.E(op, "cannot import model from local user, try --owner to switch the model owner")
+	}
 	ownerUser := dbmodel.User{}
 	ownerUser.SetTag(ownerTag)
 	err = j.Database.GetUser(ctx, &ownerUser)
@@ -554,9 +557,13 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
 
 	var usersExcludingLocalUsers []dbmodel.UserModelAccess
 	for _, userAccess := range model.Users {
-		if !strings.Contains(userAccess.User.Username, "@") {
-			// If the username doesn't contain an "@" the user is local
-			// to the controller and we don't want to propagate it.
+		username, err := names.ParseUserTag(userAccess.User.Username)
+		if err != nil {
+			zapctx.Error(ctx, "failed to parse user tag", zap.String("username", userAccess.User.Username))
+			continue
+		}
+		if username.IsLocal() {
+			// Don't propogate local users into JIMM.
 			continue
 		}
 		usersExcludingLocalUsers = append(usersExcludingLocalUsers, userAccess)