Made changes in the proposal to match the behaviour when no matching …

…sa is found

Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>

docs/operator-manual/app-sync-using-impersonation.md

@@ -10,9 +10,9 @@
 
 Argo CD supports syncing `Application` resources using the same service account used for its control plane operations. This feature enables users to decouple service account used for application sync from the service account used for control plane operations.
 
-By default, application syncs in Argo CD have the same privileges as the Argo CD control plane. As a consequence, in a multi-tenant setup, the Argo CD control plane privileges needs to match the tenant that needs the highest privileges. As an example, if an Argo CD instance has 10 Applications and only one of them requires admin privileges, then the Argo CD control plane must have admin privileges in order to be able to sync that one Application. Argo CD provides a multi-tenancy model to restrict what each Application can do using `AppProjects`, even though the control plane has higher privileges, however that creates a large attack surface since if Argo CD is compromised, attackers will gain cluster-admin access to the cluster.
+By default, application syncs in Argo CD have the same privileges as the Argo CD control plane. As a consequence, in a multi-tenant setup, the Argo CD control plane privileges needs to match the tenant that needs the highest privileges. As an example, if an Argo CD instance has 10 Applications and only one of them requires admin privileges, then the Argo CD control plane must have admin privileges in order to be able to sync that one Application. This provides an opportunity for malicious tenants to gain admin level access. Argo CD provides a multi-tenancy model to restrict what each `Application` is authorized to do using `AppProjects`, however it is not secure enough and if Argo CD is compromised, attackers will easily gain `cluster-admin` access to the cluster.
 
-Some manual steps will need to be performed by the Argo CD administrator in order to enable this feature. 
+Some manual steps will need to be performed by the Argo CD administrator in order to enable this feature, as it is disabled by default.
 
 !!! note
     This feature is considered alpha as of now. Some of the implementation details may change over the course of time until it is promoted to a stable status. We will be happy if early adopters use this feature and provide us with bug reports and feedback.
@@ -27,11 +27,11 @@ Impersonation requests first authenticate as the requesting user, then switch to
 
 In a multi-team/multi-tenant environment, a team/tenant is typically granted access to a target namespace to self-manage their kubernetes resources in a declarative way.
 A typical tenant onboarding process looks like below:
-1. The platform admin creates a tenant namespace and the service account to be used for creating the resources in that namespace is created.
+1. The platform admin creates a tenant namespace and the service account to be used for creating the resources is also created in the same tenant namespace.
 2. The platform admin creates one or more Role(s) to manage kubernetes resources in the tenant namespace
-3. The platform admin creates one or more RoleBinding(s) to map the service account to the role(s) created in previous steps.
-4. The platform admin configures ArgoCD to support apps-in-any-namespace feature, so that tenants can self-service their Argo applications in their respective tenant namespaces.
-5. If apps-in-any-namespace feature is not used, then the platform admin can provide access to tenants to manage ArgoCD Applications in the ArgoCD control plane namespace.
+3. The platform admin creates one or more RoleBinding(s) to map the service account to the role(s) created in the previous steps.
+4. The platform admin can choose to use either the [apps-in-any-namespace](./app-any-namespace.md) feature or provide access to tenants to create applications in the ArgoCD control plane namespace.
+5. If the platform admin chooses apps-in-any-namespace feature, tenants can self-service their Argo applications in their respective tenant namespaces and no additional access needs to be provided for the control plane namespace.
 
 ## Implementation details
 
@@ -65,6 +65,9 @@ data:
 !!! note
     This feature is disabled by default.
 
+!!! note
+    This feature can be enabled/disabled only at the system level and once enabled/disabled it is applicable to all Applications managed by ArgoCD.
+
 ## Configuring destination service accounts
 
 Destination service accounts can be added to the `AppProject` under `.spec.destinationServiceAccounts`. Specify the target destination `server` and `namespace` and provide the service account to be used for the sync operation using `defaultServiceAccount` field. Applications that refer this `AppProject` will use the corresponding service account configured for its destination.

docs/proposals/decouple-application-sync-user-using-impersonation.md

@@ -68,8 +68,9 @@ This proposal would allow ArgoCD administrators to manage the cluster permission
 
 ### Goals
 - Applications may only impersonate ServiceAccounts that live in the same namespace as the destination namespace configured in the application.If the service account is created in a different namespace, then the user can provide the service account name in the format `<namespace>:<service_account_name>` . ServiceAccount to be used for syncing each application is determined by the target destination configured in the `AppProject` associated with the `Application`.
