Merge branch 'feature-rebac' into vault-relation-fix

internal/jimm/model_status_parser.go

@@ -2,9 +2,7 @@ package jimm
 
 import (
 	"context"
-	"database/sql"
 	"encoding/json"
-	"strings"
 
 	"github.com/CanonicalLtd/jimm/api/params"
 	"github.com/CanonicalLtd/jimm/internal/dbmodel"
@@ -16,16 +14,17 @@ 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 model available to a given user.
+// QueryModels queries every specified model in modelUUIDs.
 //
 // 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, modelUUIDs []string, jqQuery string) (params.CrossModelQueryResponse, error) {
+func (j *JIMM) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuery string) (params.CrossModelQueryResponse, error) {
 	op := errors.Op("QueryModels")
 	results := params.CrossModelQueryResponse{
 		Results: make(map[string][]any),
@@ -34,25 +33,19 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, modelUUIDs []string, jqQuery s
 
 	query, err := gojq.Parse(jqQuery)
 	if err != nil {
-		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]
+		return results, errors.E(op, "failed to parse jq query", err)
 	}
 
 	// Set up a formatterParamsRetriever to handle the heavy lifting
 	// of each facade call and type conversion.
 	retriever := newFormatterParamsRetriever(j)
 
-	for _, id := range modelUUIDs {
-
-		params, err := retriever.GetParams(ctx, id)
+	for _, model := range models {
+		modelUUID := model.UUID.String
+		params, err := retriever.GetParams(ctx, model)
 		if err != nil {
-			zapctx.Error(ctx, "failed to get status formatter params", zap.String("model-uuid", id))
-			results.Errors[id] = append(results.Errors[id], err.Error())
+			zapctx.Error(ctx, "failed to get status formatter params", zap.String("model-uuid", modelUUID))
+			results.Errors[modelUUID] = append(results.Errors[modelUUID], err.Error())
 			continue
 		}
 
@@ -62,17 +55,17 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, modelUUIDs []string, jqQuery s
 
 		formattedStatus, err := formatter.Format()
 		if err != nil {
-			zapctx.Error(ctx, "failed to format status", zap.String("model-uuid", id))
-			results.Errors[id] = append(results.Errors[id], err.Error())
+			zapctx.Error(ctx, "failed to format status", zap.String("model-uuid", modelUUID))
+			results.Errors[modelUUID] = append(results.Errors[modelUUID], 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", id))
-			results.Errors[id] = append(results.Errors[id], err.Error())
+			zapctx.Error(ctx, "failed to marshal formatted status", zap.String("model-uuid", modelUUID))
+			results.Errors[modelUUID] = append(results.Errors[modelUUID], err.Error())
 			continue
 		}
 		tempMap := make(map[string]any)
@@ -91,11 +84,11 @@ func (j *JIMM) QueryModelsJq(ctx context.Context, modelUUIDs []string, jqQuery s
 			// 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[id] = append(results.Errors[id], "jq error: "+err.Error())
+				results.Errors[modelUUID] = append(results.Errors[modelUUID], "jq error: "+err.Error())
 				continue
 			}
 
-			results.Results[id] = append(results.Results[id], v)
+			results.Results[modelUUID] = append(results.Results[modelUUID], v)
 		}
 	}
 	return results, nil
@@ -124,10 +117,8 @@ 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, modelUUID string) (*status.NewStatusFormatterParams, error) {
-	if err := f.loadModel(ctx, modelUUID); err != nil {
-		return nil, err
-	}
+func (f *formatterParamsRetriever) GetParams(ctx context.Context, model dbmodel.Model) (*status.NewStatusFormatterParams, error) {
+	f.model = &model
 
 	err := f.dialModel(ctx)
 	if err != nil {
@@ -154,24 +145,13 @@ func (f *formatterParamsRetriever) GetParams(ctx context.Context, modelUUID stri
 	}, 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 {
-	api, err := f.jimm.dial(ctx, &f.model.Controller, f.model.ResourceTag())
+	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)
 	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

@@ -17,7 +17,6 @@ import (
 	"github.com/CanonicalLtd/jimm/internal/errors"
 	"github.com/CanonicalLtd/jimm/internal/jimm"
 	"github.com/CanonicalLtd/jimm/internal/jimmtest"
-	"github.com/CanonicalLtd/jimm/internal/openfga"
 	ofga "github.com/CanonicalLtd/jimm/internal/openfga"
 	ofganames "github.com/CanonicalLtd/jimm/internal/openfga/names"
 	jimmnames "github.com/CanonicalLtd/jimm/pkg/names"
@@ -58,6 +57,9 @@ 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
@@ -72,6 +74,9 @@ 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
@@ -86,6 +91,9 @@ 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
@@ -112,6 +120,8 @@ 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{
@@ -133,7 +143,7 @@ func getFullStatus(
 		Applications:       applications,
 		RemoteApplications: remoteApps,
 		Offers:             map[string]jujuparams.ApplicationOfferStatus{},
-		Relations:          []jujuparams.RelationStatus(nil),
+		Relations:          modelRelations,
 		Branches:           map[string]jujuparams.BranchStatus{},
 	}
 }
@@ -198,6 +208,22 @@ 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.
@@ -283,16 +309,19 @@ 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) {
@@ -411,6 +440,8 @@ 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",
@@ -463,20 +494,17 @@ func TestQueryModelsJq(t *testing.T) {
 		}...,
 	), qt.Equals, nil)
 
-	alice := openfga.NewUser(
-		&dbmodel.User{
-			Username: "alice@external",
-		},
-		client,
-	)
-
 	// Tests:
 
 	// Query for all models only.
-	userModelUUIDs, err := alice.ListModels(ctx)
+	usersModels, err := j.Database.GetUserModels(ctx, &user)
 	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, userModelUUIDs, ".model")
+	res, err := j.QueryModelsJq(ctx, models, ".model")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -528,9 +556,6 @@ 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"
 			]
@@ -539,10 +564,7 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query all applications across all models.
-	userModelUUIDs, err = alice.ListModels(ctx)
-	c.Assert(err, qt.IsNil)
-
-	res, err = j.QueryModelsJq(ctx, userModelUUIDs, ".applications")
+	res, err = j.QueryModelsJq(ctx, models, ".applications")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -678,9 +700,6 @@ 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"
 			]
