diff --git a/agent/agentbootstrap/bootstrap.go b/agent/agentbootstrap/bootstrap.go index 2cc727f5585..4ef061d5c5c 100644 --- a/agent/agentbootstrap/bootstrap.go +++ b/agent/agentbootstrap/bootstrap.go @@ -220,12 +220,16 @@ func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller controllerModelType = modeldomain.TypeCAAS } + // Add initial Admin user to the database. This will return Admin user UUID + // and a function to insert it into the database. + adminUserUUID, addAdminUser := userbootstrap.AddUserWithPassword(b.adminUser.Name(), auth.NewPassword(info.Password)) + controllerUUID := modeldomain.UUID( stateParams.ControllerModelConfig.UUID(), ) controllerModelArgs := modeldomain.ModelCreationArgs{ Name: stateParams.ControllerModelConfig.Name(), - Owner: b.adminUser.Name(), + Owner: adminUserUUID, Cloud: stateParams.ControllerCloud.Name, CloudRegion: stateParams.ControllerCloudRegion, Credential: credential.IdFromTag(cloudCredTag), @@ -237,18 +241,16 @@ func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller stateParams.ControllerInheritedConfig, stateParams.RegionInheritedConfig[stateParams.ControllerCloudRegion]) - // Add initial Admin user to the database. This will return Admin user UUID - // and a function to insert it into the database. - _, addAdminUser := userbootstrap.AddUserWithPassword(b.adminUser.Name(), auth.NewPassword(info.Password)) - databaseBootstrapConcerns := []database.BootstrapConcern{ database.BootstrapControllerConcern( ccbootstrap.InsertInitialControllerConfig(stateParams.ControllerConfig), + // The admin user needs to be added before everything else that + // requires being owned by a Juju user. cloudbootstrap.InsertCloud(stateParams.ControllerCloud), credbootstrap.InsertCredential(credential.IdFromTag(cloudCredTag), cloudCred), cloudbootstrap.SetCloudDefaults(stateParams.ControllerCloud.Name, stateParams.ControllerInheritedConfig), - modelbootstrap.CreateModel(controllerUUID, controllerModelArgs), addAdminUser, + modelbootstrap.CreateModel(controllerUUID, controllerModelArgs), ), database.BootstrapModelConcern(controllerUUID, modelconfigbootstrap.SetModelConfig(stateParams.ControllerModelConfig, controllerModelDefaults), diff --git a/api/package_test.go b/api/package_test.go index 4afb2033450..c77897d601e 100644 --- a/api/package_test.go +++ b/api/package_test.go @@ -46,6 +46,7 @@ func (*ImportSuite) TestImports(c *gc.C) { "core/secrets", "core/status", "core/trace", + "core/user", "core/watcher", "domain/credential", // Imported by environs/envcontext. "environs/envcontext", diff --git a/cmd/containeragent/initialize/package_test.go b/cmd/containeragent/initialize/package_test.go index 28f7a058398..258633c8cbf 100644 --- a/cmd/containeragent/initialize/package_test.go +++ b/cmd/containeragent/initialize/package_test.go @@ -72,6 +72,7 @@ func (*importSuite) TestImports(c *gc.C) { "core/status", "core/trace", "core/upgrade", + "core/user", "core/watcher", "domain/credential", "environs/cloudspec", diff --git a/cmd/containeragent/unit/package_test.go b/cmd/containeragent/unit/package_test.go index 26cb7aa9534..43954f7fdb5 100644 --- a/cmd/containeragent/unit/package_test.go +++ b/cmd/containeragent/unit/package_test.go @@ -115,6 +115,7 @@ func (*importSuite) TestImports(c *gc.C) { "core/secrets", "core/snap", "core/status", + "core/user", "core/watcher", "downloader", "environs", diff --git a/cmd/jujud/agent/bootstrap.go b/cmd/jujud/agent/bootstrap.go index f36e01726c4..485d78ff6ce 100644 --- a/cmd/jujud/agent/bootstrap.go +++ b/cmd/jujud/agent/bootstrap.go @@ -36,6 +36,7 @@ import ( "github.com/juju/juju/core/instance" "github.com/juju/juju/core/network" coreos "github.com/juju/juju/core/os" + coreuser "github.com/juju/juju/core/user" "github.com/juju/juju/domain/credential" "github.com/juju/juju/environs" environscloudspec "github.com/juju/juju/environs/cloudspec" @@ -64,8 +65,6 @@ var ( type BootstrapAgentFunc func(agentbootstrap.AgentBootstrapArgs) (*agentbootstrap.AgentBootstrap, error) -const adminUserName = "admin" - // BootstrapCommand represents a jujud bootstrap command. type BootstrapCommand struct { cmd.CommandBase @@ -371,7 +370,7 @@ func (c *BootstrapCommand) Run(ctx *cmd.Context) error { // We shouldn't attempt to dial peers until we have some. dialOpts.Direct = true - adminTag := names.NewLocalUserTag(adminUserName) + adminTag := names.NewLocalUserTag(coreuser.AdminUserName) bootstrap, err := c.BootstrapAgent(agentbootstrap.AgentBootstrapArgs{ AgentConfig: agentConfig, BootstrapEnviron: env, diff --git a/domain/model/defaults.go b/core/model/defaults.go similarity index 57% rename from domain/model/defaults.go rename to core/model/defaults.go index 64dc7d73780..93d8bba9563 100644 --- a/domain/model/defaults.go +++ b/core/model/defaults.go @@ -1,8 +1,10 @@ -// Copyright 2023 Canonical Ltd. +// Copyright 2024 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. package model +import "github.com/juju/juju/core/user" + const ( // ControllerModelName is the name given to the model that hosts the Juju // controller. This is a static value that we use for every Juju deployment. @@ -10,8 +12,8 @@ const ( // logic to ask questions and calculate defaults in Juju. ControllerModelName = "controller" - // ControllerModelOwner is the name of the owner that is assigned to the - // controller model. This is a static value that we ue for every Juju - // deployment. - ControllerModelOwner = "admin" + // ControllerModelOwnerUsername is the user name of the owner that is + // assigned to the controller model. This is a static value that we use for + // every Juju deployment. + ControllerModelOwnerUsername = user.AdminUserName ) diff --git a/core/multiwatcher/package_test.go b/core/multiwatcher/package_test.go index f9154cf5039..2f0ee14ccc5 100644 --- a/core/multiwatcher/package_test.go +++ b/core/multiwatcher/package_test.go @@ -31,6 +31,7 @@ func (*ImportTest) TestImports(c *gc.C) { "core/instance", "core/life", "core/model", + "core/user", "core/network", "core/permission", "core/status", diff --git a/core/user/defaults.go b/core/user/defaults.go new file mode 100644 index 00000000000..e57680a1666 --- /dev/null +++ b/core/user/defaults.go @@ -0,0 +1,10 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package user + +const ( + // AdminUsername is the default username that we give to the default admin + // user that is created as part of every Juju bootstrap. + AdminUserName = "admin" +) diff --git a/domain/cloud/state/state.go b/domain/cloud/state/state.go index faace8bebcc..11ce7f5f2dd 100644 --- a/domain/cloud/state/state.go +++ b/domain/cloud/state/state.go @@ -15,10 +15,10 @@ import ( "github.com/juju/juju/cloud" "github.com/juju/juju/core/changestream" coredatabase "github.com/juju/juju/core/database" + coremodel "github.com/juju/juju/core/model" "github.com/juju/juju/core/watcher" "github.com/juju/juju/domain" clouderrors "github.com/juju/juju/domain/cloud/errors" - "github.com/juju/juju/domain/model" "github.com/juju/juju/internal/database" ) @@ -340,9 +340,11 @@ SELECT cloud.uuid, cloud.name, cloud_type_id, auth_type.type, cloud_type.type, COALESCE((SELECT true FROM model_metadata + INNER JOIN user + ON user.uuid = model_metadata.owner_uuid WHERE cloud_uuid = cloud.uuid - AND name = ? - AND owner_uuid = ?), false) AS controller_cloud + AND model_metadata.name = ? + AND user.name = ?), false) AS controller_cloud FROM cloud LEFT JOIN cloud_auth_type ON cloud.uuid = cloud_auth_type.cloud_uuid @@ -353,8 +355,8 @@ FROM cloud ` args := []any{ - model.ControllerModelName, - model.ControllerModelOwner, + coremodel.ControllerModelName, + coremodel.ControllerModelOwnerUsername, } if name != "" { q = q + "WHERE cloud.name = ?" diff --git a/domain/cloud/state/state_test.go b/domain/cloud/state/state_test.go index 274cbc2c522..58179a0388b 100644 --- a/domain/cloud/state/state_test.go +++ b/domain/cloud/state/state_test.go @@ -4,7 +4,7 @@ package state import ( - ctx "context" + "context" "database/sql" "time" @@ -13,11 +13,12 @@ import ( jc "github.com/juju/testing/checkers" "github.com/juju/utils/v3" "github.com/juju/worker/v4/workertest" - "golang.org/x/net/context" gc "gopkg.in/check.v1" "github.com/juju/juju/cloud" "github.com/juju/juju/core/changestream" + coremodel "github.com/juju/juju/core/model" + "github.com/juju/juju/core/user" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/eventsource" "github.com/juju/juju/core/watcher/watchertest" @@ -25,6 +26,7 @@ import ( "github.com/juju/juju/domain/model" modelstate "github.com/juju/juju/domain/model/state" modeltesting "github.com/juju/juju/domain/model/testing" + userstate "github.com/juju/juju/domain/user/state" "github.com/juju/juju/internal/changestream/testing" jujudb "github.com/juju/juju/internal/database" jujutesting "github.com/juju/juju/testing" @@ -202,7 +204,7 @@ func (s *stateSuite) assertRegions(c *gc.C, cloudUUID string, expected []cloud.R } func (s *stateSuite) assertInsertCloud(c *gc.C, st *State, cloud cloud.Cloud) string { - err := st.UpsertCloud(ctx.Background(), cloud) + err := st.UpsertCloud(context.Background(), cloud) c.Assert(err, jc.ErrorIsNil) cloudUUID := s.assertCloud(c, cloud) @@ -255,7 +257,7 @@ func (s *stateSuite) TestUpsertCloudUpdateExisting(c *gc.C) { IsControllerCloud: true, } - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) cloudUUID := s.assertCloud(c, cld) @@ -267,7 +269,7 @@ func (s *stateSuite) TestUpsertCloudInvalidType(c *gc.C) { cld.Type = "mycloud" st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, gc.ErrorMatches, `.* cloud type "mycloud" not valid`) } @@ -276,7 +278,7 @@ func (s *stateSuite) TestCloudWithEmptyNameFails(c *gc.C) { cld.Name = "" st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIs, errors.NotValid) } @@ -284,7 +286,7 @@ func (s *stateSuite) TestUpdateCloudDefaults(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = st.UpdateCloudDefaults(context.Background(), cld.Name, map[string]string{ @@ -305,7 +307,7 @@ func (s *stateSuite) TestComplexUpdateCloudDefaults(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = st.UpdateCloudDefaults(context.Background(), cld.Name, map[string]string{ @@ -347,7 +349,7 @@ func (s *stateSuite) TestCloudRegionDefaults(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = st.UpdateCloudRegionDefaults( @@ -390,7 +392,7 @@ func (s *stateSuite) TestCloudRegionDefaultsComplex(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = st.UpdateCloudRegionDefaults( @@ -487,7 +489,7 @@ func (s *stateSuite) TestCloudDefaultsRemoval(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = st.UpdateCloudDefaults(context.Background(), cld.Name, map[string]string{ @@ -519,7 +521,7 @@ func (s *stateSuite) TestEmptyCloudDefaults(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) defaults, err := st.CloudDefaults(context.Background(), cld.Name) @@ -542,18 +544,18 @@ func (s *stateSuite) TestUpsertCloudInvalidAuthType(c *gc.C) { cld.AuthTypes = []cloud.AuthType{"myauth"} st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, gc.ErrorMatches, `.* auth type "myauth" not valid`) } func (s *stateSuite) TestListClouds(c *gc.C) { st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), testCloud) + err := st.UpsertCloud(context.Background(), testCloud) c.Assert(err, jc.ErrorIsNil) - err = st.UpsertCloud(ctx.Background(), testCloud2) + err = st.UpsertCloud(context.Background(), testCloud2) c.Assert(err, jc.ErrorIsNil) - clouds, err := st.ListClouds(ctx.Background(), "") + clouds, err := st.ListClouds(context.Background(), "") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 2) if clouds[0].Name == testCloud.Name { @@ -566,13 +568,25 @@ func (s *stateSuite) TestListClouds(c *gc.C) { } func (s *stateSuite) TestCloudIsControllerCloud(c *gc.C) { + userUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + + userState := userstate.NewState(s.TxnRunnerFactory()) + err = userState.AddUser( + context.Background(), userUUID, + coremodel.ControllerModelOwnerUsername, + "test user", + userUUID, + ) + c.Assert(err, jc.ErrorIsNil) + st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), testCloud) + err = st.UpsertCloud(context.Background(), testCloud) c.Assert(err, jc.ErrorIsNil) - err = st.UpsertCloud(ctx.Background(), testCloud2) + err = st.UpsertCloud(context.Background(), testCloud2) c.Assert(err, jc.ErrorIsNil) - clouds, err := st.ListClouds(ctx.Background(), "") + clouds, err := st.ListClouds(context.Background(), "") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 2) @@ -587,14 +601,14 @@ func (s *stateSuite) TestCloudIsControllerCloud(c *gc.C) { modelUUID, model.ModelCreationArgs{ Cloud: testCloud.Name, - Name: "controller", - Owner: "admin", + Name: coremodel.ControllerModelName, + Owner: userUUID, Type: model.TypeIAAS, }, ) c.Assert(err, jc.ErrorIsNil) - clouds, err = st.ListClouds(ctx.Background(), "") + clouds, err = st.ListClouds(context.Background(), "") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 2) @@ -609,16 +623,16 @@ func (s *stateSuite) TestCloudIsControllerCloud(c *gc.C) { func (s *stateSuite) TestListCloudsFilter(c *gc.C) { st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), testCloud) + err := st.UpsertCloud(context.Background(), testCloud) c.Assert(err, jc.ErrorIsNil) - err = st.UpsertCloud(ctx.Background(), testCloud2) + err = st.UpsertCloud(context.Background(), testCloud2) c.Assert(err, jc.ErrorIsNil) - clouds, err := st.ListClouds(ctx.Background(), "fluffy3") + clouds, err := st.ListClouds(context.Background(), "fluffy3") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 0) - clouds, err = st.ListClouds(ctx.Background(), "fluffy2") + clouds, err = st.ListClouds(context.Background(), "fluffy2") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, jc.DeepEquals, []cloud.Cloud{testCloud2}) } @@ -627,10 +641,10 @@ func (s *stateSuite) TestDeleteCloud(c *gc.C) { st := NewState(s.TxnRunnerFactory()) s.assertInsertCloud(c, st, testCloud) - err := st.DeleteCloud(ctx.Background(), "fluffy") + err := st.DeleteCloud(context.Background(), "fluffy") c.Assert(err, jc.ErrorIsNil) - clouds, err := st.ListClouds(ctx.Background(), "fluffy") + clouds, err := st.ListClouds(context.Background(), "fluffy") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 0) } @@ -659,10 +673,10 @@ WHERE cloud.name = ? }) c.Assert(err, jc.ErrorIsNil) - err = st.DeleteCloud(ctx.Background(), "fluffy") + err = st.DeleteCloud(context.Background(), "fluffy") c.Assert(err, gc.ErrorMatches, "cannot delete cloud as it is still in use") - clouds, err := st.ListClouds(ctx.Background(), "fluffy") + clouds, err := st.ListClouds(context.Background(), "fluffy") c.Assert(err, jc.ErrorIsNil) c.Assert(clouds, gc.HasLen, 1) c.Assert(clouds[0].Name, gc.Equals, "fluffy") @@ -701,7 +715,7 @@ func (s *stateSuite) TestWatchCloudNotFound(c *gc.C) { func (s *stateSuite) TestWatchCloud(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) var uuid string @@ -745,7 +759,7 @@ func (s *stateSuite) TestNullCloudType(c *gc.C) { func (s *stateSuite) TestSetCloudDefaults(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { @@ -785,7 +799,7 @@ func (s *stateSuite) TestSetCloudDefaultsNotFound(c *gc.C) { func (s *stateSuite) TestSetCloudDefaultsOverrides(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { @@ -821,7 +835,7 @@ func (s *stateSuite) TestSetCloudDefaultsOverrides(c *gc.C) { func (s *stateSuite) TestSetCloudDefaultsDelete(c *gc.C) { cld := testCloud st := NewState(s.TxnRunnerFactory()) - err := st.UpsertCloud(ctx.Background(), cld) + err := st.UpsertCloud(context.Background(), cld) c.Assert(err, jc.ErrorIsNil) err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { diff --git a/domain/credential/state/state_test.go b/domain/credential/state/state_test.go index f1230c869c8..1b6169d286a 100644 --- a/domain/credential/state/state_test.go +++ b/domain/credential/state/state_test.go @@ -19,18 +19,21 @@ import ( "github.com/juju/juju/cloud" "github.com/juju/juju/core/changestream" + "github.com/juju/juju/core/user" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/eventsource" "github.com/juju/juju/core/watcher/watchertest" dbcloud "github.com/juju/juju/domain/cloud/state" "github.com/juju/juju/domain/credential" "github.com/juju/juju/domain/model" + userstate "github.com/juju/juju/domain/user/state" changestreamtesting "github.com/juju/juju/internal/changestream/testing" jujutesting "github.com/juju/juju/testing" ) type credentialSuite struct { changestreamtesting.ControllerSuite + userUUID user.UUID } var _ = gc.Suite(&credentialSuite{}) @@ -38,6 +41,19 @@ var _ = gc.Suite(&credentialSuite{}) func (s *credentialSuite) SetUpTest(c *gc.C) { s.ControllerSuite.SetUpTest(c) + userUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + s.userUUID = userUUID + userState := userstate.NewState(s.TxnRunnerFactory()) + err = userState.AddUser( + context.Background(), + s.userUUID, + "test-user", + "test user", + s.userUUID, + ) + c.Assert(err, jc.ErrorIsNil) + s.addCloud(c, cloud.Cloud{ Name: "stratus", Type: "ec2", @@ -547,9 +563,10 @@ func (s *credentialSuite) TestModelsUsingCloudCredential(c *gc.C) { } result, err := tx.ExecContext(ctx, fmt.Sprintf(` INSERT INTO model_metadata (model_uuid, name, owner_uuid, model_type_id, cloud_uuid, cloud_credential_uuid) - SELECT %q, %q, "admin", 0, + SELECT %q, %q, %q, 0, (SELECT uuid FROM cloud WHERE cloud.name="stratus"), - (SELECT uuid FROM cloud_credential cc WHERE cc.name="foobar")`, modelUUID, name), + (SELECT uuid FROM cloud_credential cc WHERE cc.name="foobar")`, + modelUUID, name, s.userUUID), ) if err != nil { return err diff --git a/domain/model/service/service.go b/domain/model/service/service.go index 8ff5103d6e0..a9faf8c1da8 100644 --- a/domain/model/service/service.go +++ b/domain/model/service/service.go @@ -44,6 +44,8 @@ func NewService(st State) *Service { // - modelerrors.AlreadyExists: When the model uuid is already in use or a model // with the same name and owner already exists. // - errors.NotFound: When the cloud, cloud region, or credential do not exist. +// - [github.com/juju/juju/domain/user/errors.NotFound] when the owner of the +// mode cannot be found. func (s *Service) CreateModel( ctx context.Context, args model.ModelCreationArgs, diff --git a/domain/model/service/service_test.go b/domain/model/service/service_test.go index a22b1217177..ff8fe8453dd 100644 --- a/domain/model/service/service_test.go +++ b/domain/model/service/service_test.go @@ -7,15 +7,18 @@ import ( "context" "fmt" + "github.com/juju/collections/set" "github.com/juju/errors" "github.com/juju/testing" jc "github.com/juju/testing/checkers" . "gopkg.in/check.v1" + "github.com/juju/juju/core/user" "github.com/juju/juju/domain/credential" "github.com/juju/juju/domain/model" modelerrors "github.com/juju/juju/domain/model/errors" modeltesting "github.com/juju/juju/domain/model/testing" + usererrors "github.com/juju/juju/domain/user/errors" ) type dummyStateCloud struct { @@ -26,12 +29,14 @@ type dummyStateCloud struct { type dummyState struct { clouds map[string]dummyStateCloud models map[model.UUID]model.ModelCreationArgs + users set.Strings } type serviceSuite struct { testing.IsolationSuite modelUUID model.UUID + userUUID user.UUID state *dummyState } @@ -57,6 +62,10 @@ func (d *dummyState) Create( return fmt.Errorf("%w cloud %q", errors.NotFound, args.Cloud) } + if !d.users.Contains(args.Owner.String()) { + return fmt.Errorf("%w for owner %q", usererrors.NotFound, args.Owner) + } + hasRegion := false for _, region := range cloud.Regions { if region == args.CloudRegion { @@ -116,9 +125,13 @@ func (d *dummyState) UpdateCredential( func (s *serviceSuite) SetUpTest(c *C) { s.modelUUID = modeltesting.GenModelUUID(c) + var err error + s.userUUID, err = user.NewUUID() + c.Assert(err, jc.ErrorIsNil) s.state = &dummyState{ clouds: map[string]dummyStateCloud{}, models: map[model.UUID]model.ModelCreationArgs{}, + users: set.NewStrings(s.userUUID.String()), } } @@ -132,7 +145,7 @@ func (s *serviceSuite) TestModelCreation(c *C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", - Owner: "wallyworld", + Owner: s.userUUID.String(), } s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{ @@ -146,7 +159,7 @@ func (s *serviceSuite) TestModelCreation(c *C) { Cloud: "aws", CloudRegion: "myregion", Credential: cred, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -162,7 +175,7 @@ func (s *serviceSuite) TestModelCreationInvalidCloud(c *C) { _, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "myregion", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -179,7 +192,7 @@ func (s *serviceSuite) TestModelCreationNoCloudRegion(c *C) { _, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "noexist", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -187,6 +200,29 @@ func (s *serviceSuite) TestModelCreationNoCloudRegion(c *C) { c.Assert(err, jc.ErrorIs, errors.NotFound) } +// TestModelCreationOwnerNotFound is testing that if we make a model with an +// owner that doesn't exist we get back a [usererrors.NotFound] error. +func (s *serviceSuite) TestModelCreationOwnerNotFound(c *C) { + s.state.clouds["aws"] = dummyStateCloud{ + Credentials: map[string]credential.ID{}, + Regions: []string{"myregion"}, + } + + notFoundUser, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + + svc := NewService(s.state) + _, err = svc.CreateModel(context.Background(), model.ModelCreationArgs{ + Cloud: "aws", + CloudRegion: "myregion", + Owner: notFoundUser, + Name: "my-awesome-model", + Type: model.TypeIAAS, + }) + + c.Assert(err, jc.ErrorIs, usererrors.NotFound) +} + func (s *serviceSuite) TestModelCreationNoCloudCredential(c *C) { s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{}, @@ -200,9 +236,9 @@ func (s *serviceSuite) TestModelCreationNoCloudCredential(c *C) { Credential: credential.ID{ Cloud: "aws", Name: "foo", - Owner: "wallyworld", + Owner: s.userUUID.String(), }, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -220,7 +256,7 @@ func (s *serviceSuite) TestModelCreationNameOwnerConflict(c *C) { _, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "myregion", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -230,7 +266,7 @@ func (s *serviceSuite) TestModelCreationNameOwnerConflict(c *C) { _, err = svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "myregion", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -244,7 +280,7 @@ func (s *serviceSuite) TestUpdateModelCredentialForInvalidModel(c *C) { svc := NewService(s.state) err = svc.UpdateCredential(context.Background(), id, credential.ID{ - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foo", Cloud: "aws", }) @@ -254,7 +290,7 @@ func (s *serviceSuite) TestUpdateModelCredentialForInvalidModel(c *C) { func (s *serviceSuite) TestUpdateModelCredential(c *C) { cred := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar", } @@ -269,7 +305,7 @@ func (s *serviceSuite) TestUpdateModelCredential(c *C) { id, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "myregion", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -283,12 +319,12 @@ func (s *serviceSuite) TestUpdateModelCredential(c *C) { func (s *serviceSuite) TestUpdateModelCredentialReplace(c *C) { cred := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar", } cred2 := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar2", } @@ -305,7 +341,7 @@ func (s *serviceSuite) TestUpdateModelCredentialReplace(c *C) { Cloud: "aws", CloudRegion: "myregion", Credential: cred, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -319,7 +355,7 @@ func (s *serviceSuite) TestUpdateModelCredentialReplace(c *C) { func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *C) { cred := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar", } @@ -334,7 +370,7 @@ func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *C) { id, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", CloudRegion: "myregion", - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -348,12 +384,12 @@ func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *C) { func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *C) { cred := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar", } cred2 := credential.ID{ Cloud: "kubernetes", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar2", } @@ -375,7 +411,7 @@ func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *C) { Cloud: "aws", CloudRegion: "myregion", Credential: cred, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -389,12 +425,12 @@ func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *C) { func (s *serviceSuite) TestUpdateModelCredentialNotFound(c *C) { cred := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar", } cred2 := credential.ID{ Cloud: "aws", - Owner: "wallyworld", + Owner: s.userUUID.String(), Name: "foobar2", } @@ -410,7 +446,7 @@ func (s *serviceSuite) TestUpdateModelCredentialNotFound(c *C) { Cloud: "aws", CloudRegion: "myregion", Credential: cred, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) @@ -425,7 +461,7 @@ func (s *serviceSuite) TestDeleteModel(c *C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", - Owner: "wallyworld", + Owner: s.userUUID.String(), } s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{ @@ -439,7 +475,7 @@ func (s *serviceSuite) TestDeleteModel(c *C) { Cloud: "aws", CloudRegion: "myregion", Credential: cred, - Owner: "wallyworld", + Owner: s.userUUID, Name: "my-awesome-model", Type: model.TypeIAAS, }) diff --git a/domain/model/state/state.go b/domain/model/state/state.go index baaa0b360bb..3162969cc3a 100644 --- a/domain/model/state/state.go +++ b/domain/model/state/state.go @@ -15,6 +15,7 @@ import ( "github.com/juju/juju/domain/credential" "github.com/juju/juju/domain/model" modelerrors "github.com/juju/juju/domain/model/errors" + usererrors "github.com/juju/juju/domain/user/errors" jujudb "github.com/juju/juju/internal/database" ) @@ -108,6 +109,8 @@ func createModel(ctx context.Context, uuid model.UUID, tx *sql.Tx) error { // If the ModelCreationArgs contains a non zero value cloud credential this func // will also attempt to set the model cloud credential using updateCredential. In // this scenario the errors from updateCredential are also possible. +// If the model owner does not exist an error satisfying [usererrors.NotFound] +// will be returned. func createModelMetadata( ctx context.Context, uuid model.UUID, @@ -119,9 +122,7 @@ SELECT uuid FROM cloud WHERE name = ? ` - var ( - cloudUUID string - ) + var cloudUUID string err := tx.QueryRowContext(ctx, cloudStmt, input.Cloud).Scan(&cloudUUID) if errors.Is(err, sql.ErrNoRows) { return fmt.Errorf("%w cloud %q", errors.NotFound, input.Cloud) @@ -129,6 +130,20 @@ WHERE name = ? return fmt.Errorf("getting cloud %q uuid: %w", input.Cloud, err) } + userStmt := ` +SELECT uuid +FROM user +WHERE uuid = ? +AND removed = false +` + var userUUID string + err = tx.QueryRowContext(ctx, userStmt, input.Owner).Scan(&userUUID) + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("%w for model owner %q", usererrors.NotFound, input.Owner) + } else if err != nil { + return fmt.Errorf("getting user uuid for setting model %q owner: %w", input.Name, err) + } + stmt := ` INSERT INTO model_metadata (model_uuid, cloud_uuid, diff --git a/domain/model/state/state_test.go b/domain/model/state/state_test.go index bc9f0451dcc..d93075993b9 100644 --- a/domain/model/state/state_test.go +++ b/domain/model/state/state_test.go @@ -12,6 +12,7 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/cloud" + "github.com/juju/juju/core/user" dbcloud "github.com/juju/juju/domain/cloud/state" "github.com/juju/juju/domain/credential" credentialstate "github.com/juju/juju/domain/credential/state" @@ -19,11 +20,14 @@ import ( modelerrors "github.com/juju/juju/domain/model/errors" modeltesting "github.com/juju/juju/domain/model/testing" schematesting "github.com/juju/juju/domain/schema/testing" + usererrors "github.com/juju/juju/domain/user/errors" + userstate "github.com/juju/juju/domain/user/state" ) type modelSuite struct { schematesting.ControllerSuite - uuid model.UUID + uuid model.UUID + userUUID user.UUID } var _ = gc.Suite(&modelSuite{}) @@ -31,8 +35,23 @@ var _ = gc.Suite(&modelSuite{}) func (m *modelSuite) SetUpTest(c *gc.C) { m.ControllerSuite.SetUpTest(c) + // We need to generate a user in the database so that we can set the model + // owner. + userUUID, err := user.NewUUID() + m.userUUID = userUUID + c.Assert(err, jc.ErrorIsNil) + userState := userstate.NewState(m.TxnRunnerFactory()) + err = userState.AddUser( + context.Background(), + m.userUUID, + "test-user", + "test user", + m.userUUID, + ) + c.Assert(err, jc.ErrorIsNil) + cloudSt := dbcloud.NewState(m.TxnRunnerFactory()) - err := cloudSt.UpsertCloud(context.Background(), cloud.Cloud{ + err = cloudSt.UpsertCloud(context.Background(), cloud.Cloud{ Name: "my-cloud", Type: "ec2", AuthTypes: cloud.AuthTypes{cloud.AccessKeyAuthType, cloud.UserPassAuthType}, @@ -57,7 +76,7 @@ func (m *modelSuite) SetUpTest(c *gc.C) { _, err = credSt.UpsertCloudCredential( context.Background(), credential.ID{ Cloud: "my-cloud", - Owner: "wallyworld", + Owner: string(m.userUUID), Name: "foobar", }, cred, @@ -74,11 +93,11 @@ func (m *modelSuite) SetUpTest(c *gc.C) { CloudRegion: "my-region", Credential: credential.ID{ Cloud: "my-cloud", - Owner: "wallyworld", + Owner: string(m.userUUID), Name: "foobar", }, Name: "my-test-model", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, ) @@ -98,7 +117,7 @@ func (m *modelSuite) TestCreateModelMetadataWithNoModel(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "fantasticmodel", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, tx, @@ -119,7 +138,7 @@ func (m *modelSuite) TestCreateModelMetadataWithExistingMetadata(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "fantasticmodel", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, tx, @@ -138,7 +157,7 @@ func (m *modelSuite) TestCreateModelWithSameNameAndOwner(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "my-test-model", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, ) @@ -155,13 +174,57 @@ func (m *modelSuite) TestCreateModelWithInvalidCloudRegion(c *gc.C) { Cloud: "my-cloud", CloudRegion: "noexist", Name: "noregion", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, ) c.Assert(err, jc.ErrorIs, errors.NotFound) } +// TestCreateModelWithNonExistentOwner is here to assert that if we try and make +// a model with a user/owner that does not exist a [usererrors.NotFound] error +// is returned. +func (m *modelSuite) TestCreateModelWithNonExistentOwner(c *gc.C) { + modelSt := NewState(m.TxnRunnerFactory()) + testUUID := modeltesting.GenModelUUID(c) + err := modelSt.Create( + context.Background(), + testUUID, + model.ModelCreationArgs{ + Cloud: "my-cloud", + CloudRegion: "noexist", + Name: "noregion", + Owner: user.UUID("noexist"), // does not exist + Type: model.TypeIAAS, + }, + ) + c.Assert(err, jc.ErrorIs, usererrors.NotFound) +} + +// TestCreateModelWithRemovedOwner is here to test that if we try and create a +// new model with an owner that has been removed from the Juju user base that +// the operation fails with a [usererrors.NotFound] error. +func (m *modelSuite) TestCreateModelWithRemovedOwner(c *gc.C) { + userState := userstate.NewState(m.TxnRunnerFactory()) + err := userState.RemoveUser(context.Background(), m.userUUID) + c.Assert(err, jc.ErrorIsNil) + + modelSt := NewState(m.TxnRunnerFactory()) + testUUID := modeltesting.GenModelUUID(c) + err = modelSt.Create( + context.Background(), + testUUID, + model.ModelCreationArgs{ + Cloud: "my-cloud", + CloudRegion: "noexist", + Name: "noregion", + Owner: m.userUUID, + Type: model.TypeIAAS, + }, + ) + c.Assert(err, jc.ErrorIs, usererrors.NotFound) +} + func (m *modelSuite) TestCreateModelWithInvalidCloud(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) testUUID := modeltesting.GenModelUUID(c) @@ -172,7 +235,7 @@ func (m *modelSuite) TestCreateModelWithInvalidCloud(c *gc.C) { Cloud: "noexist", CloudRegion: "my-region", Name: "noregion", - Owner: "wallyworld", + Owner: m.userUUID, Type: model.TypeIAAS, }, ) @@ -206,7 +269,7 @@ func (m *modelSuite) TestUpdateCredentialForDifferentCloud(c *gc.C) { _, err = credSt.UpsertCloudCredential( context.Background(), credential.ID{ Cloud: "my-cloud2", - Owner: "wallyworld", + Owner: string(m.userUUID), Name: "foobar", }, cred, @@ -219,7 +282,7 @@ func (m *modelSuite) TestUpdateCredentialForDifferentCloud(c *gc.C) { m.uuid, credential.ID{ Cloud: "my-cloud2", - Owner: "wallyworld", + Owner: string(m.userUUID), Name: "foobar", }, ) @@ -253,7 +316,7 @@ func (m *modelSuite) TestSetModelCloudCredentialWithoutRegion(c *gc.C) { _, err = credSt.UpsertCloudCredential( context.Background(), credential.ID{ Cloud: "minikube", - Owner: "admin", + Owner: string(m.userUUID), Name: "foobar", }, cred, @@ -269,11 +332,11 @@ func (m *modelSuite) TestSetModelCloudCredentialWithoutRegion(c *gc.C) { Cloud: "minikube", Credential: credential.ID{ Cloud: "minikube", - Owner: "admin", + Owner: string(m.userUUID), Name: "foobar", }, Name: "controller", - Owner: "admin", + Owner: m.userUUID, Type: model.TypeCAAS, }, ) diff --git a/domain/model/types.go b/domain/model/types.go index a2cca3509d6..e49dc9f4c13 100644 --- a/domain/model/types.go +++ b/domain/model/types.go @@ -9,6 +9,7 @@ import ( "github.com/juju/errors" "github.com/juju/utils/v3" + "github.com/juju/juju/core/user" "github.com/juju/juju/domain/credential" ) @@ -32,9 +33,8 @@ type ModelCreationArgs struct { // Must not be empty for a valid struct. Name string - // Owner is the name of the owner for the model. - // Must not be empty for a valid struct. - Owner string + // Owner is the uuid of the user that owns this model in the Juju controller. + Owner user.UUID // Type is the type of the model. // Type must satisfy IsValid() for a valid struct. @@ -87,8 +87,8 @@ func (m ModelCreationArgs) Validate() error { if m.Name == "" { return fmt.Errorf("%w name cannot be empty", errors.NotValid) } - if m.Owner == "" { - return fmt.Errorf("%w owner cannot be empty", errors.NotValid) + if err := m.Owner.Validate(); err != nil { + return fmt.Errorf("%w owner: %w", errors.NotValid, err) } if !m.Type.IsValid() { return fmt.Errorf("%w model type of %q", errors.NotSupported, m.Type) diff --git a/domain/model/types_test.go b/domain/model/types_test.go index e6e97d055b8..5c401c444d7 100644 --- a/domain/model/types_test.go +++ b/domain/model/types_test.go @@ -10,6 +10,7 @@ import ( "github.com/juju/utils/v3" gc "gopkg.in/check.v1" + "github.com/juju/juju/core/user" "github.com/juju/juju/domain/credential" ) @@ -51,6 +52,8 @@ func (s *typesSuite) TestUUIDValidate(c *gc.C) { } func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { + userUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) tests := []struct { Args ModelCreationArgs ErrTest error @@ -60,7 +63,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeCAAS, }, ErrTest: errors.NotValid, @@ -80,7 +83,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: Type("ipv6-only"), }, ErrTest: errors.NotSupported, @@ -90,7 +93,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Cloud: "", CloudRegion: "my-region", Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeIAAS, }, ErrTest: errors.NotValid, @@ -100,7 +103,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Cloud: "my-cloud", CloudRegion: "", Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeIAAS, }, ErrTest: nil, @@ -113,7 +116,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Owner: "wallyworld", }, Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeIAAS, }, ErrTest: errors.NotValid, @@ -123,7 +126,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Cloud: "my-cloud", CloudRegion: "my-region", Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeIAAS, }, ErrTest: nil, @@ -138,7 +141,7 @@ func (s *typesSuite) TestModelCreationArgsValidation(c *gc.C) { Name: "mycred", }, Name: "my-awesome-model", - Owner: "wallyworld-ipv6", + Owner: userUUID, Type: TypeIAAS, }, ErrTest: nil, diff --git a/domain/schema/controller.go b/domain/schema/controller.go index 09cffb34c3e..b5df64c08bc 100644 --- a/domain/schema/controller.go +++ b/domain/schema/controller.go @@ -373,9 +373,9 @@ CREATE TABLE model_metadata ( CONSTRAINT fk_model_metadata_model_type_id FOREIGN KEY (model_type_id) REFERENCES model_type(id) --- CONSTRAINT fk_model_metadata_XXXX --- FOREIGN KEY (owner_uuid) --- REFERENCES XXXX(uuid) + CONSTRAINT fk_model_metadata_owner_uuid + FOREIGN KEY (owner_uuid) + REFERENCES user(uuid) ); CREATE UNIQUE INDEX idx_model_metadata_name_owner