Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial implementation of metabase group migration into guardian #167

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
47c3881
feat: adding table, group as metabase resource and add groups-databas…
singhvikash11 Apr 20, 2022
61580ff
fix: fix test case
singhvikash11 Apr 20, 2022
bf17893
Fix test case
singhvikash11 Apr 21, 2022
16f517b
Add constants for string literal
singhvikash11 Apr 21, 2022
e79c96e
Change type for database and collection in group
singhvikash11 Apr 21, 2022
2d93d9c
Resolve feedback
singhvikash11 Apr 22, 2022
4257dd2
fix:
singhvikash11 May 10, 2022
1c1f807
Add grant and revoke method for group, table
singhvikash11 May 10, 2022
f8e6386
Fix test
singhvikash11 May 10, 2022
eda6c8c
Fix lint
singhvikash11 May 10, 2022
76c6510
Resolve feedback and add fetch database/collection while fetching groups
singhvikash11 May 12, 2022
5d01b7c
fix: custom group naming convention for metabase provider and filter …
singhvikash11 May 12, 2022
1268389
fix: custom group naming convention for metabase provider and filter …
singhvikash11 May 12, 2022
15ba0eb
Merge remote-tracking branch 'origin/metabase_resources' into metabas…
singhvikash11 May 13, 2022
207ac02
fix: resolve merge conflict
singhvikash11 May 13, 2022
38ffeed
fix: update the log to use key/value pair logging
singhvikash11 May 18, 2022
158ea99
feat: initial implementation of metabase group migration into guardian
singhvikash11 May 19, 2022
3293d72
Merge branch 'metabase_resources' into metabase_migration to enable m…
singhvikash11 May 19, 2022
2dff81c
Update doc of Metabase resources and add `member` as role in group
singhvikash11 May 23, 2022
291c036
Merge branch 'metabase_resources' into metabase_migration
singhvikash11 May 23, 2022
55af1a2
Add migrate command to migrate access for a given provider
singhvikash11 May 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func InitServices(deps ServiceDeps) (*Services, error) {

providerClients := []providers.Client{
bigquery.NewProvider(domain.ProviderTypeBigQuery, deps.Crypto),
metabase.NewProvider(domain.ProviderTypeMetabase, deps.Crypto),
metabase.NewProvider(domain.ProviderTypeMetabase, deps.Crypto, deps.Logger),
grafana.NewProvider(domain.ProviderTypeGrafana, deps.Crypto),
tableau.NewProvider(domain.ProviderTypeTableau, deps.Crypto),
gcloudiam.NewProvider(domain.ProviderTypeGCloudIAM, deps.Crypto),
Expand Down
3 changes: 2 additions & 1 deletion cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
var cliConfig *Config

type Config struct {
Host string `mapstructure:"host"`
Host string `mapstructure:"host"`
EncryptionSecretKey string `mapstructure:"encryption_secret_key"`
}

func LoadConfig() (*Config, error) {
Expand Down
202 changes: 202 additions & 0 deletions cmd/migration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package cmd

import (
"context"
"errors"
"fmt"

"github.com/MakeNowJust/heredoc"
. "github.com/odpf/guardian/api/proto/odpf/guardian/v1beta1"
"github.com/odpf/guardian/domain"
"github.com/odpf/guardian/internal/crypto"
"github.com/odpf/guardian/plugins/migrations"
mb "github.com/odpf/guardian/plugins/migrations/metabase"
"github.com/spf13/cobra"
"google.golang.org/protobuf/types/known/structpb"
)

const (
pending = "pending"
active = "active"
)

func MigrationCmd(config *Config) *cobra.Command {
cmd := &cobra.Command{
Use: "migrate",
Short: "Guardian migration",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rahmatrhd @singhvikash11 I think we need to think of a better approach than attaching it on migrate. One possible suggestion could be to call it import and scope it with the provider itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisuhag this is a good idea, we need to implement migrate for bigquery provider and other providers too. Then we could expose it as a sub-command on the provider

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisuhag @rahmatrhd If you folks are okay with Metabase migrate for now then I can move this as sub-command to provider.

Copy link
Member

@ravisuhag ravisuhag May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singhvikash11 What do you mean okay with metabase provider?

Also, i would recommend checking out https://github.com/GoogleCloudPlatform/terraformer for some inspiration on how command could look like.

guardian provider import <provide-name> -r <resources>

what do you guys think? cc @rahmatrhd

Copy link
Member Author

@singhvikash11 singhvikash11 Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisuhag we already have the command guardian provider create command to register the resources with guardian. Let's use the following command as suggested by you to import ACL of existing resources from a source system:

guardian provider import <provide-name> acl
guardian provider import <provide-name> acl -r <resources>

what do you guys think? cc @rahmatrhd

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singhvikash11 What does acl represent in this command?

Copy link
Member Author

@singhvikash11 singhvikash11 Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisuhag Bigquery, Metabase resources like table, dataset, collection etc are defined by a set of permissions associated with a set of people, groups, service-account in the source system. By importing access-control of these resources into our system we'll replicate it in the model of appeal-resource in Guardian.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singhvikash11 Yes, understood. But what i meant is that we don't need acl word in the comamnd right? the command can plainly be guardian provider import <provide-name> -r <resources> where -r flag will take the kind of resources you want to import from that provider.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravisuhag This makes sense, we can go ahead with this command.

cc @rahmatrhd

Long: heredoc.Doc(`
Migrate target system ACL into Guardian.
`),
Example: heredoc.Doc(`
$ guardian migration <provider-urn>
`),
Annotations: map[string]string{
"group:core": "true",
},
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
providerId := args[0]

say := crypto.NewAES(config.EncryptionSecretKey)
client, cancel, err := createClient(cmd)
if err != nil {
return err
}
defer cancel()

context := cmd.Context()
provider, err := getProviderConfig(client, context, providerId, say)
if err != nil {
return err
}

resources, err := getResources(client, context, provider)
if err != nil {
return err
}

appealResponse, appeals, err := getActiveAndPendingAppeals(client, context, provider)
if err != nil {
return err
}

var migration migrations.Client
if provider.Type == migrations.Metabase {
migration = mb.NewMigration(provider.Config, resources, appeals)
} else {
return errors.New(fmt.Sprintf("Migration not supported for provider %s", provider.Type))
}

appealRequests, err := migration.PopulateAccess()
if err != nil {
return err
}

//migrate past-run pending appeals
for _, a := range appealResponse {
if a.Status == pending {
err := approveAppeal(a, client, context)
if err != nil {
return err
} else {
fmt.Println(a.Resource.Name, a.AccountId)
}
}
}

//migrate pending appeals
for _, appealRequest := range appealRequests {
resource := appealRequest.Resource
option, _ := structpb.NewStruct(map[string]interface{}{migrations.Duration: resource.Duration})

accountID := appealRequest.AccountID
appeal, err := client.CreateAppeal(context, &CreateAppealRequest{
AccountId: accountID,
Resources: []*CreateAppealRequest_Resource{
{Id: resource.ID, Role: resource.Role, Options: option}},
AccountType: "",
})
if err != nil {
return err
} else {
appeals := appeal.GetAppeals()
for _, appeal := range appeals {
err := approveAppeal(appeal, client, context)
if err != nil {
return err
}
}
}
}

return nil
},
}

bindFlagsFromConfig(cmd)

return cmd
}

func getActiveAndPendingAppeals(client GuardianServiceClient, context context.Context, provider *domain.Provider) ([]*Appeal, []domain.Appeal, error) {
appeals := make([]domain.Appeal, 0)
listAppeals, err := client.ListAppeals(context, &ListAppealsRequest{ProviderUrns: []string{provider.URN}, Statuses: []string{pending, active}})
if err != nil {
return nil, appeals, err
}

appealResponses := listAppeals.GetAppeals()
for _, a := range appealResponses {
appeals = append(appeals, domain.Appeal{
ID: a.Id,
ResourceID: a.ResourceId,
Status: a.Status,
AccountID: a.AccountId,
AccountType: a.AccountType,
Role: a.Role,
})
}
return appealResponses, appeals, nil
}

func getResources(client GuardianServiceClient, context context.Context, provider *domain.Provider) ([]domain.Resource, error) {
listResources, err := client.ListResources(context, &ListResourcesRequest{ProviderUrn: provider.URN, IsDeleted: false})
resources := make([]domain.Resource, 0)
if err != nil {
return resources, err
}
for _, r := range listResources.GetResources() {
resources = append(resources, domain.Resource{
ID: r.Id,
ProviderType: r.ProviderType,
ProviderURN: r.ProviderUrn,
Type: r.Type,
URN: r.Urn,
Name: r.Name,
})
}
return resources, nil
}

func getProviderConfig(client GuardianServiceClient, context context.Context, providerId string, say *crypto.AES) (*domain.Provider, error) {
providerResponse, err := client.GetProvider(context, &GetProviderRequest{Id: providerId})
if err != nil {
return nil, err
}

provider := providerResponse.GetProvider()
fields := provider.Config.Credentials.GetStructValue().GetFields()
abc, err := say.Decrypt(fields[migrations.Password].GetStringValue())

return &domain.Provider{
ID: providerId,
Type: provider.Type,
URN: provider.Urn,
Config: &domain.ProviderConfig{
Type: provider.Config.Type,
URN: provider.Config.Urn,
Credentials: map[string]string{
migrations.Username: fields[migrations.Username].GetStringValue(),
migrations.Password: abc,
migrations.Host: fields[migrations.Host].GetStringValue(),
},
},
}, err
}

func approveAppeal(appeal *Appeal, client GuardianServiceClient, context context.Context) error {
approvals := appeal.Approvals
for _, approval := range approvals {
if approval.Status == pending {
_, err := client.UpdateApproval(context, &UpdateApprovalRequest{
Id: appeal.Id,
ApprovalName: approval.Name,
Action: &UpdateApprovalRequest_Action{Action: "approve", Reason: "Metabase migration"},
})
if err != nil {
return err
}
}
}
return nil
}
3 changes: 3 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func New(cfg *Config) *cobra.Command {
cmd.AddCommand(configCommand())
cmd.AddCommand(VersionCmd())

//Migration command
cmd.AddCommand(MigrationCmd(cliConfig))

// Help topics
cmdx.SetHelp(cmd)
cmd.AddCommand(cmdx.SetCompletionCmd("guardian"))
Expand Down
2 changes: 1 addition & 1 deletion core/provider/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (s *Service) ValidateAppeal(a *domain.Appeal, p *domain.Provider) error {
return err
}

isRoleExists := false
isRoleExists := len(roles) == 0
for _, role := range roles {
if a.Role == role.ID {
isRoleExists = true
Expand Down
24 changes: 21 additions & 3 deletions docs/docs/providers/metabase.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ resources:
name: Editor
permissions:
- write
- type: table
policy:
id: policy_id
version: 1
roles:
- id: viewer
name: Viewer
permissions:
- all
- type: group
policy:
id: policy_id
version: 1
roles:
- id: member
name: Member
```

### `MetabaseCredentials`
Expand All @@ -85,10 +101,12 @@ resources:
### `MetabaseResourceType`

- `database`
- `table`
- `collection`
- `group`

### `MetabaseResourcePermission`

| Type | Details |
| :----------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Required. `string` | Metabase permission mapping **Possible values:** - `database`: `schemas:all` \(read table\), `native:write` \(run SQL query\) **Note**: Metabase requires `schemas:all` permission for `native:write` to be able to work - `collection`: `read`, `write` |
| Type | Details |
| :----------------- |:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Required. `string` | Metabase permission mapping **Possible values:** - `database`: `schemas:all` \(read table\), `native:write` \(run SQL query\) **Note**: Metabase requires `schemas:all` permission for `native:write` to be able to work - `collection`: `read`, `write` **Note**: Metabase table requires `all` permission to read table, no write permission on table level **Note**: Metabase group requires no specific permission to be a member of group ||
Loading