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

Kafka security config params not being picked up from env file #9947

Closed
wjglerum opened this issue Jun 11, 2020 · 13 comments · Fixed by #13968
Closed

Kafka security config params not being picked up from env file #9947

wjglerum opened this issue Jun 11, 2020 · 13 comments · Fixed by #13968
Assignees
Labels
Milestone

Comments

@wjglerum
Copy link
Contributor

Describe the bug
Not all Kafka security options are being picked up when using a .env file. We need this as we use SASL authentication. Other parameters do work as expected when provided in a .env file.

Expected behavior
Quarkus picks up all parameters from a .env file.

Actual behavior
The following parameters work as expected in a property file, yaml config and environment variable, hower they do not work when specified in a .env file:

  • KAFKA_SECURITY_PROTOCOL=SASL_SSL
  • KAFKA_SASL_MECHANISM=PLAIN
  • KAFKA_SASL_JAAS_CONFIG=<config>

Example of parameters that do work with a .env file:

  • KAFKA_BOOTSTRAP_SERVERS=<host>
  • KAFKA_REQUEST_TIMEOUT_MS=60000

To Reproduce
Steps to reproduce the behavior:

  1. Follow the Kafka Guide to setup a project with Kafka
  2. Follow the Configuration Guide to override variables with environment variables and/or .env files
  3. Run the application

Configuration
application.yaml -> works as expected

kafka:
  bootstrap:
    servers: <host>
  request:
    timeout:
      ms: 60000
  security:
    protocol: SASL_SSL
  sasl:
    mechanism: PLAIN
    jaas:
      config: <config>

.env -> doesn't work for security and sasl parameters

KAFKA_BOOTSTRAP_SERVERS=<host>
KAFKA_REQUEST_TIMEOUT_MS=60000
KAFKA_SECURITY_PROTOCOL=SASL_SSL
KAFKA_SASL_MECHANISM=PLAIN
KAFKA_SASL_JAAS_CONFIG=<config>

Manually exporting those environment variables -> does work

export KAFKA_SECURITY_PROTOCOL=SASL_SSL
export KAFKA_SASL_MECHANISM=PLAIN
export KAFKA_SASL_JAAS_CONFIG=<config>

Screenshots
Security parameters from .env file are not picked up:

11:16:58 INFO  [or.ap.ka.cl.co.ConsumerConfig] (Quarkus Main Thread) ConsumerConfig values: 
...
        sasl.jaas.config = null
...
        sasl.mechanism = GSSAPI
	security.protocol = PLAINTEXT
...

After manually exporting it does work:

11:19:55 INFO  [or.ap.ka.cl.co.ConsumerConfig] (Quarkus Main Thread) ConsumerConfig values: 
...
	sasl.jaas.config = [hidden]
...
	sasl.mechanism = PLAIN
	security.protocol = SASL_SSL
...

Environment (please complete the following information):

  • Output of uname -a or ver:
Darwin Willems-MacBook-Pro.local 19.4.0 Darwin Kernel Version 19.4.0: Wed Mar  4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64
  • Output of java -version:
openjdk version "11.0.5" 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.5+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.5+10, mixed mode)
  • GraalVM version (if different from Java): N/A
  • Quarkus version or git rev: 1.5.0.Final
  • Build tool (ie. output of mvnw --version or gradlew --version):
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /Users/wjglerum/.m2/wrapper/dists/apache-maven-3.6.3-bin/1iopthnavndlasol9gbrbg6bf2/apache-maven-3.6.3
Java version: 11.0.5, vendor: AdoptOpenJDK, runtime: /Library/Java/JavaVirtualMachines/adoptopenjdk-11.jdk/Contents/Home
Default locale: en_NL, platform encoding: UTF-8
OS name: "mac os x", version: "10.15.4", arch: "x86_64", family: "mac"

Additional context
Let me know if you need more information or examples.

@wjglerum wjglerum added the kind/bug Something isn't working label Jun 11, 2020
@geoand
Copy link
Contributor

geoand commented Jun 11, 2020

Unfortunately this is a consequence of the Kafka config not being part of the regular Quarkus config system (@dmlloyd was soooooooooooo right when he insisted that all configuration go through the Quarkus config system - unfortunately not every extension headed his suggestion)

@wjglerum
Copy link
Contributor Author

Aah that makes kinda sense, thanks. Is there an easy way to fix this in the extensions? Or plans to enforce this in the future in Quarkus?

@wjglerum
Copy link
Contributor Author

It's mainly useful for during development/debugging when you want to connect and switch locally to another Kafka broker. For production we use env vars anyway for the secrets.

@geoand
Copy link
Contributor

geoand commented Jun 11, 2020

Aah that makes kinda sense, thanks. Is there an easy way to fix this in the extensions? Or plans to enforce this in the future in Quarkus?

It's not a strict rules, so it's really up to each extension.
You might want to send an email to Quarkus mailing list to bring (general) issue to the forefront of people's attention

@wjglerum
Copy link
Contributor Author

Alright thanks for the background info and suggestion, just posted it to the mailing list too.

@emmanuelbernard
Copy link
Member

Wouldn't it make sense to just send a pull request to add the properties you need?

Are you using io.quarkus:quarkus-kafka-client or io.quarkus:quarkus-smallrye-reactive-messaging-kafka ?

I am not familiar with the .env thing, do you mean literally environment variables or something different? What I am wondering is if these properties should be build time vs runtime. For example I did not imagine people would want to swap the SASL mechanism or security protocol depending on the deployment environment.

@geoand
Copy link
Contributor

geoand commented Jun 17, 2020

@emmanuelbernard the .env file is meant to be a way for users to easily configure "environment variables", so yeah it's basically meant for runtime config.

The issue here is that because the kafka config is not part of the Quarkus config (it reads properties manually), it doesn't pick up the DotEnvConfigSource.
We can make it work for sure (but then people might start asking for YAML, or custom converters, or ...), but it would be ideal from a uniformity perspective if the extension moved to Quarkus config.

@wjglerum
Copy link
Contributor Author

wjglerum commented Jun 17, 2020

Yeah indeed, the weird thing for me here is that YAML, env vars, application properties all work, except for the .env file for these specific options. That's not something I expected when working with this. Now that I know I can simply use env vars instead locally.

We override the security config for connecting to the prod broker for debugging for example. And during local development we don't want to deal with a SASL set-up etc and simply use a docker-compose with kafka and zookeeper.

For me it would be nice from a usability point for Quarkus users that all configuration works the same, regardless of the extension. That's just my take on it though.

I'm happy to help on adding this to the extions, just don't know where to start 😄

@wjglerum
Copy link
Contributor Author

Using quarkus-smallrye-reactive-messaging-kafka btw at the moment, haven't tried io.quarkus:quarkus-kafka-client

@geoand
Copy link
Contributor

geoand commented Jun 17, 2020

I'm happy to help on adding this to the extions, just don't know where to start

Let's see what @cescoffier has to say before proceeding any further

@emmanuelbernard
Copy link
Member

Using quarkus-smallrye-reactive-messaging-kafka btw at the moment, haven't tried io.quarkus:quarkus-kafka-client

And thanks for let me know. I did look at the kafka client code and what you reported was not making sense. I'm happy it was not my brain disagreeing with reality.

@FroMage
Copy link
Member

FroMage commented Dec 3, 2020

@cescoffier would you welcome a PR to fix this?

@cescoffier
Copy link
Member

I need to check., but I believe it's already fixed by smallrye/smallrye-reactive-messaging#801.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants