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

http URL validation is too restrictive #5

Closed acidprime closed 9 years ago

acidprime commented 9 years ago

The regex here:

https://github.com/abrader/abrader-gms/blob/master/lib/puppet/type/git_webhook.rb#L36

will fail if someone uses http://user:password@server:8080/payload

I am wondering is something more like

^(https?:\/\/).*:?.*\/?$ would still work but not be as finicky with possible permutations

acidprime commented 9 years ago

example error for people who search

Error: Failed to apply catalog: Parameter webhook_url failed on Git_webhook[puppet_webhook]: Git webhook URL must be fully qualified, not 'https://puppet:im-a-password@foo.wallcity.org:8089/payload' at /etc/puppetlabs/puppet/environments/production/site/profile/manifests/master.pp:12
Wrapped exception:
Git webhook URL must be fully qualified, not 'https://puppet:im-a-password@foo.wallcity.org:8089/payload'
abrader commented 9 years ago

Weakened the validation per your suggest, as seen in this commit: https://github.com/abrader/abrader-gms/commit/61def084caf1fbd8c620dee63be9ec5139336c0a

abrader commented 9 years ago

Or...rather this commit: https://github.com/abrader/abrader-gms/commit/9ae51c141db808e5a3b83f4c48fee9e7e952e61d

acidprime commented 9 years ago

yeah I found it ;)

oznah commented 9 years ago

I am still getting error:

Failed to apply catalog: Parameter webhook_url failed on Git_webhook[web_post_receive_webhook]: Git webhook URL must be fully qualified, not 'puppet:puppet@fqdn.puppetmaster.com:8088/payload' at /etc/puppetlabs/puppet/environments/production/modules/profiles/manifests/r10k.pp:113 Wrapped exception: Git webhook URL must be fully qualified, not 'puppet:puppet@fqdn.puppetmaster.com:8088/payload'

r10k code

 git_webhook { 'web_post_receive_webhook' :
    ensure             => present,
    webhook_url        => 'https://puppet:puppet@fqdn.puppetmaster.com:8088/payload',
    token              =>  hiera('gitlab_api_token'),
    project_name       => 'puppet/control',
    server_url         => 'https://fqdn.gitlab.com',
    disable_ssl_verify => true,
    provider           => 'gitlab',
}

Shouldn't matter but, I'm using gitlab.

abrader commented 9 years ago

My apologies for the delay. Confirmed the issue. Thanks for the submission. I will be getting to the refactoring of the regex shortly.

abrader commented 9 years ago

@oznah Please try the validation from this release: https://github.com/abrader/abrader-gms/blob/master/lib/puppet/type/git_webhook.rb#L36

Worked for me, would like your input.

oznah commented 9 years ago

I will give it a try tomorrow.

On Thu, Apr 16, 2015, 3:57 PM Andrew Brader notifications@github.com wrote:

@oznah https://github.com/oznah Please try the validation from this release: https://github.com/abrader/abrader-gms/blob/master/lib/puppet/type/git_webhook.rb#L36

Worked for me, would like your input.

— Reply to this email directly or view it on GitHub https://github.com/abrader/abrader-gms/issues/5#issuecomment-93826339.

abrader commented 9 years ago

Confirmed with other interested parties this is now working for them as well. Committed changes to a new forge release v.0.0.8: https://forgeapi.puppetlabs.com/v3/files/abrader-gms-0.0.8.tar.gz

oznah commented 9 years ago

Sorry for the delay. I confirmed that this is working now. No more error.

abrader commented 9 years ago

No worries. Thanks for checking.