Skip to content

Commit

Permalink
chore: remove unneeded getters (#1502)
Browse files Browse the repository at this point in the history
Remove getters that allowed accessing the database or authorization client from outside the application layer.
  • Loading branch information
kian99 authored Dec 18, 2024
1 parent 10c5461 commit 71cdcfc
Show file tree
Hide file tree
Showing 9 changed files with 11 additions and 28 deletions.
2 changes: 1 addition & 1 deletion cmd/jaas/cmd/updatecredentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (s *updateCredentialsSuite) TestUpdateCredentialsWithLocalCredentials(c *gc
models: []
`)

ofgaUser := openfga.NewUser(sa, s.JIMM.AuthorizationClient())
ofgaUser := openfga.NewUser(sa, s.JIMM.OpenFGAClient)
cloudCredentialTag := names.NewCloudCredentialTag("test-cloud/" + clientIDWithDomain + "/test-credentials")
cloudCredential2, err := s.JIMM.GetCloudCredential(ctx, ofgaUser, cloudCredentialTag)
c.Assert(err, gc.IsNil)
Expand Down
15 changes: 0 additions & 15 deletions internal/jimm/jimm.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,21 +276,11 @@ func (j *JIMM) ResourceTag() names.ControllerTag {
return names.NewControllerTag(j.UUID)
}

// DB returns the database used by JIMM.
func (j *JIMM) DB() *db.Database {
return j.Database
}

// PubsubHub returns the pub-sub hub used for buffering model summaries.
func (j *JIMM) PubSubHub() *pubsub.Hub {
return j.Pubsub
}

// AuthorizationClient return the OpenFGA client used by JIMM.
func (j *JIMM) AuthorizationClient() *openfga.OFGAClient {
return j.OpenFGAClient
}

// RoleManager returns a manager that enables role management.
func (j *JIMM) RoleManager() RoleManager {
return j.roleManager
Expand All @@ -301,11 +291,6 @@ func (j *JIMM) GroupManager() GroupManager {
return j.groupManager
}

// GetCredentialStore returns the credential store used by JIMM.
func (j *JIMM) GetCredentialStore() credentials.CredentialStore {
return j.CredentialStore
}

type permission struct {
resource string
relation string
Expand Down
2 changes: 1 addition & 1 deletion internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,7 @@ func (j *JIMM) ListModels(ctx context.Context, user *openfga.User) ([]base.UserM
}

// Get the models from the database
models, err := j.DB().GetModelsByUUID(ctx, uuids)
models, err := j.Database.GetModelsByUUID(ctx, uuids)
if err != nil {
return nil, errors.E(op, err, "failed to get models by uuid")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/jimm/model_cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (j *JIMM) CleanupDyingModels(ctx context.Context) (err error) {
durationObserver := servermon.DurationObserver(servermon.JimmMethodsDurationHistogram, string(op))
defer durationObserver()

err = j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error {
err = j.Database.ForEachModel(ctx, func(m *dbmodel.Model) error {
if m.Life != state.Dying.String() {
return nil
}
Expand All @@ -37,7 +37,7 @@ func (j *JIMM) CleanupDyingModels(ctx context.Context) (err error) {
if err := api.ModelInfo(ctx, &jujuparams.ModelInfo{UUID: m.UUID.String}); err != nil {
// Some versions of juju return unauthorized for models that cannot be found.
if errors.ErrorCode(err) == errors.CodeNotFound || errors.ErrorCode(err) == errors.CodeUnauthorized {
if err := j.DB().DeleteModel(ctx, m); err != nil {
if err := j.Database.DeleteModel(ctx, m); err != nil {
zapctx.Error(ctx, fmt.Sprintf("cannot delete model %s: %s\n", m.UUID.String, err))
} else {
return nil
Expand Down
6 changes: 3 additions & 3 deletions internal/jimm/model_cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (s *modelCleanupSuite) TestPollModelsDying(c *qt.C) {
Valid: true,
},
}
err = s.jimm.DB().GetModel(ctx, &model)
err = s.jimm.Database.GetModel(ctx, &model)
c.Assert(err, qt.ErrorMatches, "model not found")

model = dbmodel.Model{
Expand All @@ -144,7 +144,7 @@ func (s *modelCleanupSuite) TestPollModelsDying(c *qt.C) {
Valid: true,
},
}
err = s.jimm.DB().GetModel(ctx, &model)
err = s.jimm.Database.GetModel(ctx, &model)
c.Assert(err, qt.IsNil)
}

Expand Down Expand Up @@ -174,7 +174,7 @@ func (s *modelCleanupSuite) TestPollModelsDyingControllerErrors(c *qt.C) {
Valid: true,
},
}
err = s.jimm.DB().GetModel(ctx, &model)
err = s.jimm.Database.GetModel(ctx, &model)
c.Assert(err, qt.IsNil)
c.Assert(model.Life, qt.Equals, state.Dying.String())
}
Expand Down
2 changes: 1 addition & 1 deletion internal/jimmhttp/httpproxy_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (hph *HTTPProxyHandler) ProxyHTTP(w http.ResponseWriter, req *http.Request)
writeError(ctx, w, http.StatusNotFound, err, "cannot get model")
return
}
u, p, err := hph.jimm.GetCredentialStore().GetControllerCredentials(ctx, model.Controller.Name)
u, p, err := hph.jimm.CredentialStore.GetControllerCredentials(ctx, model.Controller.Name)
if err != nil {
writeError(ctx, w, http.StatusNotFound, err, "cannot retrieve credentials")
return
Expand Down
4 changes: 2 additions & 2 deletions internal/jimmhttp/httpproxy_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ func (s *httpProxySuite) SetUpTest(c *gc.C) {
err := s.JIMM.Database.GetModel(ctx, model)
c.Assert(err, gc.IsNil)
s.model = model
err = s.JIMM.GetCredentialStore().PutControllerCredentials(ctx, model.Controller.Name, "user", "psw")
err = s.JIMM.CredentialStore.PutControllerCredentials(ctx, model.Controller.Name, "user", "psw")
c.Assert(err, gc.IsNil)
}

func (s *httpProxySuite) TestHTTPProxyHandler(c *gc.C) {
ctx := context.Background()
httpProxier := jimmhttp.NewHTTPProxyHandler(s.JIMM)
expectU, expectP, err := s.JIMM.GetCredentialStore().GetControllerCredentials(ctx, s.model.Controller.Name)
expectU, expectP, err := s.JIMM.CredentialStore.GetControllerCredentials(ctx, s.model.Controller.Name)
c.Assert(err, gc.IsNil)
// we expect the controller to respond with TLS
fakeController := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
2 changes: 1 addition & 1 deletion internal/jujuapi/access_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ func (s *accessControlSuite) TestListRelationshipTuplesNoUUIDResolution(c *gc.C)
c.Assert(err, jc.ErrorIsNil)

groupOrange := dbmodel.GroupEntry{Name: "orange"}
err = s.JIMM.DB().GetGroup(ctx, &groupOrange)
err = s.JIMM.Database.GetGroup(ctx, &groupOrange)
c.Assert(err, jc.ErrorIsNil)
expected := []apiparams.RelationshipTuple{{
Object: "group-" + groupOrange.UUID + "#member",
Expand Down
2 changes: 0 additions & 2 deletions internal/jujuapi/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/canonical/jimm/v3/internal/db"
"github.com/canonical/jimm/v3/internal/dbmodel"
"github.com/canonical/jimm/v3/internal/jimm"
"github.com/canonical/jimm/v3/internal/jimm/credentials"
"github.com/canonical/jimm/v3/internal/openfga"
ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
"github.com/canonical/jimm/v3/internal/pubsub"
Expand Down Expand Up @@ -45,7 +44,6 @@ type JIMM interface {
GetCloud(ctx context.Context, u *openfga.User, tag names.CloudTag) (dbmodel.Cloud, error)
GetCloudCredential(ctx context.Context, user *openfga.User, tag names.CloudCredentialTag) (*dbmodel.CloudCredential, error)
GetCloudCredentialAttributes(ctx context.Context, u *openfga.User, cred *dbmodel.CloudCredential, hidden bool) (attrs map[string]string, redacted []string, err error)
GetCredentialStore() credentials.CredentialStore
RoleManager() jimm.RoleManager
GroupManager() jimm.GroupManager
GetJimmControllerAccess(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error)
Expand Down

0 comments on commit 71cdcfc

Please sign in to comment.