search

redpanda-data / kminion

KMinion is a feature-rich Prometheus exporter for Apache Kafka written in Go. It is lightweight and highly configurable so that it will meet your requirements.
MIT License
610 stars 122 forks source link

Can't set kafka.tls.ca #191

Closed TheMeier closed 1 year ago

TheMeier commented 1 year ago

In https://github.com/redpanda-data/kminion/pull/147/files a feature was introduced to ad ca, cert, key as inline config. I am unable to use this feature. Anytime something is configured here the following error is logged:

{"level":"fatal","ts":1677509554.5259843,"caller":"kminion/main.go:43","msg":"failed to parse config","error":"1 error(s) decoding:\n\n* cannot parse 'kafka.tls.key[0]' as uint: strconv.ParseUint: parsing \"<STRING FROM config: kafka.tls.ca>\" invalid syntax","stacktrace":"main.main\n\t/home/runner/work/kminion/kminion/main.go:43\nruntime.main\n\t/opt/hostedtoolcache/go/1.19.3/x64/src/runtime/proc.go:250"}
eric-sap commented 1 year ago

I learned something less than thrilling about this today. if I set the KAFKA_TLS_CA to this string in the kminion deployment:

        - name: KAFKA_TLS_CA
          value: 45,45,45,45,45,66,69,71,73,78,32,67,69,82,84,73,  ...

Where those numbers are literally the byte-values of every single character in the string "-----BEGIN CERTIFICATE----- MIIGD ....." .... then it works. (Yes, that's a 7,000-character-plus environment variable I've got in my deployment) I traced through how the code in mitchellh/mapstructure/mapstructure.go::decode() ends up in a case reflect.Uint: err = d.decodeUint(name, input, outVal) (the technical reason for the error you're seeing), and it has something to do with the fact that in the TLSConfig part of the Kafka config in the main config struct defines

    Ca                    []byte `koanf:"ca"`

The koanf processing is using reflection to figure out the type but it thinks it's just "byte" and doesn't care about the array ... unless you manually provide an array via the impressive CSV shown.

I'm not familiar enough with any of this to say that this is or isn't functioning as intended, I just thought I'd share the results of my manual code tracing in this area.

TheMeier commented 1 year ago

@eric-sap thanks for checking. I had a short look a while back ago and suspected the byte[] in the struct definition to be the culprit. I can try to provide a MR which replaces that with with string...