voxpupuli / webhook-go

Puppet Webhook port in Golang
Apache License 2.0
16 stars 12 forks source link

Support git branches containing slashes #163

Closed adoom42 closed 3 months ago

adoom42 commented 3 months ago

After upgrading the puppet-r10k module, I had to migrate from the built-in webhook to webhook-go. After doing so, r10k stopped working for some branches.

If the branch name contains one or more slashes, webhook-go only handles the part after the final slash. Ex. when the branch is named feature/TICKET-1234, the webhook-go systemd log shows this:

Jun 18 13:44:35 puppet-server-01 webhook-go[3046986]: time="2024-06-18T13:44:35-07:00" level=error msg="failed to execute local command `[/opt/puppetlabs/puppet/bin/r10k deploy environment TICKET-1234 --config=/etc/puppetlabs/r10k/r10k.yaml --verbose --generate-types --modules]` with error: `exit status 1` 
<snipped>
ERROR\t -> Environment(s) 'TICKET_1234' cannot be found in any source and will not be deployed.\n

It is common to use slashes in branch names. This post has several good examples. r10k itself handles slashes by replacing them with underscores like it does with hyphens, ex. the feature/TICKET-1234 branch becomes known to Puppet agents as the feature_TICKET_1234 environment.

Looking through the source code, it appears the behavior is caused by this: https://github.com/voxpupuli/webhook-go/blob/fef55c882e825f130d63b68308604e1dbd53ae5e/lib/parsers/github.go#L26

From the Go documentation:

Base returns the last element of path.

This is a major regression from the webhook daemon that used to be included in the puppet-r10k module and is effectively a blocker for me. Looking at the logs from the old Ruby webhook daemon, it runs r10k like this, retaining the full branch name:

umask 0022; r10k deploy environment feature/TICKET-1234 -pv branch: feature/TICKET-1234

This is how the old webhook determined the branch name: https://github.com/voxpupuli/puppet-r10k/blob/b565bd104ee9826e443b5461dad1075404774b75/templates/webhook.bin.erb#L168-L176

    branch = (
        data['ref']                                               ||  # github & gitlab
        data['refChanges'][0]['refId']                 rescue nil ||  # stash
        data['push']['changes'][0]['new']['name']      rescue nil ||  # bitbucket
        data['changes'][0]['ref']['displayId']         rescue nil ||  # bitbucket server repository event
        data['pullRequest']['fromRef']['displayId']    rescue nil ||  # bitbucket server pull request event
        data['resource']['refUpdates'][0]['name']      rescue nil ||  # TFS/VisualStudio-Git
        data['repository']['default_branch']                          # github tagged release; no ref.
      ).sub('refs/heads/', '') rescue nil

It stripped 'refs/heads/' from the string.

Please parse the branch name with something other than path.Base(), or at least provide a configuration option to do so.

dhollinger commented 3 months ago

Should be merged. I'll get a release out later

fffialho commented 1 month ago

@dhollinger Do we have any idea when a release with this change might be coming out?

dhollinger commented 1 month ago

@fffialho Sorry, for some reason I thought I had already triggered a release. I'll get that done soon