kong icon indicating copy to clipboard operation
kong copied to clipboard

Validation of OpenTelemetry plugin's "config.resource_attributes" field is too strict

Open seh opened this issue 2 years ago • 5 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Kong version ($ kong version)

3.2.2

Current Behavior

Following the documentation for the OpenTelemetry plugin, writing a KongPlugin manifest to be translated by the Kong ingress controller (version 2.9.2), I attempted to set a few attributes in the "config.resource_attributes" field, which is of type "map," per the schema. If any of the attribute names include a period—such as "service.name," or "k8s.namespace.name" per the OpenTelemetry semantic conventions—then the ingress controller's validating Webhook rejects the manifest as follows:

admission webhook “validations.kong.konghq.com” denied the request: plugin failed schema validation: schema violation (config.resource_attributes: expected a string)

This comes from a manifest like the following:

apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: opentelemetry-to-collector
  namespace: &ns some-app
plugin: opentelemetry
config: 
  endpoint: http://opentelemetry-collector.some-ns.svc.cluster.local:4318/v1/traces
  resource_attributes:
    k8s.namespace.name: *ns

I tried a few variations, placing the "k8s.namespace.name" mapping key name within double quotation marks, and inserting a backslash before the periods, but neither changed the outcome.

If I remove the periods and use, say, "k8snamespacename" as the resource attribute name, Kong's validation procedure accepts the manifest, and the ingress controller programs the proxies accordingly.

Expected Behavior

Kong's validation procedure should admit just about any string here for the attribute name, as the OpenTelemetry specification is still vague on the syntactic constraints for such names.

Steps To Reproduce

  1. (Optionally) Enable the OpenTelemetery plugin within Kong's configuration.
  2. Within Kubernetes, enable Kong's validating admission Webhook via a ValidatingWebhookConfiguration.
  3. Attempt to create a KongPlugin object by applying a manifest like the one shown earlier, via kubectl apply --filename <name> --dry-run=server or kubectl apply --filename <name> --server-side=true.
  4. Observe that the validating Webhook rejects the manifest due to the "config.resource_attributes" mapping entry.

Anything else?

I first mentioned this problem in the "kong" channel of the "Kubernetes" Slack workspace.

This plugin entered Kong's code base in #8826 by @mayocream.

seh avatar Apr 11 '23 16:04 seh

@seh I used exactly the same Kong ingress controller and Kong version (KIC 2.9.2, Kong 3.2.2) and same manifest

apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: opentelemetry-to-collector
  namespace: default
plugin: opentelemetry
config: 
  endpoint: http://opentelemetry-collector.some-ns.svc.cluster.local:4318/v1/traces
  resource_attributes:
    k8s.namespace.name: default

And I could successfully create the plugin. Would you please confirm again the versions of KIC and Kong, and also the manifest?

randmonkey avatar Apr 27 '23 03:04 randmonkey

As stated earlier, I'm using Kong proxy version 3.2.2 and ingress controller version 2.9.2.

Here's another manifest:

KongPlugin manifest
apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: opentelemetry-to-collector
  namespace: default
plugin: opentelemetry
config: 
  endpoint: http://opentelemetry-collector.some-ns.svc.cluster.local:4318/v1/traces
  resource_attributes:
    k8s.namespace.name: default

Attempting to apply that manifest yields the following failure reported by kubectl:

Error from server: error when creating "/tmp/kong-plugin.yaml":
  admission webhook "validations.kong.konghq.com" denied the request:
    plugin failed schema validation:
      schema violation (config.resource_attributes: expected a string)

Asking for the server to validate in via --dry-run=server provokes the same response from the validating Webhook.

seh avatar Apr 28 '23 17:04 seh

@randmonkey, do you have a ValidatingWebhookConfiguration in place to validate creation requests for KongPlugin objects?

seh avatar Apr 28 '23 17:04 seh

Hi. I have the exact same problem on Kong 3.2.1 and (after just upgrading) also on Kong 3.3.0 without using Kong Ingress Controller.

Even the example from the docs already failing for me https://docs.konghq.com/hub/kong-inc/opentelemetry/#configure-the-opentelemetry-plugin

curl -X POST http://localhost:8001/plugins \
    --data "name=opentelemetry"  \
    --data "config.endpoint=http://some-service-name.some-namespace.svc.cluster.local:4318/v1/traces" \
    --data "config.resource_attributes.service.name=kong-dev"

Error: {"message":"schema violation (config.resource_attributes: expected a string)","code":2,"fields":{"config":{"resource_attributes":"expected a string"}},"name":"schema violation"}

reneeckstein avatar Jun 05 '23 20:06 reneeckstein

Hi @seh I just found a way to set it with decK

Plugin Config in decK

- config:
    batch_flush_delay: 3
    batch_span_count: 200
    connect_timeout: 1000
    endpoint: http://jaeger-collector.some-namespace.svc.cluster.local:4318/v1/traces
    headers: null
    read_timeout: 5000
    resource_attributes:
      service.name: kong-dev
    send_timeout: 5000
  enabled: true
  name: opentelemetry
  protocols:
  - grpc
  - grpcs
  - http
  - https

image

reneeckstein avatar Jun 05 '23 21:06 reneeckstein

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 11 '23 23:07 stale[bot]

This issue is still not fixed.

seh avatar Jul 12 '23 00:07 seh

@seh The issue is sort of conditionally fixed. That is, when you send a JSON request it works (after this PR #11091), but when you send a form data request it doesn't work because the variable name in the form data contains the . symbol, which is recognized by Kong as nested keys, and then normalize dotted keys in objects which will cause schema violation.

ms2008 avatar Jul 19 '23 08:07 ms2008

Thank you for the detail, but this still strikes me as unnecessarily pedantic: a distinction without a difference. To us users, this is still broken, with no apparent way for us to "fix it" by doing something different.

If the Kubernetes validating admission Webhook is sending HTTP form data and it should not, then that fix belongs in the Webhook. Do you think that we should migrate this issue—or file a related one—over to Kong/kubernetes-ingress-controller? It would help to hear what you think would work better instead.

seh avatar Jul 19 '23 11:07 seh

If the Kubernetes validating admission Webhook is sending HTTP form data and it should not, then that fix belongs in the Webhook.

I think Webhook sends a JSON request, can @randmonkey confirm?

Do you think that we should migrate this issue—or file a related one—over to Kong/kubernetes-ingress-controller?

I don't think it's necessary, because this issue is only triggered if you enable the KIC's admission webhook feature, and it won't happen if you disable the admission Webhook. It's essentially a Kong gateway issue.

If you're using a version of Kong gateway after the PR I mentioned above, then the problem is actually fixed.

ms2008 avatar Jul 21 '23 02:07 ms2008

@ms2008 @seh ALL requests sent from KIC are filled with JSON body, because they are sent by Kong/go-kong: https://github.com/Kong/go-kong/blob/main/kong/request.go#L14-59

randmonkey avatar Jul 21 '23 02:07 randmonkey

@ms2008 We are also experiencing this with the file-log plugin, am I right to think the fix will be part of the upcoming 3.4 release? https://github.com/Kong/kong/commits/3.4.0. The commit above is at least part of the tag history, but there's no changelog yet.

dlouzan avatar Aug 09 '23 16:08 dlouzan

As a temporary fix, we could disable the admission controller with:

helm upgrade ingress kong
    ...
    --set ingressController.admissionWebhook.enabled=false

dlouzan avatar Aug 09 '23 18:08 dlouzan

@dlouzan Yes, that fix has been included in 3.4, the exact commit is here https://github.com/Kong/kong/commit/866a15c693038d4e57ffc9d21c2042975dd821d3 and you can see that the changelog has been updated as well!

As a temporary fix, we could disable the admission controller

That's right, this only affects the webhook, so temporarily disabling the webhook is a valid workaround.

ms2008 avatar Aug 10 '23 02:08 ms2008

@seh @dlouzan 3.4 was released, could you confirm whether you can still reporoduce the issue on that version?

samugi avatar Aug 16 '23 09:08 samugi

@samugi Sure, we will comment back here when we update our systems (probably next week).

dlouzan avatar Aug 16 '23 11:08 dlouzan

@samugi But I'm realizing, we won't probably be able to test it properly, as I think the helm chart has not yet updated the default kong version to 3.4, isn't it? We tend to wait until that is updated and not keep a custom version for the values.

dlouzan avatar Aug 16 '23 11:08 dlouzan

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 16 '23 18:09 stale[bot]

Unfortunately, I am no longer equipped to test the fix, as I am no longer using Kong.

seh avatar Sep 16 '23 19:09 seh

At the time of our latest system update, the helm chart did not still incorporate a version that included this fix. We can give feedback later this month.

dlouzan avatar Sep 17 '23 20:09 dlouzan

This issue is marked as stale because it has been open for 14 days with no activity.

github-actions[bot] avatar Oct 12 '23 01:10 github-actions[bot]

Dear contributor,

We are automatically closing this issue because it has not seen any activity for three weeks. We're sorry that your issue could not be resolved. If any new information comes up that could help resolving it, please feel free to reopen it.

Your contribution is greatly appreciated!

Please have a look our pledge to the community for more information.

Sincerely, Your Kong Gateway team

github-actions[bot] avatar Oct 19 '23 01:10 github-actions[bot]

With the most recent helm chart which uses kong 3.4+, this seems to be working properly now 👍

dlouzan avatar Oct 30 '23 16:10 dlouzan