Skip to content

MINIFICPP-2718 Windows based docker tests#2133

Open
martinzink wants to merge 7 commits intoapache:mainfrom
martinzink:MINIFICPP-2718
Open

MINIFICPP-2718 Windows based docker tests#2133
martinzink wants to merge 7 commits intoapache:mainfrom
martinzink:MINIFICPP-2718

Conversation

@martinzink
Copy link
Copy Markdown
Member

This PR allows windows based docker containers to run the behave test suite with some limitations:

  • I only enabled a handful of features because some of the tests are requiring thirdparty containers (linux only and windows doesnt allow mixed (windows and linux) containers)
  • Even the tests that dont require thirdparty containers might rely on extra linux commands that needs to be replicated in powershell
  • Its unfeasable to enable these in github CI, because this requires nested virtualization (which is quite rare for VMs).

So this is only the MVP for this feature, but it is still a huge help for future developments especially for using it to develop external extensions using the upcoming C api. It cuts down the local verification time.

I've also replaced the previous m2crypto based ssl_utils with the industry standard cryptography, this should help with the setup on macs and windows systems (m2crypto was quite difficult to install on these systems)


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.

Comment on lines +15 to +21
[tool.setuptools]
package-dir = {"" = "src"}
packages = ["minifi_test_framework"]

[tool.setuptools.packages.find]
where = ["src"]
include = ["minifi_test_framework*"]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is required for wheel building

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enables running a subset of the Behave integration test suite with Windows-based Docker containers, and refactors the test framework container abstractions to support both Linux and Windows. It also replaces the previous M2Crypto/PyOpenSSL-based SSL helpers with cryptography, improving cross-platform dependency installation.

Changes:

  • Add Windows container support (new Windows container implementation, Windows MiNiFi container, and scenario tag gating via @SUPPORTS_WINDOWS).
  • Refactor test containers to explicitly target Linux (LinuxContainer) and extract controller/config helpers.
  • Replace SSL utilities implementation and dependencies with cryptography, updating affected test containers accordingly.

Reviewed changes

