Revert "backports changes back to feature-rebac (#1003)"

This reverts commit 19d3e80.

internal/jimm/model_status_parser.go

@@ -2,7 +2,9 @@ package jimm
 
 import (
 	"context"
+	"database/sql"
 	"encoding/json"
+	"strings"
 
 	"github.com/canonical/jimm/api/params"
 	"github.com/canonical/jimm/internal/dbmodel"
@@ -14,17 +16,16 @@ import (
 	"github.com/juju/juju/cmd/juju/status"
 	"github.com/juju/juju/cmd/juju/storage"
 	rpcparams "github.com/juju/juju/rpc/params"
-	"github.com/juju/names/v4"
 	"github.com/juju/zaputil/zapctx"
 )
 
-// QueryModels queries every specified model in modelUUIDs.
+// QueryModels queries every model available to a given user.
 //
 // The jqQuery must be a valid jq query and can return every result, even iterative listings.
 // If a result is erroneous, for example, bad data type parsing, the resulting struct field
 // Errors will contain a map from model UUID -> []error. Otherwise, the Results field
 // will contain model UUID -> []Jq result.
-func (j *JIMM) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuery string) (params.CrossModelQueryResponse, error) {
+func (j *JIMM) QueryModelsJq(ctx context.Context, modelUUIDs []string, jqQuery string) (params.CrossModelQueryResponse, error) {
 	op := errors.Op("QueryModels")
 	results := params.CrossModelQueryResponse{
 		Results: make(map[string][]any),
@@ -33,19 +34,25 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuer
 
 	query, err := gojq.Parse(jqQuery)
 	if err != nil {
-		return results, errors.E(op, "failed to parse jq query", err)
+		return results, errors.E(op, err)
+	}
+
+	// We remove "model:" from the UUIDs, unfortunately that's what OpenFGA returns now after
+	// recent versions.
+	for i := range modelUUIDs {
+		modelUUIDs[i] = strings.Split(modelUUIDs[i], ":")[1]
 	}
 
 	// Set up a formatterParamsRetriever to handle the heavy lifting
 	// of each facade call and type conversion.
 	retriever := newFormatterParamsRetriever(j)
 
-	for _, model := range models {
-		modelUUID := model.UUID.String
-		params, err := retriever.GetParams(ctx, model)
+	for _, id := range modelUUIDs {
+
+		params, err := retriever.GetParams(ctx, id)
 		if err != nil {
-			zapctx.Error(ctx, "failed to get status formatter params", zap.String("model-uuid", modelUUID))
-			results.Errors[modelUUID] = append(results.Errors[modelUUID], err.Error())
+			zapctx.Error(ctx, "failed to get status formatter params", zap.String("model-uuid", id))
+			results.Errors[id] = append(results.Errors[id], err.Error())
 			continue
 		}
 
@@ -55,17 +62,17 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuer
 
 		formattedStatus, err := formatter.Format()
 		if err != nil {
-			zapctx.Error(ctx, "failed to format status", zap.String("model-uuid", modelUUID))
-			results.Errors[modelUUID] = append(results.Errors[modelUUID], err.Error())
+			zapctx.Error(ctx, "failed to format status", zap.String("model-uuid", id))
+			results.Errors[id] = append(results.Errors[id], err.Error())
 			continue
 		}
 		// We could use output.NewFormatter() from 3.0+ juju/juju, but ultimately
 		// we just want some JSON output, regardless of user formatting. As such json.Marshal
 		// *should* be OK. But TODO: make sure this is fine.
 		fb, err := json.Marshal(formattedStatus)
 		if err != nil {
-			zapctx.Error(ctx, "failed to marshal formatted status", zap.String("model-uuid", modelUUID))
-			results.Errors[modelUUID] = append(results.Errors[modelUUID], err.Error())
+			zapctx.Error(ctx, "failed to marshal formatted status", zap.String("model-uuid", id))
+			results.Errors[id] = append(results.Errors[id], err.Error())
 			continue
 		}
 		tempMap := make(map[string]any)
@@ -84,11 +91,11 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuer
 			// query. As such, we simply append all to the errors field and continue to collect
 			// both erreoneous and valid query results.
 			if err, ok := v.(error); ok {
-				results.Errors[modelUUID] = append(results.Errors[modelUUID], "jq error: "+err.Error())
+				results.Errors[id] = append(results.Errors[id], "jq error: "+err.Error())
 				continue
 			}
 
-			results.Results[modelUUID] = append(results.Results[modelUUID], v)
+			results.Results[id] = append(results.Results[id], v)
 		}
 	}
 	return results, nil
@@ -117,8 +124,10 @@ func newFormatterParamsRetriever(j *JIMM) *formatterParamsRetriever {
 
 // GetParams retrieves the required parameters for the Juju status formatter from the currently
 // loaded model. See formatterParamsRetriever.LoadModel for more information.
-func (f *formatterParamsRetriever) GetParams(ctx context.Context, model dbmodel.Model) (*status.NewStatusFormatterParams, error) {
-	f.model = &model
+func (f *formatterParamsRetriever) GetParams(ctx context.Context, modelUUID string) (*status.NewStatusFormatterParams, error) {
+	if err := f.loadModel(ctx, modelUUID); err != nil {
+		return nil, err
+	}
 
 	err := f.dialModel(ctx)
 	if err != nil {
@@ -145,13 +154,24 @@ func (f *formatterParamsRetriever) GetParams(ctx context.Context, model dbmodel.
 	}, nil
 }
 
+// LoadModel loads the model by UUID from the database into the formatterParamsRetriever.
+// This MUST be called before attempting to GetParams().
+func (f *formatterParamsRetriever) loadModel(ctx context.Context, modelUUID string) error {
+	model := dbmodel.Model{
+		UUID: sql.NullString{String: modelUUID, Valid: true},
+	}
+
+	if err := f.jimm.Database.GetModel(ctx, &model); err != nil {
+		zapctx.Error(ctx, "failed to retrieve model", zap.String("model-uuid", modelUUID))
+		return err
+	}
+	f.model = &model
+	return nil
+}
+
 // dialModel dials the model currently loaded into the formatterParamsRetriever.
 func (f *formatterParamsRetriever) dialModel(ctx context.Context) error {
-	modelTag, ok := f.model.Tag().(names.ModelTag)
-	if !ok {
-		return errors.E(errors.Op("failed to parse model tag"))
-	}
-	api, err := f.jimm.dial(ctx, &f.model.Controller, modelTag)
+	api, err := f.jimm.dial(ctx, &f.model.Controller, f.model.ResourceTag())
 	if err != nil {
 		zapctx.Error(ctx, "failed to dial controller for model", zap.String("controller-uuid", f.model.Controller.UUID), zap.String("model-uuid", f.model.UUID.String), zap.Error(err))
 	}

internal/jimm/model_status_parser_test.go

@@ -57,9 +57,6 @@ models:
     status: available
     info: "OK!"
     since: 2020-02-20T20:02:20Z
-  users:
-  - user: alice@external
-    access: admin
 - name: model-2
   type: iaas
   uuid: 20000000-0000-0000-0000-000000000000
@@ -74,9 +71,6 @@ models:
     status: available
     info: "OK!"
     since: 2020-02-20T20:02:20Z
-  users:
-  - user: alice@external
-    access: admin
 - name: model-3
   type: iaas
   uuid: 30000000-0000-0000-0000-000000000000
@@ -91,9 +85,6 @@ models:
     status: available
     info: "OK!"
     since: 2020-02-20T20:02:20Z
-  users:
-  - user: alice@external
-    access: admin
 - name: model-5
   type: iaas
   uuid: 50000000-0000-0000-0000-000000000000
@@ -120,8 +111,6 @@ func getFullStatus(
 	modelName string,
 	applications map[string]jujuparams.ApplicationStatus,
 	remoteApps map[string]jujuparams.RemoteApplicationStatus,
-	modelRelations []jujuparams.RelationStatus,
-
 ) jujuparams.FullStatus {
 	return jujuparams.FullStatus{
 		Model: jujuparams.ModelStatusInfo{
@@ -143,7 +132,7 @@ func getFullStatus(
 		Applications:       applications,
 		RemoteApplications: remoteApps,
 		Offers:             map[string]jujuparams.ApplicationOfferStatus{},
-		Relations:          modelRelations,
+		Relations:          []jujuparams.RelationStatus(nil),
 		Branches:           map[string]jujuparams.BranchStatus{},
 	}
 }
@@ -208,22 +197,6 @@ var model1 = getFullStatus("model-1", map[string]jujuparams.ApplicationStatus{
 			},
 		},
 	},
-	[]jujuparams.RelationStatus{
-		{
-			Id:        0,
-			Key:       "myapp",
-			Interface: "db",
-			Scope:     "regular",
-			Endpoints: []jujuparams.EndpointStatus{
-				{
-					ApplicationName: "myapp",
-					Name:            "db",
-					Role:            "myrole",
-					Subordinate:     false,
-				},
-			},
-		},
-	},
 )
 
 // Model2 holds a model that is running against a K8S controller and has two apps.
@@ -309,19 +282,16 @@ var model2 = getFullStatus("model-2", map[string]jujuparams.ApplicationStatus{
 	},
 },
 	nil,
-	nil,
 )
 
 // Model3 holds an empty model
 var model3 = getFullStatus("model-3", map[string]jujuparams.ApplicationStatus{},
 	nil,
-	nil,
 )
 
 // Model5 holds an empty model, but it's API returns an error for storage
 var model5 = getFullStatus("model-5", map[string]jujuparams.ApplicationStatus{},
 	nil,
-	nil,
 )
 
 func TestQueryModelsJq(t *testing.T) {
@@ -440,8 +410,6 @@ func TestQueryModelsJq(t *testing.T) {
 	env := jimmtest.ParseEnvironment(c, crossModelQueryEnv)
 	env.PopulateDB(c, j.Database, nil)
 
-	user := env.User("alice@external").DBObject(c, j.Database, nil)
-
 	modelUUIDs := []string{
 		"10000000-0000-0000-0000-000000000000",
 		"20000000-0000-0000-0000-000000000000",
@@ -494,17 +462,20 @@ func TestQueryModelsJq(t *testing.T) {
 		}...,
 	), qt.Equals, nil)
 
+	alice := openfga.NewUser(
+		&dbmodel.User{
+			Username: "alice@external",
+		},
+		client,
+	)
+
 	// Tests:
 
 	// Query for all models only.
-	usersModels, err := j.Database.GetUserModels(ctx, &user)
+	userModelUUIDs, err := alice.ListModels(ctx)
 	c.Assert(err, qt.IsNil)
-	models := make([]dbmodel.Model, len(usersModels))
-	for i, m := range usersModels {
-		models[i] = m.Model_
-	}
 
-	res, err := j.QueryModelsJq(ctx, models, ".model")
+	res, err := j.QueryModelsJq(ctx, userModelUUIDs, ".model")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -556,6 +527,9 @@ func TestQueryModelsJq(t *testing.T) {
 			]
 		},
 		"errors": {
+			"40000000-0000-0000-0000-000000000000": [
+				"model not found"
+			],
 			"50000000-0000-0000-0000-000000000000": [
 				"forcing an error on model 5"
 			]
@@ -564,7 +538,10 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query all applications across all models.
-	res, err = j.QueryModelsJq(ctx, models, ".applications")
+	userModelUUIDs, err = alice.ListModels(ctx)
+	c.Assert(err, qt.IsNil)
+
+	res, err = j.QueryModelsJq(ctx, userModelUUIDs, ".applications")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -700,6 +677,9 @@ func TestQueryModelsJq(t *testing.T) {
 		  ]
 		},
 		"errors": {
+			"40000000-0000-0000-0000-000000000000": [
+				"model not found"
+			],
 			"50000000-0000-0000-0000-000000000000": [
 				"forcing an error on model 5"
 			]
@@ -708,7 +688,10 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query specifically for models including the app "nginx-ingress-integrator"
-	res, err = j.QueryModelsJq(ctx, models, ".applications | with_entries(select(.key==\"nginx-ingress-integrator\"))")
+	userModelUUIDs, err = alice.ListModels(ctx)
+	c.Assert(err, qt.IsNil)
+
+	res, err = j.QueryModelsJq(ctx, userModelUUIDs, ".applications | with_entries(select(.key==\"nginx-ingress-integrator\"))")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -762,6 +745,9 @@ func TestQueryModelsJq(t *testing.T) {
 		  ]
 		},
 		"errors": {
+			"40000000-0000-0000-0000-000000000000": [
+				"model not found"
+			],
 			"50000000-0000-0000-0000-000000000000": [
 				"forcing an error on model 5"
 			]
@@ -770,7 +756,10 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query specifically for storage on this model.
-	res, err = j.QueryModelsJq(ctx, models, ".storage")
+	userModelUUIDs, err = alice.ListModels(ctx)
+	c.Assert(err, qt.IsNil)
+
+	res, err = j.QueryModelsJq(ctx, userModelUUIDs, ".storage")
 	c.Assert(err, qt.IsNil)
 
 	// Not the cleanest thing in the world, but this field needs ignoring,
@@ -815,10 +804,14 @@ func TestQueryModelsJq(t *testing.T) {
 		  ]
 		},
 		"errors": {
+		  "40000000-0000-0000-0000-000000000000": [
+			"model not found"
+		  ],
 		  "50000000-0000-0000-0000-000000000000": [
 			"forcing an error on model 5"
-		  ]
+		]
 		}
 	}
 	`, qt.JSONEquals, res)
+
 }

internal/jujuapi/jimm.go

@@ -496,18 +496,13 @@ func (r *controllerRoot) RemoveCloudFromController(ctx context.Context, req apip
 // The query will run against output exactly like "juju status --format json", but for each of their models.
 func (r *controllerRoot) CrossModelQuery(ctx context.Context, req apiparams.CrossModelQueryRequest) (apiparams.CrossModelQueryResponse, error) {
 	const op = errors.Op("jujuapi.CrossModelQuery")
-
-	usersModels, err := r.jimm.Database.GetUserModels(ctx, r.user.User)
+	modelUUIDs, err := r.user.ListModels(ctx)
 	if err != nil {
 		return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.Code("failed to get models for user"))
 	}
-	models := make([]dbmodel.Model, len(usersModels))
-	for i, m := range usersModels {
-		models[i] = m.Model_
-	}
 	switch strings.TrimSpace(strings.ToLower(req.Type)) {
 	case "jq":
-		return r.jimm.QueryModelsJq(ctx, models, req.Query)
+		return r.jimm.QueryModelsJq(ctx, modelUUIDs, req.Query)
 	case "jimmsql":
 		return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.CodeNotImplemented)
 	default: