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

(golang/v4) Generate local webhooks for make run #4477

Open
damsien opened this issue Jan 9, 2025 · 8 comments
Open

(golang/v4) Generate local webhooks for make run #4477

damsien opened this issue Jan 9, 2025 · 8 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@damsien
Copy link
Contributor

damsien commented Jan 9, 2025

What do you want to happen?

Description of the needs

As a k8s operator developer, I would like to test my webhooks by using make run.

The "classic way" to quickly test the manager logic is to execute make run, create some resources, check the logs at the same time, CTRL+C, change the code, make run, etc... However, as for today, we cannot test the webhook server logic by executing make run since the the manager is launched within the shell and not as a Pod in a container. Therefore, the MutatingWebhookConfiguration & ValidatingWebhookConfiguration objects cannot request the Service attached to the manager's Pod (since there is neither Service nor Pod).

Is this really needed?

Today, the only way to test the webhook server logic is either to write tests in the my_webhook_test.go file or by deploying the operator logic as a Pod in a cluster by using make deploy. The problem of these two methods is that it takes some time to be run whereas make run is fast (because it is meant to be fast to run) and allow us to test our operator the "classic way".

Also, as we can we see in this issue #400, even if it has been closed, people still have a discussion about an implementation of this feature.

Solution proposal

My solution is based on this issue comment #400 (comment).

1. Update the Makefile

Set the LOCALHOST_BRIDGE variable. This is the docker0 bridge address that allows any container to make a request the loopback address of the host.

  • On MacOS, you can reach the host from inside your docker containers by using the hostname host.docker.internal
  • On Linux, you can reach the host from inside your docker containers by using the IP Address 172.17.0.1 (this might change depending on your docker network configuration, but by default this is the one).

Execute a script to generate the caBundle & the server certificate (make gen-local-certs ?). This script can be located in /hack?

2. Certificate generation script

This script will first generate a CA bundle & certificate using the docker0 bridge host as SAN

[alt_names]
DNS.1 = $LOCALHOST_BRIDGE
IP.1 = $LOCALHOST_BRIDGE

The ca.crt & tls.crt will be located int the /tmp/k8s-webhook-server/serving-certs directory since controller-runtime will look at it first. Therefore, we do not have to modify our main.go to specify a temporary custom cert dir.

https://github.com/kubernetes-sigs/controller-runtime/blob/2484715d39199d4e02628c79e9be7c6f76fae28b/pkg/webhook/server.go#L143-L145

And then, it will generate a local webhook manifest on the flight based on the config/webhook/manifests.yaml. Thus, make manifests must be executed before (the same way as we use make deploy). The generation will basically copy paste the content of the manifests.yaml and update the clientConfig part.

Convert

clientConfig:
  service:
    name: webhook-service
    namespace: system
    path: /validate-my-domain-apiversion-my-kind

into

clientConfig:
  caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0....
  url: https://$LOCALHOST_BRIDGE/validate-my-domain-apiversion-my-kind

This part of the script removes the clientConfig.service specification and place url instead. Also, it will inject the generated CA bundle.

3. Apply the webhook

Finally, we have to apply the local webhook manifest on the cluster.

TL;DR

Create a script that generate a certificate and inject it in the webhook object. Then, the webhook make the request toward the docker bridge. The manager that runs within the shell (so it listen on the loopback address) will receive the request an can process the webhook request.

Do no hesitate to ask me questions since I already implemented this mechanism in the operator I am working on.

Extra Labels

No response

@damsien damsien added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 9, 2025
@camilamacedo86
Copy link
Member

camilamacedo86 commented Jan 9, 2025

Hi @damsien,

Thank you for raising this issue. Note that now is possible to pass the certs path.

make run ARGS="--webhook-cert-path=/tmp/certs --webhook-cert-name=tls.crt --webhook-cert-key=tls.key"

So, I think that what you are suggesting here would be something like


Proposed Solution (Example)

  • Update the Makefile