Copilot reviewed 42 out of 44 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
extensions/standard-processors/tests/features/steps/steps.py Switch SSL setup to shared helper function.
extensions/standard-processors/tests/features/steps/minifi_controller_steps.py Route controller actions through a controller helper object; update retry decorator args.
extensions/standard-processors/tests/features/steps/minifi_c2_server_container.py Switch to cryptography-based cert/key serialization; base class to LinuxContainer.
extensions/standard-processors/tests/features/replace_text.feature Mark feature as supporting Windows.
extensions/standard-processors/tests/features/environment.py Avoid Alpine image inspection on Windows host.
extensions/standard-processors/tests/features/core_functionality.feature Mark feature as supporting Windows; increase timeouts.
extensions/standard-processors/tests/features/containers/tcp_client_container.py Base class switched to LinuxContainer.
extensions/standard-processors/tests/features/containers/syslog_container.py Base class switched to LinuxContainer.
extensions/standard-processors/tests/features/containers/diag_slave_container.py Base class switched to LinuxContainer.
extensions/sql/tests/features/containers/postgress_server_container.py Base class switched to LinuxContainer.
extensions/splunk/tests/features/containers/splunk_container.py Base class switched to LinuxContainer; switch to cryptography dump helpers.
extensions/prometheus/tests/features/containers/prometheus_container.py Base class switched to LinuxContainer; switch to cryptography dump helpers.
extensions/opc/tests/features/containers/opc_ua_server_container.py Base class switched to LinuxContainer.
extensions/mqtt/tests/features/containers/mqtt_broker_container.py Base class switched to LinuxContainer.
extensions/kafka/tests/features/containers/kafka_server_container.py Base class switched to LinuxContainer; switch keystore/truststore material to cryptography dump helpers.
extensions/grafana-loki/tests/features/containers/reverse_proxy_container.py Base class switched to LinuxContainer.
extensions/grafana-loki/tests/features/containers/grafana_loki_container.py Base class switched to LinuxContainer; switch to cryptography dump helpers.
extensions/gcp/tests/features/containers/fake_gcs_server_container.py Base class switched to LinuxContainer.
extensions/elasticsearch/tests/features/containers/opensearch_container.py Switch to cryptography dump helpers.
extensions/elasticsearch/tests/features/containers/elasticsearch_container.py Switch to cryptography dump helpers.
extensions/elasticsearch/tests/features/containers/elastic_base_container.py Base class switched to LinuxContainer.
extensions/couchbase/tests/features/containers/couchbase_server_container.py Base class switched to LinuxContainer; switch to cryptography dump helpers; update retry decorator arg names.
extensions/azure/tests/features/containers/azure_server_container.py Base class switched to LinuxContainer.
extensions/aws/tests/features/containers/s3_server_container.py Base class switched to LinuxContainer.
extensions/aws/tests/features/containers/kinesis_server_container.py Base class switched to LinuxContainer.
docker/installed/win.Dockerfile New Windows MiNiFi installed-image Dockerfile.
docker/RunBehaveTests.sh Improve feature file collection portability (avoid mapfile).
behave_framework/src/minifi_test_framework/steps/configuration_steps.py Switch from container methods to protocol helper functions.
behave_framework/src/minifi_test_framework/core/ssl_utils.py Replace M2Crypto/PyOpenSSL SSL utilities with cryptography.
behave_framework/src/minifi_test_framework/core/minifi_test_context.py Introduce OS-/image-based MiNiFi container selection (Windows vs Linux, FHS vs normal).
behave_framework/src/minifi_test_framework/core/hooks.py Skip scenarios on Windows unless tagged SUPPORTS_WINDOWS; guard cleanup if setup didn’t run.
behave_framework/src/minifi_test_framework/containers/nifi_container.py Base class switched to LinuxContainer; switch to cryptography dump helpers.
behave_framework/src/minifi_test_framework/containers/minifi_win_container.py New Windows MiNiFi container implementation.
behave_framework/src/minifi_test_framework/containers/minifi_protocol.py New protocol + shared property-setting helpers.
behave_framework/src/minifi_test_framework/containers/minifi_linux_container.py New Linux MiNiFi container implementation (normal vs FHS deployment).
behave_framework/src/minifi_test_framework/containers/minifi_controller.py New helper encapsulating minifi-controller interactions.
behave_framework/src/minifi_test_framework/containers/minifi_container.py Removed monolithic MiNiFi container (replaced by linux/windows split).
behave_framework/src/minifi_test_framework/containers/http_proxy_container.py Base class switched to LinuxContainer.
behave_framework/src/minifi_test_framework/containers/container_windows.py New Windows container implementation for file ops + exec + log helpers.
behave_framework/src/minifi_test_framework/containers/container_protocol.py New protocol defining container interface used by the framework.
behave_framework/src/minifi_test_framework/containers/container_linux.py Rename/reshape base container into LinuxContainer; move shared helpers.
behave_framework/pyproject.toml Bump framework version; swap SSL deps to cryptography; adjust package discovery.

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

martinzink added a commit that referenced this pull request Mar 31, 2026
martinzink added a commit to martinzink/nifi-minifi-cpp that referenced this pull request Apr 7, 2026
martinzink added a commit to martinzink/nifi-minifi-cpp that referenced this pull request Apr 8, 2026
@szaszm szaszm self-requested a review April 13, 2026 13:28

Then at least one file with the content "first_custom_text" is placed in the "/tmp/output" directory in less than 20 seconds
And at least one file with the content "second_custom_text" is placed in the "/tmp/output" directory in less than 20 seconds
Then at least one file with the content "first_custom_text" is placed in the "/tmp/output" directory in less than 200 seconds
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

why?

@fgerlits fgerlits self-requested a review April 14, 2026 10:15
Comment on lines +489 to +492
exit_code, output = self.exec_run(["awk", "/VmRSS/ { printf \"%d\\n\", $2 }", "/proc/1/status"])
if exit_code != 0:
return None
memory_usage_in_bytes = int(output.strip()) * 1024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

very minor, but why do we add the \n in line 489 only to strip it out in line 492? also, I would include "kB" in the regex, just in case the format of /proc/1/status changes in the future:

Suggested change
exit_code, output = self.exec_run(["awk", "/VmRSS/ { printf \"%d\\n\", $2 }", "/proc/1/status"])
if exit_code != 0:
return None
memory_usage_in_bytes = int(output.strip()) * 1024
exit_code, output = self.exec_run(["awk", "/VmRSS.*kB/ { printf \"%d\", $2 }", "/proc/1/status"])
if exit_code != 0:
return None
memory_usage_in_bytes = int(output) * 1024

Comment on lines +39 to +40
def directory_contains_file_with_content(self, directory_path: str, expected_content: str) -> bool:
...
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a bit strange that we say these stubs return bool, str or int, but in fact we return None. Maybe raise an exception instead?

self.client: docker.DockerClient = docker.from_env()
self.container: Optional[Container] = None
self.files: List[File] = []
self.dirs: List[Directory] = []
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor, but List is deprecated, we should use list instead: https://docs.python.org/3/library/typing.html#aliases-to-built-in-types

Comment on lines +58 to +66
def _normalize_path(self, path: str) -> str:
clean_path = path.strip().replace("/", "\\")
if clean_path.startswith("\\"):
clean_path = clean_path[1:]

# If it doesn't already have a drive letter, assume C:
if ":" not in clean_path:
return f"C:\\{clean_path}"
return clean_path
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would os.path.normpath() work instead of this function?

Comment on lines +369 to +371
exit_code, _ = self._run_powershell(ps_script)

return exit_code == 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we could log the error (if any) here, too, as in the other functions

Comment on lines +364 to +366
f"if ((Test-Path -LiteralPath '{win_path}' -PathType Container) "
f"-and (Get-ChildItem -LiteralPath '{win_path}' -Force | Select-Object -First 1)) "
f"{{ exit 0 }} else {{ exit 1 }}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When I run this on a test directory, I get True on both empty and non-empty directories. A StackOverflow answer suggests adding Measure-Object:

Suggested change
f"if ((Test-Path -LiteralPath '{win_path}' -PathType Container) "
f"-and (Get-ChildItem -LiteralPath '{win_path}' -Force | Select-Object -First 1)) "
f"{{ exit 0 }} else {{ exit 1 }}"
f"if ((Test-Path -LiteralPath '{win_path}' -PathType Container) "
f"-and ((Get-ChildItem -LiteralPath '{win_path}' -Force | "
f" Select-Object -First 1 | Measure-Object).Count -gt 0)) "
f"{{ exit 0 }} else {{ exit 1 }}"

Comment on lines +44 to +47
cert = x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key()).serial_number(
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.timezone.utc)).not_valid_after(
datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(days=3650)).add_extension(
x509.SubjectKeyIdentifier.from_public_key(key.public_key()), critical=False, ).add_extension(x509.BasicConstraints(ca=True, path_length=None),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpicking, but I think breaking into lines like this would be easier to read:

Suggested change
cert = x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key()).serial_number(
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.timezone.utc)).not_valid_after(
datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(days=3650)).add_extension(
x509.SubjectKeyIdentifier.from_public_key(key.public_key()), critical=False, ).add_extension(x509.BasicConstraints(ca=True, path_length=None),
cert = (x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key())
.serial_number(x509.random_serial_number())
.not_valid_before(datetime.datetime.now(datetime.timezone.utc))
.not_valid_after(datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(days=3650))
.add_extension(x509.SubjectKeyIdentifier.from_public_key(key.public_key()), critical=False)
.add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True)
.sign(key, hashes.SHA256()))

Comment on lines +36 to +37
Then at least one file with the content "first_custom_text" is placed in the "/tmp/output" directory in less than 200 seconds
And at least one file with the content "second_custom_text" is placed in the "/tmp/output" directory in less than 200 seconds
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this much slower on Windows, or was the timeout only changed for testing and it can be reverted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants