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

EVEREST-1502 | Remove admin policy from ConfigMap, hard-code it and load it internally #702

Merged
merged 5 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .github/workflows/dev-be-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ jobs:

- name: Add CI user to admin role
run: |
kubectl patch configmap everest-rbac -n everest-system --patch "$(kubectl get configmap everest-rbac -n everest-system -o json | jq '.data["policy.csv"] += "\ng, everest_ci, admin:role"' | jq '{data: { "policy.csv": .data["policy.csv"] } }')"
kubectl patch configmap everest-rbac -n everest-system --patch "$(kubectl get configmap everest-rbac -n everest-system -o json | jq '.data["policy.csv"] += "\ng, everest_ci, role:admin"' | jq '{data: { "policy.csv": .data["policy.csv"] } }')"
kubectl get configmap everest-rbac -n everest-system -ojsonpath='{.data.policy\.csv}'

- name: Run integration tests
Expand Down
10 changes: 5 additions & 5 deletions api-tests/generated/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ export interface paths {
* *Example:*
* Assume the following RBAC policy, and users `alice` and `bob`:
* ```
* p, admin:role, namespaces, read, *
* p, admin:role, database-engines, *, *\/*
* p, admin:role, database-clusters, *, *\/*
* p, role:dev, namespaces, read, *
* p, role:dev, database-engines, *, *\/*
* p, role:dev, database-clusters, *, *\/*
* p, bob, database-clusters, *, *\/*
* g, alice, admin:role
* g, alice, role:dev
* ```
* The API will return the following permissions for `alice`:
* ```
Expand Down Expand Up @@ -844,7 +844,7 @@ export interface components {
};
NamespaceList: string[];
UserPermissions: {
enabled?: boolean;
enabled: boolean;
permissions?: string[][];
};
UserCredentials: {
Expand Down
24 changes: 12 additions & 12 deletions api/everest-server.gen.go

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

24 changes: 12 additions & 12 deletions client/everest-client.gen.go

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

4 changes: 2 additions & 2 deletions commands/settings/rbac/can.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ $ everestctl settings rbac can alice read database-clusters my-namespace/cluster
# Check if user 'alice' can perform all/any actions on 'cluster-1' in namespace 'my-namespace'
$ everestctl settings rbac can alice '*' database-clusters my-namespace/cluster-1

# Check if role 'admin:role' can update backup 'prod-backup-1' in namespace 'prod-namespace'
$ everestctl settings rbac can admin:role update database-cluster-backups prod-namespace/prod-backup-1
# Check if role 'role:admin' can update backup 'prod-backup-1' in namespace 'prod-namespace'
$ everestctl settings rbac can role:admin update database-cluster-backups prod-namespace/prod-backup-1

# Check if user 'bob' can delete all/any backups in namespace 'prod-namespace'
$ everestctl settings rbac can bob delete database-cluster-backups prod-namespace/*
Expand Down
10 changes: 1 addition & 9 deletions deploy/quickstart-k8s.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,7 @@ metadata:
data:
enabled: "false"
policy.csv: |
p, admin:role, namespaces, *, *
p, admin:role, database-engines, *, */*
p, admin:role, database-clusters, *, */*
p, admin:role, database-cluster-backups, *, */*
p, admin:role, database-cluster-restores, *, */*
p, admin:role, database-cluster-credentials, *, */*
p, admin:role, backup-storages, *, */*
p, admin:role, monitoring-instances, *, */*
g, admin, admin:role
g, admin, role:admin
---
apiVersion: apps/v1
kind: Deployment
Expand Down
8 changes: 4 additions & 4 deletions docs/spec/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ paths:
*Example:*
Assume the following RBAC policy, and users `alice` and `bob`:
```
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, admin:role, database-clusters, *, */*
p, role:dev, namespaces, read, *
p, role:dev, database-engines, *, */*
p, role:dev, database-clusters, *, */*
p, bob, database-clusters, *, */*
g, alice, admin:role
g, alice, role:dev
```
The API will return the following permissions for `alice`:
```
Expand Down
4 changes: 4 additions & 0 deletions pkg/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ const (
// EverestJWTPublicKeyFile is the path to the JWT public key.
EverestJWTPublicKeyFile = "/etc/jwt/id_rsa.pub"

// EverestRBACRolePrefix is the prefix for roles.
EverestRBACRolePrefix = "role:"
// EverestAdminUser is the name of the admin user.
EverestAdminUser = "admin"
// EverestAdminRole is the name of the admin role.
EverestAdminRole = EverestRBACRolePrefix + "admin"

// EverestSettingsConfigMapName is the name of the Everest settings ConfigMap.
EverestSettingsConfigMapName = "everest-settings"
Expand Down
29 changes: 29 additions & 0 deletions pkg/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ func refreshEnforcerInBackground(
if err := validatePolicy(enforcer); err != nil {
panic("invalid policy detected - " + err.Error())
}
// Calling LoadPolicy() re-writes the entire model, so we need to add back the admin role.
if err := loadAdminPolicy(enforcer); err != nil {
panic("failed to load admin policy - " + err.Error())
}
enforcer.EnableEnforce(IsEnabled(cm))
})
if inf.Start(ctx, &corev1.ConfigMap{}) != nil {
Expand All @@ -116,6 +120,9 @@ func newEnforcer(adapter persist.Adapter, enableLogs bool) (*casbin.Enforcer, er
if err != nil {
return nil, err
}
if err := loadAdminPolicy(enf); err != nil {
return nil, errors.Join(err, errors.New("failed to load admin policy"))
}
if err := validatePolicy(enf); err != nil {
return nil, err
}
Expand Down Expand Up @@ -178,6 +185,28 @@ func GetUser(c echo.Context) (string, error) {
return subject, nil
}

func loadAdminPolicy(enf casbin.IEnforcer) error {
paths, _, err := buildPathResourceMap("") // reads the swagger API definition
if err != nil {
return err
}
resources := make(map[string]struct{})
for _, resource := range paths {
resources[resource] = struct{}{}
}
action := "*"
for resource := range resources {
object := "*/*"
if resource == ResourceNamespaces {
object = "*"
}
if _, err := enf.AddPolicy(common.EverestAdminRole, resource, action, object); err != nil {
return err
}
}
return nil
}

// buildPathResourceMap builds a map of paths to resources and a list of resources.
// Returns: (resourceMap, skipPaths, error) .
func buildPathResourceMap(basePath string) (map[string]string, []string, error) {
Expand Down
48 changes: 24 additions & 24 deletions pkg/rbac/testdata/policy-1-good.csv
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, admin:role, database-clusters, *, */*
p, admin:role, database-cluster-backups, *, */*
p, admin:role, database-cluster-restores, *, */*
p, admin:role, backup-storages, *, */*
p, admin:role, monitoring-instances, *, */*
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
p, role:admin, database-clusters, *, */*
p, role:admin, database-cluster-backups, *, */*
p, role:admin, database-cluster-restores, *, */*
p, role:admin, backup-storages, *, */*
p, role:admin, monitoring-instances, *, */*

p, readonly:role, namespaces, read, *
p, readonly:role, database-engines, read, */*
p, readonly:role, database-clusters, read, */*
p, readonly:role, database-cluster-backups, read, */*
p, readonly:role, database-cluster-restores, read, */*
p, readonly:role, backup-storages, read, */*
p, readonly:role, monitoring-instances, read, */*
p, role:readonly, namespaces, read, *
p, role:readonly, database-engines, read, */*
p, role:readonly, database-clusters, read, */*
p, role:readonly, database-cluster-backups, read, */*
p, role:readonly, database-cluster-restores, read, */*
p, role:readonly, backup-storages, read, */*
p, role:readonly, monitoring-instances, read, */*

p, devteam:role, namespaces, *, *
p, devteam:role, database-engines, *, dev/*
p, devteam:role, database-clusters, *, dev/*
p, devteam:role, database-cluster-backups, *, dev/*
p, devteam:role, database-cluster-restores, *, dev/*
p, devteam:role, backup-storages, *, */*
p, devteam:role, monitoring-instances, *, */*
p, role:devteam, namespaces, *, *
p, role:devteam, database-engines, *, dev/*
p, role:devteam, database-clusters, *, dev/*
p, role:devteam, database-cluster-backups, *, dev/*
p, role:devteam, database-cluster-restores, *, dev/*
p, role:devteam, backup-storages, *, */*
p, role:devteam, monitoring-instances, *, */*

p, alice, database-clusters, create, alice/*

g, admin, admin:role
g, alice, readonly:role
g, bob, devteam:role
g, admin, role:admin
g, alice, role:readonly
g, bob, role:devteam
4 changes: 2 additions & 2 deletions pkg/rbac/testdata/policy-2-bad.csv
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
this is a bad policy
xxxxxxxxxx
10 changes: 5 additions & 5 deletions pkg/rbac/testdata/policy-3-bad.csv
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, admin:role, monitoring-instances, *, */*
p, admin:role, monitoring instances, *, */*
g, admin, admin:role
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
p, role:admin, monitoring-instances, *, */*
p, role:admin, monitoring instances, *, */*
g, admin, role:admin

10 changes: 5 additions & 5 deletions pkg/rbac/testdata/policy-4-bad.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, admin:role, monitoring-instances, *, */*
p, notexists:role, monitoring-instances, *, */*
g, admin, admin:role
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
p, role:admin, monitoring-instances, *, */*
p, role:notexists, monitoring-instances, *, */*
g, admin, role:admin
8 changes: 4 additions & 4 deletions pkg/rbac/testdata/policy-5-bad.csv
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, admin:role, monitoring-instances, *, */*, asdfasdf
g, admin, admin:role
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
p, role:admin, monitoring-instances, *, */*, asdfasdf
g, admin, role:admin
6 changes: 3 additions & 3 deletions pkg/rbac/testdata/policy-6-bad.csv
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
p, alice, non-existent-resource, *, */*
g, admin, admin:role
g, admin, role:admin
6 changes: 3 additions & 3 deletions pkg/rbac/testdata/policy-7-bad.csv
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
p, admin:role, namespaces, read, *
p, admin:role, database-engines, *, */*
a, admin:role, database-clusters, *, */*
p, role:admin, namespaces, read, *
p, role:admin, database-engines, *, */*
a, role:admin, database-clusters, *, */*

7 changes: 6 additions & 1 deletion pkg/rbac/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/casbin/casbin/v2"
"go.uber.org/zap"

"github.com/percona/everest/pkg/common"
"github.com/percona/everest/pkg/kubernetes"
)

Expand Down Expand Up @@ -82,7 +83,11 @@ func checkResourceNames(policies [][]string) error {
func checkRoles(roles []string, policies [][]string) error {
for _, policy := range policies {
roleName := policy[0]
if !strings.HasSuffix(roleName, ":role") {
if !strings.HasPrefix(roleName, common.EverestRBACRolePrefix) {
continue
}
if roleName == common.EverestAdminRole {
// Its fine to not assign the admin role to any user.
continue
}
if !slices.Contains(roles, roleName) {
Expand Down
Loading
Loading