argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.48k stars 5.32k forks source link

Destination permitted check does not check both server and namespace #19804

Open mmb opened 2 weeks ago

mmb commented 2 weeks ago

Checklist:

Describe the bug

Project destinations:

destinations:
  - namespace: test
     server: *
  - namespace: other
    server: !https://test-server

An app in this project trying to deploy to https://test-server in namespace test will be denied.

The logic in https://github.com/argoproj/argo-cd/blob/master/pkg/apis/application/v1alpha1/app_project_types.go#L474 denies if the server is in any deny destination regardless of the namespace.

To Reproduce

This is reproducible by adding a unit test to types_test.go.

Expected behavior

I would expect the denial to take the namespace into consideration. For me the "any allow permits and no deny rejects" semantics are hard to reason about. It seems like this check should be a simple "if any rule matches", although this would be a breaking change with security implications.

Screenshots

Version

v2.11.3+3f344d5

Logs

Paste any relevant application logs here.
blakepettersson commented 2 days ago

After giving this a bit of thought, this could be made more intuitive. I'll need to revisit the code and see if this can be enhanced without risking any breaking changes.