pipeline icon indicating copy to clipboard operation
pipeline copied to clipboard

It is not possible to have the webhook controller _not_ use TLS

Open abayer opened this issue 1 year ago • 1 comments

Expected Behavior

If the WEBHOOK_SECRET_NAME env var isn't set on the webhook container, it should listen on plain HTTP, with no TLS.

Actual Behavior

If the WEBHOOK_SECRET_NAME env var isn't set, a default value of webhook-certs is used instead. If that secret does not exist, it'll error out. As a result, there's no way to have the webhook controller serve plain HTTP.

Steps to Reproduce the Problem

  1. Delete the webhook-certs secret
  2. Remove the WEBHOOK_SECRET_NAME env var from the tekton-pipelines-webhook deployment.
  3. Change the WEBHOOK_PORT env var in the tekton-pipelines-webhook deployment to something other than 8443, like 8088 (8080 is already in use), just for clarity. =)
  4. Change the https-webhook in ports in the tekton-pipelines-webhook deployment to http-webhook, with the value set to the same port you used in the WEBHOOK_PORT env var above.
  5. In the tekton-pipelines-webhook service, add a new entry in ports with name set to http-webhook, port set to 80, and targetPort set to http-webhook.
  6. This part requires that you have some sort of TLS termination setup - we've got an internal system that does this via a sidecar, and I don't have enough familiarity with, say, how you would do this with Istio, but the end result should be that the TLS setup should serve on 443 (with the https-webhook targetPort in the tekton-pipelines-webhook service changed from https-webhook to whatever port your TLS terminator is serving on), in front of the WEBHOOK_PORT you specified above. This is because admission controller webhook services have to be on https on port 443.
  7. Now that everything's set up, an attempt to create a new Tekton resource should fail because port 8088 or whatever isn't actually open on the webhook container.

Additional Info

  • Tekton Pipeline version:

Tested directly on v0.45.0 and the latest release, v0.61.1.

We're trying to switch from the k8s secret webhook-certs for TLS to instead using our internal TLS proxying tooling, and that, well, does not work. The reason for this is https://github.com/tektoncd/pipeline/blob/95fbf318460a2f73ac505bbdb13786a788d1c092/cmd/webhook/main.go#L238-L241 - if WEBHOOK_SECRET_NAME is empty, it is defaulted to webhook-certs, rather than leaving it empty, and the Knative webhook tooling will always serve on TLS, trying to use that secret.

abayer avatar Jul 25 '24 15:07 abayer

fwiw, I'd like to propose that we change that code to:

	secretName, present := os.LookupEnv("WEBHOOK_SECRET_NAME")
	if !present {
		secretName = "webhook-certs" // #nosec
	}

This would retain the current behavior in any case where the WEBHOOK_SECRET_NAME env var isn't present at all, but would let a user explicitly set WEBHOOK_SECRET_NAME to "" in order to just serve HTTP without TLS.

abayer avatar Jul 25 '24 15:07 abayer