From 71cdcfce082b23e256ad2e0863fb0ac0d7e00588 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:00:03 +0200 Subject: [PATCH] chore: remove unneeded getters (#1502) Remove getters that allowed accessing the database or authorization client from outside the application layer. --- cmd/jaas/cmd/updatecredentials_test.go | 2 +- internal/jimm/jimm.go | 15 --------------- internal/jimm/model.go | 2 +- internal/jimm/model_cleanup.go | 4 ++-- internal/jimm/model_cleanup_test.go | 6 +++--- internal/jimmhttp/httpproxy_handler.go | 2 +- internal/jimmhttp/httpproxy_handler_test.go | 4 ++-- internal/jujuapi/access_control_test.go | 2 +- internal/jujuapi/interface.go | 2 -- 9 files changed, 11 insertions(+), 28 deletions(-) diff --git a/cmd/jaas/cmd/updatecredentials_test.go b/cmd/jaas/cmd/updatecredentials_test.go index f7899e3fa..f41119c2b 100644 --- a/cmd/jaas/cmd/updatecredentials_test.go +++ b/cmd/jaas/cmd/updatecredentials_test.go @@ -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) diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index fdadd7977..2995f3c34 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -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 @@ -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 diff --git a/internal/jimm/model.go b/internal/jimm/model.go index ea6c16d7d..482086160 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -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") } diff --git a/internal/jimm/model_cleanup.go b/internal/jimm/model_cleanup.go index 8b5aafa87..2f3f04310 100644 --- a/internal/jimm/model_cleanup.go +++ b/internal/jimm/model_cleanup.go @@ -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 } @@ -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 diff --git a/internal/jimm/model_cleanup_test.go b/internal/jimm/model_cleanup_test.go index 3241a9dbd..9e8354ffd 100644 --- a/internal/jimm/model_cleanup_test.go +++ b/internal/jimm/model_cleanup_test.go @@ -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{ @@ -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) } @@ -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()) } diff --git a/internal/jimmhttp/httpproxy_handler.go b/internal/jimmhttp/httpproxy_handler.go index 5e8070bf5..9067aa0ec 100644 --- a/internal/jimmhttp/httpproxy_handler.go +++ b/internal/jimmhttp/httpproxy_handler.go @@ -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 diff --git a/internal/jimmhttp/httpproxy_handler_test.go b/internal/jimmhttp/httpproxy_handler_test.go index b84c94358..a9d07e669 100644 --- a/internal/jimmhttp/httpproxy_handler_test.go +++ b/internal/jimmhttp/httpproxy_handler_test.go @@ -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) { diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index d8e5b8491..a26006411 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -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", diff --git a/internal/jujuapi/interface.go b/internal/jujuapi/interface.go index 1565abf9d..e57b1341c 100644 --- a/internal/jujuapi/interface.go +++ b/internal/jujuapi/interface.go @@ -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" @@ -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)