Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toYaml removes quotes from keys which causes failure in keys with a dot #12606

Closed
ogarcia opened this issue Dec 1, 2023 · 8 comments
Closed

Comments

@ogarcia
Copy link

ogarcia commented Dec 1, 2023

I have this:

librdkafka_options:
  "security.protocol": sasl_ssl

Once the filter is passed it is modified by this other one:

librdkafka_options:
  security.protocol: sasl_ssl

By removing the quotes, the final system ends up interpreting it in this way:

librdkafka_options:
  security:
    protocol: sasl_ssl

To reproduce simply try to use following yaml:

testyaml:
  librdkafka_options:
    "security.protocol": sasl_ssl

With this template:

apiVersion: v1
kind: ConfigMap
metadata:
  name:  test
  labels:
    alabel
data:
  config.yaml: |
{{ tpl (toYaml .Values.testyaml) . | indent 4 }}
@ogarcia ogarcia changed the title Dot in value name is misinterpreted by toYaml toYaml removes quotes from keys which causes failure in keys with a dot Dec 1, 2023
@Ithrael
Copy link
Contributor

Ithrael commented Dec 1, 2023

By removing the quotes, the final system ends up interpreting it in this way:

librdkafka_options:
  security:
    protocol: sasl_ssl

At first, I was somewhat confused about this, because I thought for k8s, it doesn't matter whether the key has quotes or not. The tests I did also seem to support this.

image Up to this point, it seems that the helm template method of stripping quotes doesn't pose a problem.

However, if we treat the quoted part as a string value of a key, then the situation seems to be different.
image

@ogarcia
Copy link
Author

ogarcia commented Dec 1, 2023

By removing the quotes, the final system ends up interpreting it in this way:

librdkafka_options:
  security:
    protocol: sasl_ssl

This statement is what I think is happening, but it may not be exactly like that. Don't pay too much attention to that.

Actually the problem can be seen in this issue.

With the complete configuration:

role: Agent
service:
    enabled: false
serviceHeadless:
    enabled: false
customConfig:
    data_dir: /vector-data-dir
    api:
        enabled: true
        address: 127.0.0.1:8686
        playground: false
    sources:
        kubernetes_logs:
            type: kubernetes_logs
    sinks:
        kafka:
            type: kafka
            inputs:
                - kubernetes_logs
            bootstrap_servers: the.kafka.server:9093
            topic: aprettytopic
            encoding:
                codec: json
            sasl:
                enabled: true
                mechanism: PLAIN
                username: user
                password: pass
            librdkafka_options:
                "security.protocol": sasl_ssl

What the chart does is to apply the filter that I mentioned before ({{ tpl (toYaml .Values.testyaml) . | indent 4 }}) on the field customConfig and ends up generating the following:

apiVersion: v1
data:
  vector.yaml: |
    api:
      address: 127.0.0.1:8686
      enabled: true
      playground: false
    data_dir: /vector-data-dir
    sinks:
      kafka:
        bootstrap_servers: the.kafka.server:9093
        encoding:
          codec: json
        inputs:
        - kubernetes_logs
        librdkafka_options:
          security.protocol: sasl_ssl
        sasl:
          enabled: true
          mechanism: PLAIN
          username: user
          password: pass
        topic: aprettytopic
        type: kafka
    sources:
      kubernetes_logs:
        type: kubernetes_logs
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: vector
    meta.helm.sh/release-namespace: monitoring
  creationTimestamp: "2023-11-27T10:52:37Z"
  labels:
    app.kubernetes.io/component: Agent
    app.kubernetes.io/instance: vector
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: vector
    app.kubernetes.io/version: 0.34.1-distroless-libc
    helm.sh/chart: vector-0.29.0
  name: vector
  namespace: monitoring
  resourceVersion: "314255883"
  uid: anuuid

Which removes the quotation marks from "security.protocol" which then causes Vector not to interpret it correctly (this is documented here).

Of course, if I edit the configmap by hand and put the quotation marks around it, everything works correctly.

It's not really clear to me whose problem it is. If it's in Helm for removing the quotes, if it's in the chart because it should be declared otherwise or if the problem is really that Vector should maybe pick up those extra settings from a Yaml list or a literal.

@gjenkins8
Copy link
Contributor

I think this is a bug in the system which is consuming the YAML here. I'm fairly certain that e.g. security.protocol: sasl_ssl is correct Yaml, and should not be interpreted as security: protocol: sasl_ssl. A parser which requires quotes around yaml keys doesn't seem to be conformant.

I think it would be necessary to reference the Yaml spec, where is says keys with dots (".") must be quoted otherwise.

You might be able to use toJson in your chart, as JSON will quote keys.

@ogarcia
Copy link
Author

ogarcia commented Dec 2, 2023

@gjenkins8 I have just opened an issue in Vector, let's see if we can find a solution. Thank you very much for your help.

@jszwedko
Copy link

jszwedko commented Dec 7, 2023

(chiming in from Vector, where vectordotdev/vector#19289 prompted this issue)

I think this is a bug in the system which is consuming the YAML here. I'm fairly certain that e.g. security.protocol: sasl_ssl is correct Yaml, and should not be interpreted as security: protocol: sasl_ssl. A parser which requires quotes around yaml keys doesn't seem to be conformant.
I think it would be necessary to reference the Yaml spec, where is says keys with dots (".") must be quoted otherwise.

To clarify this bit a bit more, the reason the "security.protocol" key is quoted is exactly becaue we don't want it to be interpreted as a nested object. That is we want to interpret:

librdkafka_options:
  "security.protocol": sasl_ssl

as

{
  "librdkafka_options": { "security.protocol": "sasl_ssl" }
}

and not:

{
  "librdkafka_options": { "security": {"protocol": "sasl_ssl" } }
}

@gjenkins8
Copy link
Contributor

@jszwedko -- yes, I agree. Issue here is different / I mean that:

librdkafka_options:
  security.protocol: sasl_ssl   # no quotes

Is valid yaml, that should also be interpreted as:

{
  "librdkafka_options": { "security.protocol": "sasl_ssl" }
}

I don't believe yaml spec requires keys containing periods (".") to be quoted. Not should any parser interpret keys with periods "as multiple keys".

@jszwedko
Copy link

Ah, yes, I see, you are right! I was thinking of TOML. It seems like there might be something in Vector that is interpreting the .s as nested keys. We'll look into that.

@ogarcia
Copy link
Author

ogarcia commented Dec 11, 2023

In the end the incident has been a a series of unfortunate events, it has been solved, thank you all very much!

@ogarcia ogarcia closed this as completed Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants