gitVersion results in inconsistent string values

[imjasonh]

Ref. ko-build/ko#315 (comment)

pkg/version/base.go includes:

// NOTE: The $Format strings are replaced during 'git archive' thanks to the
// companion .gitattributes file containing 'export-subst' in this same
// directory.  See also https://git-scm.com/docs/gitattributes
gitVersion   string = "v0.0.0-master+$Format:%h$"

This $Format:%h$ directive, activated by git archive and the .gitattributes containing the line base.go export-subst, results in file contents served by GitHub Archives that can change in unexpected ways. For example, the length of the commit SHA can expand over time as there are collisions.

This non-determinism can lead to surprises, such as ko-build/ko#315, where a client-go dependency resulted in unexpectedly different tar hashes, which was detected when a new version of the ko binary was being added to Homebrew.

tl;dr: though the client-go dependency itself didn't change for ko, later runs of git archive (via GitHub Archives) resulted in different contents for the gitVersion string to avoid a new collision for the previous prefix length, which cascaded into different content SHAs and shady-looking binary SHAs.

I'm not sure, but after reading docs I believe you could naively solve this by replacing $Format:%h$ with $Format:%H$ so the gitVersion string includes the full commit hash instead of the abbreviated one.

I have no idea if anything depends on gitVersion being a shorter string, but presumably things are fine with it having been a variable-length string so 🤷‍♂️ . Given enough time/collisions, I suppose theoretically %h will slow grow to be equal to %H, but I don't think that will happen during any of our lifetimes 👴

cc @Bo98

[k8s-ci-robot]

@imjasonh: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[liggitt]

Thanks for the report. Moving to kubernetes/kubernetes where this source comes from.

There's not a lot we can do for past releases, but I'd be in favor of switching to %H going forward. Reproducibility of source archives is important.

[neolit123]

/sig release

[neolit123]

/kind bug

[imjasonh]

Thanks for taking this on @liggitt !! 🎉

[jonyhy96]

@liggitt @neolit123 since we don't actually use git archive to full-fill the commit info as version, should we just delete that .gitattributes file?
containerd faced a issue about checksum mismatch containerd/containerd#6387 due to this.

[afbjorklund]

containerd faced a issue about checksum mismatch containerd/containerd#6387 due to this.

and before that it was cri-o/cri-o#4820 and before that containers/podman#9355, so everywhere

[nikhita]

should we just delete that .gitattributes file?

@jonyhy96 @afbjorklund I have created kubernetes/publishing-bot#285 to stop including the .gitattributes files in k8s libraries. For anyone vendoring these libraries in the future, you should no longer be affected by this issue.