diff --git a/pkg/services/authz/zanzana/client.go b/pkg/services/authz/zanzana/client.go index cd63995e89c4..5a769323de8d 100644 --- a/pkg/services/authz/zanzana/client.go +++ b/pkg/services/authz/zanzana/client.go @@ -22,10 +22,15 @@ type Client interface { } func NewClient(ctx context.Context, cc grpc.ClientConnInterface, cfg *setting.Cfg) (*client.Client, error) { + stackID := cfg.StackID + if stackID == "" { + stackID = "default" + } + return client.New( ctx, cc, - client.WithTenantID(fmt.Sprintf("stack-%s", cfg.StackID)), + client.WithTenantID(fmt.Sprintf("stacks-%s", stackID)), client.WithLogger(log.New("zanzana-client")), ) } diff --git a/pkg/services/authz/zanzana/client/client.go b/pkg/services/authz/zanzana/client/client.go index 6f4e27fc50fb..cc29199270fd 100644 --- a/pkg/services/authz/zanzana/client/client.go +++ b/pkg/services/authz/zanzana/client/client.go @@ -6,14 +6,12 @@ import ( "fmt" openfgav1 "github.com/openfga/api/proto/openfga/v1" - "github.com/openfga/language/pkg/go/transformer" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "google.golang.org/grpc" "google.golang.org/protobuf/types/known/wrapperspb" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/authz/zanzana/schema" ) var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/authz/zanzana/client") @@ -32,16 +30,9 @@ func WithLogger(logger log.Logger) ClientOption { } } -func WithSchema(modules []transformer.ModuleFile) ClientOption { - return func(c *Client) { - c.modules = modules - } -} - type Client struct { logger log.Logger client openfgav1.OpenFGAServiceClient - modules []transformer.ModuleFile tenantID string storeID string modelID string @@ -61,21 +52,17 @@ func New(ctx context.Context, cc grpc.ClientConnInterface, opts ...ClientOption) } if c.tenantID == "" { - c.tenantID = "stack-default" + c.tenantID = "stacks-default" } - if len(c.modules) == 0 { - c.modules = schema.SchemaModules - } - - store, err := c.getOrCreateStore(ctx, c.tenantID) + store, err := c.getStore(ctx, c.tenantID) if err != nil { return nil, err } c.storeID = store.GetId() - modelID, err := c.loadModel(ctx, c.storeID, c.modules) + modelID, err := c.loadModel(ctx, c.storeID) if err != nil { return nil, err } @@ -119,24 +106,6 @@ func (c *Client) Write(ctx context.Context, in *openfgav1.WriteRequest) error { return err } -func (c *Client) getOrCreateStore(ctx context.Context, name string) (*openfgav1.Store, error) { - store, err := c.getStore(ctx, name) - - if errors.Is(err, errStoreNotFound) { - var res *openfgav1.CreateStoreResponse - res, err = c.client.CreateStore(ctx, &openfgav1.CreateStoreRequest{Name: name}) - if res != nil { - store = &openfgav1.Store{ - Id: res.GetId(), - Name: res.GetName(), - CreatedAt: res.GetCreatedAt(), - } - } - } - - return store, err -} - var errStoreNotFound = errors.New("store not found") func (c *Client) getStore(ctx context.Context, name string) (*openfgav1.Store, error) { @@ -170,52 +139,21 @@ func (c *Client) getStore(ctx context.Context, name string) (*openfgav1.Store, e } } -func (c *Client) loadModel(ctx context.Context, storeID string, modules []transformer.ModuleFile) (string, error) { - var continuationToken string - - model, err := schema.TransformModulesToModel(modules) - if err != nil { - return "", err - } - - for { - // ReadAuthorizationModels returns authorization models for a store sorted in descending order of creation. - // So with a pageSize of 1 we will get the latest model. - res, err := c.client.ReadAuthorizationModels(ctx, &openfgav1.ReadAuthorizationModelsRequest{ - StoreId: storeID, - PageSize: &wrapperspb.Int32Value{Value: 20}, - ContinuationToken: continuationToken, - }) - - if err != nil { - return "", fmt.Errorf("failed to load authorization model: %w", err) - } - - for _, m := range res.GetAuthorizationModels() { - // If provided dsl is equal to a stored dsl we use that as the authorization id - if schema.EqualModels(m, model) { - return m.GetId(), nil - } - } - - // If we have not found any matching authorization model we break the loop and create a new one - if res.GetContinuationToken() == "" { - break - } - - continuationToken = res.GetContinuationToken() - } - - writeRes, err := c.client.WriteAuthorizationModel(ctx, &openfgav1.WriteAuthorizationModelRequest{ - StoreId: c.storeID, - TypeDefinitions: model.GetTypeDefinitions(), - SchemaVersion: model.GetSchemaVersion(), - Conditions: model.GetConditions(), +func (c *Client) loadModel(ctx context.Context, storeID string) (string, error) { + // ReadAuthorizationModels returns authorization models for a store sorted in descending order of creation. + // So with a pageSize of 1 we will get the latest model. + res, err := c.client.ReadAuthorizationModels(ctx, &openfgav1.ReadAuthorizationModelsRequest{ + StoreId: storeID, + PageSize: &wrapperspb.Int32Value{Value: 1}, }) if err != nil { - return "", fmt.Errorf("failed to load authorization model: %w", err) + return "", fmt.Errorf("failed to load latest authorization model: %w", err) } - return writeRes.GetAuthorizationModelId(), nil + if len(res.AuthorizationModels) != 1 { + return "", fmt.Errorf("failed to load latest authorization model") + } + + return res.AuthorizationModels[0].GetId(), nil } diff --git a/pkg/services/authz/zanzana/client/client_test.go b/pkg/services/authz/zanzana/client/client_test.go deleted file mode 100644 index cd9e43088105..000000000000 --- a/pkg/services/authz/zanzana/client/client_test.go +++ /dev/null @@ -1,117 +0,0 @@ -package client - -import ( - "context" - "testing" - - openfgav1 "github.com/openfga/api/proto/openfga/v1" - "github.com/openfga/language/pkg/go/transformer" - - "github.com/fullstorydev/grpchan/inprocgrpc" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/tests/testsuite" - - zserver "github.com/grafana/grafana/pkg/services/authz/zanzana/server" - zstore "github.com/grafana/grafana/pkg/services/authz/zanzana/store" - "github.com/grafana/grafana/pkg/services/sqlstore/migrator" -) - -func TestMain(m *testing.M) { - testsuite.Run(m) -} - -func TestIntegrationClient(t *testing.T) { - conn := zanzanaServerIntegrationTest(t) - - var ( - prevStoreID string - prevModelID string - ) - - t.Run("should create default store and authorization model on first startup", func(t *testing.T) { - c, err := New(context.Background(), conn) - require.NoError(t, err) - - assert.NotEmpty(t, c.storeID) - assert.NotEmpty(t, c.modelID) - - prevStoreID, prevModelID = c.storeID, c.modelID - }) - - t.Run("should reuse existing store and authorization model", func(t *testing.T) { - c, err := New(context.Background(), conn) - require.NoError(t, err) - - assert.Equal(t, prevStoreID, c.storeID) - assert.Equal(t, prevModelID, c.modelID) - }) - - t.Run("should create new store and authorization model when new tenant id is used", func(t *testing.T) { - c, err := New(context.Background(), conn, WithTenantID("new")) - require.NoError(t, err) - - assert.NotEmpty(t, c.storeID) - assert.NotEmpty(t, c.modelID) - - assert.NotEqual(t, prevStoreID, c.storeID) - assert.NotEqual(t, prevModelID, c.modelID) - - prevStoreID, prevModelID = c.storeID, c.modelID - }) - - t.Run("should update authorization model if it has new changes", func(t *testing.T) { - dsl := ` -module core - -type user - ` - modules := []transformer.ModuleFile{ - {Name: "core.fga", Contents: dsl}, - } - c, err := New(context.Background(), conn, WithTenantID("new"), WithSchema(modules)) - require.NoError(t, err) - - assert.Equal(t, prevStoreID, c.storeID) - assert.NotEqual(t, prevModelID, c.modelID) - }) - - t.Run("should load older authorization model", func(t *testing.T) { - c, err := New(context.Background(), conn, WithTenantID("new")) - require.NoError(t, err) - - assert.Equal(t, prevStoreID, c.storeID) - assert.Equal(t, prevModelID, c.modelID) - }) -} - -func zanzanaServerIntegrationTest(tb testing.TB) *inprocgrpc.Channel { - if testing.Short() { - tb.Skip("skipping integration test") - } - - db, cfg := db.InitTestDBWithCfg(tb) - - // Hack to skip these tests on mysql 5.7 - if db.GetDialect().DriverName() == migrator.MySQL { - if supported, err := db.RecursiveQueriesAreSupported(); !supported || err != nil { - tb.Skip("skipping integration test") - } - } - - logger := log.NewNopLogger() - - store, err := zstore.NewEmbeddedStore(cfg, db, logger) - require.NoError(tb, err) - - srv, err := zserver.NewOpenFGA(&cfg.Zanzana, store, logger) - require.NoError(tb, err) - - channel := &inprocgrpc.Channel{} - openfgav1.RegisterOpenFGAServiceServer(channel, srv) - - return channel -} diff --git a/pkg/services/authz/zanzana/server.go b/pkg/services/authz/zanzana/server.go index 18e2c76ccab8..25cb2a9508d3 100644 --- a/pkg/services/authz/zanzana/server.go +++ b/pkg/services/authz/zanzana/server.go @@ -26,7 +26,7 @@ func NewAuthzServer(cfg *setting.Cfg, openfga openfgav1.OpenFGAServiceServer) (* return zserver.NewAuthz( openfga, - zserver.WithTenantID(fmt.Sprintf("stack-%s", stackID)), + zserver.WithTenantID(fmt.Sprintf("stacks-%s", stackID)), ) } diff --git a/pkg/services/authz/zanzana/server/authz_server.go b/pkg/services/authz/zanzana/server/authz_server.go index c28e281c9ebe..a982c0308d00 100644 --- a/pkg/services/authz/zanzana/server/authz_server.go +++ b/pkg/services/authz/zanzana/server/authz_server.go @@ -68,7 +68,7 @@ func NewAuthz(openfga openfgav1.OpenFGAServiceServer, opts ...ServerOption) (*Se } if s.tenantID == "" { - s.tenantID = "stack-default" + s.tenantID = "stacks-default" } if len(s.modules) == 0 { diff --git a/pkg/services/dashboards/service/zanzana_integration_test.go b/pkg/services/dashboards/service/zanzana_integration_test.go index c10d4ce1b8bd..2db1f20d4cf4 100644 --- a/pkg/services/dashboards/service/zanzana_integration_test.go +++ b/pkg/services/dashboards/service/zanzana_integration_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/quota/quotatest" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/tag/tagimpl" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -31,12 +32,16 @@ func TestIntegrationDashboardServiceZanzana(t *testing.T) { } t.Run("Zanzana enabled", func(t *testing.T) { - // t.Helper() - features := featuremgmt.WithFeatures(featuremgmt.FlagZanzana) - db, cfg := db.InitTestDBWithCfg(t) + // Hack to skip these tests on mysql 5.7 + if db.GetDialect().DriverName() == migrator.MySQL { + if supported, err := db.RecursiveQueriesAreSupported(); !supported || err != nil { + t.Skip("skipping integration test") + } + } + // Enable zanzana and run in embedded mode (part of grafana server) cfg.Zanzana.ZanzanaOnlyEvaluation = true cfg.Zanzana.Mode = setting.ZanzanaModeEmbedded