argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

proposal: Applications outside argocd namespace

Open jannfis opened this issue 3 years ago • 16 comments

This is a proposal to enable Application resources to exist outside the Argo CD control plane's namespace.

Should be considered an alternative to #6405

Related issues:

  • https://github.com/argoproj/argo-cd/issues/3474

Signed-off-by: jannfis [email protected]

Checklist:

  • [ ] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • [ ] The title of the PR states what changed and the related issues number (used for the release note).
  • [ ] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • [ ] Does this PR require documentation updates?
  • [ ] I've updated documentation as required by this PR.
  • [ ] Optional. My organization is added to USERS.md.
  • [ ] I have signed off all my commits as required by DCO
  • [ ] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • [ ] My build is green (troubleshooting builds).

jannfis avatar Jun 05 '21 13:06 jannfis

Codecov Report

Merging #6409 (a50f20d) into master (c0f16fb) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6409   +/-   ##
=======================================
  Coverage   45.98%   45.98%           
=======================================
  Files         226      226           
  Lines       27413    27413           
=======================================
  Hits        12605    12605           
  Misses      13094    13094           
  Partials     1714     1714           
Impacted Files Coverage Δ
util/settings/settings.go 51.42% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jun 05 '21 13:06 codecov[bot]

I've gone through the whole proposal and it looks like a fairly simple enhancement to the existing paradigm!

sbose78 avatar Jun 17 '21 16:06 sbose78

@jessesuen @alexmt Can we please revisit this topic? I think it'd be an important feature to have.

jannfis avatar Sep 02 '21 10:09 jannfis

FWIW I really look forward to seeing this ship.

tiagomeireles avatar Sep 15 '21 18:09 tiagomeireles

looking for this feature

it makes "apps of apps" such a mess if want to have tenant namespaces but having the application definition elsewhere from the actual deployments

bloodsper avatar Nov 03 '21 20:11 bloodsper

Thanks. This is indeed a great feature which we are looking for. Right now we are trying to do a similar thing with a ton of hacks:

We have written a k8s controller to watch for Application CRs in all namespaces except for argocd ns, and create a new corresponding app CR in argocd ns (using parts of uid for avoiding duplication). Also we have written a validation webhook to enforce and reject App CRs created in other ns, by checking and validating the .spec.project of apps based on a a label on namespaces which identifies the argo project, e.g. ns1 with the label argo.snappcloud.io/project: proj1 will cause creating any Application CR with .spec.project != proj1 in ns1 namespace to be rejected.

m-yosefpor avatar Nov 04 '21 23:11 m-yosefpor

Thanks @m-yosefpor for reviewing and picking up the discussion.

jannfis avatar Nov 12 '21 09:11 jannfis

LGTM

We've been going back and forth on this proposal for many months. Recently all maintainers agreed this should be implemented.

I think we should merge proposal,start implementing it and handle implementation details as we go.

@jessesuen , @jannfis , @wtam2018 let me know if you have any objections please.

I have commented some notes on this proposal. @jannfis has replied them with valid answers. However I think it would be great if those answers are somehow mentioned in the proposal as well.

Also I think the hyphen separator mentioned in https://github.com/argoproj/argo-cd/pull/6409#discussion_r743308908 should be deleted due to described duplication error.

m-yosefpor avatar Mar 10 '22 23:03 m-yosefpor

Will RBAC be addressed? When I create an ArgoCD Application via the UI, I am constrained by the RBAC rules effecting my user. If I create an ArgoCD application by creating a YAML for an Application object and applying, it will not know my ArgoCD user and I suspect will not enforce RBAC. My use case is that multiple teams share a cluster. I want teams to be able to use the App of Apps pattern, but I want them to be restricted into only creating applications within the projects their RBAC allows and only deploying to clusters which their RBAC allows.

mkaiserincomm avatar Apr 07 '22 16:04 mkaiserincomm

Hi all, Was just curious and wanted to know where are we on this?

Avni-Sharma avatar Jun 17 '22 19:06 Avni-Sharma

Hello, any news when this feature will be released?

Paulo-B avatar Jul 12 '22 15:07 Paulo-B

@Paulo-B it's expected for 2.5 in August. The PR is under review.

crenshaw-dev avatar Jul 13 '22 02:07 crenshaw-dev

@jannfis I very much agree with the concerns of @mkaiserincomm. There is a significant risk involved if people can create Applications linking to any project in their own namespace. This would grant them permissions by-proxy that they should not have according to the RBAC rules.

I think that some explicit configuration is required to allow this. One idea would be to only allow Applications in namespaces that are explicitly linked to Projects that explicitly contain configuration to read Application objects from that namespace.

For example:

apiVersion: argoproj.io/v1alpha1
kind: AppProject
metadata:
  name: cicd
  finalizers:
    - resources-finalizer.argocd.argoproj.io
spec:
  description: Project CICD owned by Team CICD

  destinations:
    - namespace: 'cicd'
      allowApplications: true
      server: 'https://kubernetes.default.svc'
    - namespace: 'cicd-*'
      allowApplications: false #default
      server: 'https://kubernetes.default.svc'

  sourceRepos:
    - '*'

This configuration would trigger ArgoCD to read any Application CRD's from the cicd namespace, and explicitly 'parent' them into the CICD project regardless of the project property in the Application Resource.

FireDrunk avatar Jul 28 '22 09:07 FireDrunk

@FireDrunk This problem is solved with the new sourceNamespaces property in the AppProject. This defines a list of namespaces where Applications are allowed to exist in order to associate to the given project.

For example, with an AppProject foobar that has

spec:
  sourceNamespaces:
  - ns-foo
  - ns-bar

Then an Application created in either namespace ns-foo or ns-bar may set their own .spec.project to foobar, thus associating to the project. If an Application is created in namespace ns-baz and has .spec.project set to foobar, the application controller will not reconcile this application.

jannfis avatar Jul 28 '22 10:07 jannfis

@FireDrunk This problem is solved with the new sourceNamespaces property in the AppProject. This defines a list of namespaces where Applications are allowed to exist in order to associate to the given project.

For example, with an AppProject foobar that has

spec:
  sourceNamespaces:
  - ns-foo
  - ns-bar

Then an Application created in either namespace ns-foo or ns-bar may set their own .spec.project to foobar, thus associating to the project. If an Application is created in namespace ns-baz and has .spec.project set to foobar, the application controller will not reconcile this application.

That looks like a perfect alternative! I would be happy to test this functionality, since we are anxiously awaiting these kinds of improvements!

Is there any kind of 'event' or notification stream or any kind of monitoring solution to make it easy to find these kinds of mishaps?

FireDrunk avatar Jul 28 '22 12:07 FireDrunk

(The PR for testing if you want @FireDrunk :-) https://github.com/argoproj/argo-cd/pull/9755)

crenshaw-dev avatar Jul 28 '22 13:07 crenshaw-dev

The PR #9755 implementing this proposal will be merged soon eventually.

jannfis avatar Aug 08 '22 16:08 jannfis