@@ -689,10 +708,7 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query specifically for models including the app "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\"))")
+	res, err = j.QueryModelsJq(ctx, models, ".applications | with_entries(select(.key==\"nginx-ingress-integrator\"))")
 	c.Assert(err, qt.IsNil)
 	c.Assert(`
 	{
@@ -746,9 +762,6 @@ 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"
 			]
@@ -757,10 +770,7 @@ func TestQueryModelsJq(t *testing.T) {
 	`, qt.JSONEquals, res)
 
 	// Query specifically for storage on this model.
-	userModelUUIDs, err = alice.ListModels(ctx)
-	c.Assert(err, qt.IsNil)
-
-	res, err = j.QueryModelsJq(ctx, userModelUUIDs, ".storage")
+	res, err = j.QueryModelsJq(ctx, models, ".storage")
 	c.Assert(err, qt.IsNil)
 
 	// Not the cleanest thing in the world, but this field needs ignoring,
@@ -805,14 +815,10 @@ 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

@@ -480,13 +480,18 @@ 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")
-	modelUUIDs, err := r.user.ListModels(ctx)
+
+	usersModels, err := r.jimm.Database.GetUserModels(ctx, r.user.User)
 	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, modelUUIDs, req.Query)
+		return r.jimm.QueryModelsJq(ctx, models, req.Query)
 	case "jimmsql":
 		return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.CodeNotImplemented)
 	default: