Skip to content

Commit

Permalink
feat: add permitOnlyProjectScopedClusters flag
Browse files Browse the repository at this point in the history
This commit adds a new flag, `permitOnlyProjectScopedClusters`, which
prevents any application from syncing to clusters which are not a part
of the same project. Fixes #10220.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
  • Loading branch information
blakepettersson committed Aug 19, 2022
1 parent 2fb2c23 commit 35174e4
Show file tree
Hide file tree
Showing 18 changed files with 735 additions and 460 deletions.
4 changes: 4 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -4925,6 +4925,10 @@
"orphanedResources": {
"$ref": "#/definitions/v1alpha1OrphanedResourcesMonitorSettings"
},
"permitOnlyProjectScopedClusters": {
"type": "boolean",
"title": "PermitOnlyProjectScopedClusters determines whether destinations can only reference clusters which are project-scoped"
},
"roles": {
"type": "array",
"title": "Roles are user defined RBAC roles associated with this project",
Expand Down
38 changes: 29 additions & 9 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,10 @@ func (ctrl *ApplicationController) getResourceTree(a *appv1.Application, managed
})
} else {
err := ctrl.stateCache.IterateHierarchy(a.Spec.Destination.Server, kube.GetResourceKey(live), func(child appv1.ResourceNode, appName string) bool {
if !proj.IsResourcePermitted(schema.GroupKind{Group: child.ResourceRef.Group, Kind: child.ResourceRef.Kind}, child.Namespace, a.Spec.Destination) {
permitted, _ := proj.IsResourcePermitted(schema.GroupKind{Group: child.ResourceRef.Group, Kind: child.ResourceRef.Kind}, child.Namespace, a.Spec.Destination, func(project string) ([]*appv1.Cluster, error) {
return ctrl.db.GetProjectClusters(context.TODO(), project)
})
if !permitted {
return false
}
nodes = append(nodes, child)
Expand All @@ -478,7 +481,16 @@ func (ctrl *ApplicationController) getResourceTree(a *appv1.Application, managed
belongToAnotherApp = true
}
}
if belongToAnotherApp || !proj.IsResourcePermitted(schema.GroupKind{Group: child.ResourceRef.Group, Kind: child.ResourceRef.Kind}, child.Namespace, a.Spec.Destination) {

if belongToAnotherApp {
return false
}

permitted, _ := proj.IsResourcePermitted(schema.GroupKind{Group: child.ResourceRef.Group, Kind: child.ResourceRef.Kind}, child.Namespace, a.Spec.Destination, func(project string) ([]*appv1.Cluster, error) {
return ctrl.db.GetProjectClusters(context.TODO(), project)
})

if !permitted {
return false
}
orphanedNodes = append(orphanedNodes, child)
Expand Down Expand Up @@ -798,7 +810,7 @@ func (ctrl *ApplicationController) processAppOperationQueueItem() (processNext b
app := origApp.DeepCopy()

if app.Operation != nil {
// If we get here, we are about process an operation but we cannot rely on informer since it might has stale data.
// If we get here, we are about to process an operation, but we cannot rely on informer since it might have stale data.
// So always retrieve the latest version to ensure it is not stale to avoid unnecessary syncing.
// We cannot rely on informer since applications might be updated by both application controller and api server.
freshApp, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(app.ObjectMeta.Namespace).Get(context.Background(), app.ObjectMeta.Name, metav1.GetOptions{})
Expand All @@ -812,7 +824,9 @@ func (ctrl *ApplicationController) processAppOperationQueueItem() (processNext b
if app.Operation != nil {
ctrl.processRequestedAppOperation(app)
} else if app.DeletionTimestamp != nil && app.CascadedDeletion() {
_, err = ctrl.finalizeApplicationDeletion(app)
_, err = ctrl.finalizeApplicationDeletion(app, func(project string) ([]*appv1.Cluster, error) {
return ctrl.db.GetProjectClusters(context.Background(), project)
})
if err != nil {
ctrl.setAppCondition(app, appv1.ApplicationCondition{
Type: appv1.ApplicationConditionDeletionError,
Expand Down Expand Up @@ -926,21 +940,27 @@ func (ctrl *ApplicationController) shouldBeDeleted(app *appv1.Application, obj *
return !kube.IsCRD(obj) && !isSelfReferencedApp(app, kube.GetObjectRef(obj))
}

func (ctrl *ApplicationController) getPermittedAppLiveObjects(app *appv1.Application, proj *appv1.AppProject) (map[kube.ResourceKey]*unstructured.Unstructured, error) {
func (ctrl *ApplicationController) getPermittedAppLiveObjects(app *appv1.Application, proj *appv1.AppProject, projectClusters func(project string) ([]*appv1.Cluster, error)) (map[kube.ResourceKey]*unstructured.Unstructured, error) {
objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{})
if err != nil {
return nil, err
}
// Don't delete live resources which are not permitted in the app project
for k, v := range objsMap {
if !proj.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name) {
permitted, err := proj.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name, projectClusters)

if err != nil {
return nil, err
}

if !permitted {
delete(objsMap, k)
}
}
return objsMap, nil
}

func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Application) ([]*unstructured.Unstructured, error) {
func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Application, projectClusters func(project string) ([]*appv1.Cluster, error)) ([]*unstructured.Unstructured, error) {
logCtx := log.WithField("application", app.QualifiedName())
logCtx.Infof("Deleting resources")
// Get refreshed application info, since informer app copy might be stale
Expand Down Expand Up @@ -981,7 +1001,7 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
if validDestination {
// ApplicationDestination points to a valid cluster, so we may clean up the live objects

objsMap, err := ctrl.getPermittedAppLiveObjects(app, proj)
objsMap, err := ctrl.getPermittedAppLiveObjects(app, proj, projectClusters)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1016,7 +1036,7 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
return objs, err
}

objsMap, err = ctrl.getPermittedAppLiveObjects(app, proj)
objsMap, err = ctrl.getPermittedAppLiveObjects(app, proj, projectClusters)
if err != nil {
return nil, err
}
Expand Down
16 changes: 12 additions & 4 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,9 @@ func TestFinalizeAppDeletion(t *testing.T) {
patched = true
return true, nil, nil
})
_, err := ctrl.finalizeApplicationDeletion(app)
_, err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*argoappv1.Cluster, error) {
return []*argoappv1.Cluster{}, nil
})
assert.NoError(t, err)
assert.True(t, patched)
})
Expand Down Expand Up @@ -602,7 +604,9 @@ func TestFinalizeAppDeletion(t *testing.T) {
patched = true
return true, nil, nil
})
objs, err := ctrl.finalizeApplicationDeletion(app)
objs, err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*argoappv1.Cluster, error) {
return []*argoappv1.Cluster{}, nil
})
assert.NoError(t, err)
assert.True(t, patched)
objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{})
Expand Down Expand Up @@ -634,7 +638,9 @@ func TestFinalizeAppDeletion(t *testing.T) {
patched = true
return true, nil, nil
})
_, err := ctrl.finalizeApplicationDeletion(app)
_, err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*argoappv1.Cluster, error) {
return []*argoappv1.Cluster{}, nil
})
assert.NoError(t, err)
assert.True(t, patched)
})
Expand All @@ -657,7 +663,9 @@ func TestFinalizeAppDeletion(t *testing.T) {
fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
return defaultReactor.React(action)
})
_, err := ctrl.finalizeApplicationDeletion(app)
_, err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*argoappv1.Cluster, error) {
return []*argoappv1.Cluster{}, nil
})
assert.NoError(t, err)
}

Expand Down
12 changes: 11 additions & 1 deletion controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,17 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap

// filter out all resources which are not permitted in the application project
for k, v := range liveObjByKey {
if !project.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name) {
permitted, err := project.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name, func(project string) ([]*appv1.Cluster, error) {
return m.db.GetProjectClusters(context.TODO(), project)
})

if err != nil {
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
failedToLoadObjs = true
continue
}

if !permitted {
delete(liveObjByKey, k)
}
}
Expand Down
14 changes: 12 additions & 2 deletions controller/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,18 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
if !proj.IsGroupKindPermitted(un.GroupVersionKind().GroupKind(), res.Namespaced) {
return fmt.Errorf("resource %s:%s is not permitted in project %s", un.GroupVersionKind().Group, un.GroupVersionKind().Kind, proj.Name)
}
if res.Namespaced && !proj.IsDestinationPermitted(v1alpha1.ApplicationDestination{Namespace: un.GetNamespace(), Server: app.Spec.Destination.Server, Name: app.Spec.Destination.Name}) {
return fmt.Errorf("namespace %v is not permitted in project '%s'", un.GetNamespace(), proj.Name)
if res.Namespaced {
permitted, err := proj.IsDestinationPermitted(v1alpha1.ApplicationDestination{Namespace: un.GetNamespace(), Server: app.Spec.Destination.Server, Name: app.Spec.Destination.Name}, func(project string) ([]*v1alpha1.Cluster, error) {
return m.db.GetProjectClusters(context.TODO(), project)
})

if err != nil {
return err
}

if !permitted {
return fmt.Errorf("namespace %v is not permitted in project '%s'", un.GetNamespace(), proj.Name)
}
}
return nil
}),
Expand Down
27 changes: 27 additions & 0 deletions docs/user-guide/projects.md
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,30 @@ stringData:
```

All the examples above talk about Git repositories, but the same principles apply to clusters as well.

With cluster-scoped clusters we can also restrict projects to only allow applications whose destinations belong to the
same project. The default behavior allows for applications to be installed onto clusters which are not a part of the same
project, as the example below demonstrates:

```yaml
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
name: "some-ns"
spec:
destination:
# This destination might not actually be a cluster which belongs to `foo-project`
server: https://some-k8s-server/
namespace: "some-ns"
project: foo-project
```

To prevent this behavior, we can set the attribute `permitOnlyProjectScopedClusters` on a project.

```yaml
spec:
permitOnlyProjectScopedClusters: true
```

With this set, the application above would no longer be allowed to be synced to any cluster other than the ones which
are a part of the same project.
4 changes: 4 additions & 0 deletions manifests/core-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9063,6 +9063,10 @@ spec:
for apps which have orphaned resources
type: boolean
type: object
permitOnlyProjectScopedClusters:
description: PermitOnlyProjectScopedClusters determines whether destinations
can only reference clusters which are project-scoped
type: boolean
roles:
description: Roles are user defined RBAC roles associated with this
project
Expand Down
4 changes: 4 additions & 0 deletions manifests/crds/appproject-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ spec:
for apps which have orphaned resources
type: boolean
type: object
permitOnlyProjectScopedClusters:
description: PermitOnlyProjectScopedClusters determines whether destinations
can only reference clusters which are project-scoped
type: boolean
roles:
description: Roles are user defined RBAC roles associated with this
project
Expand Down
4 changes: 4 additions & 0 deletions manifests/ha/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9063,6 +9063,10 @@ spec:
for apps which have orphaned resources
type: boolean
type: object
permitOnlyProjectScopedClusters:
description: PermitOnlyProjectScopedClusters determines whether destinations
can only reference clusters which are project-scoped
type: boolean
roles:
description: Roles are user defined RBAC roles associated with this
project
Expand Down
4 changes: 4 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9063,6 +9063,10 @@ spec:
for apps which have orphaned resources
type: boolean
type: object
permitOnlyProjectScopedClusters:
description: PermitOnlyProjectScopedClusters determines whether destinations
can only reference clusters which are project-scoped
type: boolean
roles:
description: Roles are user defined RBAC roles associated with this
project
Expand Down
34 changes: 27 additions & 7 deletions pkg/apis/application/v1alpha1/app_project_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,18 +336,18 @@ func (proj AppProject) IsGroupKindPermitted(gk schema.GroupKind, namespaced bool
}

// IsLiveResourcePermitted returns whether a live resource found in the cluster is permitted by an AppProject
func (proj AppProject) IsLiveResourcePermitted(un *unstructured.Unstructured, server string, name string) bool {
return proj.IsResourcePermitted(un.GroupVersionKind().GroupKind(), un.GetNamespace(), ApplicationDestination{Server: server, Name: name})
func (proj AppProject) IsLiveResourcePermitted(un *unstructured.Unstructured, server string, name string, projectClusters func(project string) ([]*Cluster, error)) (bool, error) {
return proj.IsResourcePermitted(un.GroupVersionKind().GroupKind(), un.GetNamespace(), ApplicationDestination{Server: server, Name: name}, projectClusters)
}

func (proj AppProject) IsResourcePermitted(groupKind schema.GroupKind, namespace string, dest ApplicationDestination) bool {
func (proj AppProject) IsResourcePermitted(groupKind schema.GroupKind, namespace string, dest ApplicationDestination, projectClusters func(project string) ([]*Cluster, error)) (bool, error) {
if !proj.IsGroupKindPermitted(groupKind, namespace != "") {
return false
return false, nil
}
if namespace != "" {
return proj.IsDestinationPermitted(ApplicationDestination{Server: dest.Server, Name: dest.Name, Namespace: namespace})
return proj.IsDestinationPermitted(ApplicationDestination{Server: dest.Server, Name: dest.Name, Namespace: namespace}, projectClusters)
}
return true
return true, nil
}

// HasFinalizer returns true if a resource finalizer is set on an AppProject
Expand Down Expand Up @@ -384,7 +384,27 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
}

// IsDestinationPermitted validates if the provided application's destination is one of the allowed destinations for the project
func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination) bool {
func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination, projectClusters func(project string) ([]*Cluster, error)) (bool, error) {
destinationMatched := proj.isDestinationMatched(dst)
if destinationMatched && proj.Spec.PermitOnlyProjectScopedClusters {
clusters, err := projectClusters(proj.Name)
if err != nil {
return false, fmt.Errorf("could not retrieve project clusters: %s", err)
}

for _, cluster := range clusters {
if cluster.Name == dst.Name || cluster.Server == dst.Server {
return true, nil
}
}

return false, nil
}

return destinationMatched, nil
}

func (proj AppProject) isDestinationMatched(dst ApplicationDestination) bool {
anyDestinationMatched := false
noDenyDestinationsMatched := true

Expand Down

0 comments on commit 35174e4

Please sign in to comment.