Skip to content

Commit

Permalink
Add config setting to preserve V2 update/delete RBAC (argoproj#20600)
Browse files Browse the repository at this point in the history
A breaking change was introduced in a previous commit that is planned to
be a part of the next major version of Argo CD (v3) where it's okay to
introduce breaking changes. We want this feature before we hit v3, so
we add a config setting that allows us to explicitly turn this new v3
behavior on in v2. The current v2 behavior is the default, so this
change will not affect folks who do not explicitly opt in.

This commit to add the gating code is added separately so it will be
easy to either cherry pick that pervious commit or revert this one.

(see also argoproj#18124, argoproj#19988)

Signed-off-by: Matt Finkel <[email protected]>
  • Loading branch information
fffinkel committed Nov 5, 2024
1 parent 6ab13bd commit 62574fa
Show file tree
Hide file tree
Showing 4 changed files with 352 additions and 10 deletions.
35 changes: 29 additions & 6 deletions docs/operator-manual/rbac.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ The `applications` resource is an [Application-Specific Policy](#application-spe

#### Fine-grained Permissions for `update`/`delete` action

The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself but not its resources.
The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself **and** all of its resources.
It can be desirable to only allow `update` or `delete` on specific resources within an application.

To do so, when the action if performed on an application's resource, the `<action>` will have the `<action>/<group>/<kind>/<ns>/<name>` format.
Expand All @@ -138,12 +138,35 @@ p, example-user, applications, delete, default/prod-app, deny
p, example-user, applications, delete/*/Pod/*, default/prod-app, allow
```

If we want to explicitly allow updates to the application, but deny updates to any sub-resources:
!!! note

```csv
p, example-user, applications, update, default/prod-app, allow
p, example-user, applications, update/*, default/prod-app, deny
```
It is not possible to deny fine-grained permissions for a sub-resource if the action was **explicitly allowed on the application**.
For instance, the following policies will **allow** a user to delete the Pod and any other resources in the application:

```csv
p, example-user, applications, delete, default/prod-app, allow
p, example-user, applications, delete/*/Pod/*, default/prod-app, deny
```

!!! note

In v3, RBAC will have a breaking change. The `udpate` and `delete` actions
(without a `/*`) will no longer include sub-resources. This allows you to
explicitly allow or deny access to an application without affecting its
sub-resources. For example, you may want to allow enable/disable of auto-sync
by allowing update on an application, but disallow the editing of deployment
manifests for that application.

To enable this behavior before v3, you can set the config value
`server.rbac.enablev3` to `true` in the Argo CD ConfigMap argocd-cm.

Once you do so, you can explicitly allow updates to the application, but deny
updates to any sub-resources:

```csv
p, example-user, applications, update, default/prod-app, allow
p, example-user, applications, update/*, default/prod-app, deny
```

#### The `action` action

Expand Down
11 changes: 10 additions & 1 deletion server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -1333,10 +1333,19 @@ func (s *Server) getAppResources(ctx context.Context, a *appv1.Application) (*ap
}

func (s *Server) getAppLiveResource(ctx context.Context, action string, q *application.ApplicationResourceRequest) (*appv1.ResourceNode, *rest.Config, *appv1.Application, error) {
if action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate {
enableV3, err := s.settingsMgr.GetServerRBACEnableV3()
if err != nil {
return nil, nil, nil, errors.New("asdfasdfasdfasdf: " + err.Error())
}

if enableV3 && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) {
action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName())
}
a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName())
if !enableV3 && err != nil && errors.Is(err, permissionDeniedErr) && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) {
action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName())
a, _, err = s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName())
}
if err != nil {
return nil, nil, nil, err
}
Expand Down
Loading

0 comments on commit 62574fa

Please sign in to comment.