Merge branch 'v3' of https://github.com/canonical/jimm into fix-k8s-t…

…ests

internal/jimm/access.go

@@ -23,6 +23,7 @@ import (
 	"github.com/canonical/jimm/v3/internal/jimmjwx"
 	"github.com/canonical/jimm/v3/internal/openfga"
 	ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
+	"github.com/canonical/jimm/v3/internal/servermon"
 	jimmnames "github.com/canonical/jimm/v3/pkg/names"
 )
 
@@ -723,10 +724,13 @@ func (j *JIMM) parseAndValidateTag(ctx context.Context, key string) (*ofganames.
 //
 // This approach to cleaning up tuples is intended to be temporary while we implement
 // a better approach to eventual consistency of JIMM's database objects and OpenFGA tuples.
-func (j *JIMM) OpenFGACleanup(ctx context.Context) error {
+func (j *JIMM) OpenFGACleanup(ctx context.Context) (err error) {
+	const op = errors.Op("jimm.CleanupDyingModels")
+	zapctx.Info(ctx, string(op))
+	durationObserver := servermon.DurationObserver(servermon.JimmMethodsDurationHistogram, string(op))
+	defer durationObserver()
 	var (
 		continuationToken string
-		err               error
 		tuples            []ofga.Tuple
 	)
 	for {

internal/jimm/model.go

@@ -761,10 +761,10 @@ func (m *modelSummariesMap) addModelSummary(summary jujuparams.ModelSummaryResul
 	m.modelSummaries[summary.Result.UUID] = summary
 }
 
-// ModelSummaries returns the list of modelsummary the user has access to.
+// ListModelSummaries returns the list of modelsummary the user has access to.
 // It queries the controllers and then merge the info from the JIMM db.
-func (j *JIMM) ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
-	const op = errors.Op("jimm.ModelSummaries")
+func (j *JIMM) ListModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
+	const op = errors.Op("jimm.ListModelSummaries")
 
 	modelSummariesSafeMap := modelSummariesMap{}
 	modelSummaryResults := []jujuparams.ModelSummaryResult{}
@@ -1147,10 +1147,10 @@ func (j *JIMM) RevokeModelAccess(ctx context.Context, user *openfga.User, mt nam
 		return errors.E(op, errors.CodeBadRequest, fmt.Sprintf("failed to recognize given access: %q", access), err)
 	}
 
-	requiredAccess := "admin"
+	requiredAccess := ofganames.AdministratorRelation
 	if user.Tag() == ut {
 		// If the user is attempting to revoke their own access.
-		requiredAccess = "read"
+		requiredAccess = ofganames.ReaderRelation
 	}
 
 	err = j.doModel(ctx, user, mt, requiredAccess, func(_ *dbmodel.Model, _ API) error {
@@ -1326,7 +1326,7 @@ func (j *JIMM) ValidateModelUpgrade(ctx context.Context, user *openfga.User, mt
 // returned from the dial operation. If the given function returns an error
 // that error will be returned with the code unmasked.
 func (j *JIMM) doModelAdmin(ctx context.Context, user *openfga.User, mt names.ModelTag, f func(*dbmodel.Model, API) error) error {
-	return j.doModel(ctx, user, mt, "admin", f)
+	return j.doModel(ctx, user, mt, ofganames.AdministratorRelation, f)
 }
 
 // GetUserModelAccess returns the access level a user has against a specific model.
@@ -1335,7 +1335,7 @@ func (j *JIMM) GetUserModelAccess(ctx context.Context, user *openfga.User, model
 	return ToModelAccessString(accessLevel), nil
 }
 
-func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTag, access string, f func(*dbmodel.Model, API) error) error {
+func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTag, requireRelation openfga.Relation, f func(*dbmodel.Model, API) error) error {
 	const op = errors.Op("jimm.doModel")
 	zapctx.Info(ctx, string(op))
 
@@ -1346,11 +1346,12 @@ func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTa
 		return errors.E(op, err)
 	}
 
-	accessLevel, err := j.GetUserModelAccess(ctx, user, mt)
+	hasAccess, err := user.HasModelRelation(ctx, mt, requireRelation)
 	if err != nil {
 		return errors.E(op, err)
 	}
-	if !allowedModelAccess[access][accessLevel] {
+
+	if !hasAccess {
 		// If the user doesn't have correct access on the model return
 		// an unauthorized error.
 		return errors.E(op, errors.CodeUnauthorized, "unauthorized")
@@ -1367,21 +1368,6 @@ func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTa
 	return nil
 }
 
-var allowedModelAccess = map[string]map[string]bool{
-	"admin": {
-		"admin": true,
-	},
-	"write": {
-		"admin": true,
-		"write": true,
-	},
-	"read": {
-		"admin": true,
-		"write": true,
-		"read":  true,
-	},
-}
-
 // ChangeModelCredential changes the credential used with a model on both
 // the controller and the local database.
 func (j *JIMM) ChangeModelCredential(ctx context.Context, user *openfga.User, modelTag names.ModelTag, cloudCredentialTag names.CloudCredentialTag) error {

internal/jimm/model_cleanup.go

@@ -12,15 +12,18 @@ import (
 
 	"github.com/canonical/jimm/v3/internal/dbmodel"
 	"github.com/canonical/jimm/v3/internal/errors"
+	"github.com/canonical/jimm/v3/internal/servermon"
 )
 
 // CleanupDyingModels loops over dying models, contacting the respective controller.
 // And deleting the model from our database if the error is `NotFound` which means the model was successfully deleted.
-func (j *JIMM) CleanupDyingModels(ctx context.Context) error {
+func (j *JIMM) CleanupDyingModels(ctx context.Context) (err error) {
 	const op = errors.Op("jimm.CleanupDyingModels")
 	zapctx.Info(ctx, string(op))
+	durationObserver := servermon.DurationObserver(servermon.JimmMethodsDurationHistogram, string(op))
+	defer durationObserver()
 
-	err := j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error {
+	err = j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error {
 		if m.Life != state.Dying.String() {
 			return nil
 		}

internal/jimm/model_test.go

@@ -1973,7 +1973,7 @@ func TestModelSummaries(t *testing.T) {
 					},
 				},
 			}
-			summaries, err := j.ModelSummaries(ctx, alice, "")
+			summaries, err := j.ListModelSummaries(ctx, alice, "")
 			c.Check(err, qt.IsNil)
 			c.Check(summaries.Results, qt.HasLen, t.expectedSummariesSize)
 			c.Check(summaries.Results, qt.DeepEquals, t.expectedSummaries)

internal/jujuapi/cloud.go

@@ -17,6 +17,7 @@ import (
 	"github.com/canonical/jimm/v3/internal/jimm"
 	"github.com/canonical/jimm/v3/internal/jujuapi/rpc"
 	"github.com/canonical/jimm/v3/internal/openfga"
+	ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
 )
 
 func init() {
@@ -276,30 +277,18 @@ func collapseUpdateCredentialResults(args jujuparams.TaggedCredentials, updateRe
 	return results
 }
 
-func userModelAccess(ctx context.Context, user *openfga.User, model names.ModelTag) (string, error) {
-	isModelAdmin, err := openfga.IsAdministrator(ctx, user, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if isModelAdmin {
-		return "admin", nil
-	}
-	hasWriteAccess, err := user.IsModelWriter(ctx, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if hasWriteAccess {
-		return "write", nil
-	}
-	hasReadAccess, err := user.IsModelReader(ctx, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if hasReadAccess {
-		return "read", nil
+func userModelAccess(ctx context.Context, user *openfga.User, model names.ModelTag) string {
+	userRelation := user.GetModelAccess(ctx, model)
+	switch userRelation {
+	case ofganames.AdministratorRelation:
+		return "admin"
+	case ofganames.WriterRelation:
+		return "write"
+	case ofganames.ReaderRelation:
+		return "read"
+	default:
+		return ""
 	}
-
-	return "", nil
 }
 
 // CredentialContents implements the CredentialContents method of the Cloud (v5) facade.
@@ -326,13 +315,9 @@ func getIdentityCredentials(ctx context.Context, user *openfga.User, j JIMM, arg
 		}
 		mas := make([]jujuparams.ModelAccess, len(c.Models))
 		for i, m := range c.Models {
-			userModelAccess, err := userModelAccess(ctx, user, m.ResourceTag())
-			if err != nil {
-				return nil, errors.E(err)
-			}
 			mas[i] = jujuparams.ModelAccess{
 				Model:  m.Name,
-				Access: userModelAccess,
+				Access: userModelAccess(ctx, user, m.ResourceTag()),
 			}
 		}
 		return &jujuparams.ControllerCredentialInfo{

internal/jujuapi/modelmanager.go

@@ -71,7 +71,7 @@ type ModelManager interface {
 	ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
 	ModelDefaultsForCloud(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error)
 	ModelInfo(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelInfo, error)
-	ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
+	ListModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
 	ModelStatus(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelStatus, error)
 	QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error)
 	SetModelDefaults(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, configs map[string]interface{}) error
@@ -113,7 +113,7 @@ func (r *controllerRoot) ListModelSummaries(ctx context.Context, _ jujuparams.Mo
 	if r.controllerUUIDMasking {
 		maskingControllerUUID = r.params.ControllerUUID
 	}
-	res, err := r.jimm.ModelSummaries(ctx, r.user, maskingControllerUUID)
+	res, err := r.jimm.ListModelSummaries(ctx, r.user, maskingControllerUUID)
 	if err != nil {
 		return jujuparams.ModelSummaryResults{}, errors.E(op, err)
 	}

internal/openfga/user.go

@@ -79,6 +79,15 @@ func (u *User) IsModelWriter(ctx context.Context, resource names.ModelTag) (bool
 	return isWriter, nil
 }
 
+// HasModelRelation returns true if user has the specified relation to the model.
+func (u *User) HasModelRelation(ctx context.Context, resource names.ModelTag, relation Relation) (bool, error) {
+	hasRelation, err := checkRelation(ctx, u, resource, relation)
+	if err != nil {
+		return false, errors.E(err)
+	}
+	return hasRelation, nil
+}
+
 // IsServiceAccountAdmin returns true if the user has administrator relation to the service account.
 func (u *User) IsServiceAccountAdmin(ctx context.Context, clientID jimmnames.ServiceAccountTag) (bool, error) {
 	isAdmin, err := checkRelation(ctx, u, clientID, ofganames.AdministratorRelation)

internal/openfga/user_test.go

@@ -133,6 +133,18 @@ func (s *userTestSuite) TestModelAccess(c *gc.C) {
 	allowed, err = adamUser.IsModelWriter(ctx, model)
 	c.Assert(err, gc.IsNil)
 	c.Assert(allowed, gc.Equals, false)
+
+	allowed, err = eveUser.HasModelRelation(ctx, model, ofganames.ReaderRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, true)
+
+	allowed, err = eveUser.HasModelRelation(ctx, model, ofganames.AdministratorRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, true)
+
+	allowed, err = adamUser.HasModelRelation(ctx, model, ofganames.ReaderRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, false)
 }
 
 func (s *userTestSuite) TestSetModelAccess(c *gc.C) {

internal/servermon/monitoring.go

@@ -31,6 +31,13 @@ var (
 		Help:      "Histogram of database query time in seconds",
 		Buckets:   []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10},
 	}, []string{"method"})
+	JimmMethodsDurationHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{
+		Namespace: "jimm",
+		Subsystem: "jimm",
+		Name:      "op_duration_seconds",
+		Help:      "Histogram of jimm operations time in seconds",
+		Buckets:   []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10},
+	}, []string{"method"})
 	DBQueryErrorCount = promauto.NewCounterVec(prometheus.CounterOpts{
 		Namespace: "jimm",
 		Subsystem: "db",
@@ -101,12 +108,6 @@ var (
 		Name:      "models_created_fail_total",
 		Help:      "The number of fails attempting to create models.",
 	})
-	MonitorDeltasReceivedCount = promauto.NewCounterVec(prometheus.CounterOpts{
-		Namespace: "jimm",
-		Subsystem: "monitor",
-		Name:      "deltas_received_total",
-		Help:      "The number of watcher deltas received.",
-	}, []string{"controller"})
 	MonitorErrorsCount = promauto.NewCounterVec(prometheus.CounterOpts{
 		Namespace: "jimm",
 		Subsystem: "monitor",

internal/testutils/jimmtest/mocks/model.go

@@ -25,7 +25,7 @@ type ModelManager struct {
 	ForEachModel_           func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error
 	ForEachUserModel_       func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error
 	FullModelStatus_        func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, patterns []string) (*jujuparams.FullStatus, error)
-	ModelSummaries_         func(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
+	ListModelSummaries_     func(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
 	GetModel_               func(ctx context.Context, uuid string) (dbmodel.Model, error)
 	ImportModel_            func(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
 	IdentityModelDefaults_  func(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error)
@@ -129,11 +129,11 @@ func (j *ModelManager) ModelStatus(ctx context.Context, u *openfga.User, mt name
 	return j.ModelStatus_(ctx, u, mt)
 }
 
-func (j *ModelManager) ModelSummaries(ctx context.Context, u *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
-	if j.ModelSummaries_ == nil {
+func (j *ModelManager) ListModelSummaries(ctx context.Context, u *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
+	if j.ListModelSummaries_ == nil {
 		return jujuparams.ModelSummaryResults{}, errors.E(errors.CodeNotImplemented)
 	}
-	return j.ModelSummaries_(ctx, u, maskingControllerUUID)
+	return j.ListModelSummaries_(ctx, u, maskingControllerUUID)
 }
 
 func (j *ModelManager) QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error) {