CommitStatus Global Resolution Re-Think?

[gnunn1]

CommitStatuses are globally resolved by the controller, this means it is possible to have a CommitStatus in one namespace be used for a PromotionStrategy in a different namespace. If a CommitStatus key and sha matches the list of keys in the PromotionStrategy as well as the commit sha it will be used. This could be useful for distributed teams, say different teams authoring microservices in different namespaces, to coordinate deployments.

Note though there is no authentication or opt-in to this process, as long as the CommitStatus matches the key and sha, neither of which is considered sensitive, it will get used. At first glance this doesn't appear to be a major security issue since the worse a bad actor could do is block a deployment.

However it's conceivable that someone could author a matching (on key and sha) imposter commit status and through a race condition trigger a promotion of a change. If the same bad actor had managed to get access to the git repository and author a commit on the DRY branch they could potentially leverage this to promote an attack across clusters.

While this can feel like a bit of a stretch, given the bad actor needs access to the cluster to make it work, it illustrates one possible problem and where one problem exists there could be others.

In general in Kubernetes when items need to be globally resolved they are cluster-scoped whereas CommitStatus is namespace scoped. While this behaviour is implied in the docs via a note (https://gitops-promoter.readthedocs.io/en/latest/gating-promotions/), it is neither intuitive nor obvious to users and I know for me it was a bit of a surprise.

In terms of solutions there are a couple of possible solutions that came out of some brief discussion @crenshaw-dev:

1. Stop resolving CommitStatus globally and only resolve them locally, this would require CommitStatus to always be in the same namespace as PromotionStrategy.
2. Same as 1 but if a globally required CommitStatus is required then add support for a ClusterCommitStatus that is cluster scoped. The challenge here is most regular tenants on a cluster don't have access to cluster scoped resources so these cluster scoped CommitStatus resources would likely require the Platform team to manage them rather then tenants. Additionally the user created CommitStatuses (ArgoCDCommitStatus, WebRequestCommitStatus, etc) would need to have cluster scoped equivalents as well.
3. Add an opt-in mechanism to PromotionStrategy, similar to Argo CD Apps-In-Any-Namespaces, where the PromotionStrategy specifies the namespaces from which CommitStatuses can be contributed.

[gnunn1]

BTW the note I'm referring to in the docs where the global scoping is more implied then explicitly stated is this one:

Multiple controller installs: When more than one Promoter controller runs
in the same cluster, label gate CRs, PromotionStrategy resources, and 
user-created roots (GitRepository, ScmProvider, ClusterScmProvider) with 
promoter.argoproj.io/instance-id so each install only reconciles its own gates
and CommitStatuses. 
See [Multiple Controller Installs](https://gitops-promoter.readthedocs.io/en/latest/multi-install/).

I haven't seen it mentioned anywhere else though I could be missing something.

[zachaller]

I think I can get behind not globally finding all CommitStatues and by default namespace scope them, but I think there are probably enough use cases for the global lookups that I think a 4th option of creating a namespace label something like promoter.argoproj.io/commit-status-producer: true sites well. Namespaces are generally admin controlled and this pattern is also common with a few other built in k8s resources like pod-security enforce labels.

• A CommitStatus in the same namespace as the PromotionStrategy always counts — no label needed.

• A CommitStatus in any other namespace counts only if that namespace carries the label.

[gnunn1]

While I am OK with the using a namespace label IMHO the value of the label needs to have granularity and not be global, i.e CommitStatuses in Namespace A can be used in Namespace B but not Namespace C.

[zachaller]

A key use case for a globally admin-controlled CommitStatus is a team managing a deployment freeze window who wants to block all deployments at once. The simplest approach: one controller that watches every CTP and/or PS, pulls the proposed hashes, and reports a block.

We're already living the alternative at Intuit. Our HCAP windows get checked from every single PS via a WRCS, which doesn't scale — it piles load onto the APIs we're hitting just to ask "are we in HCAP?" The single-controller approach above is the simpler design.

With the namespace label it restricts who can report report status to k8s admins only because generally namespace create/updates are not end user controlled and are more a platform level permission.

I think going down the fine grained access is something I don't want to take on today, without more feedback and actual use cases that show a restriction is needed. I think today our separation of two users, end users and admins, buys us a lot without a lot of complexity.