abrader / abrader-gms

Git management system types and providers for Puppet utilizing Github, Gitlab, Stash APIs
Apache License 2.0
26 stars 32 forks source link

Webhook for merges as well as pushes? #4

Closed esalberg closed 9 years ago

esalberg commented 9 years ago

Is there a way to modify the API call to create a webhook entry that fires on merge events as well as push events?

I'd be willing to submit a pull request if it's possible and not implemented, but I'm not quite sure where to look - your provider, the r10k module webhook itself, etc. Is this something that you've ever investigated?

esalberg commented 9 years ago

A bit more digging, and it's definitely possible: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/api/projects.md

However, it looks like the type (definitely) and provider (possibly) would need to be updated - I'm assuming that the default is for the webhook to be created for pushes since nothing is specified now that I can see.

I can look into doing a pull request, but if someone who's better at ruby wants to make the change first, I won't object.

abrader commented 9 years ago

Please test the follow commit to see if it resolves this issue/request:

https://github.com/abrader/abrader-gms/commit/8230a8e0936d6f8674d028b20176e7e07060ad55

I also added coverage for 'Tag Push Events' and 'Issues Events' since they were similar in implementation.

If all is good, I make sure it's in the next release to the Puppet Forge.

esalberg commented 9 years ago

I'm getting this: Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Invalid parameter merge_request_events on Git_webhook[web_post_receive_webhook_control] at [...]

My hook: git_webhook {'web_post_receive_webhook_control': ensure => 'present', webhook_url => "http://${::fqdn}:8088/payload", token => hiera('gitlab_api_token'), project_name => 'puppet/puppet_control', server_url => $gitlab_server, provider => 'gitlab', merge_request_events => true, require => Git_deploy_key['add_deploy_key_to_control'], }

I doublechecked in the environments/modules directory to make sure I'm getting the latest code. It did get updated, i.e. contains:

newparam(:merge_request_events, :boolean => true, :parent => Puppet::Parameter::Boolean) do
  desc 'The URL in the webhook_url parameter will be triggered when a merge request is created. NOTE: GitLab only'

[etc.]

abrader commented 9 years ago

I have feeling you got the type but not the provider code. I would suggest considering removing the module and performing a git clone from my aformentioned commit if you don't see the following lines in 'lib/puppet/provider/gitlab.rb':

https://github.com/abrader/abrader-gms/blob/master/lib/puppet/provider/git_webhook/gitlab.rb#L124L126

Look for the following code in your version of the gitlab.rb file:

if resource.merge_request_events?
  opts['merge_requests_events'] = resource[:merge_request_events]
end

To confirm, I completely removed the module and did a fresh clone. Performed a test w/o issue.

abrader commented 9 years ago

Since I know this is working and well tested at this point, I will include this commit in my future Forge release. Thanks for the feature request.

esalberg commented 9 years ago

I had to restart pe-puppetserver. Welcome to Puppet 3.7 for me - I beat my head against this all day.

It looks great - thanks for the quick update!

abrader commented 9 years ago

v0.0.6 (including these params) now available: https://forge.puppetlabs.com/abrader/gms