refactor(controller): overhaul git repo interfaces #2483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
✅ Deploy Preview for docs-kargo-akuity-io canceled.
|
krancour
changed the title
krancour
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2483 +/- ##
==========================================
+ Coverage 48.27% 48.79% +0.51%
==========================================
Files 246 250 +4
Lines 17739 17969 +230
==========================================
+ Hits 8564 8768 +204
+ Misses 8749 8721 -28
- Partials 426 480 +54 ☔ View full report in Codecov by Sentry. |
hiddeco
approved these changes
Sep 2, 2024
internal/controller/git/work_tree.go
Outdated
| } | ||
| // The root of the bare repo will be three levels up from the path returned | ||
| // by git rev-parse --git-dir | ||
| repoPath := filepath.Join(strings.TrimSpace(string(res)), "..", "..", "..", "repo") |
There was a problem hiding this comment.
Non-blocking: this feels prone to errors.
There was a problem hiding this comment.
I didn't love this either and I'd be happy to take any suggestion you might have.
There was a problem hiding this comment.
Refactored this in b4a47b0
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
hiddeco
approved these changes
Sep 3, 2024
krancour
added a commit
to krancour/kargo
that referenced
this pull request
Sep 3, 2024
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a bit of an overhaul of the
internal/controller/gitpackage that kills a few birds with one stone.Promotion directives will require support for bare repos with multiple working trees associated with them.
The capabilities of a normal repo with a single working tree vs a bare repo vs a working tree associated with a bare repo are all a little different. It made sense to expose three distinct interfaces -- Repo, BareRepo, and WorkTree. (The Repo interface is a superset of the WorkTree interface while the BareRepo interface lacks all the capabilities of a WorkTree, but has some distinct capabilities like AddWorkTree or RemoveWorkTree.)
Git-based promotion directives will rely heavily on these new interfaces. A clone directive may create a bare clone with one or more working trees.
Subsequent directives, such as commit and push will require the ability to interact with bare repos and working trees that already exist in the FS. Thus, obtaining interfaces for existing repos and working trees is another new capability introduced by this PR.
Directives will also introduce the possibility that, for instance, pushing a commit occurs long after cloning a repo, and in the intervening time, the credentials used to clone may have expired and will not be useful for pushing. This introduces a requirement that when obtaining interfaces for repos or working trees that already exist on the file system, the credentials must be refreshed.
This PR also takes #2400 into account. I anticipate the image-hardening effort will soon result in Kargo images that lack a shell. Kargo Render has already been through this in akuity/kargo-render#297. The store credential helper that we previously relied on requires a shell to function properly. So while already ensuring credentials are refreshed whenever an interface is obtained for an existing repo or working tree from the file system, I decided it was advisable to cut over to the custom credential helper strategy that Kargo Render also uses now.
A couple noteworthy aspects of this PR:
The existing Repo interface barely breaks, so the impact to existing code is minimal.
The implementation of existing functionality barely changes.
The PR adds new tests where none existed before to aid in asserting that nothing breaks.
I've also executed a battery of e2e tests against these changes and everything checks out.