-- If impersonation feature is enabled, and no service account name is provided in the associated `AppProject`, then the default service account of the destination namespace of the `Application` should be used.
+- If impersonation feature is enabled, and no service account name is provided in the associated `AppProject`, then the sync operation would fail with an appropriate error message. Users can configure a catch all service account matching all destinations to avoid such sync errors.
 - Access restrictions implemented through properties in AppProject (if done) must have the existing behavior. From a security standpoint, any restrictions that were available before switching to a service account based approach should continue to exist even when the impersonation feature is enabled.
+- The feature can be enabled/disabled only at the system level. Once enabled/disabled, it is applicable to all ArgoCD `Applications`.
 
 ### Non-Goals
 
@@ -81,7 +82,7 @@ As part of this proposal, it would be possible for an ArgoCD Admin to specify a
 
 When applications gets synced, based on its destination (target cluster and namespace combination), the `defaultServiceAccount` configured in the `AppProject` will be selected and used for impersonation when executing the kubectl commands for the sync operation.
 
-We would be introducing a new element `destinationServiceAccounts` in `AppProject.spec`. This element is used for the sole purpose of specifying the impersonation configuration. The `defaultServiceAccount` configured for the `AppProject` would be used for the sync operation for a particular destination cluster and namespace. If impersonation feature is enabled and no specific service account is provided in the `AppProject` CR, then the `default` service account in the destination namespace would be used for impersonation.
+We would be introducing a new element `destinationServiceAccounts` in `AppProject.spec`. This element is used for the sole purpose of specifying the impersonation configuration. The `defaultServiceAccount` configured for the `AppProject` would be used for the sync operation for a particular destination cluster and namespace. If impersonation feature is enabled and no specific service account is provided in the `AppProject` CR, then the sync operation will fail with an error. Users can configure a catch all service account matching all destinations to avoid such sync errors.
 
 ```yaml
 apiVersion: argoproj.io/v1alpha1
@@ -108,6 +109,9 @@ spec:
     - server: https://kubernetes.default.svc
       namespace: guestbook-stage
       defaultServiceAccount: guestbook-stage-deployer
+    - server: '*
+      namespace: '*'
+      defaultServiceAccount: default # catch all service account to be used when all other matches fail.
 ```
 
 ### Structure of DestinationServiceAccount:
@@ -510,6 +514,55 @@ If application resources have hardcoded namespaces in the git repository, would
 
 The service account to be used for impersonation is determined on a per Application level rather than on per resource level. The value specified in `Application.spec.destination.namespace` would be used to determine the service account to be used for the sync operation of all resources present in the `Application`.
 
+#### Application does not have a `spec.destination.namespace` field
+`spec.destination.namespace` is an optional field in an `Application`. If the user does not specify it, the application controller will use the service account in the Application's namespace for the sync operation. User's also have the option of specifying the service account along with its namespace, in which case the service account in the user specified namespace will be used for the sync operation.
+
+eg:
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: Application
+metadata:
+  name: guestbook
+  namespace: argocd
+spec:
+  project: my-project
+  source:
+    repoURL: https://github.com/argoproj/argocd-example-apps.git
+    targetRevision: HEAD
+    path: guestbook
+  destination:
+    server: https://kubernetes.default.svc
+---
+apiVersion: argoproj.io/v1alpha1
+kind: AppProject
+metadata:
+  name: my-project
+  namespace: argocd
+  finalizers:
+    - resources-finalizer.argocd.argoproj.io
+spec:
+  description: Example Project
+  # Allow manifests to deploy from any Git repos
+  sourceRepos:
+    - '*'
+  destinations:
+    - namespace: guestbook
+      server: https://kubernetes.default.svc
+    - namespace: guestbook-ui
+      server: https://kubernetes.default.svc
+  destinationServiceAccounts:
+    - namespace: guestbook
+      server: https://kubernetes.default.svc
+      defaultServiceAccount: guestbook-deployer
+    - namespace: guestbook-ui
+      server: https://kubernetes.default.svc
+      defaultServiceAccount: guestbook-ui-deployer
+```
+In the above example, since `spec.destination.namespace` is not specified, Application's namespace `argocd` is used for scoping the service account. So the service account `system:serviceaccount:argocd:guestbook-deployer` will be used for the sync operation.
+
+In the above example, If the matching service account is specified with a namespace, eg: `guestbook:guestbook-deployer`, then the service account `system:serviceaccount:guestbook:guestbook-deployer` will be used for the sync operation.
+
 ### Security Considerations
 
 * How does this proposal impact the security aspects of Argo CD workloads ?