KongPlugin Konnect sync fails when Konnect sanitizer enabled and configFrom used #5692
Comments
|
I think both of the options (omitting whole plugin vs filling with defaults) will bring some confusion. However, when users are using |
|
If you encounter this issue, a temporary workaround could be turning |
|
Re
I think upstream is the best way to handle this. I can't think of much that we can do to handle this on our end as-is: if we have to provide a valid value for any field that doesn't accept a vault reference, we're going to be playing whack-a-mole tracking down every non-referenceable field and writing code to generate a placeholder value that conforms to the field schema. Using patches instead of Initial review of how the upstream API handles this found Kong/koko#449, but it doesn't look like that's where the "if reference, use special validation" logic lives. It defines how to recognize the reference magic string, but doesn't have obvious general-case validation of it. I did find https://github.com/Kong/koko-private/blob/main/internal/plugin/validators/lua_validator.go and https://github.com/Kong/goks while searching around, and that suggests Koko is offloading schema validation to the Lua, which makes sense--not much reason to maintain parallel schemas, and Lua's easy to embed. If that's indeed the case, we'd need to add something similar to the existing If we can swing that, we'd pass a That said, we'd still have a bit of an issue on our end: we don't quite know what fields we need to mask. Lastly, it looks like this Lua offload only applies to plugins, and that plugins simply hold the vast majority of |
We could use Admin API's Example of a plugin schema that includes a referenceable field: $ curl --request GET \
--url http://localhost:8001/schemas/plugins/session \
--header 'accept: */*' | jq
...
"fields": [
{
"secret": {
"encrypted": true,
"default": "rxXaQBYq2IkINwzRZgXKDNEsneUReYBIMzLHBzaPc6Uj",
"type": "string",
"required": false,
"referenceable": true,
"description": "The secret that is used in keyed HMAC generation."
}
},
... |
I agree with this viewpoint. Although we can currently achieve our goals through some solutions, if this is not clearly communicated with the upstream, it may be disrupted at some point in the future. |
|
I have added a blocked label for this issue, because I want to get some response from @mheap https://kongstrong.slack.com/archives/C02KEASTTRC/p1716306680429769 |
|
We could maybe create a stopgap that doesn't require fancy field generation by creating "reference" copies of every plugin, where we just write a static minimum viable configuration for each and send that for plugins with secret fields? For I don't think plugins are allowed to declare This would not handle custom plugins however, and AFAIK Konnect does have some mechanism that allows those. |
|
For @mheap to decide there are essentially four options, laid out below. My preference is trying to go for the Koko-side break glass skip plugin validation display whatever approach. If viable it's less work than building out either the generators or placeholder plugin configurations. If we opt for a KIC-only approach, my preference is for the placeholder plugin approach. It's simple, albeit rather stupid, and arguably a stopgap--we should commit to some sort of upstream handling in the future if we go that route. Send placeholder plugins KIC team writes static minimum viable plugin configurations for each plugin. When a plugin uses In addition to the overlay code, this requires some busywork to build out all the stand-in configs. We can maybe do so iteratively with patch releases that include additional plugins if we want to target a subset first. Konnect will display fake values as if they were actual values. There will be no Konnect-side indication that these are actually masked fields. Fill plugin fields from schema-aware generators KIC team writes generators for Kong DAO schema types. KIC reads plugin schemas and tries to generate appropriate garbage values for strings, ints, and more complex types like PEM. Required fields absent from This takes more code work, but covers all plugins, including custom, once complete. It has a higher chance of failure for Upstream gateway changes Update the Kong DAO to recognize something akin to the existing Upstream Koko changes We add some Koko-only plugin flag that tells Koko it shouldn't try to validate a given plugin instance and instead just accept and display whatever configuration we submit (either empty configuration for This is probably much more reasonable than the gateway approach, since we know that Konnect doesn't really need to validate KIC-submitted configuration, as Konnect is never going to send that configuration to a DP. |
|
The correct solution here is for Koko not to validate empty fields in a KIC backed configuration. However, this was too much work for the team during initial discussions. I think we should proceed as follows:
This will not catch all sensitive data, and that's ok. The initial feature request for this was tightly scoped to "certificate private keys", and somehow expanded to "anything from a secret". Long term, we should have a way in Gateway to mark fields as sensitive in a schema, which would then allow KIC to send a zero value, and Koko to skip validation for empty values for KIC in Konnect |
|
Unassigning self, #5932 going to consume more time than expected. Re:
We'd just be shifting complex work from one team to another with the above proposal. Is it too much work in the "we will never have time to implement this" sense or in the "we don't have time to implement this now" sense? The "correct. but too much" bit suggests we're looking for more of a stopgap, and I don't think we have a ready-made schema walker and merger that we can use to produce one quickly KIC-side. The merger bit that smooshes the actual and fake fill-in-the-gap plugin you need for either KIC-side solution. IDK if you're likely to get both that and the schema-walker to figure out which specific fields to replace in the 3.2 release timeline. |
|
It's closer to "we will never have time to implement this". The correct way to do this is have Gateway mark items as sensitive, then have Koko use that marker with KIC in Konnect to loosen validations. Given that takes schema changes across two products, I think we fix in KIC for now and work on getting schema changes in to Gateway in the future |
|
@mheap and I synced up here. It seems like the best compromise between complexity/time is to go back to the original requirement and only obfuscate certificate private keys and not any secrets. Users will come back complaining about their secrets not being obfuscated anymore and we can direct them towards:
|
Is there an existing issue for this?
Current Behavior
KIC fails to sync sanitized (when
SanitizeKonnectConfigDumpsfeature gate is on) KongPlugins with Konnect if KongPlugin usesconfigFromto load whole configuration from a Secret.Expected Behavior
Expected behavior is to be defined. If the whole configuration is kept in a Secret, we cannot tell which part of it really is sensitive so we assume we cannot take any part of it and push it to Konnect. We sanitize the whole config, replacing every field's value with {vault://redacted-value} which fails the validation for some of them which Kong doesn't expect to be loaded from vaults.
Two possible solutions I suggest we could implement so this doesn't block syncs if someone makes this mistake:
configFromwithSanitizeKonnectConfigDumpsbeing on.sanitized-with-defaultsto signal that.In docs, we should encourage use of
configPatchesas a preferred way to define Secret parts of configuration only.Steps To Reproduce
echo " apiVersion: configuration.konghq.com/v1 kind: KongPlugin metadata: name: rate-limiting-example namespace: omniva plugin: rate-limiting configFrom: secretKeyRef: name: rate-limit-redis key: config " | kubectl apply -f - echo " apiVersion: v1 kind: Secret metadata: name: rate-limit-redis namespace: omniva stringData: config: | minute: 10 policy: redis redis_host: redis-master redis_password: PASSWORD type: Opaque " | kubectl apply -f - echo " apiVersion: networking.k8s.io/v1 kind: Ingress metadata: name: echo namespace: omniva annotations: konghq.com/strip-path: 'true' konghq.com/plugins: rate-limiting-example spec: ingressClassName: kong rules: - http: paths: - path: /echo pathType: ImplementationSpecific backend: service: name: echo port: number: 1027 " | kubectl apply -f -Kong Ingress Controller version
Acceptance Criteria
KongPlugins usingconfigFromto fetch configurations from secrets are correctly uploaded, with only fields came from private key of TLS sanitizedKubernetes version
No response
Anything else?
No response
The text was updated successfully, but these errors were encountered: