Verify SSL certificates by default

[johannes-huther]

Merging this PR closes #20.

The current implementation does not verify SSL certificates, if I correctly understand the curl docs on option -k. This parameter was introduced in 16b7ccd as a response to #3.

I can see how some people do not have the time, patience or even the option to use verifiable SSL certificates, but I do and so will others.

The code in this PR makes verification of SSL certificates the default (corresponds to approach 1 mentioned in #20). This has the benefit that the most secure option is used by default, but might break other peoples' workflows if they use unsigned certificates and would require them to update their workflow config.

If you want certificate verification not to be the default, I can modifiy this PR. If you want to do the change yourself, feel free to do so as well.

This PR also cleans up some of the code by storing the options in a variable and therefore allowing one of the curl sections to be removed. Also: &> /dev/null is not needed if -v is omitted and -s is used and therefore removed.

Disclaimer: I'm not fully able to test this as a proper GitHub Action in one of my projects. I therefore tested the code by running the docker container with the same environment variables, that I believe GitHub would use.

Code fully tested using:

    - name: Invoke deployment hook
      uses: johannes-huther/workflow-webhook@feature/verify-certificates
      env:
        webhook_url: ${{ secrets.WEBHOOK_URL }}
        webhook_secret: ${{ secrets.WEBHOOK_SECRET }}

Also tested with verify_ssl: false and silent: true.

[distributhor]

Thank you for these contributions, I appreciate any help to improve the project, and these are valuable additions. I have a busy week ahead, but will make some time during the week to review and merge.

[distributhor]

I have merged the pull requests and released a new version (v2.0.0). I mentioned you in the release note, if you prefer me to remove that, please let me know. I am still busy testing the features myself, and will continue to do so over the weekend. If it needs tweaking to sort out any issues (I don't think it will), then I'll patch it as I go. But for now a v2 tag exists and can be used - the previous v1 tag obviously also still exists, for those who currently use it.

[johannes-huther]

I have merged the pull requests and released a new version (v2.0.0). I mentioned you in the release note, if you prefer me to remove that, please let me know. I am still busy testing the features myself, and will continue to do so over the weekend. If it needs tweaking to sort out any issues (I don't think it will), then I'll patch it as I go. But for now a v2 tag exists and can be used - the previous v1 tag obviously also still exists, for those who currently use it.

Great 👍! I noticed that the tags in the examples in README.md still contain v1, e.g:

workflow-webhook/README.md

Line 50 in dc09356

uses: distributhor/workflow-webhook@v1

That should probably be adjusted. Edit: Just noticed that you already fixed it :)

---

On another note: I didn't like that the docker image was rebuilt for each run of the action and I was unsure how to use the caching feature. Therefore I switched to a composite action instead of a docker action in my fork, this means that CI would only work on linux (at least I suppose so), but as all my workflows run on ubuntu-latest this is no issue for me.

If you find some time and are interested you can take a look at johannes-huther@af1465f. It runs significantly faster for me now. I also had to replace the way how the request id is generated as the previous method seems to be incompatible with ubuntu, but that should be no problem (johannes-huther@20d7312).

I don't know if this is relevant for you or other users, or if you need the platform independence and therefore I didn't yet create a PR. If you ever got the caching feature working I would also be interested in an example workflow file :)

[distributhor]

I updated the v1 reference thanks. I'm aware that a caching solution is needed, and hoping to again look into the available options sometime soon. Would be great to improve that part for sure.

[johannes-huther]

Would you mind if I release my fork as an "using: 'composite'-alternative" action on the marketplace as well and casually merge your upstream changes and PR my changes/ contributions to my fork back to your repository? I do think that both (the cross-platform compatible docker version and the linux/ubuntu only, but faster composite version) would have their justification to co-exist.

Another approach would be to have both of them in your repository, but on different branches and tagging them as v2.0.0 and v2.0.0-composite, but I'm unsure if the marketplace can handle these tags correctly.

I'd of course also provide a link back to your repository such as:

---

Cross-platform alternative

This is a composite-action fork of the docker based cross-platform compatible project distributhor/workflow-webhook. If you need to run on anything different than ubuntu check out distributhor's project.

[distributhor]

I'm totally fine with it. If someone can benefit, it's all good. Once I've had a look at all the options available for caching and performance improvements, I will decide on an appropriate course of action for the project, but would lean towards supporting wider, as opposed to being more restrictive. Will definitely still look at what you've done and take it into consideration. But in the meantime you are more than welcome to create an alternative project and pulling upstream changes. No problem with that.

[distributhor]

Just a quick update. I've tested several options over the weekend, to determine best way forward for performance improvements. I first tried to play with docker image caching, but it didn't result in a major improvement - it was only fractionally faster.

I then looked into the composite actions, as you have in the fork, and this does make a substantial difference. The only "downside" is that you are then tied a little closer to the environment in which it turns, and I'd like (as far as possible) to remain agnostic on that aspect.

The last thing I tried was to pre-build the docker image, and host it on docker hub. The action then won't have to build the image, but can simply pull it. This is also a lot faster, and is almost comparable to the composite actions in terms of speed.

After exploring all those avenues, I am going to update the project such that it will now use the pre-built docker image, so that here is no docker build step. The pre-built image uses the same Dockerfile that is in the project, so it all feels very clean, and the performance gains are good.

[johannes-huther]

Great, I didn't think about that option at all.

I think I will stick with the composite variant for my projects for now, as it is the method with least overhead and i run everything on ubuntu-latest anyway, even though the difference is minimal.

Some things you might consider (if you haven't already):

• Use a github action to deploy a new image to the registry on each push to master (or each release) and tag the container automatically.
• Try to use an environment variable for the docker tag in action.yml. Not sure if this is possible though :D
• Try using the github container registry (not sure if this has any benefits, but they might be hosted in the same datacenter as GitHub actions, so maybe the download time is slightly faster?; also you would have repo and images in one place)