From 7700358764753bbceb62688b22ba2d62ce58972f Mon Sep 17 00:00:00 2001 From: Jan Novak Date: Sat, 8 Aug 2026 13:18:45 +0200 Subject: [PATCH] Remove unused cert-manager/webhook scaffolding kubebuilder's generic scaffold defensively wires up webhook TLS-cert machinery and an unconditional cert-manager install in the e2e suite, in case a project grows admission webhooks later. This one never will -- the spec's non-goals explicitly rule out admission webhooks and cert-manager wiring -- so none of it does anything. Verified before removing: no config/webhook/, no +kubebuilder:webhook markers anywhere, and config/*/kustomization.yaml's [CERTMANAGER] blocks are all inert (never uncommented). cmd/main.go: drops the webhook import, the three webhook-cert-* flags, and the WebhookServer wiring on ctrl.Options -- the manager now runs with no webhook server, correctly, since nothing registers one. Left the metrics-cert flags alone; those are unrelated to webhooks. test/e2e/e2e_suite_test.go: drops the unconditional cert-manager install/uninstall around the suite. test/utils/utils.go: drops the now-dead InstallCertManager/ UninstallCertManager/IsCertManagerCRDsInstalled and their warnError helper, plus UncommentCode -- unrelated to cert-manager, but found to have zero callers even before this cleanup. Left the inert commented-out [WEBHOOK]/[CERTMANAGER] kustomize blocks and kubebuilder's scaffold marker comments alone: pure comments, no runtime behavior, unlike the cert-manager install this actually removed. Verified clean with both build tags (go build/vet, and -tags=e2e for test/e2e). make test unchanged and green. Co-Authored-By: Claude --- .claude/settings.json | 3 +- cmd/main.go | 33 +---- .../2026-08-07-1747-proxy-operator.md | 67 +++++++++ test/e2e/e2e_suite_test.go | 53 +------ test/utils/utils.go | 135 ------------------ 5 files changed, 76 insertions(+), 215 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 0d2c4d9..658ab11 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -55,7 +55,8 @@ "Bash(go doc *)", "Bash(go list *)", "Bash(gofmt -w internal/provider/config.go)", - "Bash(gofmt -l .)" + "Bash(gofmt -l .)", + "Bash(git restore *)" ], "additionalDirectories": [ "/Users/jan.novak/srv/go/egress-proxies-operator/.claude", diff --git a/cmd/main.go b/cmd/main.go index 7d9e2e7..14eb9d2 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -33,7 +33,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" crawlv1alpha1 "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/api/v1alpha1" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/controller" @@ -56,7 +55,6 @@ func init() { func main() { var metricsAddr string var metricsCertPath, metricsCertName, metricsCertKey string - var webhookCertPath, webhookCertName, webhookCertKey string var enableLeaderElection bool var probeAddr string var secureMetrics bool @@ -70,15 +68,12 @@ func main() { "Enabling this will ensure there is only one active controller manager.") flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") - flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.") - flag.StringVar(&webhookCertName, "webhook-cert-name", "tls.crt", "The name of the webhook certificate file.") - flag.StringVar(&webhookCertKey, "webhook-cert-key", "tls.key", "The name of the webhook key file.") flag.StringVar(&metricsCertPath, "metrics-cert-path", "", "The directory that contains the metrics server certificate.") flag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.") flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") flag.BoolVar(&enableHTTP2, "enable-http2", false, - "If set, HTTP/2 will be enabled for the metrics and webhook servers") + "If set, HTTP/2 will be enabled for the metrics server") opts := zap.Options{ Development: true, } @@ -102,23 +97,6 @@ func main() { tlsOpts = append(tlsOpts, disableHTTP2) } - // Initial webhook TLS options - webhookTLSOpts := tlsOpts - webhookServerOptions := webhook.Options{ - TLSOpts: webhookTLSOpts, - } - - if len(webhookCertPath) > 0 { - setupLog.Info("Initializing webhook certificate watcher using provided certificates", - "webhook-cert-path", webhookCertPath, "webhook-cert-name", webhookCertName, "webhook-cert-key", webhookCertKey) - - webhookServerOptions.CertDir = webhookCertPath - webhookServerOptions.CertName = webhookCertName - webhookServerOptions.KeyName = webhookCertKey - } - - webhookServer := webhook.NewServer(webhookServerOptions) - // Metrics endpoint is enabled in 'config/default/kustomization.yaml'. The Metrics options configure the server. // More info: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.24.1/pkg/metrics/server @@ -139,12 +117,8 @@ func main() { // If the certificate is not specified, controller-runtime will automatically // generate self-signed certificates for the metrics server. While convenient for development and testing, - // this setup is not recommended for production. - // - // TODO(user): If you enable certManager, uncomment the following lines: - // - [METRICS-WITH-CERTS] at config/default/kustomization.yaml to generate and use certificates - // managed by cert-manager for the metrics server. - // - [PROMETHEUS-WITH-CERTS] at config/prometheus/kustomization.yaml for TLS certification. + // this setup is not recommended for production. This project doesn't use cert-manager (no admission + // webhooks, no other consumer of managed certs) -- pass real certs via the flags below if needed. if len(metricsCertPath) > 0 { setupLog.Info("Initializing metrics certificate watcher using provided certificates", "metrics-cert-path", metricsCertPath, "metrics-cert-name", metricsCertName, "metrics-cert-key", metricsCertKey) @@ -157,7 +131,6 @@ func main() { mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, Metrics: metricsServerOptions, - WebhookServer: webhookServer, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "b47711d1.example.com", diff --git a/docs/plans-executions/2026-08-07-1747-proxy-operator.md b/docs/plans-executions/2026-08-07-1747-proxy-operator.md index 8baac0d..868be21 100644 --- a/docs/plans-executions/2026-08-07-1747-proxy-operator.md +++ b/docs/plans-executions/2026-08-07-1747-proxy-operator.md @@ -410,3 +410,70 @@ meaningfully uncovered function is `New()` itself, deliberately. `make test` green across the whole repo (`go build`/`go vet` clean, `internal/provider` 96.0%, `internal/provider/kubernetes` 77.6%, `internal/provider/registry` 100%, unchanged). + +## Cleanup — removed unused cert-manager/webhook scaffolding + +Not a plan step; the user asked for this directly after I explained what +`make test-e2e` currently does, and didn't want unused scaffold machinery +carried forward into later work. + +kubebuilder's generic scaffold assumes a project *might* grow admission +webhooks later, so it wires up webhook TLS-cert machinery and an +unconditional cert-manager install in the e2e suite defensively. Checked +whether any of it was actually load-bearing before touching anything: + +```bash +grep -rn "cert-manager\|certmanager\|CertManager" config/ +# only inside commented-out [CERTMANAGER] blocks in kustomization.yaml — +# the whole block is inert, never uncommented +grep -n "webhook" PROJECT +# no output — kubebuilder create webhook was never run +``` + +Confirmed nothing here does anything for this project — no `config/webhook/` +exists, no `+kubebuilder:webhook` markers exist anywhere, and the spec's +own non-goals explicitly rule out admission webhooks and cert-manager +wiring forever, not just "not yet." + +Removed: + +- **`cmd/main.go`**: the `webhook` import, the three `webhook-cert-*` + flags, the `webhookServerOptions`/`webhookServer` construction, and the + `WebhookServer:` field on `ctrl.Options` — the manager runs with no + webhook server at all now, which is correct since nothing registers one. + Left the metrics-cert flags alone (`--metrics-cert-path` etc.) — those + let real certs be mounted for the metrics endpoint without cert-manager, + which is unrelated to webhooks and still useful. Reworded a comment that + said "TODO(user): If you enable certManager..." since that will never + happen here. +- **`test/e2e/e2e_suite_test.go`**: the unconditional cert-manager install + in `BeforeSuite` and matching uninstall in `AfterSuite` + (`setupCertManager`/`teardownCertManager`/`shouldCleanupCertManager`), + and the doc comment claiming the suite "requires Kind and CertManager" + (it only requires Kind now). +- **`test/utils/utils.go`**: `InstallCertManager`, `UninstallCertManager`, + `IsCertManagerCRDsInstalled`, and their now-unused `warnError` helper and + `certmanagerVersion`/`certmanagerURLTmpl` constants. Also removed + `UncommentCode` — grepped first and confirmed it had zero callers even + before this cleanup; it was dead scaffold code from the start, unrelated + to cert-manager, just found while in there. + +Left the inert commented-out `[WEBHOOK]`/`[CERTMANAGER]` blocks in the +`config/*/kustomization.yaml` files and the +`+kubebuilder:scaffold:e2e-webhooks-checks`-style marker comments in +`test/e2e/e2e_test.go` alone — those are standard kubebuilder codegen +anchors and pure comments with no runtime behavior, unlike the cert-manager +install this cleanup actually removed. Stripping every trace of "webhook" +from every scaffold comment across the tree would be a much bigger, purely +cosmetic diff for no behavioral benefit; this cleanup targeted the things +that were actually *doing* something. + +Verified with both build tags, since `test/e2e` only compiles under `-tags=e2e`: + +```bash +go build ./... && go vet ./... +go build -tags=e2e ./... && go vet -tags=e2e ./... +``` + +Both clean. `make test` green across the whole repo, unchanged from before +the cleanup. diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index cdb7926..73efcbe 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -31,19 +31,14 @@ import ( "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/test/utils" ) -var ( - // managerImage is the manager image to be built and loaded for testing. - managerImage = "example.com/egress-proxies-operator:v0.0.1" - // shouldCleanupCertManager tracks whether CertManager was installed by this suite. - shouldCleanupCertManager = false -) +// managerImage is the manager image to be built and loaded for testing. +var managerImage = "example.com/egress-proxies-operator:v0.0.1" -// TestE2E runs the e2e test suite to validate the solution in an isolated environment. -// The default setup requires Kind and CertManager. +// TestE2E runs the e2e test suite to validate the solution in an isolated +// environment. The default setup requires Kind. // // To enable kubectl kuberc (use custom kubectl configurations), set: KUBECTL_KUBERC=true // By default, kuberc is disabled to ensure consistent test behavior across different environments. -// To skip CertManager installation, set: CERT_MANAGER_INSTALL_SKIP=true func TestE2E(t *testing.T) { RegisterFailHandler(Fail) _, _ = fmt.Fprintf(GinkgoWriter, "Starting egress-proxies-operator e2e test suite\n") @@ -56,18 +51,11 @@ var _ = BeforeSuite(func() { _, err := utils.Run(cmd) ExpectWithOffset(1, err).NotTo(HaveOccurred(), "Failed to build the manager image") - // TODO(user): If you want to change the e2e test vendor from Kind, - // ensure the image is built and available, then remove the following block. By("loading the manager image on Kind") err = utils.LoadImageToKindClusterWithName(managerImage) ExpectWithOffset(1, err).NotTo(HaveOccurred(), "Failed to load the manager image into Kind") configureKubectlKubeRC() - setupCertManager() -}) - -var _ = AfterSuite(func() { - teardownCertManager() }) // Disable kubectl kuberc by default for test isolation. @@ -84,36 +72,3 @@ func configureKubectlKubeRC() { _, _ = fmt.Fprintf(GinkgoWriter, "kubectl kuberc enabled (KUBECTL_KUBERC=true)\n") } } - -// setupCertManager installs CertManager if needed for webhook tests. -// Skips installation if CERT_MANAGER_INSTALL_SKIP=true or if already present. -func setupCertManager() { - if os.Getenv("CERT_MANAGER_INSTALL_SKIP") == "true" { - _, _ = fmt.Fprintf(GinkgoWriter, "Skipping CertManager installation (CERT_MANAGER_INSTALL_SKIP=true)\n") - return - } - - By("checking if CertManager is already installed") - if utils.IsCertManagerCRDsInstalled() { - _, _ = fmt.Fprintf(GinkgoWriter, "CertManager is already installed. Skipping installation.\n") - return - } - - // Mark for cleanup before installation to handle interruptions and partial installs. - shouldCleanupCertManager = true - - By("installing CertManager") - Expect(utils.InstallCertManager()).To(Succeed(), "Failed to install CertManager") -} - -// teardownCertManager uninstalls CertManager if it was installed by setupCertManager. -// This ensures we only remove what we installed. -func teardownCertManager() { - if !shouldCleanupCertManager { - _, _ = fmt.Fprintf(GinkgoWriter, "Skipping CertManager cleanup (not installed by this suite)\n") - return - } - - By("uninstalling CertManager") - utils.UninstallCertManager() -} diff --git a/test/utils/utils.go b/test/utils/utils.go index a408630..54a9ddb 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -17,8 +17,6 @@ limitations under the License. package utils import ( - "bufio" - "bytes" "fmt" "os" "os/exec" @@ -28,17 +26,10 @@ import ( ) const ( - certmanagerVersion = "v1.20.2" - certmanagerURLTmpl = "https://github.com/cert-manager/cert-manager/releases/download/%s/cert-manager.yaml" - defaultKindBinary = "kind" defaultKindCluster = "kind" ) -func warnError(err error) { - _, _ = fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) -} - // Run executes the provided command within this context func Run(cmd *exec.Cmd) (string, error) { dir, _ := GetProjectDir() @@ -59,80 +50,6 @@ func Run(cmd *exec.Cmd) (string, error) { return string(output), nil } -// UninstallCertManager uninstalls the cert manager -func UninstallCertManager() { - url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) - cmd := exec.Command("kubectl", "delete", "-f", url) - if _, err := Run(cmd); err != nil { - warnError(err) - } - - // Delete leftover leases in kube-system (not cleaned by default) - kubeSystemLeases := []string{ - "cert-manager-cainjector-leader-election", - "cert-manager-controller", - } - for _, lease := range kubeSystemLeases { - cmd = exec.Command("kubectl", "delete", "lease", lease, - "-n", "kube-system", "--ignore-not-found", "--force", "--grace-period=0") - if _, err := Run(cmd); err != nil { - warnError(err) - } - } -} - -// InstallCertManager installs the cert manager bundle. -func InstallCertManager() error { - url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) - cmd := exec.Command("kubectl", "apply", "-f", url) - if _, err := Run(cmd); err != nil { - return err - } - // Wait for cert-manager-webhook to be ready, which can take time if cert-manager - // was re-installed after uninstalling on a cluster. - cmd = exec.Command("kubectl", "wait", "deployment.apps/cert-manager-webhook", - "--for", "condition=Available", - "--namespace", "cert-manager", - "--timeout", "5m", - ) - - _, err := Run(cmd) - return err -} - -// IsCertManagerCRDsInstalled checks if any Cert Manager CRDs are installed -// by verifying the existence of key CRDs related to Cert Manager. -func IsCertManagerCRDsInstalled() bool { - // List of common Cert Manager CRDs - certManagerCRDs := []string{ - "certificates.cert-manager.io", - "issuers.cert-manager.io", - "clusterissuers.cert-manager.io", - "certificaterequests.cert-manager.io", - "orders.acme.cert-manager.io", - "challenges.acme.cert-manager.io", - } - - // Execute the kubectl command to get all CRDs - cmd := exec.Command("kubectl", "get", "crds") - output, err := Run(cmd) - if err != nil { - return false - } - - // Check if any of the Cert Manager CRDs are present - crdList := GetNonEmptyLines(output) - for _, crd := range certManagerCRDs { - for _, line := range crdList { - if strings.Contains(line, crd) { - return true - } - } - } - - return false -} - // LoadImageToKindClusterWithName loads a local docker image to the kind cluster func LoadImageToKindClusterWithName(name string) error { cluster := defaultKindCluster @@ -172,55 +89,3 @@ func GetProjectDir() (string, error) { wd = strings.ReplaceAll(wd, "/test/e2e", "") return wd, nil } - -// UncommentCode searches for target in the file and remove the comment prefix -// of the target content. The target content may span multiple lines. -func UncommentCode(filename, target, prefix string) error { - // false positive - // nolint:gosec - content, err := os.ReadFile(filename) - if err != nil { - return fmt.Errorf("failed to read file %q: %w", filename, err) - } - strContent := string(content) - - idx := strings.Index(strContent, target) - if idx < 0 { - return fmt.Errorf("unable to find the code %q to be uncommented", target) - } - - out := new(bytes.Buffer) - _, err = out.Write(content[:idx]) - if err != nil { - return fmt.Errorf("failed to write to output: %w", err) - } - - scanner := bufio.NewScanner(bytes.NewBufferString(target)) - if !scanner.Scan() { - return nil - } - for { - if _, err = out.WriteString(strings.TrimPrefix(scanner.Text(), prefix)); err != nil { - return fmt.Errorf("failed to write to output: %w", err) - } - // Avoid writing a newline in case the previous line was the last in target. - if !scanner.Scan() { - break - } - if _, err = out.WriteString("\n"); err != nil { - return fmt.Errorf("failed to write to output: %w", err) - } - } - - if _, err = out.Write(content[idx+len(target):]); err != nil { - return fmt.Errorf("failed to write to output: %w", err) - } - - // false positive - // nolint:gosec - if err = os.WriteFile(filename, out.Bytes(), 0644); err != nil { - return fmt.Errorf("failed to write file %q: %w", filename, err) - } - - return nil -}