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

feat: give webhooks the option for accepting invalid HTTPS certs #71

Closed
JosephKav opened this issue May 26, 2022 · 16 comments · Fixed by #74
Closed

feat: give webhooks the option for accepting invalid HTTPS certs #71

JosephKav opened this issue May 26, 2022 · 16 comments · Fixed by #74
Labels
enhancement New feature or request

Comments

@JosephKav
Copy link
Collaborator

webhooks currently fail to send if the cert is invalid (e.g. self-signed). This'll add an allow_invalid_certs key to each webhook (with support for it in defaults/globally like notify)

Triggered by:
Originally posted by @samcro1967 in #61 (comment)

So for fun (I mean who doesn't love certs), I changed webhook to use https. I get a different error in the webhook log when I execute the webhook from Argus, but I am guessing it is related to how I generated the self signed cert. Thought I would share in case it sheds any new light on the topic.

webhook log

[webhook] 2022/05/26 15:30:37 http: TLS handshake error from 172.18.0.14:41156: tls: oversized record received with length 21536

cert generation command

openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes -keyout key.pem -days 9999 -out cert.pem`
@JosephKav JosephKav added the enhancement New feature or request label May 26, 2022
@jrhbcn
Copy link

jrhbcn commented May 27, 2022

OK, I have tried to use https (through my reverse proxy config, currently using swag) but still does not seems to trigger the webhook in my setup. I think it might be a problem of my config, a couple of questions:

  1. I see I can define webhooks in argus on the webhook section and inside each service. Right now I am only doing it inside the service (actually, I haven't got any generic webhook: definition outside the service: definition). Is this correct?
  2. Just to clarify, when a webhook is called manually through the gui, does it trigger a call to all deployed_versions of all other services? When I trigger a webhook manually, I don't see anything related to it on the webhook logs but I see it triggers all deployed_versions ones (i also use webhooks to check for deployed versions).

@JosephKav
Copy link
Collaborator Author

1. I see I can define webhooks in argus on the webhook section and inside each service. Right now I am _only_ doing it inside the service (actually, I haven't got any generic `webhook:` definition outside the `service:` definition). Is this correct?

This shouldn't matter. It will try and use the vars in the services webhook, and any that don't exist it'll try and look up in webhook.same_name. My testing was just with it under a service, but outside of that, I do have webhooks that get their vars from that base webhook definition

2. Just to clarify, when a webhook is called manually through the gui, does it trigger a call to all deployed_versions of all other services? When I trigger a webhook manually, I don't see anything related to it on the webhook logs but I see it triggers all deployed_versions ones (i also use webhooks to check for deployed versions).

The webhook sends shouldn't do anything about deployed_version. That's just queried separately at the same frequency it checks github/url

@jrhbcn
Copy link

jrhbcn commented May 27, 2022

The webhook sends shouldn't do anything about deployed_version. That's just queried separately at the same frequency it checks github/url

OK, I think I found a possible explanation. When I execute "Resend webhook" from the GUI Argus seems to crash, then it reboots and that is why all my deployed_version webhooks where called. I don't know why is crashing in my setup though. I am running argus using docker-compose from ghcr.io/release-argus/argus:latest and this is the DEBUG output of the crash:

DEBUG: WebSocket (192.168.1.7), READ {"version":1,"page":"APPROVALS","type":"WEBHOOK","sub_type":"SUMMARY","service_data":{"id":"Argus"}}
VERBOSE: wsWebHook (192.168.1.7), -
DEBUG: WebSocket (192.168.1.7), READ {"version":1,"page":"APPROVALS","type":"VERSION","target":"ARGUS_ALL","service_data":{"id":"Argus","status":{"latest_version":"0.4.1"}}}
VERBOSE: wsServiceAction (192.168.1.7), -
INFO: wsServiceAction (192.168.1.7), Argus Release approved - "ALL" WebHook
INFO: Argus, Sending WebHooks for "0.4.1"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x808389]

goroutine 95 [running]:
github.com/release-argus/Argus/webhook.(*WebHook).GetType(...)
        /build/webhook/init.go:156
github.com/release-argus/Argus/webhook.(*WebHook).GetRequest(0xc000247e00?)
        /build/webhook/init.go:138 +0x89
github.com/release-argus/Argus/webhook.(*WebHook).try(0xc000247e00, {{0xc000224a08?, 0x16?}, {0xc0002249b0?, 0x4c?}})
        /build/webhook/send.go:116 +0x77
github.com/release-argus/Argus/webhook.(*WebHook).Send(0xc000247e00, {{0xc0002249b0, 0x5}, {0xc00057a210, 0x26}, {0xc0006d8080, 0x3f}, {0xc000224ab0, 0x5}}, 0x0)
        /build/webhook/send.go:74 +0x259
github.com/release-argus/Argus/webhook.(*Slice).Send.func1(0x1?)
        /build/webhook/send.go:42 +0x4b
created by github.com/release-argus/Argus/webhook.(*Slice).Send
        /build/webhook/send.go:41 +0xe5

@jrhbcn
Copy link

jrhbcn commented May 27, 2022

Should I move to :master docker image? I thought that was too bleeding edge :)

@JosephKav
Copy link
Collaborator Author

That crash looks like it's from you not setting the type of the webhook. The only type atm is github. I'll make this a hard default for now, so when type isn't set, it defaults it to a github-style webhook

@jrhbcn
Copy link

jrhbcn commented May 27, 2022

Finally, it works!!! It was indeed the type:github missing from my part. Now it works with http / https.

Thanks @JosephKav

JosephKav added a commit that referenced this issue May 27, 2022
- default to `github` + error if defined as something other than `github`
- switched around the invalid param texts so the found var is before the "<invalid>"
#71 (comment)
@JosephKav JosephKav changed the title feat(webhooks): support for invalid HTTPS certs feat: give webhooks the option for accepting invalid HTTPS certs May 27, 2022
@JosephKav JosephKav linked a pull request May 27, 2022 that will close this issue
@samcro1967
Copy link

samcro1967 commented May 27, 2022

I setup webhooks on my reverse proxy and tested again. The TLS error is gone and the webhook is successfully sent, but I am getting got matched, but didn't get triggered because the trigger rules were not satisfied in webhook logs.

It is not the first match as I am using SECRET as secret and copying and pasting, so it must be in the second match? Perhaps something to do with the fact I am running on the latest commit of argus master that is causing the second match to fail???

  trigger-rule:
    and:
    - match:
        type: payload-hmac-sha256
        secret: SECRET
        parameter:
          source: header
          name: X-Hub-Signature-256
    - match:
        type: value
        value: refs/heads/master
        parameter:
          source: payload
          name: ref

@jrhbcn
Copy link

jrhbcn commented May 27, 2022

@samcro1967 have you tried removing the matches completely from the webhook config? It should work as parameters are only triggered if a match rule is defined.

@samcro1967
Copy link

@jrhbcn I had tied removing the second match since I believe it was the one failing and it did not work. I just removed both and it did work. I am using http with allow_invalid_certs: false in the webhook based on the latest commit so http is working how. I suspect https with a self signed cert would also work.

So I would say there is an issue with a newer commit that is impacting the triggers, but not a requirement for me. The only missing piece would be the ability send a parameter with the webhook to send to the script as an argument.

@jrhbcn
Copy link

jrhbcn commented May 27, 2022

So I would say there is an issue with a newer commit that is impacting the triggers, but not a requirement for me. The only missing piece would be the ability send a parameter with the webhook to send to the script as an argument.

I am solving that now using the parameter directly in the webhook url. Such as:

    webhook:
      update_docker:
        type: github
        url: http://192.168.9.9/hooks/update-docker?container=argus
        secret: SECRET

@JosephKav
Copy link
Collaborator Author

JosephKav commented May 27, 2022

So I would say there is an issue with a newer commit that is impacting the triggers, but not a requirement for me.

I'm not sure about that as you haven't ever got it to work with those match rules, but it still works fine for me with the rules you posted. Not sure what could be different in our setups

The only missing piece would be the ability send a parameter with the webhook to send to the script as an argument.

This I'll do in #61
(add custom_headers that you can give the webhook)

@samcro1967
Copy link

@jrhbcn I do not see anything in your hooks.yml for that. So does that just become $1 for the script or am I missing something?

@samcro1967
Copy link

@JosephKav I added the below and it is working now. Thanks!

  pass-arguments-to-command:
    - source: url
      name: container

@samcro1967
Copy link

@JosephKav So I setup Wireshark and enabled debugging on webhook. Was all set to capture what Argus sent and what webhook received and now it is working. Will open a new issue if I see it again with more detail hopefully this time.

@samcro1967
Copy link

As I was reviewing logs for dpkg this morning, I realized that yesterday I did remove the community packaged version of webhook and built from source. The community packaged version is not maintained and was at 2.6.x if I remember correctly. The current version is 2.8.x. My guess is that was the solution if you also built from source.

@JosephKav
Copy link
Collaborator Author

JosephKav commented May 28, 2022

XD I did think you might not be on 2.8.0, but then I saw that it came out in Dec 2020, so I thought you must have been :/. I didn't build it from source, I just use the binary on the releases page. It does also have a Docker tag for 2.8.0

Glad we've gotten to the bottom of it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants