Project deny destinations results in destinations being allowed when not matched

[agaudreault]

Describe the bug

When "deny" rules are used in AppProject, if a destination is NOT matched by a DENY rule, then it will result in that destination being ALLOWED. In general, a destination should

• if any deny rule matches, then the destination is denied
• if any allow rule matches, then the destination is allowed
• if no rule match, the destination is DENIED

However, there is currently no concept of "deny" rules. Only a concept of "negative matching". This negative matching has a significant different behavior than the repo URL negative matching "https://argo-cd.readthedocs.io/en/stable/user-guide/projects/#managing-projects" which will actually deny.

The documentation states that

As with sources, a destination is considered valid if the following conditions hold:

Any allow destination rule (i.e. a rule which isn't prefixed with !) permits the destination
AND no deny destination (i.e. a rule which is prefixed with !) rejects the destination

However, this is false.

To Reproduce

1. Create the following project

apiVersion: argoproj.io/v1alpha1
kind: AppProject
metadata:
  name: example
spec:
  destinations:
    # This is a deny rule because server contains "!"
    - namespace: *
      server: !https://kubernetes.default.svc*
    - namespace: guestbook-ui
      server: https://my-cluster

2. Create an Application with resources deployed in namespace test of cluster https://my-cluster
3. Observe that the Application have access to this destination because test is matched by * and https://my-cluster correctly does not match https://kubernetes.default.svc.

Expected behavior

The destination should only be allowed when a project destination is allowed. If a deny rule exist, it should only be evaluated for "access denial" and not for "grating access"

Proposal

1. Option 1: Add a new feature flag to change the behavior

   • A deny rule is defined whenever one or more element contains !
   • Breaking changes
   • Still confusing, but work as expected.

2. Option 2: Add a new parameter on the AppProject destination deny: true to specify when a rule is to deny access.

   • Should validate that ! is not used in deny rules to avoid confusion caused by the negation of negation
   • Backward compatible
   • Current "negative matching" still exist and significantly differ from sourceRepos matching

Problematic code

argo-cd/pkg/apis/application/v1alpha1/app_project_types.go

Lines 484 to 507 in 8841b0d

	func (proj AppProject) isDestinationMatched(dst ApplicationDestination) bool {
	anyDestinationMatched := false
	for _, item := range proj.Spec.Destinations {
	dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name, true)
	dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server, true)
	dstNamespaceMatched := globMatch(item.Namespace, dst.Namespace, true)
	matched := (dstServerMatched || dstNameMatched) && dstNamespaceMatched
	if matched {
	anyDestinationMatched = true
	} else if (!dstNameMatched && isDenyPattern(item.Name)) || (!dstServerMatched && isDenyPattern(item.Server)) && dstNamespaceMatched {
	return false
	} else if !dstNamespaceMatched && isDenyPattern(item.Namespace) && dstServerMatched {
	return false
	}
	}
	return anyDestinationMatched
	}
	func isDenyPattern(pattern string) bool {
	return strings.HasPrefix(pattern, "!")
	}