fix: make the kustomize manifests deployable and consistent - #2835
Open
AlinsRan wants to merge 4 commits into
Open
fix: make the kustomize manifests deployable and consistent#2835AlinsRan wants to merge 4 commits into
AlinsRan wants to merge 4 commits into
Conversation
`kustomize build config/default` produces a deployment that cannot talk to its own sidecar and a metrics Service that points at nothing: - The manager container does not set `ADC_SERVER_URL`, so the controller falls back to `http://127.0.0.1:3000` while the ADC sidecar only listens on `unix:/sockets/adc.sock`. The Helm chart sets it. - `metrics_service.yaml` exposes 8443/https, but the controller serves plain HTTP metrics on 8080 (`metrics_addr: ":8080"`, `secure_metrics: false`), and the manager declares no container port at all. The ServiceMonitor scrapes the same missing port. - The webhook Service selects `app.kubernetes.io/name: apisix-ingress-controller`, a label the pod template never carried, so it matches no endpoints. The webhook NetworkPolicy has the same mismatch. - `manager_webhook_patch.yaml` passes `--webhook-cert-path`, a flag the controller does not have, and mounts the serving certificate at `/tmp/k8s-webhook-server/serving-certs` instead of the `/certs` directory `webhook.tls_cert_dir` points at. The cert-manager CA injection that the patch depends on was never wired up either. - `certificate-metrics.yaml` issues a certificate the controller has no way to load: metrics TLS has no cert directory option. The webhook stays opt-in and `webhook.enable` stays false, as before; the [WEBHOOK] sections in config/default now describe how to turn it on and, unlike before, produce a working webhook when uncommented. Move the controller ConfigMap source from config/samples to config/manager, since it is part of the deployment rather than a sample, and let config/samples hold sample custom resources only. Complete the samples for all twelve CRDs and fill in the empty specs: the GatewayProxy sample was missing the required `provider` field, and the ApisixGlobalRule and ApisixPluginConfig samples were missing the required `plugins` field, so applying them was rejected outright. Add the apiVersion/kind header to every kustomization.yaml, drop the leftover kubebuilder scaffolding comments and use `apisix-ingress-controller` consistently for `app.kubernetes.io/name`.
AlinsRan
force-pushed
the
chore/refresh-kustomize-manifests
branch
from
August 5, 2026 05:32
eeb2b13 to
9a8a715
Compare
The kustomize manifests under config/ had no coverage: a broken patch, a Service pointing at a port nothing listens on and samples that the API server rejects outright all survived because nothing ever rendered or applied them. Build every kustomization, then bring up a kind cluster and server-side apply the rendered installer, which validates every object against the API server including the CRD schemas. The samples are then dry-run applied against those CRDs, so a sample missing a required field or violating a CEL rule fails the job. Also check that config/crd/bases and config/webhook/manifests.yaml match what controller-gen produces. config/rbac/role.yaml is left out of that check: it carries a pre-existing rule ordering difference against the pinned controller-gen version.
The pipeline swallowed both a failed `kubectl get crd` and an empty grep result, so the step passed without waiting for anything.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
kustomize build config/defaultcurrently produces a deployment that cannot work. This PR fixes the manifests. No existing default is flipped — the webhook stays opt-in andwebhook.enablestaysfalse.Broken today:
ADC_SERVER_URL, so the controller falls back tohttp://127.0.0.1:3000while the ADC sidecar only listens onunix:/sockets/adc.sock. The Helm chart sets it.8443/https, while the controller serves plain HTTP metrics on8080(metrics_addr: ":8080",secure_metrics: false), and the manager declares no container port at all. TheServiceMonitorscrapes the same missing port.app.kubernetes.io/name: apisix-ingress-controller, but the pod template only carriescontrol-plane: controller-manager, so the Service matches no endpoints. The webhookNetworkPolicyhas the same mismatch.manager_webhook_patch.yamlpasses--webhook-cert-path, a flag the controller does not have, and mounts the serving certificate at/tmp/k8s-webhook-server/serving-certsinstead of/certs, which is whatwebhook.tls_cert_dirpoints at. The cert-manager CA injection the patch depends on was never wired up either, so uncommenting the[WEBHOOK]sections did not yield a working webhook.certificate-metrics.yamlissues a certificate the controller cannot load — metrics TLS has no cert-directory option, so theCertificateis dead weight. Removed.The
[WEBHOOK]/[CERTMANAGER]sections stay commented out exactly as before; they now carry a correct patch and the cert-managerreplacements, so uncommenting them (plus settingwebhook.enable: true) produces a working webhook.Layout and samples:
config/samples/config.yamltoconfig/manager/config.yaml. It is part of the deployment, not a sample. The file content is unchanged.config/samplesnow covers all twelve CRDs (addedBackendTrafficPolicy,L4RoutePolicy,PluginConfig) and the six# TODO(user): Add fields herestubs are filled in. Three samples were invalid against their own CRD schema:GatewayProxywas missing the requiredprovider, andApisixGlobalRule/ApisixPluginConfigwere missing the requiredplugins.apiVersion/kindheader to everykustomization.yaml, dropped leftover kubebuilder scaffolding comments, and madeapp.kubernetes.io/nameconsistentlyapisix-ingress-controller.Verification
kustomize build config/default,config/crdandconfig/samplesall succeed, andmake build-installerregeneratesdist/install.yaml.config/defaultoutput against master. Apart from theapp.kubernetes.io/namerename, the only changes are the four fixes above: the metrics Service now targets the namedmetricsport on 8080, the manager declares that port,ADC_SERVER_URLis set, and the pod carries the label the webhook Service and NetworkPolicy select on. The resource set is otherwise identical — no webhook or cert-manager objects are added.[WEBHOOK]sections renders aValidatingWebhookConfigurationwith thecert-manager.io/inject-ca-fromannotation and aCertificatewhosednsNamesresolve to the prefixed webhook Service.config/samplesagainst the CRD OpenAPI schema and its CELx-kubernetes-validationsrules (via a throwaway test built on the existingcrdSchemaValidatorharness inapi/v2); all twelve pass.CI
.github/workflows/kustomize-checker.ymlguardsconfig/from here on. It runs whenconfig/**,api/**or theMakefilechanges and:make build-installerand buildsconfig/crdandconfig/samples;config/crd/basesandconfig/webhook/manifests.yamlmatch what controller-gen produces (config/rbac/role.yamlis excluded — it has a pre-existing rule ordering difference against the pinned controller-gen);Every step above was run locally against a kind cluster before pushing: the installer applies cleanly and all twelve samples pass the server-side dry run.