Support ${COMMIT_SHA} (et al)

[mattmoor]

In GCB, $GIT_COMMIT is a fairly widely used substitution, e.g. it can be used to give Docker images unique tags (with a hint of provenance).

I'd posit that this class of substitution is most useful in the context of a trigger (specifically a Git trigger), which is something we haven't meaningfully discussed and may want to keep out of scope for now. I'm illustrating it here more to show my thinking than anything else:

apiVersion: cloudbuild.dev/v1alpha1
kind: GithubTrigger
spec:
  repo: # overload of GitSourceSpec
    url: https://github.com/foo/bar
    branch: <regexp, e.g. master>
    tag: <regexp, e.g. v[0-9.]+>
    refs: <regexp, e.g. refs/pulls/.*>

  # TODO(mattmoor): Additional conditions, e.g. PR from admin, comment from admin on PR.

  template:
    name: dockerfile-build-push
    namespace: cloud-builders
    arguments:
    - name: IMAGE
       value: gcr.io/foo/bar:${GIT_COMMIT}

/cc @ImJasonH

[imjasonh]

I don't want to dive too deeply into triggers too far just yet. I think we can say that if a build has a gitSource, it has the git-related parameters added. Git triggers happen to specify a gitSource when they fire and instantiate builds.

A template that specifies gitSource can override a git-related parameter with a default if it wants. A non-templated build that specifies gitSource can override a git-related parameter value by specifying an argument with a value.

WDYT?

[mattmoor]

Right, a complication is accessing this information at a point in the process that we can perform these substitutions reasonably. This was my poor attempt at continuing to skirt this issue by trying to make it a part of another resource that might have an outside signal that can drive these values.

[imjasonh]

In GCB we look up the commit referenced by the branch/tag/ref before accepting the build, which lets us validate that the branch/whatever exists and is accessible to the builder service account. While we're at it, we apply substitutions.

I'm not sure how best to support this in CRD-land, especially if the goal is totally service-agnostic Git support. Briefly run a container with the requisite secrets and have it report existence/resolution of and access to the ref? That feels weird, but it should work. Is it weird to run a container during operation of an admission controller webhook?

[mattmoor]

Running the container would be awkward. What I'd been thinking was that we could do this in a library in the controller for built-in types. This might be worth a quick GVC?

[mattmoor]

@ImJasonH do you want to capture our discussion here?

[imjasonh]

Sure. If a build specifies gitSource with branch/tag/ref, the controller will try to reach out to figure out what commit is currently referenced by that branch/tag/ref, and will populate that in the gitSource.commitSha, and inject a parameter for ${COMMIT_SHA} so it can be replaced in builds. We'll also populate ${BRANCH_NAME} and ${TAG_NAME} as appropriate.

This will also act as validation that the branch/tag/ref exists in the repo. We could also do that check if the user specifies an exact commit SHA.

If the source requires a secret, the controller will ask the API server for that secret and use it to authorize with the remote repo.

Did I miss anything?

[mattmoor]

Not that we discussed, but for completeness (since the CRD also supports refs) we should include ${REF_NAME}.

[imjasonh]

SGTM.

If a build specifies ref: "refs/heads/master" should we be smart and realize they mean branch: "master", and populate ${BRANCH_NAME}?

Should we populate ${REF_NAME} if the user specifies a branch or tag, with "refs/heads/foo" or "refs/tags/bar"?

[mattmoor]

I'd prefer to be dumb and consistent. If you want ${BRANCH_NAME} use branch: . Don't feel super strongly. If we start being smart, then maybe we always set ${REF_NAME} (except commit:).

[imjasonh]

We can get information about refs without invoking the git CLI, using a package like go-git, for instance:

func main() {
        repo, _ := git.Init(memory.NewStorage(), nil)
        remo, _ := repo.CreateRemote(&config.RemoteConfig{
                Name: "origin",
                URLs: []string{"ssh://[email protected]:user/repo.git"},
                // or: URLs: []string{ "https://github.com/user/repo"},
        })
        refs, _ := remo.List(&git.ListOptions{ /* Auth goes here */})
        for _, ref := range refs {
                log.Println(ref.Name(), ref.Hash().String())
        }
}

[knative-housekeeping-robot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale