MINIFICPP-2644 Add proxy controller service support for AWS and Azure processors #2045
Conversation
lordgamez
force-pushed
the
MINIFICPP-2644_aws_azure
branch
from
93b6f3b to
a1a832e
Compare
|
The clang tidy errors are already fixed in #2040 |
There was a problem hiding this comment.
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
ProxyConfigurationServicecontroller 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.
|
The issues listed in the clang tidy checks are fixed in the following PR: #2040 |
lordgamez
force-pushed
the
MINIFICPP-2644_aws_azure
branch
from
a1a832e to
cd31ed2
Compare
CONTROLLERS.md
Outdated
|
|
||
| ### Description | ||
|
|
||
| Provides a set of configurations for different MiNiFi C++ components to use a proxy server. |
There was a problem hiding this comment.
The proxy protocol is not specified anywhere. Typical options include http, https, socks4, socks4a, socks5. We should document what this service uses and leave the door open for more protocols in the future.
There was a problem hiding this comment.
Good point, we only support HTTP proxy at the moment. The controller is based on the NiFi counterpart https://nifi.apache.org/components/org.apache.nifi.proxy.StandardProxyConfigurationService/ where the proxy type can be specified if there are additional options. We can add that in the future if we want to support more protocols. Currently the cloud providers usually support only HTTP proxies, the newer AWS SDK should support HTTPS as well, we can introduce that later if needed. Added additional info to the description in 2b46713
| struct ProxyConfiguration { | ||
| std::string proxy_host; | ||
| std::optional<uint16_t> proxy_port; | ||
| std::optional<std::string> proxy_user; | ||
| std::optional<std::string> proxy_password; | ||
| }; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
As we only support HTTP proxies for now only these parameters are used, they can be extended or refactored when other protocols are introduced.
There was a problem hiding this comment.
If we add this type definition to the API, we can't change it later without breaking API.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
2 participants
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:
For documentation related changes:
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.