AlinsRan commented on issue #2832:
URL:
https://github.com/apache/apisix-ingress-controller/issues/2832#issuecomment-5450970890
Thanks for writing this up, and for laying out both options.
We'd rather take the second one: add `secretRef` to `PluginConfig`, and not
make `ApisixPluginConfig` usable as an `ExtensionRef` target.
The problem with the first option is that the two CRDs decide ownership in
different ways, and referencing one from the other puts those two ways in
conflict.
`ApisixPluginConfig` carries `spec.ingressClassName` and is reconciled
against it (`FindMatchingIngressClass` in `apisixpluginconfig_controller.go`).
`PluginConfig` has no class field at all. It is only ever reached through an
`ExtensionRef`, so it inherits ownership from the route's parent Gateway and
that Gateway's `GatewayClass`.
Once an HTTPRoute can point at an `ApisixPluginConfig`, the two can
disagree, and there is no good answer when they do. Take a route attached to a
Gateway of class `a` that references an `ApisixPluginConfig` with
`ingressClassName: b`. If we read it, `ingressClassName` stops meaning anything
on that path. If we refuse it, we report `ResolvedRefs=False` for an object
that plainly exists and that another controller is happily reconciling. Either
way the field means one thing for the apiv2 path and something else for the
Gateway API path, and the object's status ends up being written by a controller
that isn't the one consuming it.
There's a smaller reason too: `ApisixPluginConfig` isn't published as an
independent resource today. It's inlined during `ApisixRoute` translation
(`internal/adc/translator/apisixroute.go`). Making it an `ExtensionRef` target
would hand it a lifecycle it doesn't currently have, which is a fair amount of
new surface for something `PluginConfig` can cover directly.
Roughly what we have in mind:
```yaml
apiVersion: v1
kind: Secret
metadata:
name: oidc-credentials
namespace: default
stringData:
client_id: my-client
client_secret: "s3cr3t"
session.secret: "8f2a...at least 32 bytes..."
---
apiVersion: apisix.apache.org/v1alpha1
kind: PluginConfig
metadata:
name: oidc
namespace: default
spec:
plugins:
- name: openid-connect
secretRef:
name: oidc-credentials
config:
discovery: https://idp.example.com/.well-known/openid-configuration
scope: openid profile
bearer_only: false
```
The `HTTPRoute` side is unchanged:
```yaml
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: httpbin
namespace: default
spec:
parentRefs:
- name: apisix
rules:
- matches:
- path:
type: PathPrefix
value: /
filters:
- type: ExtensionRef
extensionRef:
group: apisix.apache.org
kind: PluginConfig
name: oidc
backendRefs:
- name: httpbin
port: 80
```
Each key in the Secret is merged into `config`, so the plugin ends up
receiving:
```json
{
"discovery": "https://idp.example.com/.well-known/openid-configuration";,
"scope": "openid profile",
"bearer_only": false,
"client_id": "my-client",
"client_secret": "s3cr3t",
"session": { "secret": "8f2a..." }
}
```
A few things worth knowing about that merge, since they'll shape what you
can put in the Secret:
A key containing dots nests, which is how `session.secret` above lands under
`session`. Secret values are always merged as strings, so fields that need a
number or a boolean (`session.timeout`, `ssl_verify`) have to stay in `config`
— APISIX's schema will reject them otherwise. Keys in the Secret win over the
same key in `config`.
Two behaviours we want to be deliberate about: a `secretRef` pointing at a
Secret that doesn't exist should fail rather than publish the plugin without
its credentials — the precedent is `validateSecrets` on the `ApisixRoute` path,
not the quieter handling on the consumer path. And the field will be a
`SecretReference` (`name` plus optional `namespace`) to match the rest of
v1alpha1, with cross-namespace references honouring `ReferenceGrant` the same
way `Consumer` already does.
In the meantime, `$env://` works today without any change on our side, since
`config` is passed through to APISIX as-is. The catch is that the value has to
live in the data plane's environment, so it can't vary per `PluginConfig` —
probably fine for a shared session secret, not for per-tenant client
credentials.
Worth mentioning that this doesn't overlap with #2831 as much as it might
look. `secretRef` reads the Secret and materialises the plaintext into the
pushed configuration; `$secret://` leaves the value out of the configuration
entirely and has the data plane fetch it at request time. Different trade-offs,
and we'd like both — this one because it only needs a Kubernetes Secret, that
one because the secret never has to be in the config at all.
--
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]
