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

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

Open
abayer opened this issue Jul 25, 2024 · 1 comment
Open

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

abayer opened this issue Jul 25, 2024 · 1 comment
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@abayer
Copy link
Contributor

abayer commented Jul 25, 2024

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

secretName := os.Getenv("WEBHOOK_SECRET_NAME")
if secretName == "" {
secretName = "webhook-certs" // #nosec
}
- 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 abayer added the kind/bug Categorizes issue or PR as related to a bug. label Jul 25, 2024
@abayer
Copy link
Contributor Author

abayer commented Jul 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

1 participant