Skip to content

Commit

Permalink
Merge pull request juju#16813 from tlm/juju-5345-user-model-foreign-key
Browse files Browse the repository at this point in the history
juju#16813

Until now we haven't had any strong referential integrity for model owners onto the user table. Now that the user DDL has been established this sets up the checks and test to assert that when adding a model the owner exists and is a valid user in the system (not removed).

As part of this PR I have also wired up the bootstrap process to set the admin user as the owner of the controller model.

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

Run unit tests under `service` and `state` in the model domain package.

Also bootstrap a controller to lxd and make sure it succeeds with no error output around creating the controller model.

## Documentation changes

N/A

## Links

**Jira card:** JUJU-5345
  • Loading branch information
jujubot authored Jan 25, 2024
2 parents 03c73de + 0962f64 commit 094e14a
Show file tree
Hide file tree
Showing 18 changed files with 281 additions and 112 deletions.
14 changes: 8 additions & 6 deletions agent/agentbootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down
1 change: 1 addition & 0 deletions api/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions cmd/containeragent/initialize/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func (*importSuite) TestImports(c *gc.C) {
"core/status",
"core/trace",
"core/upgrade",
"core/user",
"core/watcher",
"domain/credential",
"environs/cloudspec",
Expand Down
1 change: 1 addition & 0 deletions cmd/containeragent/unit/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func (*importSuite) TestImports(c *gc.C) {
"core/secrets",
"core/snap",
"core/status",
"core/user",
"core/watcher",
"downloader",
"environs",
Expand Down
5 changes: 2 additions & 3 deletions cmd/jujud/agent/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 7 additions & 5 deletions domain/model/defaults.go → core/model/defaults.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
// 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.
// It provides a common reference point that we can leverage in business
// 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
)
1 change: 1 addition & 0 deletions core/multiwatcher/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func (*ImportTest) TestImports(c *gc.C) {
"core/instance",
"core/life",
"core/model",
"core/user",
"core/network",
"core/permission",
"core/status",
Expand Down
10 changes: 10 additions & 0 deletions core/user/defaults.go
Original file line number Diff line number Diff line change
@@ -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"
)
12 changes: 7 additions & 5 deletions domain/cloud/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -353,8 +355,8 @@ FROM cloud
`

args := []any{
model.ControllerModelName,
model.ControllerModelOwner,
coremodel.ControllerModelName,
coremodel.ControllerModelOwnerUsername,
}
if name != "" {
q = q + "WHERE cloud.name = ?"
Expand Down
Loading

0 comments on commit 094e14a

Please sign in to comment.