Skip to content

Commit

Permalink
Merge pull request juju#18034 from Aflynn50/remove-user-permission-state
Browse files Browse the repository at this point in the history
juju#18034

Remove users and permissions from state.
<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0 
-->

<!-- Why this change is needed and what it does. -->

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [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

## QA steps
Unit tests pass. This PR doesn't actually change any production code so no further QA should be needed.
<!-- Describe steps to verify that the change works. -->

## Links

<!-- Link to all relevant specification, documentation, bug, issue or JIRA card. -->

**Jira card:** [JUJU-6700](https://warthogs.atlassian.net/browse/JUJU-6700), [ JUJU-6701](https://warthogs.atlassian.net/browse/JUJU-6701)



[JUJU-6700]: https://warthogs.atlassian.net/browse/JUJU-6700?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Sep 13, 2024
2 parents f16eb44 + 7802c8e commit be6ce33
Show file tree
Hide file tree
Showing 25 changed files with 30 additions and 1,232 deletions.
3 changes: 0 additions & 3 deletions agent/agentbootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,6 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
modelTag := model.Tag().(names.ModelTag)
controllerTag := names.NewControllerTag(controllerCfg.ControllerUUID())
s.assertCanLogInAsAdmin(c, modelTag, controllerTag, testing.DefaultMongoPassword)
user, err := st.User(model.Owner())
c.Assert(err, jc.ErrorIsNil)
c.Check(user.PasswordValid(testing.DefaultMongoPassword), jc.IsTrue)

// Check that controller model configuration has been added, and
// model constraints set.
Expand Down
2 changes: 0 additions & 2 deletions apiserver/common/modelmanagerinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/juju/juju/core/credential"
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/objectstore"
"github.com/juju/juju/core/permission"
"github.com/juju/juju/core/status"
"github.com/juju/juju/core/watcher"
environscloudspec "github.com/juju/juju/environs/cloudspec"
Expand Down Expand Up @@ -92,7 +91,6 @@ type Model interface {
// needs a Model with this model. Once this is gone ControllerUUID can be
// removed from this interface.
ControllerUUID() string
AddUser(state.UserAccessSpec) (permission.UserAccess, error)
SetCloudCredential(tag names.CloudCredentialTag) (bool, error)
}

Expand Down
2 changes: 0 additions & 2 deletions apiserver/facades/client/client/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/juju/juju/apiserver/common/storagecommon"
"github.com/juju/juju/core/crossmodel"
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/permission"
"github.com/juju/juju/core/status"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/internal/charm"
Expand Down Expand Up @@ -64,7 +63,6 @@ type Model interface {
CloudCredentialTag() (names.CloudCredentialTag, bool)
Config() (*config.Config, error)
Owner() names.UserTag
AddUser(state.UserAccessSpec) (permission.UserAccess, error)
StatusHistory(status.StatusHistoryFilter) ([]status.StatusInfo, error)
LatestToolsVersion() version.Number
Status() (status.StatusInfo, error)
Expand Down
40 changes: 0 additions & 40 deletions apiserver/facades/client/client/package_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 0 additions & 36 deletions apiserver/facades/client/modelmanager/modelinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,6 @@ type mockState struct {
cloudUsers map[string]permission.Access
model *mockModel
controllerModel *mockModel
users []permission.UserAccess
machines []common.Machine
controllerNodes []common.ControllerNode
cfgDefaults config.ModelDefaultAttributes
Expand Down Expand Up @@ -988,36 +987,6 @@ func (st *mockState) Close() error {
return st.NextErr()
}

func (st *mockState) AddControllerUser(spec state.UserAccessSpec) (permission.UserAccess, error) {
st.MethodCall(st, "AddControllerUser", spec)
return permission.UserAccess{}, st.NextErr()
}

func (st *mockState) UserAccess(tag names.UserTag, target names.Tag) (permission.UserAccess, error) {
st.MethodCall(st, "ModelUser", tag, target)
for _, user := range st.users {
if user.UserTag != tag {
continue
}
nextErr := st.NextErr()
if nextErr != nil {
return permission.UserAccess{}, nextErr
}
return user, nil
}
return permission.UserAccess{}, st.NextErr()
}

func (st *mockState) RemoveUserAccess(subject names.UserTag, target names.Tag) error {
st.MethodCall(st, "RemoveUserAccess", subject, target)
return st.NextErr()
}

func (st *mockState) SetUserAccess(subject names.UserTag, target names.Tag, access permission.Access) (permission.UserAccess, error) {
st.MethodCall(st, "SetUserAccess", subject, target, access)
return permission.UserAccess{}, st.NextErr()
}

func (st *mockState) ModelConfigDefaultValues(cloud string) (config.ModelDefaultAttributes, error) {
st.MethodCall(st, "ModelConfigDefaultValues", cloud)
return st.cfgDefaults, nil
Expand Down Expand Up @@ -1305,11 +1274,6 @@ func (m *mockModel) MigrationMode() state.MigrationMode {
return m.migrationStatus
}

func (m *mockModel) AddUser(spec state.UserAccessSpec) (permission.UserAccess, error) {
m.MethodCall(m, "AddUser", spec)
return permission.UserAccess{}, m.NextErr()
}

func (m *mockModel) SetCloudCredential(tag names.CloudCredentialTag) (bool, error) {
m.MethodCall(m, "SetCloudCredential", tag)
return m.setCloudCredentialF(tag)
Expand Down
5 changes: 1 addition & 4 deletions apiserver/facades/client/modelmanager/modelmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1383,17 +1383,14 @@ func (s *modelManagerStateSuite) TestAdminCanCreateModelForSomeoneElse(c *gc.C)
c.Assert(model.OwnerTag, gc.Equals, owner.String())
c.Assert(model.Name, gc.Equals, "test-model")
c.Assert(model.Type, gc.Equals, "iaas")
// Make sure that the environment created does actually have the correct
// owner, and that owner is actually allowed to use the environment.

newState, err := s.StatePool().Get(model.UUID)
c.Assert(err, jc.ErrorIsNil)
defer newState.Release()

newModel, err := newState.Model()
c.Assert(err, jc.ErrorIsNil)
c.Assert(newModel.Owner(), gc.Equals, owner)
_, err = newState.UserAccess(owner, newModel.ModelTag())
c.Assert(err, jc.ErrorIsNil)
}

func (s *modelManagerStateSuite) TestNonAdminCannotCreateModelForSomeoneElse(c *gc.C) {
Expand Down
9 changes: 1 addition & 8 deletions cmd/jujud-controller/agent/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,6 @@ func (s *BootstrapSuite) TestInitialPassword(c *gc.C) {
err = adminDB.Login("admin", info.Password)
c.Assert(err, jc.ErrorIsNil)

// Check that the admin user has been given an appropriate password
st, closer := s.getSystemState(c)
defer closer()
u, err := st.User(names.NewLocalUserTag("admin"))
c.Assert(err, jc.ErrorIsNil)
c.Assert(u.PasswordValid(testPassword), jc.IsTrue)

// Check that the machine configuration has been given a new
// password and that we can connect to mongo as that machine
// and that the in-mongo password also verifies correctly.
Expand All @@ -408,7 +401,7 @@ func (s *BootstrapSuite) TestInitialPassword(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
defer session.Close()

st, closer = s.getSystemState(c)
st, closer := s.getSystemState(c)
defer closer()

node, err := st.ControllerNode("0")
Expand Down
35 changes: 23 additions & 12 deletions state/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,11 @@ func (m *Model) EnqueueAction(operationID string, receiver names.Tag,
return nil, errors.New("action name required")
}

checkNotDead := true
receiverCollectionName, receiverId, err := m.st.tagToCollectionAndId(receiver)
if err != nil {
if errors.Is(err, errors.NotImplemented) {
checkNotDead = false
} else if err != nil {
return nil, errors.Trace(err)
}
doc, ndoc, err := newActionDoc(m.st, operationID, receiver, actionName, payload, parallel, executionGroup)
Expand All @@ -695,11 +698,16 @@ func (m *Model) EnqueueAction(operationID string, receiver names.Tag,
doc.Status = ActionError
doc.Message = actionError.Error()
}
ops := []txn.Op{{
C: receiverCollectionName,
Id: receiverId,
Assert: notDeadDoc,
}, {

var ops []txn.Op
if checkNotDead {
ops = append(ops, txn.Op{
C: receiverCollectionName,
Id: receiverId,
Assert: notDeadDoc,
})
}
ops = append(ops, []txn.Op{{
C: operationsC,
Id: m.st.docID(operationID),
Assert: txn.DocExists,
Expand All @@ -708,7 +716,7 @@ func (m *Model) EnqueueAction(operationID string, receiver names.Tag,
Id: doc.DocId,
Assert: txn.DocMissing,
Insert: doc,
}}
}}...)
if actionError == nil {
ops = append(ops, txn.Op{
C: actionNotificationsC,
Expand All @@ -719,11 +727,14 @@ func (m *Model) EnqueueAction(operationID string, receiver names.Tag,
}

buildTxn := func(attempt int) ([]txn.Op, error) {
if notDead, err := isNotDead(m.st, receiverCollectionName, receiverId); err != nil {
return nil, err
} else if !notDead {
return nil, stateerrors.ErrDead
} else if attempt != 0 {
if checkNotDead {
if notDead, err := isNotDead(m.st, receiverCollectionName, receiverId); err != nil {
return nil, err
} else if !notDead {
return nil, stateerrors.ErrDead
}
}
if attempt != 0 {
_, err := m.Operation(operationID)
if err != nil {
return nil, errors.Trace(err)
Expand Down
24 changes: 0 additions & 24 deletions state/allcollections.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,6 @@ func allCollections() CollectionSchema {
// migration minions.
migrationsMinionSyncC: {global: true},

// This collection holds user information that's not specific to any
// one model.
usersC: {
global: true,
},

// This collection holds users that are relative to controllers.
controllerUsersC: {
global: true,
},

// This collection is used as a unique key restraint. The _id field is
// a concatenation of multiple fields that form a compound index,
// allowing us to ensure users cannot have the same name for two
Expand All @@ -144,16 +133,6 @@ func allCollections() CollectionSchema {
global: true,
},

// This collection is basically a standard SQL intersection table; it
// references the global records of the users allowed access to a
// given operation.
permissionsC: {
global: true,
indexes: []mgo.Index{{
Key: []string{"object-global-key", "subject-global-key"},
}},
},

// -----------------

// Local collections
Expand Down Expand Up @@ -511,7 +490,6 @@ const (
containerRefsC = "containerRefs"
controllersC = "controllers"
controllerNodesC = "controllerNodes"
controllerUsersC = "controllerusers"
dockerResourcesC = "dockerResources"
filesystemAttachmentsC = "filesystemAttachments"
filesystemsC = "filesystems"
Expand All @@ -531,7 +509,6 @@ const (
openedPortsC = "openedPorts"
operationsC = "operations"
payloadsC = "payloads"
permissionsC = "permissions"
providerIDsC = "providerIDs"
relationScopesC = "relationscopes"
relationsC = "relations"
Expand All @@ -556,7 +533,6 @@ const (
unitStatesC = "unitstates"
upgradeInfoC = "upgradeInfo"
usermodelnameC = "usermodelname"
usersC = "users"
volumeAttachmentsC = "volumeattachments"
volumeAttachmentPlanC = "volumeattachmentplan"
volumesC = "volumes"
Expand Down
11 changes: 0 additions & 11 deletions state/applicationoffers.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ import (
"github.com/juju/juju/internal/uuid"
)

const (
// applicationOfferGlobalKey is the key for an application offer.
applicationOfferGlobalKey = "ao"
)

// applicationOfferKey will return the key for a given offer using the
// offer uuid and the applicationOfferGlobalKey.
func applicationOfferKey(offerUUID string) string {
return fmt.Sprintf("%s#%s", applicationOfferGlobalKey, offerUUID)
}

// applicationOfferDoc represents the internal state of a application offer in MongoDB.
type applicationOfferDoc struct {
DocID string `bson:"_id"`
Expand Down
11 changes: 0 additions & 11 deletions state/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package state

import (
"fmt"

"github.com/juju/errors"
"github.com/juju/mgo/v3"
"github.com/juju/mgo/v3/bson"
Expand All @@ -18,17 +16,8 @@ import (
const (
// ControllerSettingsGlobalKey is the key for the controller and its settings.
ControllerSettingsGlobalKey = "controllerSettings"

// controllerGlobalKey is the key for controller.
controllerGlobalKey = "c"
)

// controllerKey will return the key for a given controller using the
// controller uuid and the controllerGlobalKey.
func controllerKey(controllerUUID string) string {
return fmt.Sprintf("%s#%s", controllerGlobalKey, controllerUUID)
}

// Controller encapsulates state for the Juju controller as a whole,
// as opposed to model specific functionality.
//
Expand Down
Loading

0 comments on commit be6ce33

Please sign in to comment.