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

Return commit for revision instead of HEAD #2410

Merged
merged 3 commits into from
May 22, 2024

Conversation

0xavi0
Copy link
Contributor

@0xavi0 0xavi0 commented May 6, 2024

Changing the commit retrieved when applying gitrepos with revisions. It will show the commit for that revision/tag instead to the latest commit for the default branch (master)

It also refactors the code of the pkg/git package in order to make it more testable. (Now more than 95% of the code is covered by unit tests)

It splits the previous code into different entities and functionalities.

  • remote.go: this file holds the main functionalities to obtain the latest commit for branches and tags.
  • fetch.go: this file already existed. It is the entry point to the package
  • netutils.go: this file holds the functionalities to obtain HTTP clients and transports with the given credentials stored in a secret. (Functions are exported as they could be useful for other packages, maybe?)
  • vendor.go: This file contains functions for retrieving the latest commit from a branch faster than running a whole ls-remote but that are tied to vendors and their APIs, it is not generic and right now it only supports github and git.rancher.io
  • validate.go: Previously existing file that contains validations for branches, commits and basic url checks.

It also changes how the latest commit is obtained to use the function using the vendor's API if available and falling back to ls-remote.
Reason why is because the function was already called with the following Headers:

  • "Accept" - "application/vnd.github.v3.sha" : This is still used to retrieve just the latest commit.
  • "If-None-Match" - "referenceCommit": This was used before which made the query return code 200 AND the latest commit if it was different to the given reference and code 304 (not modified) otherwise. Now it always returns the latest commit, no matter if it's equal to the reference one or not. This way we can save the ls-remote query which is more expensive as we already know the latest commit for that branch.

It also changes the error messages when a commit could not be retrieved, showing the branch or revision used.
This way the use is able to see, for example, that Fleet's defaults to master as default branch if the user didn't provide any branch in the gitrepo definition and his/her repo has main as the default branch.

Refers to: #2403
Refers to: #2404

@0xavi0 0xavi0 requested a review from a team as a code owner May 6, 2024 07:29
@0xavi0 0xavi0 self-assigned this May 6, 2024
@0xavi0 0xavi0 added the kind/bug label May 6, 2024
@0xavi0 0xavi0 added this to the v2.9.0 milestone May 6, 2024
@0xavi0 0xavi0 force-pushed the 2403-return-commit-for-revision branch from c8faecf to 15f1912 Compare May 6, 2024 07:31
@0xavi0 0xavi0 marked this pull request as draft May 6, 2024 07:34
Changing the commit retrieved when applying `gitrepo`s with revisions.
It will show the commit for that revision/tag instead to the latest commit for the default branch (`master`)

Refers to: rancher#2403

Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
@0xavi0 0xavi0 force-pushed the 2403-return-commit-for-revision branch from 15f1912 to 0fe19f9 Compare May 6, 2024 07:48
@0xavi0 0xavi0 marked this pull request as ready for review May 6, 2024 08:16
@0xavi0 0xavi0 marked this pull request as draft May 6, 2024 09:49
@0xavi0 0xavi0 force-pushed the 2403-return-commit-for-revision branch 2 times, most recently from 3324f8d to f24ea88 Compare May 13, 2024 14:41
@0xavi0 0xavi0 marked this pull request as ready for review May 14, 2024 06:53
@0xavi0 0xavi0 force-pushed the 2403-return-commit-for-revision branch 2 times, most recently from 174a77d to 2780914 Compare May 16, 2024 08:05
This is a refactoring of the code in `pkg/git` package in order to make it more testeable.
It basically splits things into smaller entities and follows the interface/struct approach to
make visible (and mockable) some previously internal code.

Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
@0xavi0 0xavi0 force-pushed the 2403-return-commit-for-revision branch from 2780914 to 31a8bc1 Compare May 17, 2024 08:18
pkg/git/remote.go Outdated Show resolved Hide resolved
pkg/git/remote.go Outdated Show resolved Hide resolved
pkg/git/vendor_test.go Outdated Show resolved Hide resolved
Co-authored-by: Mario Manno <mario.manno@gmail.com>
@0xavi0 0xavi0 requested a review from manno May 22, 2024 10:31
@manno manno merged commit b0492ca into rancher:main May 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants