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

Support reference name in GitRepositoryRef #1017

Closed
haarchri opened this issue Feb 2, 2023 · 10 comments · Fixed by #1026
Closed

Support reference name in GitRepositoryRef #1017

haarchri opened this issue Feb 2, 2023 · 10 comments · Fixed by #1026
Labels
area/git Git related issues and pull requests enhancement New feature or request

Comments

@haarchri
Copy link

haarchri commented Feb 2, 2023

we want to discuss first change in the GitRepository the GitRepositoryRef

today we have the following types: branch, tag, semver and commit https://github.com/fluxcd/source-controller/blob/main/api/v1beta2/gitrepository_types.go#L157 for us using flux in combination with crossplane its very hard to use it this way

implementation today:

type GitRepositoryRef struct {
	// Branch to check out, defaults to 'master' if no other field is defined.
	//
	// When GitRepositorySpec.GitImplementation is set to 'go-git', a shallow
	// clone of the specified branch is performed.
	// +optional
	Branch string `json:"branch,omitempty"`

	// Tag to check out, takes precedence over Branch.
	// +optional
	Tag string `json:"tag,omitempty"`

	// SemVer tag expression to check out, takes precedence over Tag.
	// +optional
	SemVer string `json:"semver,omitempty"`

	// Commit SHA to check out, takes precedence over all reference fields.
	//
	// When GitRepositorySpec.GitImplementation is set to 'go-git', this can be
	// combined with Branch to shallow clone the branch, in which the commit is
	// expected to exist.
	// +optional
	Commit string `json:"commit,omitempty"`
}

we have crossplane compositions running to input branch or tag but this lead to the following problem:

1.) specifiy branch: (feature development)

Spec:
  Git Implementation:  go-git
  Interval:            10m0s
  Ref:
    Branch:  feature/AWSP-1733-nginx-ssh-gitlab

2.) specify tag: (feature development finished and release by tag):

Spec:
  Git Implementation:  go-git
  Interval:            10m0s
  Ref:
    Branch:  feature/AWSP-1733-nginx-ssh-gitlab
    Tag:  v1.2.0

3.) specify new branch for new feature
its not possible to switch to Branch anymore - you need to delete the Tag first - and normally a kubectl merge is running - so you need to adjust the manifest by hand - wich is not allowed in our company

Spec:
  Git Implementation:  go-git
  Interval:            10m0s
  Ref:
    Branch: feature/AWSP-1734-kyverno-update
    Tag:  v1.2.0

Implementation in future:

type GitRepositoryRef struct {
         // type, default is branch
	// +kubebuilder:validation:Enum=branch,tag,semver,commit
	Type string `json:"type,omitempty"`

	// name, default is main
	// +optional
	Name string `json:"name,omitempty"`
}

any chance to discuss this kind of change before bumping to GA v1 ? #947

@stefanprodan
Copy link
Member

stefanprodan commented Feb 2, 2023

and normally a kubectl merge is running

Apply the GitRepository with Flux and the branch will be gone, unlike kubectl, kustomize-controller knows how to remove fields deleted from Git.

@haarchri
Copy link
Author

haarchri commented Feb 2, 2023

but no cli is involved in our case for hundreds of bubernetes clusters - and we looking for a flow with Helm,Kubectl or Crossplane for example- what is against a type field ?

@stefanprodan
Copy link
Member

what is against a type field ?

Breaking everyone's clusters by changing a core API that we promised users we'll not do.

@haarchri
Copy link
Author

haarchri commented Feb 2, 2023

we can add this fields in parallel and make the flow much more easier for kubernetes tooling

@stefanprodan
Copy link
Member

stefanprodan commented Feb 2, 2023

The enum doesn't cover the use-case where you want to pic a specific commit from a branch, you can do that with the current API by specifying both the branch and commit.

@hiddeco
Copy link
Member

hiddeco commented Feb 2, 2023

I think this could also be solved by adding another field which takes precedence over all existing fields which would allow the definition of a plain Git Reference. This would prevent having to define behavior describing enums which would cause problems as laid out by Stefan, while allowing ambiguous control over fetching branches, tags, or even pull request references (as has been reported as a use case elsewhere).

Internally, this would likely just result in the controller resolving the reference to a specific commit, which would then be fetched.

@stefanprodan
Copy link
Member

I think this could also be solved by adding another field which takes precedence over all existing fields which would allow the definition of a plain Git Reference.

+1 something like ref.name which takes precedence over branch/tag/etc would solve the issue here and not break Flux for everyone else.

@haarchri
Copy link
Author

haarchri commented Feb 2, 2023

sounds good i will have a look to open a PR

haarchri pushed a commit to haarchri/source-controller that referenced this issue Feb 6, 2023
Signed-off-by: Christopher Paul Haar <christopherpaul.haar@dkb.de>
@haarchri
Copy link
Author

haarchri commented Feb 6, 2023

@stefanprodan @hiddeco we added an draft PR: #1022 any chance to get a review before we finish this implementation and adding tests ? thats would be very helpful! thanks for your time

@aryan9600
Copy link
Member

hey @haarchri, its not very straightforward to fix this as it requires more changes in different areas. i'm happy to pick it up and take it off your hands.

@hiddeco hiddeco changed the title GitRepositoryRef change to type and name Support reference name in GitRepositoryRef Feb 16, 2023
@hiddeco hiddeco added enhancement New feature or request area/git Git related issues and pull requests labels Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants