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

[bitnami/kafka] Issue 32224- Allow empty values in Kafka config as environment variable #42309

Merged
merged 3 commits into from
Aug 4, 2023
Merged

Conversation

arushi315
Copy link
Contributor

Description of the change

Remove the 'Empty String' check for Kafka configuration values in the kafka_configure_from_environment_variables function in the libkafka.sh script for the 3.2, 3.3 and 3.4 Kafka containers

Benefits

This allows the 'ssl.endpoint.identification.algorithm' configuration value to be set to an empty string.
By default, the broker will the 'HTTPS' as the default value. To choose to disable this check, the configuration value must be set to an empty string.
There is a similar setting to configure zookeeper.

Possible drawbacks

This change allows any parameter to be set to an empty string.

Applicable issues

fixes #32224

@github-actions github-actions bot added the kafka label Jul 25, 2023
@github-actions github-actions bot added the triage Triage is needed label Jul 25, 2023
arushir added 3 commits July 25, 2023 15:58
Signed-off-by: arushir <arushir@vmware.com>
This reverts commit 53f46db.

Signed-off-by: arushir <arushir@vmware.com>
Signed-off-by: arushir <arushir@vmware.com>
@javsalgar javsalgar changed the title Issue 32224- Allow empty values in Kafka config as environment variable [bitnami/kafka] Issue 32224- Allow empty values in Kafka config as environment variable Jul 26, 2023
@javsalgar javsalgar added the verify Execute verification workflow for these changes label Jul 26, 2023
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Jul 26, 2023
@bitnami-bot bitnami-bot removed the request for review from carrodher July 26, 2023 12:11
@FraPazGal
Copy link
Contributor

Thanks for the PR @arushi315! The PR LGTM but I'll run some internal checks before giving the final approval and merging.

@arushi315
Copy link
Contributor Author

Hi @FraPazGal
Any update on this?

Copy link
Contributor

@FraPazGal FraPazGal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay @arushi315, our internal testing threw some errors and I wanted to double-check them first. They were due to transient issues on our end and the changes LGTM.

Thanks for the contribution!

@FraPazGal FraPazGal merged commit e74c35b into bitnami:main Aug 4, 2023
15 checks passed
@jekanik
Copy link

jekanik commented Aug 4, 2023

Hi! @FraPazGal @arushi315
Looks like this PR breaks our docker compose.

Exception:

2023-08-04 16:47:10 Exception in thread "main" org.apache.kafka.common.config.ConfigException: Listener with name  defined in inter.broker.listener.name not found in listener.security.protocol.map.
2023-08-04 16:47:10     at kafka.server.KafkaConfig.$anonfun$getInterBrokerListenerNameAndSecurityProtocol$1(KafkaConfig.scala:2004)
2023-08-04 16:47:10     at scala.collection.immutable.Map$Map2.getOrElse(Map.scala:236)
2023-08-04 16:47:10     at kafka.server.KafkaConfig.getInterBrokerListenerNameAndSecurityProtocol(KafkaConfig.scala:2003)
2023-08-04 16:47:10     at kafka.server.KafkaConfig.interBrokerListenerName(KafkaConfig.scala:1875)
2023-08-04 16:47:10     at kafka.server.KafkaConfig.validateValues(KafkaConfig.scala:2198)
2023-08-04 16:47:10     at kafka.server.KafkaConfig.<init>(KafkaConfig.scala:2063)
2023-08-04 16:47:10     at kafka.server.KafkaConfig.<init>(KafkaConfig.scala:1494)
2023-08-04 16:47:10     at kafka.tools.StorageTool$.$anonfun$main$1(StorageTool.scala:41)
2023-08-04 16:47:10     at scala.Option.flatMap(Option.scala:271)
2023-08-04 16:47:10     at kafka.tools.StorageTool$.main(StorageTool.scala:41)
2023-08-04 16:47:10     at kafka.tools.StorageTool.main(StorageTool.scala)

Part of compose:

  kafka:
    image: 'bitnami/kafka:3.3.2'
    hostname: kafka
    container_name: kafka
    networks:
      - test
    environment:
      - KAFKA_ENABLE_KRAFT=yes
      - KAFKA_CFG_PROCESS_ROLES=broker,controller
      - KAFKA_CFG_CONTROLLER_LISTENER_NAMES=CONTROLLER
      - KAFKA_CFG_LISTENERS=PLAINTEXT://:9092,CONTROLLER://:9093
      - KAFKA_CFG_LISTENER_SECURITY_PROTOCOL_MAP=CONTROLLER:PLAINTEXT,PLAINTEXT:PLAINTEXT
      - KAFKA_CFG_ADVERTISED_LISTENERS=PLAINTEXT://kafka:9092
      - KAFKA_CFG_BROKER_ID=1
      - KAFKA_CFG_NODE_ID=1
      - KAFKA_CFG_CONTROLLER_QUORUM_VOTERS=1@kafka:9093
      - ALLOW_PLAINTEXT_LISTENER=yes
      - KAFKA_KRAFT_CLUSTER_ID=r4zt_wrqTRuT7W2NJsB_GA
      - KAFKA_CFG_DELETE_TOPIC_ENABLE=true
      - KAFKA_CFG_AUTO_CREATE_TOPICS_ENABLE=true

We fix it with adding KAFKA_CFG_INTER_BROKER_LISTENER_NAME=PLAINTEXT or hardcoding image to bitnami/kafka:3.3.2-debian-11-r181.
Just want you to know that issue has appeared.

Thanks

@github-actions github-actions bot removed the solved label Aug 4, 2023
@github-actions github-actions bot added the triage Triage is needed label Aug 4, 2023
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Aug 4, 2023
@bitnami-bot bitnami-bot assigned dgomezleon and unassigned FraPazGal Aug 4, 2023
@carrodher carrodher removed the request for review from dgomezleon August 4, 2023 17:05
@carrodher carrodher assigned FraPazGal and unassigned dgomezleon Aug 4, 2023
@OneCricketeer
Copy link

This doesn't seem to work for the following

kafka:
    image: bitnami/kafka:3.5.1
    restart: unless-stopped
    environment:
      KAFKA_KRAFT_CLUSTER_ID: WNfE3WMTRRGBs35BikbfRg # Run 'kafka-storage random-uuid'
      BITNAMI_DEBUG: yes
      ALLOW_PLAINTEXT_LISTENER: yes
      KAFKA_ENABLE_KRAFT: yes
      KAFKA_CFG_CONTROLLER_LISTENER_NAMES: CONTROLLER
      KAFKA_CFG_DELETE_TOPIC_ENABLE: 'true'
      KAFKA_CFG_LOG_RETENTION_HOURS: 48  # 2 days of retention for demo purposes
      KAFKA_CFG_PROCESS_ROLES: controller
      KAFKA_CFG_NODE_ID: 1
      KAFKA_CFG_CONTROLLER_QUORUM_VOTERS: 1@kafka-controller:9093
      KAFKA_CFG_LISTENERS: CONTROLLER://:9093
      KAFKA_CFG_LISTENER_SECURITY_PROTOCOL_MAP: PLAINTEXT:PLAINTEXT,CONTROLLER:PLAINTEXT
      KAFKA_CFG_ADVERTISED_LISTENERS: ""  # here

Getting error when starting

The advertised.listeners config must be empty when process.roles=controller

@FraPazGal
Copy link
Contributor

Thanks for the reports. As @jekanik said, this change has introduced unwanted breaking changes. This is caused by having some of the KAFKA_CFG envars set to empty in kafka-env.sh. Before this PR, these envars were not taken into account and no changes were made to the config file. Now that every set environment variable is added to the config file, we are adding some of them as "" instead of leaving the default value.

We are investigating a hotfix for this issue and we'll try to have more info by the end of the day.

@arushi315
Copy link
Contributor Author

arushi315 commented Aug 7, 2023

Hi @FraPazGal
Thank you for looking into this.
For the breaking changes, were you able to merge the hotfix and do you have a ticket to track that?
Edit: Read your message again. Looking for an update end of the day!

@FraPazGal
Copy link
Contributor

Hi @arushi315 @jekanik,

After investigating possible workarounds and the changes' nature we have opened an internal task to work on a permanent solution for this. In the meantime a working workaround is to explicitly setting a value for any faulty "empty" environment variable so that is uses Kafka's default value. As such, adding the following to your docker-compose should solve the issues:

kafka:
    image: bitnami/kafka:3.5.1
    restart: unless-stopped
    environment:
      ...
      - KAFKA_CFG_LISTENERS=PLAINTEXT://:9092
      - KAFKA_CFG_INTER_BROKER_LISTENER_NAME=PLAINTEXT # mandatory
      - KAFKA_CFG_LISTENER_SECURITY_PROTOCOL_MAP=PLAINTEXT:PLAINTEXT # removed SSL procotols for readability
      - KAFKA_CFG_SASL_MECHANISM_INTER_BROKER_PROTOCOL=GSSAPI
      - KAFKA_CFG_SASL_MECHANISM_CONTROLLER_PROTOCOL=GSSAPI
      - KAFKA_CFG_MAX_REQUEST_SIZE=1048576
      - KAFKA_CFG_MAX_PARTITION_FETCH_BYTES=1048576

Unless stated, the values used above are Kafka's default ones. Remember you only need to use these values if you are not already using these envars in your setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kafka solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bitnami/kafka] Kafka config via ENV no longer accepts empty values
8 participants