LOCALHOST_BRIDGE ?= $(shell docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

gen-local-certs:
	./hack/gen-certs.sh $(LOCALHOST_BRIDGE)

run-local: gen-local-certs
	@echo "Updating webhook manifests with sed..."
	CA_BUNDLE=$$(cat /tmp/k8s-webhook-server/serving-certs/ca.crt | base64 -w 0) && \
	sed -e "s|service:.*|url: https://$(LOCALHOST_BRIDGE)|" \
	    -e "s|caBundle:.*|caBundle: $$CA_BUNDLE|" \
	    config/webhook/manifests.yaml > config/webhook/local-manifests.yaml
	kubectl apply -f config/webhook/local-manifests.yaml
	ARGS="--webhook-cert-path=/tmp/k8s-webhook-server/serving-certs" make run
  • Add a script to generate the certs locally (for dev purpose)

Example hack/gen-certs.sh

#!/bin/bash

set -e

LOCALHOST_BRIDGE=$1
CERT_DIR="/tmp/k8s-webhook-server/serving-certs"
mkdir -p ${CERT_DIR}

# Generate CA
openssl genrsa -out ${CERT_DIR}/ca.key 2048
openssl req -x509 -new -nodes -key ${CERT_DIR}/ca.key -sha256 -days 365 \
  -subj "/CN=Webhook-CA" -out ${CERT_DIR}/ca.crt

# Generate server cert
openssl genrsa -out ${CERT_DIR}/tls.key 2048
cat > ${CERT_DIR}/server.conf <<EOF
[req]
distinguished_name = req_distinguished_name
x509_extensions = v3_req
[req_distinguished_name]
[v3_req]
keyUsage = keyEncipherment, dataEncipherment
extendedKeyUsage = serverAuth
subjectAltName = @alt_names
[alt_names]
IP.1 = ${LOCALHOST_BRIDGE}
EOF

openssl req -new -key ${CERT_DIR}/tls.key -out ${CERT_DIR}/tls.csr -config ${CERT_DIR}/server.conf
openssl x509 -req -in ${CERT_DIR}/tls.csr -CA ${CERT_DIR}/ca.crt -CAkey ${CERT_DIR}/ca.key \
  -CAcreateserial -out ${CERT_DIR}/tls.crt -days 365 -extensions v3_req -extfile ${CERT_DIR}/server.conf

Then, we could execute make run

Key Questions

However, I believe we might want to think about:

  • Should we provide a patch or overlay that allows projects to work without requiring cert-manager? This could address scenarios like those discussed in request for guides/examples on using existing self-signed certificates in wehhook instead of relying on cert-manager #4292.
  • Should we create a specific patch/overlay for development purposes, enabling webhooks to run with locally generated certificates instead of the SED approach?
  • Alternatively, should we adopt a straightforward solution as suggested, where the make run command scaffolds webhooks specifically for development purposes? PS.:For projects without webhooks, this would mean no additional scaffolding is added. IHMO: If we take this route, implementation should happen under the webhook subcommand instead of the init command.

Perhaps we could start by identifying the best and recommended approaches for the concerns raised in #4292. From there, we can evaluate whether changes are needed in the scaffold and decide on the best course of action.

I am looking forward to your thoughts.

@damsien
Copy link
Contributor Author

damsien commented Jan 10, 2025

Hello @camilamacedo86 , thanks for you answer 😀

Should we provide a patch/overlay?

For me, kubebuilder provides 2 levels of testing: run manager directly through the shell (make run) and run manager in a Pod (make deploy).

First, I think that this feature is necessary for the make run command because we do not have another way to test webhooks. It is not necessary for the make deploy command (because we can install cert-manager) but I don't think that people use it for real production deployment. I think that they usually use it for development purpose as well, but this time for testing the operator within a cluster with real resources.

That leads to a question: Do we consider the make deploy command to serve development purposes?

If the answer is yes, then I think that we could provide a patch/overlay to allow projects to work without cert-manager. I think it is better than writing a documentation that explains how to generate a certificate and use it (especially if it can be automated).

If the answer is no, then we can just keep the implementation example of your comment for the make run-local command.

In both of the case, I think that we still need to let the option to use cert-manager.

When do we scaffold?

I don't think that scaffolding at the kubebuilder init time is a big deal since we generate the certs with the make run-local command and not the make run one. I don't see this feature as a plugin like helm since it is a base component of kubernetes.

However, if you consider scaffolding at the first kubebuilder create webhook command execution, then we can take this route. I don't really have an opinion about this one to be honest 😅

@camilamacedo86
Copy link
Member

camilamacedo86 commented Jan 10, 2025

Hi @damsien

That leads to a question: Do we consider the make deploy command to serve development purposes?

Yes, it is.
After you develop your solution and you want to distribute it you should not provide the source code.
Ideally, you should provide only the YAML and host the image, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4/README.md#project-distribution

Also, you can package it in other ways. But the make deploy is ideally for dev purposes only.

@damsien
Copy link
Contributor Author

damsien commented Jan 12, 2025

Hello @camilamacedo86

Okay, it confirms what I thought.

Patch/overlay

Concerning the patch/overlay, I think we could place it in a config/local folder? I think that if we put in a different folder, it avoids any confusion between the dev manifests and the ones that will be provided for distribution. WDYT?
The config/local/kustomization.yaml must not be referenced in the resources: section of the config/default/kustomization.yaml to avoid providing it in the dist folder when executing make build-installer.

In the make run & make deploy, we need to build the local webhooks if the folder exists.

@if [ -d "config/local" ]; then \
	$(KUSTOMIZE) build config/local | $(KUBECTL) apply -f - \
fi

Webhook target

Also, we have to think that the webhook should target the docker bridge when running make run BUT it should target the manager's Service when running make deploy. In the case of make deploy, we only need to inject the caBundle without modifying the rest of the structure.

Also, in the case of make deploy, we have to remove the caBundle part when running make undeploy.
It is the same problem as the validating webhook

Conversion webhooks

I also wanted to point out that we have to think about CRD's conversion webhooks. We have to do the same injection based on the config/crd/patches/webhook_in_my-crd.yaml instead.

@camilamacedo86
Copy link
Member

Hi @damsien,

What do you think would be the best approach for users who don’t want to use a cert-manager?

I believe we might need to address this scenario (see: #4292).

To help us make a decision, here are a couple of considerations:

  • Should we provide a default configuration that works without a cert-manager?
  • Generating certificates directly doesn’t seem appropriate for production use. However, a configuration that supports this approach might suffice for make-run scenarios, with the certificates generated via a hack script.

WDYT?

@appiepollo14
Copy link

To make matters more complex, maybe. Please take into account that there are multiple containertails configurable in the makefile. I'm using Podman, which results into an error when using:

LOCALHOST_BRIDGE ?= $(shell docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')
How to keep this a generic working option or is this out of scope?

@damsien
Copy link
Contributor Author

damsien commented Jan 13, 2025

Hello @camilamacedo86,

Yes I think we should provide a default configuration for dev purposes that works without cert-manager. The goal is to make kubebuilder the most user-friendly as possible. Therefore, when an user starts using kubebuilder for creating webhooks, he should be able to easily test it.

And yes, we need to make a clear distinction between the development purpose and the production use.

Hello @appiepollo14,

Concerning your use case, we have two options:

  • Harden LOCALHOST_BRIDGE ?= 172.17.0.1 and let the user change to host.docker.internal if it uses a MAC or another IP address it it uses another container tool
  • Harden LOCALHOST_BRIDGE ?= $(shell docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}') and let the use change to $(shell podman network inspect podman --format='{{(index (index (index .plugins 0).plugins 0).ipam.ranges 0).gateway}}') (if using podman)

In both use cases, we should document the way to change this variable depending on the user's environment.

@camilamacedo86
Copy link
Member

I think it might be worth considering a configuration option for production that operates without CertManager. This wouldn't mean that the configuration itself generates certificates, but rather that it includes the necessary scaffolding to integrate certificates when provided externally.

Given this, I believe it's a good idea to explore and study the available alternatives to achieve the same functionality without CertManager. This investigation will provide a better foundation for our discussions on this topic. For reference, see: kubebuilder issue #4292.

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

No branches or pull requests

3 participants