fix: preserve the source Git index during version detection

[tsnaik]

What type of PR is this?
bug

Which issue does this PR fix?
No issue filed. Repro steps are below.

What does this PR do / Why do we need it?
During rootless builds, GitVersion runs git status inside a user namespace. git status refreshes the index as an optional side effect, so it writes the namespace UID/GID (0:0) into the source repo's .git/index. After that, host Git sees a stat mismatch on every tracked file and rereads them, which is slow on large repos. File contents and ownership don't change.

This passes --no-optional-locks to git status so it no longer writes the index. -dirty detection works the same as before.

If an issue # is not available please add repro steps and logs showing the issue:
Run as a non-root user:

cd "$(mktemp -d)" && git init -q
echo hello > payload
cat > stacker.yaml <<'YAML'
repro:
  from: {type: scratch}
  imports:
    - {path: payload, dest: /payload}
YAML
git add . && git commit -qm init
git ls-files --debug payload | grep uid   # host uid/gid
stacker build                             # as non-root
git ls-files --debug payload | grep uid   # now 0/0

Testing done on this change:
Ran the repro above with both binaries:

Binary      uid/gid before build    uid/gid after build
Unpatched   1003/1002               0/0
Patched     1003/1002               1003/1002

Same-revision rootless builds, clean and dirty:

Binary           Source index    Host rereads payload after build
Unpatched        Rewritten       Yes (on every status)
Patched, clean   Byte-identical  No
Patched, dirty   Byte-identical  No

OCI Git annotations were correct for both clean and dirty builds.

Automation added to e2e:

• test/git-index.bats (unprivileged): creates a Git repo and checks that the build user owns the committed file, then runs a rootless stacker build -f, and checks two things: the file's cached uid/gid in the index still matches the file's real owner, and the hash of .git/index is unchanged.
• Unit tests: TestGitVersionPreservesIndex, plus a control test, TestGitStatusRefreshesStaleIndex, which shows that plain git status does rewrite a stale index.

Without the fix:

running tests in modes: unpriv
1..1
build user: tanaik uid=1003, payload owner: 1003:1002
index owner before: 1003:1002 after: 0:0
not ok 1 rootless build does not rewrite the source git index in 402ms
# (in test file test/git-index.bats, line 49)
#   `[ "$owner_after" = "$owner" ]' failed
# usernsexec-ing [u 0 1003 1 1 265538 365535 g 0 1002 1 1 265538 365535 -- .../stacker --internal-userns --debug build -f .../source-git/stacker.yaml]

With the fix:

running tests in modes: unpriv
1..1
build user: tanaik uid=1003, payload owner: 1003:1002
index owner before: 1003:1002 after: 1003:1002
ok 1 rootless build does not rewrite the source git index in 386ms

Will this break upgrades or downgrades?
No. No image format, config, or CLI changes.

Does this PR introduce any user-facing change?
Yes, a bug fix.

Rootless builds no longer rewrite the source repo's Git index, which caused host Git to reread unchanged files.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.97%. Comparing base (bcbe638) to head (62a0ff9).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #805      +/-   ##
==========================================
+ Coverage   54.87%   54.97%   +0.10%     
==========================================
  Files          55       55              
  Lines        5910     5910              
==========================================
+ Hits         3243     3249       +6     
+ Misses       2094     2088       -6     
  Partials      573      573              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[raharper]

Thanks for finding this. Can we add a bats test for this one; it certainly seems like we can catch it rewriting the index today with a test case; and then with your change confirm that we no longer update the index.

[raharper]

Thanks for updating the go test and adding the bats test. couple more things in the bats test to confirm.

[raharper]

This might need to use it's own tmpdir directory so it doesn't end up getting interaction with the stacker git dir in which the test is running. see @test "git version annotation matches stackerfile repository" in basic.bats for setting up a TMPDIR separate git repo.

[raharper]

I'm not sure if on the github runner the user may be "ubuntu" or it might be "root" -- we should verify what the UID/GID of the user is first.

Then where ever we create the git repo for this test, confirm that it's owned by the correct uid/gid/user before running stacker build.

Then, after running stacker build we should print what the uid/gid/user value is, and then expect that it match the user.

If you can; disable the fix and confirm the test catches the failure. Posting a log of that failure as a comment I think is sufficient.

[tsnaik]

Fair enough. On local machine I did run the test without the fix and it failed as expected. I missed updating the PR description with that snippet then, updated now.

And yeah, for github CI it makes sense to check the user before the test and verify it after.