MINIFICPP-2666 Move Kafka tests to modular docker tests #2059

lordgamez · 2025-11-06T16:39:23Z

Create SSL certificates for each MiNiFi container for SSL tests

https://issues.apache.org/jira/browse/MINIFICPP-2666

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?

For code changes:

If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE file?
If applicable, have you updated the NOTICE file?

For documentation related changes:

Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

Copilot

Pull Request Overview

This PR migrates Kafka integration tests from the legacy test framework to the new modular docker-based testing framework, enabling SSL certificate generation for each MiNiFi container and modernizing the test infrastructure.

Key Changes:

Introduces a new KafkaServer container class with SSL/SASL authentication support
Migrates all Kafka test scenarios to use the new step definition framework
Adds SSL certificate generation utilities for secure communication testing

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
extensions/kafka/tests/features/steps/kafka_server_container.py	New KafkaServer container implementation with SSL/SASL configuration
extensions/kafka/tests/features/steps/steps.py	Migrated Kafka test step definitions to new framework
extensions/kafka/tests/features/publishkafka.feature	Updated PublishKafka scenarios to use new test framework
extensions/kafka/tests/features/consumekafka.feature	Updated ConsumeKafka scenarios to use new test framework
extensions/kafka/tests/features/environment.py	Added Kafka helper Docker image builder and scenario hooks
behave_framework/src/minifi_test_framework/core/ssl_utils.py	New SSL certificate generation utilities
behave_framework/src/minifi_test_framework/containers/minifi_container.py	Enhanced to generate SSL certificates for each container
behave_framework/src/minifi_test_framework/containers/container.py	Added support for binary files and container lifecycle management
behave_framework/src/minifi_test_framework/steps/flow_building_steps.py	Added property removal support and SSL context service setup
behave_framework/src/minifi_test_framework/steps/checking_steps.py	Added regex matching and enhanced file verification capabilities
behave_framework/pyproject.toml	Added SSL-related dependencies (m2crypto, pyopenssl, pyjks)
docker/test/integration/minifi/processors/ConsumeKafka.py	Removed old ConsumeKafka processor class
docker/test/integration/cluster/containers/KafkaBrokerContainer.py	Removed old KafkaBrokerContainer implementation
docker/test/integration/cluster/checkers/KafkaHelper.py	Removed old KafkaHelper class
docker/RunBehaveTests.sh	Added Kafka tests to the test execution script

Comments suppressed due to low confidence (6)

extensions/kafka/tests/features/steps/steps.py:91

There are duplicate function definitions with the same name step_impl in this file. Python will only keep the last definition, causing earlier step definitions to be overridden. Each step function should have a unique name.

For example, lines 32, 37, 67, 74, 81, 88 all define def step_impl(context): or similar signatures. These should be renamed to unique names like step_impl_kafka_server_setup, step_impl_consume_kafka_setup, etc.
extensions/kafka/tests/features/steps/steps.py:215

There are two different functions with the same name wait_for_consumer_registration defined on lines 203 and 211. Python will only keep the last definition, making the first function (line 203) unreachable. These functions should have unique names like wait_for_consumer_initial_registration and wait_for_consumer_reregistration.
extensions/kafka/tests/features/steps/steps.py:22
Import of 'checking_steps' is not used.
extensions/kafka/tests/features/steps/steps.py:23
Import of 'configuration_steps' is not used.
extensions/kafka/tests/features/steps/steps.py:24
Import of 'core_steps' is not used.
extensions/kafka/tests/features/steps/steps.py:25
Import of 'flow_building_steps' is not used.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

behave_framework/src/minifi_test_framework/containers/minifi_container.py

behave_framework/src/minifi_test_framework/steps/flow_building_steps.py

behave_framework/src/minifi_test_framework/containers/container.py

extensions/kafka/tests/features/steps/kafka_server_container.py

behave_framework/src/minifi_test_framework/containers/container.py

fgerlits · 2025-12-02T10:50:49Z

behave_framework/src/minifi_test_framework/containers/container.py

+    def start(self):
+        if self.container:
+            self.container.start()
+
+    def stop(self):
+        if self.container:
+            self.container.stop()
+
+    def kill(self):
+        if self.container:
+            self.container.kill()
+
+    def restart(self):
+        if self.container:
+            self.container.restart()


Could we log something if self.container does not exist? I assume that shouldn't happen normally, so a log would help with debugging if it does.

fgerlits · 2025-12-02T10:58:35Z

behave_framework/src/minifi_test_framework/containers/container.py

+
+            return os.path.join(temp_dir, os.path.basename(directory_path.strip('/')))
+        except Exception as e:
+            logging.error(f"Error extracting files from container: {e}")


Does e contain the container's name and directory_path? If it doesn't, then I would add these two to the error message.

fgerlits · 2025-12-02T18:05:49Z

extensions/kafka/tests/features/steps/kafka_server_container.py

+    def create_topic(self, topic_name: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-topics.sh --create --topic {topic_name} --bootstrap-server {self.container_name}:9092"])
+        logging.info("Create topic output: %s", output)
+        return code == 0
+
+    def produce_message(self, topic_name: str, message: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message}'"])
+        logging.info("Produce message output: %s", output)
+        return code == 0
+
+    def produce_message_with_key(self, topic_name: str, message: str, message_key: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --property 'key.separator=:' --property 'parse.key=true' --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message_key}:{message}'"])
+        logging.info("Produce message with key output: %s", output)
+        return code == 0


Can we log code, as well, please?

fgerlits · 2025-12-02T18:19:57Z

extensions/kafka/tests/features/consumekafka.feature

    And a message with content "<message 2>" is published to the "ConsumeKafkaTest" topic with key "consume_kafka_test_key"

-    Then two flowfiles with the contents "<message 1>" and "<message 2>" are placed in the monitored directory in less than 45 seconds
+    Then the contents of "/tmp/output" in less than 30 seconds are: "<message 1>" and "<message 2>"


The old phrasing sounds more natural and clearer to me: "happens in less than x seconds" requires an event ("flowfiles are placed"), not a state ("the contents are"). Can we change it back?

fgerlits · 2025-12-02T18:22:29Z

extensions/kafka/tests/features/consumekafka.feature

    Examples: Topic names and formats to test
      | message 1            | message 2           | topic names              | topic name format |
-      | Ulysses              | James Joyce         | ConsumeKafkaTest         | (not set)         |
      | The Great Gatsby     | F. Scott Fitzgerald | ConsumeKafkaTest         | Names             |


why was the first example line removed?

szaszm · 2025-12-02T17:22:31Z

extensions/kafka/tests/features/steps/kafka_server_container.py

+        return code == 0
+
+    def produce_message(self, topic_name: str, message: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message}'"])


optional: I'd avoid bashisms and use a standard pipe.

Suggested change

(code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message}'"])

(code, output) = self.exec_run(["/bin/sh", "-c", f"echo '{message}' | /opt/kafka/bin/kafka-console-producer.sh --topic '{topic_name}' --bootstrap-server '{self.container_name}':9092"])

This will also likely fail on Windows, with @martinzink 's changes to run the tests on windows containers.

thats moot, because we wont be able to support this on windows anyways, there is no kafka server windows container and as of now we cant mix and match windows and linux containers

szaszm · 2025-12-02T17:23:45Z

extensions/kafka/tests/features/steps/kafka_server_container.py

+        return code == 0
+
+    def produce_message_with_key(self, topic_name: str, message: str, message_key: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --property 'key.separator=:' --property 'parse.key=true' --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message_key}:{message}'"])


Suggested change

(code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --property 'key.separator=:' --property 'parse.key=true' --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message_key}:{message}'"])

(code, output) = self.exec_run(["/bin/sh", "-c", f" echo ''{message_key}:{message}'' | /opt/kafka/bin/kafka-console-producer.sh --property 'key.separator=:' --property 'parse.key=true' --topic '{topic_name}' --bootstrap-server '{self.container_name}':9092"])

szaszm · 2025-12-02T23:00:25Z

behave_framework/src/minifi_test_framework/containers/container.py

+        if self.container.status == "running":
+            return self._verify_file_contents_in_running_container(directory_path, expected_contents)
+
+        return self._verify_file_contents_in_stopped_container(directory_path, expected_contents)


what if the container stops after the check, before/during scanning the directory?

szaszm · 2025-12-03T12:58:30Z

extensions/kafka/tests/features/steps/kafka_server_container.py

+            context=None)
+
+    def create_topic(self, topic_name: str):
+        (code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-topics.sh --create --topic {topic_name} --bootstrap-server {self.container_name}:9092"])


optional: sh + quotes to handle more special characters

Suggested change

(code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-topics.sh --create --topic {topic_name} --bootstrap-server {self.container_name}:9092"])

(code, output) = self.exec_run(["/bin/sh", "-c", f"/opt/kafka/bin/kafka-topics.sh --create --topic '{topic_name}' --bootstrap-server '{self.container_name}':9092"])

szaszm · 2025-12-03T13:19:40Z

extensions/kafka/tests/features/consumekafka.feature

+    And RouteOnAttribute is EVENT_DRIVEN
+    And a LogAttribute processor
+    And LogAttribute is EVENT_DRIVEN
+    And a PutFile processor with the "Directory" property set to "/tmp/output"
+    And PutFile is EVENT_DRIVEN


Shouldn't it be default in the test framework to have non-source processors use event driven scheduling? to reduce boilerplate

szaszm · 2025-12-03T13:23:30Z

extensions/kafka/tests/features/consumekafka.feature

    And the publisher performs a <transaction type> transaction publishing to the "ConsumeKafkaTest" topic these messages: <messages sent>

-    Then <number of flowfiles expected> flowfiles are placed in the monitored directory in less than 15 seconds
+    Then there are <number of flowfiles expected> files in the "/tmp/output" directory in less than 15 seconds


@fgerlits comment applies to this too: it's clearer to expect an event than a state, so the old version was better.

lordgamez added depends-on-another-PR low-impact Test only or trivial change that's most likely not gonna introduce any new bugs labels Nov 6, 2025

lordgamez force-pushed the MINIFICPP-2666 branch 2 times, most recently from e172fa7 to 4c9af78 Compare November 10, 2025 09:25

lordgamez mentioned this pull request Nov 11, 2025

MINIFICPP-2668 Move standard processor tests to modular docker tests #2061

Open

8 tasks

fgerlits removed the depends-on-another-PR label Nov 14, 2025

lordgamez added the depends-on-another-PR label Nov 17, 2025

lordgamez force-pushed the MINIFICPP-2624_opcua branch from dc13c8e to c95e615 Compare November 17, 2025 09:55

lordgamez force-pushed the MINIFICPP-2666 branch from 4c9af78 to 2640d9b Compare November 17, 2025 09:55

lordgamez requested a review from Copilot November 17, 2025 12:23

Copilot started reviewing on behalf of lordgamez November 17, 2025 12:23 View session

Copilot finished reviewing on behalf of lordgamez November 17, 2025 12:26

Copilot AI reviewed Nov 17, 2025

View reviewed changes

lordgamez force-pushed the MINIFICPP-2666 branch 3 times, most recently from 9d8ce58 to 0f60d4a Compare November 17, 2025 13:17

MINIFICPP-2666 Move Kafka tests to modular docker tests

985912c

lordgamez removed the depends-on-another-PR label Nov 21, 2025

lordgamez changed the base branch from MINIFICPP-2624_opcua to main November 21, 2025 08:39

lordgamez force-pushed the MINIFICPP-2666 branch from 0f60d4a to 985912c Compare November 21, 2025 08:39

fgerlits reviewed Dec 2, 2025

View reviewed changes

szaszm reviewed Dec 2, 2025

View reviewed changes

szaszm reviewed Dec 3, 2025

View reviewed changes

	(code, output) = self.exec_run(["/bin/bash", "-c", f"/opt/kafka/bin/kafka-console-producer.sh --topic {topic_name} --bootstrap-server {self.container_name}:9092 <<< '{message}'"])
	(code, output) = self.exec_run(["/bin/sh", "-c", f"echo '{message}' \| /opt/kafka/bin/kafka-console-producer.sh --topic '{topic_name}' --bootstrap-server '{self.container_name}':9092"])

MINIFICPP-2666 Move Kafka tests to modular docker tests #2059

Are you sure you want to change the base?

MINIFICPP-2666 Move Kafka tests to modular docker tests #2059

Uh oh!

Conversation

lordgamez commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For all changes:

For code changes:

For documentation related changes:

Note:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgerlits Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lordgamez commented Nov 6, 2025 •

edited

Loading

fgerlits Dec 2, 2025 •

edited

Loading