Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use consistent length for project build ID #456

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

ginglis13
Copy link
Contributor

@ginglis13 ginglis13 commented Feb 11, 2025

Description of changes:

git describe returns a SHA1 of variable length depending on the number
of objects in a git repository
https://git-scm.com/book/en/v2/Git-Tools-Revision-Selection#Short-SHA-1

For example, at time of writing bottlerocket-kernel-kit returns a SHA1
with a length of 7, whereas the bottlerocket-core-kit returns a SHA1 of
length 8. This is due to the bottlerocket-kernel-kit having a much
smaller git history than the core-kit (or bottlerocket-os/bottlerocket)

the build ID is used in application inventory generation. Application
inventory is used by various software to evaluate vulnerability
applicability along with Bottlerocket's updateinfo.xml. The build ID
length being non-deterministic resulted in inconsistencies for
comparisons between the updateinfo.xml generated by Bottlerocket and the
application inventory embedded into Bottlerocket AMIs.

This commit sets a consistent length of 8 for commits returned by git
describe and used as project build ID. This is done for
historical purposes given advisories published to
https://advisories.bottlerocket.aws/updateinfo.xml.gz have been
referenced with their build ID.

Testing done:

cargo test

Run git describe --always --dirty --exclude '*' --abbrev=8 in various git repos

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@ginglis13 ginglis13 requested a review from yeazelm February 11, 2025 21:06
Copy link
Contributor

@yeazelm yeazelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

@koooosh koooosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The --abbrev=8 is clear in the code, but should we update either the commit message or description stating that we've chosen the consistent length to be 8 chars for historical purposes? The current description is unclear what we've chosen, at least to me.

git describe returns a SHA1 of variable length depending on the number
of objects in a git repository
https://git-scm.com/book/en/v2/Git-Tools-Revision-Selection#Short-SHA-1

For example, at time of writing bottlerocket-kernel-kit returns a SHA1
with a length of 7, whereas the bottlerocket-core-kit returns a SHA1 of
length 8. This is due to the bottlerocket-kernel-kit having a much
smaller git history than the core-kit (or bottlerocket-os/bottlerocket)

the build ID is used in application inventory generation. Application
inventory is used by various software to evaluate vulnerability
applicability along with Bottlerocket's updateinfo.xml. The build ID
length being non-deterministic resulted in inconsistencies for
comparisons between the updateinfo.xml generated by Bottlerocket and the
application inventory embedded into Bottlerocket AMIs.

This commit sets a consistent length of 8 for commits returned by git
describe and used as project build ID. This is done for
historical purposes given advisories published to
https://advisories.bottlerocket.aws/updateinfo.xml.gz have been
referenced with their build ID.

Signed-off-by: Gavin Inglis <[email protected]>
@ginglis13 ginglis13 merged commit 3e2d029 into bottlerocket-os:develop Feb 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants