source-controller: Source controller fails to clone when specifying just the revision (and default branch `main` at origin)

I think we need to come to a decision on how the API should react here. I agree with @stealthybox where specifying both a branch and a commit is redundant as a commit should exist outside of a branch construct.

From an ease-of-use perspective, the API is much more readable and easier understood if it’s an order of precedence thing,

Branch < Tag < Semver < Commit

Should we even support restricting the commit to the specified branch? I personally would have never invented or expected that behavior with my mental model and usage of git. I would always expect a detached HEAD.

I view the commit as a complete override over all other values because it’s the most specific. The only case where I see it adds value/safety is if the platform-admin deploys an OPA policy, mutating webhook, or similar to restrict the object to have a specific branch name that cannot be changed by the owner of the object.

If we continue supporting this behavior, we should really make sure that the error message is way more clear, because reporting that the object isn’t found is super misleading:

results in: git commit '16e375850ade4e5c77cc5ad37d0b643ff9aca44c' not found: object not found

The error should specify that it’s not part of the particular branch and also print the branch name.

It might be difficult to differentiate the default branch value from one that’s explicitly set if we’re trying to still support this alongside “Any Commit from any branch”.

@jonathan-innis The user must specify one of either branch, tag, semver, or commit according to gitrepo spec for type GitRepositoryRef. If the content of this struct is omitted, the struct is filled out automatically with a branch ref to master, that is the current behavior and I don’t think it is changing.

If you specify a commit hash, Flux checks out that commit hash only if it is on the branch specified. This is also currently the behavior implemented, not changing. If the user specified only a commit hash and no branch ref, Flux will no longer fill the branch ref with a default value of master.

apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: GitRepository
metadata:
  name: foo
  namespace: flux-system
spec:
  gitImplementation: go-git
  interval: 1m0s
  ref:
    branch: ''
    commit: 1b0d21cc635e28d51992d53e1ecc23a3f6cf4ac5
  timeout: 20s
  url: https://github.com/kingdonb/hephynator

If I create this source now, in 0.13.4, Flux fills out the branch ref as master anyway. I have to know what branch it’s on in order to specify the git SHA in commit, (I explicitly didn’t ask for the master branch, but Flux didn’t allow me to do that.)

Yes this is a slight breaking change but I think the number of users who are specifying a commit hash in their source and want it to be rejected if the commit hash wasn’t on the (implicit default) master branch, or reject the commit if the master branch didn’t exist (but I also don’t want to speak its name in the source YAML) … must be miniscule. If this is your use case, you should be explicit and request a commit hash from the branch you want.

This is surprising behavior and we should axe it, even if it is technically a breaking change IMHO.

@squaremo my point is that if both the branch and the commit are specified, we should set the revision as <branch>/<commit>, if the commit doesn’t exists in the specified branch we should error out. I’m ok with setting the revision to <commit> only if the branch is not present in the definition.