Allow TLS configuration for githubwebhookserver
Is your feature request related to a problem? Please describe. I would like to configure end to end TLS for the githubwebhookserver.
Describe the solution you'd like I would like to have the githubwebhookserver listen for tls connections when configured with a host and certificate bundle secret name.
Describe alternatives you've considered The only alternative I'm aware of is to terminate TLS at ingress.
There isn't much documentation around the githubwebhookserver so it's difficult to know what a typical configuration is or should look like. Browsing the code it doesn't appear that this feature is supported though.
Hello 👋
This a feature I am interested in as well and I was wondering if there is any estimation and plans for its addition?
Thanks, Dimitar
@sethamclean @DimitarHristov111 Hey! Just curious, why don't you terminate TLS at ingress?
@sethamclean @DimitarHristov111 Hey! Just curious, why don't you terminate TLS at ingress?
We do now because it's our only option. End to end TLS would be more secure since secrets are being transmitted in clear text from the point of termination to the web hook container.
@sethamclean @DimitarHristov111 Hey! Just curious, why don't you terminate TLS at ingress?
Hey 👋 Our setup is slightly different, but we again do something similar. And we still hope to have it as an end-to-end encryption which is why we are interested in the feature.
Thanks! Interesting. So you tend to do/request the same for every application that exposes an HTTP server on its own? Or is ARC special? Can it be an option for you to use a service mesh of sorts or a CNI plugin like Cilium that can provide you end-to-end between pods?
Hello @mumoshu I work with @DimitarHristov111 so I can give you a little more context. We have some clusters where only ARC is running, so:
- we didn't want to add the service mesh as we feel it's an overkill
- we don't use Cilium as CNI and our CNI can't do TLS termination
- we're "just" using the AWS Load Balancer controller to configure the LB in front of the service. This works well for other services where we can mount TLS certificates generated for example by cert-manager
The options I see here are:
- Contribute to the project to allow TLS connections
- Add a sidecar in front of the
githubwebhookserverso we do the TLS termination there. This maybe need just a change to the chart to allow us to inject sidecar container and then it will be up to us decide how to do the TLS termination.
What do you think?
@maruina Thanks for clarifying! That makes sense. Either 1 or 2 sounds good.
I'd guess the option 2 is easier to contribute/review (Probably adding sidecarContainers and volumes under githubWebhookServer would work:
https://github.com/actions-runner-controller/actions-runner-controller/blob/667764e02734938e0d0745a3098e66d3e086f7b7/charts/actions-runner-controller/values.yaml#L175)
The option 1 might be more useable though. In case you could contribute the option 1, I'd guess adding--cert-file and --key-file to the githubwebhookserver and enhancing a template in the chart to enable specifying those flags (along with additional volumes if the key and the cert is being provided via a k8s secret) would work.
githubwebhookserver's command: https://github.com/actions-runner-controller/actions-runner-controller/blob/667764e02734938e0d0745a3098e66d3e086f7b7/cmd/githubwebhookserver/main.go#L88-L104 The relevant part of the chart template: https://github.com/actions-runner-controller/actions-runner-controller/blob/667764e02734938e0d0745a3098e66d3e086f7b7/charts/actions-runner-controller/templates/githubwebhook.deployment.yaml#L38-L51
Thanks @mumoshu for your input. I will have a chat with @DimitarHristov111 and then we will come back to you :)
Hello, we think we will go with the second option. It might be a bit more difficult, but it seems like the better solution. :)
I found the following guide https://github.com/actions-runner-controller/actions-runner-controller/blob/master/CONTRIBUTING.md for contributing to the project. Do you think there is anything else we should consider as well before doing the contribution?
@DimitarHristov111 Hey! I might not have covered how to contribute to the chart but hopefully, there are not many variables there.
Probably, all you need before finally submitting a pull request would be to (1) clone the repo (2) install the chart locally by running helm upgrade --install -f values.yaml $RELEASE_NAME ./charts/actions-runner-controller (3) modify the chart locally (4) rerun the helm upgrade --install.
Note that you don't need to bump the chart version as we use a workflow to automate publishing the chart and bumping the chart version triggers that, although we want to control when to publish the version(so that end users won't receive too many notifications about new chart versions