MINIFICPP-2644 Add proxy controller service support for AWS and Azure processors by lordgamez · Pull Request #2045 · apache/nifi-minifi-cpp

lordgamez · 2025-10-09T11:19:41Z

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

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.

lordgamez · 2025-10-09T12:56:31Z

The clang tidy errors are already fixed in #2040

Copilot

Pull Request Overview

This PR adds proxy controller service support for AWS and Azure processors, allowing centralized proxy configuration management through a new ProxyConfigurationService controller service instead of requiring individual proxy properties on each processor.

Introduces a new ProxyConfigurationService controller service for centralized proxy configuration management
Updates AWS and Azure processors to support using the proxy configuration service as an alternative to individual proxy properties
Adds comprehensive test coverage for the new proxy configuration service functionality

Reviewed Changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
minifi-api/include/minifi-cpp/controllers/ProxyConfigurationServiceInterface.h	Interface definition for proxy configuration service
libminifi/include/controllers/ProxyConfigurationService.h	Implementation header for proxy configuration service
libminifi/src/controllers/ProxyConfigurationService.cpp	Core implementation of proxy configuration service
libminifi/test/unit/ProxyConfigurationServiceTests.cpp	Unit tests for proxy configuration service
extensions/azure/processors/*	Azure processor base classes updated to support proxy service
extensions/azure/storage/*	Azure storage clients updated to handle proxy configuration
extensions/azure/tests/*	Test files updated to include proxy service test cases
extensions/aws/processors/AwsProcessor.*	AWS processor updated to support proxy configuration service
extensions/aws/tests/*	AWS test files updated to test both proxy service and direct properties
docker/test/integration/*	Integration test support for proxy configuration service
PROCESSORS.md	Documentation updated to reflect new proxy configuration service property
CONTROLLERS.md	Documentation for the new ProxyConfigurationService controller

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

PROCESSORS.md

lordgamez · 2025-10-13T07:12:04Z

The issues listed in the clang tidy checks are fixed in the following PR: #2040

CONTROLLERS.md

szaszm · 2025-11-13T13:33:20Z

minifi-api/include/minifi-cpp/controllers/ProxyConfigurationServiceInterface.h

+struct ProxyConfiguration {
+  std::string proxy_host;
+  std::optional<uint16_t> proxy_port;
+  std::optional<std::string> proxy_user;
+  std::optional<std::string> proxy_password;
+};


details like this should not be on the API. username/password can be optional and replaced by other auth methods in some other proxy protocols. Possibly even hostname and port can vary.

As we only support HTTP proxies for now only these parameters are used, they can be extended or refactored when other protocols are introduced.

If we add this type definition to the API, we can't change it later without breaking API.

I'm not sure what would be the best way to define this to adjust it to the future C API. In the current form I think it will only be extended, so non of the current field will be changed in the future. We should discuss this how these cases should be handled in the future, because I think additional fields being added to a struct on the API can be common. We can either define some additional reserved fields on the C API for future extensions, we can define some accessors on the C API so the struct itself will not be published to the API, or we can just break it when needed if we can predict that it will not be changed for a long time. I think we still have some time for think about this as controller services will not be supported on the C API for a while. What do you suggest in this case?

We could change this to have separate getter function for each field like we do on the SSLContextServiceInterface, in that way that can be extended when new properties are introduced, what do you think?

I think a getter-only interface could work. I would also add a proxy type getter, even if the type enum only has one possible value today, so older processors can detect future versions of the service that are incompatible.

I think we could also do a flat struct with the first field being a type enum field, if the interface always returns a heap allocated object: that way future versions can grow the struct without breaking its layout. I'd prefer this approach.

I updated the interface to use a getter-only interface to be consistent with the SSLContextServiceInterface, I also added a type enum in 12e4262

extensions/aws/processors/AwsProcessor.cpp

CONTROLLERS.md

libminifi/include/controllers/ProxyConfigurationService.h

szaszm · 2026-02-23T12:50:11Z

extensions/aws/processors/AwsProcessor.h

+  EXTENSIONAPI static constexpr auto ProxyType = core::PropertyDefinitionBuilder<magic_enum::enum_count<minifi::controllers::ProxyType>()>::createProperty("Proxy Type")
+      .withDescription("Proxy type")
+      .withDefaultValue(magic_enum::enum_name(minifi::controllers::ProxyType::HTTP))
+      .withAllowedValues(magic_enum::enum_names<minifi::controllers::ProxyType>())
+      .build();


this belongs in the ProxyConfigurationService, not in the processors. In NiFi the same controller service specifies all proxy details, including the type/protocol.

It is also present in the controller service, but the proxy properties were already present in the AWS processors before introducing the proxy controller service, so this needs to be added here too. Maybe we can deprecate it in the future and remove them to only use the proxy controller service. In NiFi 2.0 they also removed the proxy properties leaving the proxy configuration service property. I would not suggest using the existing proxy properties for host, port, username, password, but require adding a controller service just for the proxy type.

szaszm · 2026-02-23T12:59:20Z

extensions/azure/storage/DataLakeStorageClient.h

  std::string file_system_name;
  std::string directory_name;
  std::optional<uint64_t> number_of_retries;
+  std::optional<minifi::controllers::ProxyConfiguration> proxy_configuration;


Instead of an optional member, it could default to type==DIRECT, which means no proxy. That would make one more impossible state unrepresentable, which is considered a best practice nowadays.

I'm not sure I'm convinced about this. What do you mean by "one more impossible state unrepresentable"? It seem a bit unintuitive to me. When the user adds a proxy controller service their intention is to use a proxy, if they do not want to use a proxy they should not add a service at all. What would be the benefit of adding a proxy service and configure it to not to use a proxy?

szaszm · 2026-02-23T13:57:11Z

extensions/aws/processors/AwsProcessor.cpp

+    proxy.proxy_user = proxy_controller_service->getUsername().value_or("");
+    proxy.proxy_password = proxy_controller_service->getPassword().value_or("");
+  } else {
+    proxy.proxy_type = minifi::utils::parseOptionalEnumProperty<minifi::controllers::ProxyType>(context, ProxyType).value_or(minifi::controllers::ProxyType::HTTP);


We should add a DIRECT option and make it the default.

szaszm · 2026-02-23T14:13:17Z

minifi-api/include/minifi-cpp/controllers/ProxyConfigurationServiceInterface.h

+  HTTPS
+};
+
+class ProxyConfigurationServiceInterface : public virtual core::controller::ControllerService {


I'd keep the naming consistent with NiFi and call this ProxyConfigurationService

szaszm · 2026-02-23T14:13:52Z

libminifi/include/controllers/ProxyConfigurationService.h

+
+namespace org::apache::nifi::minifi::controllers {
+
+class ProxyConfigurationService : public core::controller::ControllerServiceImpl, public ProxyConfigurationServiceInterface {


I'd keep the naming consistent with NiFi and call this StandardProxyConfigurationService

I'm not sure it's a good idea, I think we should be consistent with our own naming conventions. For example we have AzureStorageCredentialsService and SSLContextService, while these are named StandardAzureCredentialsControllerService and StandardSSLContextService in NiFi. I also think adding the "Standard" prefix just adds confusion for the user when there are no other proxy services so there is no need for the additional adjective.

szaszm · 2026-02-23T14:15:56Z

extensions/azure/tests/DeleteAzureBlobStorageTests.cpp

  REQUIRE(failed_flowfiles[0] == TEST_DATA);
 }

+TEST_CASE_METHOD(DeleteAzureBlobStorageTestsFixture, "Test Azure blob delete using proxy", "[azureBlobStorageDelete]") {


Since this logic is in shared code, I'd only add one unit test, not one for each processor.

I kept the tests for the Put and List processors as the List processors are a bit different and removed the others in f70ac21

… processors

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch from 93b6f3b to a1a832e Compare October 9, 2025 12:35

lordgamez requested a review from Copilot October 9, 2025 12:56

Copilot AI reviewed Oct 9, 2025

View reviewed changes

PROCESSORS.md Show resolved Hide resolved

PROCESSORS.md Show resolved Hide resolved

PROCESSORS.md Show resolved Hide resolved

PROCESSORS.md Show resolved Hide resolved

lordgamez marked this pull request as ready for review October 13, 2025 07:15

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch from a1a832e to cd31ed2 Compare October 18, 2025 12:50

szaszm reviewed Nov 13, 2025

View reviewed changes

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch 2 times, most recently from 12e4262 to ee61c70 Compare January 10, 2026 08:56

fgerlits reviewed Jan 12, 2026

View reviewed changes

extensions/aws/processors/AwsProcessor.cpp Outdated Show resolved Hide resolved

CONTROLLERS.md Outdated Show resolved Hide resolved

libminifi/include/controllers/ProxyConfigurationService.h Show resolved Hide resolved

lordgamez marked this pull request as draft January 14, 2026 16:31

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch from b361c6f to ee61c70 Compare January 14, 2026 16:32

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch 2 times, most recently from 530067c to fa7ded8 Compare February 18, 2026 16:19

lordgamez marked this pull request as ready for review February 18, 2026 16:20

fgerlits approved these changes Feb 18, 2026

View reviewed changes

szaszm reviewed Mar 2, 2026

View reviewed changes

lordgamez added 8 commits March 5, 2026 10:31

MINIFICPP-2644 Add proxy controller service support for AWS and Azure…

0c4679d

… processors

Clang tidy fix

fc35643

Review update

1eba545

Fix after rebase

db0915c

Review update

a0726ee

Add HTTPS proxy support for AWS processors

a590aee

Review update

a77d0f7

Review update

f70ac21

lordgamez force-pushed the MINIFICPP-2644_aws_azure branch from fa7ded8 to f70ac21 Compare March 5, 2026 09:38


		namespace org::apache::nifi::minifi::controllers {

		class ProxyConfigurationService : public core::controller::ControllerServiceImpl, public ProxyConfigurationServiceInterface {

Conversation

lordgamez commented Oct 9, 2025

For all changes:

For code changes:

For documentation related changes:

Note:

Uh oh!

lordgamez commented Oct 9, 2025

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!

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

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lordgamez Nov 13, 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!

Uh oh!

Uh oh!

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 Oct 13, 2025 •

edited

Loading

lordgamez Nov 13, 2025 •

edited

Loading