moonming opened a new pull request, #1036:
URL: https://github.com/apache/apisix-helm-chart/pull/1036
> Draft: depends on #1035. The vendored
`charts/apisix-ingress-controller-1.5.0.tgz` and `Chart.lock` in this branch
are a local build of that PR; they will be regenerated with `helm dependency
update` once 1.5.0 is released.
## Summary
The apisix chart ships a fixed, publicly known Admin API key
(`edd1c9f034335f136f87ad84b625c8f1`, viewer
`4054f7cf07e344346cd3f287985e76a2`). When the bundled ingress controller is
enabled, the chart also appends `0.0.0.0/0` and `::/0` to `allow_admin`. The
install command in the ingress controller getting-started guide produces
exactly this combination: any pod that can reach the admin Service can change
the gateway configuration.
APISIX's own "empty key → generate" fallback does not help here.
`config.yaml` is mounted read-only from a ConfigMap, so the write-back fails
and APISIX exits (tested with 3.19.0). Even if it could write, each pod would
get its own key, and the controller could not read any of them.
This PR makes the chart generate the keys, store them in a Secret, and give
the same key to APISIX and to the bundled controller.
## Changes
**Admin API keys**
- When `apisix.admin.credentials.admin` / `viewer` are empty and no
`secretName` is set, the chart generates random 32-character keys into the
Secret `<fullname>-admin-credentials`. It reads them back with `lookup`, so
`helm upgrade` keeps them. Keys set inline or through `secretName` work as
before.
- On upgrade, a generated key that is missing from that Secret fails the
release instead of being replaced. This covers releases installed with earlier
chart versions (which used the public defaults) and keys deleted by hand, so
Admin API clients are never cut off silently. The error explains what to set.
With `apisix.admin.enabled: false`, missing keys are generated again, since
nothing can use them.
- The public default keys are rejected in `apisix.admin.credentials`, in
`apisix.fullCustomConfig.config`, in the generated Secret, and in the
`secretName` Secret when the chart can read it.
**Bundled ingress controller**
- The apisix chart creates the GatewayProxy, with this release's Admin API
Service and admin key: a value, or a `secretKeyRef` to the Secret APISIX reads.
The controller's IngressClass references it through
`ingress-controller.gatewayProxy.name` (#1035). Users no longer repeat the key
under `ingress-controller`. Fixes #908.
- `ingress-controller.gatewayProxy.createDefault: true` still lets the
controller chart create its own GatewayProxy. Its key must now be set (the
public default from the controller chart is overridden with an empty value),
must not be a public default, and must come from the same place as APISIX's key
when it targets this release.
- A release without an Admin API (data_plane, `fullCustomConfig`, Admin API
disabled) fails instead of leaving the IngressClass pointing at a GatewayProxy
that does not exist. `ingress-controller.gatewayProxy.name: ""` opts out.
**Docs**: README sections on Admin API keys, GitOps, rotation, and upgrading
to 3.0.0; NOTES print where the generated keys are.
## Breaking changes (3.0.0)
- Releases that relied on the default keys fail on `helm upgrade` with
`ADMIN KEY ERROR` until new keys (or `secretName`) are set. Admin API clients
need the new key.
- `ingress-controller.gatewayProxy.createDefault: true` now needs an
explicit admin key that matches APISIX. The getting-started guide sets
`createDefault=true`; it should drop that flag (follow-up PR in
apache/apisix-ingress-controller).
- The bundled controller chart goes from 1.3.0 to 1.5.0, so the 1.4.x
changes come along.
## GitOps
`lookup` returns nothing when the chart is rendered without access to the
cluster. Tested on kind (Kubernetes 1.37) with Argo CD v3.6.1 and Flux v2.10.0:
| Setup | Result |
|---|---|
| Argo CD, generated keys | After a refresh the app is OutOfSync; each sync
writes a new key into the Secret. The APISIX pods keep the first key while the
controller follows the Secret, so the controller gets `401 ... failed to check
token` and new Ingresses stop reaching APISIX. |
| Argo CD, `secretName` | Key unchanged across syncs, app stays Synced,
controller works. |
| Argo CD, `ignoreDifferences` on the Secret's `/data` +
`RespectIgnoreDifferences=true` | The first generated key is kept, app stays
Synced. |
| Argo CD, a key generated by a named `pre-install` hook | Re-created on
every sync (no delete policy means `BeforeHookCreation`), so hooks are not an
alternative. |
| Flux, generated keys | Kept across upgrades. |
| Flux, upgrading a 2.18.2 release that used the defaults | Fails with
`ADMIN KEY ERROR`; the release stays on 2.18.2 and the old key keeps working. |
The README tells Argo CD and offline-rendering users to use `secretName`, or
the `ignoreDifferences` setting above.
## Tests
- `test/render/admin-credentials.sh`: 67 render checks covering generated,
inline, mixed and existing-Secret keys; public key rejection; the upgrade guard
(`--is-upgrade`); GatewayProxy wiring and the `createDefault` checks; releases
without an Admin API; `fullCustomConfig`. All pass on Helm 4.2.4 and Helm
3.22.1.
- `test/e2e/admin-credentials.sh`: 35 checks on kind with real APISIX 3.19.0
and apisix-ingress-controller 2.2.0, all passing:
- install with the guide's values;
- the Admin API accepts the generated key and returns 401 for the public
key;
- an Ingress route works through the controller;
- upgrade keeps the key;
- rotation: patch the Secret and restart APISIX, the old key gets 401 and
the controller picks up the new key;
- a Secret with a deleted key or a public key fails the upgrade;
- `secretName`, including a Secret that holds the public key;
- upgrade from 2.18.2: fails and leaves revision 1 untouched, then
succeeds with new keys;
- the default etcd mode.
- Revert check: the same e2e against the current master chart fails 26 of 35
checks.
- `ct lint` passes. READMEs regenerated with helm-docs v1.11.0.
## Related
- Supersedes #740 (a key generated in the ConfigMap template on every
render, without `lookup`, and not passed to the controller).
- Follow-ups filed separately: #1032 (bundled etcd has no authentication),
#1033 (the `data_encryption` keyring cannot be set and defaults to a public
value), apache/apisix#14178 (a numeric key passed through an environment
variable gets 401).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
