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

Feature request: Add git push option for arbitrary refspec #509

Closed
ghost opened this issue Apr 7, 2023 · 18 comments · Fixed by #514
Closed

Feature request: Add git push option for arbitrary refspec #509

ghost opened this issue Apr 7, 2023 · 18 comments · Fixed by #514
Labels
area/git Git related issues and pull requests enhancement New feature or request

Comments

@ghost
Copy link

ghost commented Apr 7, 2023

Hi, in my company we use Gerrit, and there's a special reference which opens the equivalent of merge/pull requests when commits are pushed to this reference:

git push origin HEAD:refs/for/master

It would be really great if we could specify such arbitrary reference, maybe like this:

spec:
  git:
    push:
      refspec: HEAD:refs/for/master
@makkes makkes added enhancement New feature or request area/git Git related issues and pull requests labels Apr 11, 2023
@makkes
Copy link
Member

makkes commented Apr 11, 2023

fluxcd/source-controller#1017 might help when implementing this.

The ref would be refs/for/master.

@brovoca
Copy link

brovoca commented Apr 24, 2023

We heavily use Gerrit in our team and suffer from not being able to create Gerrit changes, as described by @aristapimenta. When I try to configure refs/heads/refs/for/master or refs/for/master, an actual branch is created, which isn't what should happen. Surely, we can push directly to master, but we prefer approving the change first.

@stefanprodan
Copy link
Member

I think adding refs would also allow Flux to work with GitLab, but we may need a lis of refs instead of just one item. cc @hiddeco

@hiddeco
Copy link
Member

hiddeco commented May 2, 2023

GitLab integrations I was thinking about is different, and called "push options". See:

@stefanprodan
Copy link
Member

@hiddeco then we can add ref name to IAC CRD same as we did with GitRepo and for push options we can have a separate field right?

@aryan9600
Copy link
Member

aryan9600 commented May 2, 2023

yes we can modify the CRD where the ref name would be specified under spec.git.push.refName or spec.git.push.ref.

@stefanprodan
Copy link
Member

I vote for spec.git.push.ref

@thompson-shaun
Copy link

thompson-shaun commented May 7, 2023

GitLab integrations I was thinking about is different, and called "push options". See:

 * https://docs.gitlab.com/ee/user/project/push_options.html

 * [Allow specifying git push options for image automation controller flux2#3490](https://github.com/fluxcd/flux2/discussions/3490)

Found this thread looking for a way to create Merge Requests via GitLab's push options. Very exciting!

@cyclimse
Copy link

Looking forward to this feature as well for Gitlab! It looks like gogit supports push_options go-git/go-git#399

@hiddeco
Copy link
Member

hiddeco commented May 10, 2023

@hiddeco then we can add ref name to IAC CRD same as we did with GitRepo and for push options we can have a separate field right?

Yes. If we vote for .spec.git.push.ref, I think the counterpart for push options would be .spec.git.push.options. This would take a string list of options.

@aryan9600
Copy link
Member

as i pointed out in today's community meeting, the most apt name is .spec.git.push.refspec as a Git Refspec and a Git ref are two different things. (ref: https://git-scm.com/book/en/v2/Git-Internals-The-Refspec, https://git-scm.com/book/en/v2/Git-Internals-Git-References)

@brovoca
Copy link

brovoca commented Jun 15, 2023

Eagerly waiting for this...

@aryan9600
Copy link
Member

aryan9600 commented Jun 15, 2023

Even if we get this feature in, I don't think image-automation-controller will be compatible with Gerrit. Gerrit uses the commit-msg Git hook to insert a identifier called Change-Ids. Atm go-git, the Git library we use does not support Git hooks. Even if it did, the commit-msg Git hook that Gerrit requires is written in bash and shells out to default Git installation in the machine (ref: https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html).

That is a big no-no in Flux, and one of the core reasons we use a library and don't shell out ourselves to push, pull, etc.
We can still get this feature in as it might be helpful for other scenarios, but I don't think it solves the Gerrit use case. Please note that my knowledge about Gerrit is extremely limited, so feel free to correct me if I am wrong.

@brovoca
Copy link

brovoca commented Jun 16, 2023

@aryan9600 I've written my own messageTemplate that generates a Change-Id:

---
apiVersion: image.toolkit.fluxcd.io/v1beta1
kind: ImageUpdateAutomation
metadata:
  name: flux-system-master
  namespace: flux-system
spec:
  interval: 1m0s
  sourceRef:
    kind: GitRepository
    name: flux-system
    namespace: flux-system
  git:
    checkout:
      ref:
        branch: master
    push:
      branch: master
    commit:
      author:
        email: flux@localdomain
        name: flux
      messageTemplate: |
        Perform automatic image update

        Automation name: {{ .AutomationObject }}

        Files:
        {{ range $filename, $_ := .Updated.Files -}}
        - {{ $filename }}
        {{ end }}
        Objects:
        {{ range $resource, $_ := .Updated.Objects -}}
        - {{ $resource.Kind }} {{ $resource.Name }}
        {{ end }}
        Images:
        {{ range .Updated.Images -}}
        - {{ . }}
        {{ end }}
        {{- $ChangeId := .AutomationObject -}}
        {{- $ChangeId = printf "%s%s" $ChangeId ( .Updated.Files | toString ) -}}
        {{- $ChangeId = printf "%s%s" $ChangeId ( .Updated.Objects | toString ) -}}
        {{- $ChangeId = printf "%s%s" $ChangeId ( .Updated.Images | toString ) }}
        Change-Id: {{ printf "I%s" ( sha256sum $ChangeId | trunc 40 ) }}

@aryan9600
Copy link
Member

okay, then please follow the instructions provided in #514 (comment) and see if it works for you.

@brovoca
Copy link

brovoca commented Jun 22, 2023

I will likely do this once back from a long vacation. Thank you for your assistance @aryan9600!

@aryan9600
Copy link
Member

@brovoca did you get a chance to try it out?

@brovoca
Copy link

brovoca commented Aug 7, 2023

Sorry, not yet, just came back from vacation. Not sure when I'll have a moment

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.

7 participants