Skip to content

Commit

Permalink
Merge pull request #49 from ellingtonjp/granular-permissions-errors
Browse files Browse the repository at this point in the history
🔨 Fix bug for Metabase Pro instances with modified analytics permissions
  • Loading branch information
flovouin authored Apr 7, 2024
2 parents 607ca76 + 9ee8f28 commit 1611bd7
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 20 deletions.
36 changes: 32 additions & 4 deletions internal/provider/permissions_graph_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,16 @@ func makeAccessPermissionsFromDatabaseAccess(ctx context.Context, da *metabase.P
return &nullObject, diag.Diagnostics{}
}

var diags diag.Diagnostics
schemas, err := da.Schemas.AsPermissionsGraphDatabaseAccessSchemas0()
if err != nil {
diags.AddError("Unexpected permissions value. This could be caused by using granular permissions (unsupported). Remove granular permissions and try again", err.Error())
return nil, diags
}

obj, diags := types.ObjectValueFrom(ctx, accessPermissionsObjectType.AttrTypes, AccessPermissions{
Native: stringValueOrNull(da.Native),
Schemas: stringValueOrNull(da.Schemas),
Schemas: stringValueOrNull(&schemas),
})
if diags.HasError() {
return nil, diags
Expand Down Expand Up @@ -239,6 +246,11 @@ func updateModelFromPermissionsGraph(ctx context.Context, g metabase.Permissions
}

for dbId, dbPermissions := range dbPermissionsMap {
// Ignore the Metabase Analytics database until we have proper support.
if dbId == metabase.MetabaseAnalyticsDatabaseId {
continue
}

permissionsObject, objDiags := makePermissionsObjectFromDatabasePermissions(ctx, groupId, dbId, dbPermissions)
diags.Append(objDiags...)
if diags.HasError() {
Expand Down Expand Up @@ -274,7 +286,13 @@ func makeDatasetAccessFromModel(ctx context.Context, apObj types.Object, setIfNu

// Default values ("none") forbid any access.
var native *metabase.PermissionsGraphDatabaseAccessNative
schemas := metabase.PermissionsGraphDatabaseAccessSchemasNone

var schemas metabase.PermissionsGraphDatabaseAccess_Schemas
err := schemas.FromPermissionsGraphDatabaseAccessSchemas0(metabase.PermissionsGraphDatabaseAccessSchemas0None)
if err != nil {
diags.AddError("Unexpected error setting schemas to none value", err.Error())
return nil, diags
}
if setNative {
none := metabase.PermissionsGraphDatabaseAccessNativeNone
native = &none
Expand All @@ -294,7 +312,11 @@ func makeDatasetAccessFromModel(ctx context.Context, apObj types.Object, setIfNu
}

if !ap.Schemas.IsNull() {
schemas = metabase.PermissionsGraphDatabaseAccessSchemas(ap.Schemas.ValueString())
err := schemas.FromPermissionsGraphDatabaseAccessSchemas0(metabase.PermissionsGraphDatabaseAccessSchemas0(ap.Schemas.ValueString()))
if err != nil {
diags.AddError("Unexpected error setting permissions value", err.Error())
return nil, diags
}
}
}

Expand Down Expand Up @@ -417,7 +439,13 @@ func makePermissionsGraphFromModel(ctx context.Context, data PermissionsGraphRes
// If the permission does not exist in the plan but exists in the state, it should be explicitly deleted by
// creating the permission with "none" values.
nativeNone := metabase.PermissionsGraphDatabaseAccessNativeNone
schemasNone := metabase.PermissionsGraphDatabaseAccessSchemasNone

var schemasNone metabase.PermissionsGraphDatabaseAccess_Schemas
err := schemasNone.FromPermissionsGraphDatabaseAccessSchemas0(metabase.PermissionsGraphDatabaseAccessSchemas0None)
if err != nil {
diags.AddError("Unexpected error setting schema none value", err.Error())
return nil, diags
}
deletedPermissions := metabase.PermissionsGraphDatabasePermissions{
Data: &metabase.PermissionsGraphDatabaseAccess{
Native: &nativeNone,
Expand Down
20 changes: 14 additions & 6 deletions metabase-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1279,12 +1279,20 @@ components:
- write
- none
schemas:
type: string
description: Whether "Data access" is allowed.
enum:
- full
- all
- none
# The `schemas` property can either be a string or an object. The API returns an object in two cases:
# 1. Permissions are set to "granular" and some tables have different permissions than others
# 2. Permissions are modified on the Metabase Analytics database (available in pro version)
#
# Proper support for these cases requires more research. For now, we only specify the string type. It's under
# `oneOf` so the generated client doesn't error when it receives a non-string response. Application code is
# expected to detect and handle these cases.
oneOf:
- type: string
description: Whether "Data access" is allowed.
enum:
- full
- all
- none
# Sessions.
Session:
type: object
Expand Down
59 changes: 49 additions & 10 deletions metabase/client.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions metabase/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package metabase
// The default ID of the `Administrators` permissions group, created automatically by Terraform.
const AdministratorsPermissionsGroupId = 2

// The ID of the `Metabase Analytics` database, automatically created for pro plans.
const MetabaseAnalyticsDatabaseId = "13371337"

// The list of JSON attributes in a `Card` object that are needed to fully define the card, e.g. when creating it.
var DefiningCardAttributes = map[string]bool{
"cache_ttl": true,
Expand Down

0 comments on commit 1611bd7

Please sign in to comment.