From 1b4eacc58891bcda0514ec865cd5b250997f611e Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 4 Dec 2020 10:07:14 +0100 Subject: [PATCH] Refactor argument name to enable git2go Signed-off-by: Philip Laine --- api/v1beta1/gitrepository_types.go | 11 ++++++-- ...rce.toolkit.fluxcd.io_gitrepositories.yaml | 11 +++++--- controllers/gitrepository_controller.go | 7 +++-- controllers/gitrepository_controller_test.go | 20 +++++++------- docs/api/source.md | 14 +++++----- docs/spec/v1beta1/gitrepositories.md | 26 +++++++++++-------- go.sum | 2 -- pkg/git/git.go | 26 ++++++++++++------- 8 files changed, 72 insertions(+), 45 deletions(-) diff --git a/api/v1beta1/gitrepository_types.go b/api/v1beta1/gitrepository_types.go index 352b6d42..da66f7e9 100644 --- a/api/v1beta1/gitrepository_types.go +++ b/api/v1beta1/gitrepository_types.go @@ -26,6 +26,10 @@ import ( const ( // GitRepositoryKind is the string representation of a GitRepository. GitRepositoryKind = "GitRepository" + // Represents the go-git git implementation kind + GoGitImplementation = "go-git" + // Represents the gi2go git implementation kind + Git2GoImplementation = "git2go" ) // GitRepositorySpec defines the desired state of a Git repository. @@ -71,9 +75,12 @@ type GitRepositorySpec struct { // +optional Suspend bool `json:"suspend,omitempty"` - // Enables support for git servers that require v2. + // Determines which git client library to use. + // Defaults to go-git, valid values are ('go-git', 'git2go'). + // +kubebuilder:validation:Enum=go-git;git2go + // +kubebuilder:default:=go-git // +optional - GitProtocolV2Compatibility bool `json:"gitProtocolV2Compatibility"` + GitImplementation string `json:"gitImplementation,omitempty"` } // GitRepositoryRef defines the Git ref used for pull and checkout operations. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index 1f16e1fc..1c5bd925 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -49,9 +49,14 @@ spec: spec: description: GitRepositorySpec defines the desired state of a Git repository. properties: - gitProtocolV2Compatibility: - description: Enables support for git servers that require v2. - type: boolean + gitImplementation: + default: go-git + description: Determines which git client library to use. Defaults + to go-git, valid values are ('go-git', 'git2go'). + enum: + - go-git + - git2go + type: string ignore: description: Ignore overrides the set of excluded patterns in the .sourceignore format (which is the same as .gitignore). If not provided, diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 5593d7c5..1c3ae2c6 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -183,7 +183,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour // determine auth method auth := &common.Auth{} if repository.Spec.SecretRef != nil { - authStrategy, err := git.AuthSecretStrategyForURL(repository.Spec.URL, repository.Spec.GitProtocolV2Compatibility) + authStrategy, err := git.AuthSecretStrategyForURL(repository.Spec.URL, repository.Spec.GitImplementation) if err != nil { return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err } @@ -207,7 +207,10 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour } } - checkoutStrategy := git.CheckoutStrategyForRef(repository.Spec.Reference, repository.Spec.GitProtocolV2Compatibility) + checkoutStrategy, err := git.CheckoutStrategyForRef(repository.Spec.Reference, repository.Spec.GitImplementation) + if err != nil { + return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err + } commit, revision, err := checkoutStrategy.Checkout(ctx, tmpGit, repository.Spec.URL, auth) if err != nil { return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 00712b10..5ce37489 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -87,7 +87,7 @@ var _ = Describe("GitRepositoryReconciler", func() { expectMessage string expectRevision string - v2 bool + gitImplementation string } DescribeTable("Git references tests", func(t refTestCase) { @@ -284,10 +284,10 @@ var _ = Describe("GitRepositoryReconciler", func() { Namespace: key.Namespace, }, Spec: sourcev1.GitRepositorySpec{ - URL: u.String(), - Interval: metav1.Duration{Duration: indexInterval}, - Reference: t.reference, - GitProtocolV2Compatibility: t.v2, + URL: u.String(), + Interval: metav1.Duration{Duration: indexInterval}, + Reference: t.reference, + GitImplementation: t.gitImplementation, }, } Expect(k8sClient.Create(context.Background(), created)).Should(Succeed()) @@ -317,11 +317,11 @@ var _ = Describe("GitRepositoryReconciler", func() { expectMessage: "x509: certificate signed by unknown authority", }), Entry("self signed v2", refTestCase{ - reference: &sourcev1.GitRepositoryRef{Branch: "main"}, - waitForReason: sourcev1.GitOperationFailedReason, - expectStatus: metav1.ConditionFalse, - expectMessage: "error: user rejected certificate", - v2: true, + reference: &sourcev1.GitRepositoryRef{Branch: "main"}, + waitForReason: sourcev1.GitOperationFailedReason, + expectStatus: metav1.ConditionFalse, + expectMessage: "error: user rejected certificate", + gitImplementation: sourcev1.Git2GoImplementation, }), ) }) diff --git a/docs/api/source.md b/docs/api/source.md index 96e6e76d..512fdaeb 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -389,14 +389,15 @@ bool -gitProtocolV2Compatibility
+gitImplementation
-bool +string (Optional) -

Enables support for git servers that require v2.

+

Determines which git client library to use. +Defaults to go-git, valid values are (‘go-git’, ‘git2go’).

@@ -1234,14 +1235,15 @@ bool -gitProtocolV2Compatibility
+gitImplementation
-bool +string (Optional) -

Enables support for git servers that require v2.

+

Determines which git client library to use. +Defaults to go-git, valid values are (‘go-git’, ‘git2go’).

diff --git a/docs/spec/v1beta1/gitrepositories.md b/docs/spec/v1beta1/gitrepositories.md index 72355e7b..0dcc91e8 100644 --- a/docs/spec/v1beta1/gitrepositories.md +++ b/docs/spec/v1beta1/gitrepositories.md @@ -51,9 +51,12 @@ type GitRepositorySpec struct { // +optional Suspend bool `json:"suspend,omitempty"` - // Enables support for git servers that require v2. + // Determines which git client library to use. + // Defaults to go-git, valid values are ('go-git', 'git2go'). + // +kubebuilder:validation:Enum=go-git;git2go + // +kubebuilder:default:=go-git // +optional - GitProtocolV2Compatibility bool `json:"gitProtocolV2Compatibility"` + GitImplementation string `json:"gitImplementation,omitempty"` } ``` @@ -174,12 +177,13 @@ spec: When specified, `spec.ignore` overrides the default exclusion list. -## Git V2 +## Git Implementation -You should skip this unless you know that you need v2 compatibility. Enabling -this feature comes with its own set of drawbacks. +You can skip this section unless you know that you need support for either +specific git wire protocol functionality. Changing the git implementation +comes with its own set of drawbacks. -Some git providers like Azure DevOps require special features in the users git client +Some git providers like Azure DevOps require that the git client supports specific capabilities to be able to communicate. The initial library used in source-controller did not support this functionality while other libraries that did were missinging other critical functionality, specifically the ability to do shallow cloning. Shallow cloning is important as it allows @@ -188,12 +192,12 @@ For some very large repositories this means downloading GB of data that could fi and also impact the traffic costs. To be able to support Azure DevOps a compromise solution was built, giving the user the -option to enable V2 compatibility with the drawbacks it brings. +option to select the git library while accepting the drawbacks. -| V2 Compatibility | Shallow Clones | Azure DevOps Support | +| Git Implementation | Shallow Clones | Azure DevOps Support | |---|---|---| -| false | true | false | -| true | false | true | +| 'go-git' | true | false | +| 'git2go' | false | true | Pull the master branch from a repository in Azure DevOps. @@ -206,7 +210,7 @@ metadata: spec: interval: 1m url: https://dev.azure.com/org/proj/_git/repo - gitProtocolV2Compatibility: true + gitImplementation: git2go ``` ## Spec examples diff --git a/go.sum b/go.sum index 14429b36..86b696c9 100644 --- a/go.sum +++ b/go.sum @@ -224,8 +224,6 @@ github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5Kwzbycv github.com/fluxcd/pkg/apis/meta v0.3.0/go.mod h1:wOzQQx8CdtUQCGaLzqGu4QgnNxYkI6/wvdvlovxWhF0= github.com/fluxcd/pkg/apis/meta v0.4.0 h1:JChqB9GGgorW9HWKxirTVV0rzrcLyzBaVjinmqZ0iHA= github.com/fluxcd/pkg/apis/meta v0.4.0/go.mod h1:wOzQQx8CdtUQCGaLzqGu4QgnNxYkI6/wvdvlovxWhF0= -github.com/fluxcd/pkg/gittestserver v0.0.3-0.20201202222244-96033b836a6a h1:fmfbt5VrEPUb4X0UI14a0K2FWr0iv/NRUadk8X35byc= -github.com/fluxcd/pkg/gittestserver v0.0.3-0.20201202222244-96033b836a6a/go.mod h1:HWZaoib03fQeSsauCAN2iAFdr6bnjKQ+CFxMFD2mwDY= github.com/fluxcd/pkg/gittestserver v0.1.0 h1:BvIG+bBhgbmqhtpSS2qUpOXRIL1P1Ow2jauloH8X86U= github.com/fluxcd/pkg/gittestserver v0.1.0/go.mod h1:HWZaoib03fQeSsauCAN2iAFdr6bnjKQ+CFxMFD2mwDY= github.com/fluxcd/pkg/helmtestserver v0.0.1 h1:8RcLZdg7Zr9ZqyijsIIASjjMXQtF4UWP4Uds4iK2VJM= diff --git a/pkg/git/git.go b/pkg/git/git.go index 5e32f38e..b7c148ba 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -17,6 +17,8 @@ limitations under the License. package git import ( + "fmt" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/fluxcd/source-controller/pkg/git/common" gitv1 "github.com/fluxcd/source-controller/pkg/git/v1" @@ -27,18 +29,24 @@ const ( defaultBranch = "master" ) -func CheckoutStrategyForRef(ref *sourcev1.GitRepositoryRef, useGitV2 bool) common.CheckoutStrategy { - if useGitV2 { - return gitv2.CheckoutStrategyForRef(ref) +func CheckoutStrategyForRef(ref *sourcev1.GitRepositoryRef, gitImplementation string) (common.CheckoutStrategy, error) { + switch gitImplementation { + case sourcev1.GoGitImplementation: + return gitv1.CheckoutStrategyForRef(ref), nil + case sourcev1.Git2GoImplementation: + return gitv2.CheckoutStrategyForRef(ref), nil + default: + return nil, fmt.Errorf("invalid git implementation %s", gitImplementation) } - - return gitv1.CheckoutStrategyForRef(ref) } -func AuthSecretStrategyForURL(url string, useGitV2 bool) (common.AuthSecretStrategy, error) { - if useGitV2 { +func AuthSecretStrategyForURL(url string, gitImplementation string) (common.AuthSecretStrategy, error) { + switch gitImplementation { + case sourcev1.GoGitImplementation: + return gitv1.AuthSecretStrategyForURL(url) + case sourcev1.Git2GoImplementation: return gitv2.AuthSecretStrategyForURL(url) + default: + return nil, fmt.Errorf("invalid git implementation %s", gitImplementation) } - - return gitv1.AuthSecretStrategyForURL(url) }