Skip to content

Commit 8890e9f

Browse files
lordgamezlordgamez
committed
Review update
1 parent 3328292 commit 8890e9f

19 files changed

Lines changed: 173 additions & 162 deletions

‎CONTROLLERS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ In the list below, the names of required properties appear in bold. Any other pr
261261
| Proxy Server Port | | | Proxy server port number. |
262262
| Proxy User Name | | | The name of the proxy client for user authentication. |
263263
| Proxy User Password | | | The password of the proxy client for user authentication.<br/>**Sensitive Property: true** |
264-
| Proxy Type | HTTP | HTTP<br/>HTTPS | Proxy type. |
264+
| Proxy Type | HTTP | DIRECT<br/>HTTP | Proxy type. If set to DIRECT, the proxy server is not used. |
265265

266266

267267
## RocksDbStateStorage

‎PROCESSORS.md‎

Lines changed: 16 additions & 16 deletions
Large diffs are not rendered by default.

‎behave_framework/src/minifi_test_framework/steps/flow_building_steps.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,10 @@ def set_processor_properties(context: MinifiTestContext):
244244
@given("a ProxyConfigurationService controller service is set up with {proxy_type} proxy configuration in the \"{container_name}\" flow")
245245
def step_impl(context: MinifiTestContext, proxy_type: str, container_name: str):
246246
controller_service = ControllerService(class_name="ProxyConfigurationService", service_name="ProxyConfigurationService")
247-
controller_service.add_property("Proxy Server Host", f"http-proxy-{context.scenario_id}")
247+
controller_service.add_property("Proxy Server Host", f"{proxy_type.lower()}://http-proxy-{context.scenario_id}")
248248
controller_service.add_property("Proxy User Name", "admin")
249249
controller_service.add_property("Proxy User Password", "test101")
250-
controller_service.add_property("Proxy Type", proxy_type)
250+
controller_service.add_property("Proxy Type", "HTTP")
251251
if proxy_type.lower() == "http":
252252
controller_service.add_property("Proxy Server Port", "3128")
253253
else:

‎extensions/aws/processors/AwsProcessor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ minifi::controllers::ProxyConfiguration AwsProcessor::getProxy(core::ProcessCont
7777
proxy.proxy_user = proxy_controller_service->getUsername().value_or("");
7878
proxy.proxy_password = proxy_controller_service->getPassword().value_or("");
7979
} else {
80-
proxy.proxy_type = minifi::utils::parseOptionalEnumProperty<minifi::controllers::ProxyType>(context, ProxyType).value_or(minifi::controllers::ProxyType::HTTP);
8180
proxy.proxy_host = minifi::utils::parseOptionalProperty(context, ProxyHost, flow_file).value_or("");
81+
proxy.proxy_type = proxy.proxy_host.empty() ? minifi::controllers::ProxyType::DIRECT : minifi::controllers::ProxyType::HTTP;
8282
proxy.proxy_port = gsl::narrow<uint32_t>(minifi::utils::parseOptionalU64Property(context, ProxyPort, flow_file).value_or(0));
8383
proxy.proxy_user = minifi::utils::parseOptionalProperty(context, ProxyUsername, flow_file).value_or("");
8484
proxy.proxy_password = minifi::utils::parseOptionalProperty(context, ProxyPassword, flow_file).value_or("");

‎extensions/aws/processors/AwsProcessor.h‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,25 +133,20 @@ class AwsProcessor : public core::ProcessorImpl { // NOLINT(cppcoreguidelines-s
133133
.withValidator(core::StandardPropertyValidators::NON_BLANK_VALIDATOR)
134134
.supportsExpressionLanguage(true)
135135
.build();
136-
EXTENSIONAPI static constexpr auto ProxyType = core::PropertyDefinitionBuilder<magic_enum::enum_count<minifi::controllers::ProxyType>()>::createProperty("Proxy Type")
137-
.withDescription("Proxy type")
138-
.withDefaultValue(magic_enum::enum_name(minifi::controllers::ProxyType::HTTP))
139-
.withAllowedValues(magic_enum::enum_names<minifi::controllers::ProxyType>())
140-
.build();
141136
EXTENSIONAPI static constexpr auto ProxyHost = core::PropertyDefinitionBuilder<>::createProperty("Proxy Host")
142-
.withDescription("Proxy host name or IP")
137+
.withDescription("Proxy host name or IP. Use https:// prefix for HTTPS proxy. DEPRECATED, please use Proxy Configuration Service instead.")
143138
.supportsExpressionLanguage(true)
144139
.build();
145140
EXTENSIONAPI static constexpr auto ProxyPort = core::PropertyDefinitionBuilder<>::createProperty("Proxy Port")
146-
.withDescription("The port number of the proxy host")
141+
.withDescription("The port number of the proxy host. DEPRECATED, please use Proxy Configuration Service instead.")
147142
.supportsExpressionLanguage(true)
148143
.build();
149144
EXTENSIONAPI static constexpr auto ProxyUsername = core::PropertyDefinitionBuilder<>::createProperty("Proxy Username")
150-
.withDescription("Username to set when authenticating against proxy")
145+
.withDescription("Username to set when authenticating against proxy. DEPRECATED, please use Proxy Configuration Service instead.")
151146
.supportsExpressionLanguage(true)
152147
.build();
153148
EXTENSIONAPI static constexpr auto ProxyPassword = core::PropertyDefinitionBuilder<>::createProperty("Proxy Password")
154-
.withDescription("Password to set when authenticating against proxy")
149+
.withDescription("Password to set when authenticating against proxy. DEPRECATED, please use Proxy Configuration Service instead.")
155150
.supportsExpressionLanguage(true)
156151
.isSensitive(true)
157152
.build();
@@ -174,7 +169,6 @@ class AwsProcessor : public core::ProcessorImpl { // NOLINT(cppcoreguidelines-s
174169
Region,
175170
CommunicationsTimeout,
176171
EndpointOverrideURL,
177-
ProxyType,
178172
ProxyHost,
179173
ProxyPort,
180174
ProxyUsername,

‎extensions/aws/s3/S3Wrapper.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,21 @@ struct RequestParameters {
127127
Aws::Client::ClientConfiguration client_config;
128128

129129
void setClientConfig(const minifi::controllers::ProxyConfiguration& proxy, const std::string& endpoint_override_url) {
130-
client_config.proxyHost = proxy.proxy_host;
130+
client_config.endpointOverride = endpoint_override_url;
131+
if (proxy.proxy_type == minifi::controllers::ProxyType::DIRECT) {
132+
return;
133+
}
134+
client_config.proxyScheme = minifi::utils::string::startsWith(proxy.proxy_host, "https") ? Aws::Http::Scheme::HTTPS : Aws::Http::Scheme::HTTP;
135+
auto proxy_host = proxy.proxy_host;
136+
if (minifi::utils::string::startsWith(proxy_host, "https://")) {
137+
proxy_host = proxy_host.substr(8);
138+
} else if (minifi::utils::string::startsWith(proxy_host, "http://")) {
139+
proxy_host = proxy_host.substr(7);
140+
}
141+
client_config.proxyHost = proxy_host;
131142
client_config.proxyPort = proxy.proxy_port.value_or(0);
132143
client_config.proxyUserName = proxy.proxy_user.value_or("");
133144
client_config.proxyPassword = proxy.proxy_password.value_or("");
134-
client_config.proxyScheme = proxy.proxy_type == minifi::controllers::ProxyType::HTTPS ? Aws::Http::Scheme::HTTPS : Aws::Http::Scheme::HTTP;
135-
client_config.endpointOverride = endpoint_override_url;
136145
}
137146
};
138147

‎extensions/aws/tests/DeleteS3ObjectTests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ TEST_CASE_METHOD(DeleteS3ObjectTestsFixture, "Non blank validator tests") {
8585
TEST_CASE_METHOD(DeleteS3ObjectTestsFixture, "Test proxy setting", "[awsS3Proxy]") {
8686
setRequiredProperties();
8787
SECTION("Use proxy configuration service") {
88-
setProxy(true);
88+
setProxy(ProxyConfigType::ControllerServiceHttp);
8989
}
9090
SECTION("Use processor properties") {
91-
setProxy(false);
91+
setProxy(ProxyConfigType::ProcessorProperties);
9292
}
9393
test_controller.runSession(plan, true);
9494
checkProxySettings();

‎extensions/aws/tests/FetchS3ObjectTests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ TEST_CASE_METHOD(FetchS3ObjectTestsFixture, "Non blank validator tests") {
102102
TEST_CASE_METHOD(FetchS3ObjectTestsFixture, "Test proxy setting", "[awsS3Proxy]") {
103103
setRequiredProperties();
104104
SECTION("Use proxy configuration service") {
105-
setProxy(true);
105+
setProxy(ProxyConfigType::ControllerServiceHttp);
106106
}
107107
SECTION("Use processor properties") {
108-
setProxy(false);
108+
setProxy(ProxyConfigType::ProcessorProperties);
109109
}
110110
test_controller.runSession(plan, true);
111111
checkProxySettings();

‎extensions/aws/tests/ListS3Tests.cpp‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,26 @@ TEST_CASE_METHOD(ListS3TestsFixture, "Non blank validator tests") {
7575
TEST_CASE_METHOD(ListS3TestsFixture, "Test proxy setting", "[awsS3Proxy]") {
7676
setRequiredProperties();
7777
SECTION("Use proxy configuration service") {
78-
setProxy(true);
78+
setProxy(ProxyConfigType::ControllerServiceHttp);
7979
}
8080
SECTION("Use processor properties") {
81-
setProxy(false);
81+
setProxy(ProxyConfigType::ProcessorProperties);
8282
}
8383
test_controller.runSession(plan, true);
8484
checkProxySettings();
8585
}
8686

87+
TEST_CASE_METHOD(ListS3TestsFixture, "Test proxy is not configured if proxy type is direct", "[awsS3Proxy]") {
88+
setRequiredProperties();
89+
setProxy(ProxyConfigType::ControllerServiceDirect);
90+
91+
test_controller.runSession(plan, true);
92+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyHost == "");
93+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyPort == 0);
94+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyUserName == "");
95+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyPassword == "");
96+
}
97+
8798
TEST_CASE_METHOD(ListS3TestsFixture, "Test listing without versioning", "[awsS3ListObjects]") {
8899
setRequiredProperties();
89100
plan->setProperty(s3_processor, "Region", minifi::aws::processors::region::US_EAST_1);

‎extensions/aws/tests/PutS3ObjectTests.cpp‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,15 +201,26 @@ TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test multiple user metadata", "[awsS3
201201
TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test proxy setting", "[awsS3Proxy]") {
202202
setRequiredProperties();
203203
SECTION("Use proxy configuration service") {
204-
setProxy(true);
204+
setProxy(ProxyConfigType::ControllerServiceHttp);
205205
}
206206
SECTION("Use processor properties") {
207-
setProxy(false);
207+
setProxy(ProxyConfigType::ProcessorProperties);
208208
}
209209
test_controller.runSession(plan);
210210
checkProxySettings();
211211
}
212212

213+
TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test proxy is not configured if proxy type is direct", "[awsS3Proxy]") {
214+
setRequiredProperties();
215+
setProxy(ProxyConfigType::ControllerServiceDirect);
216+
217+
test_controller.runSession(plan, true);
218+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyHost == "");
219+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyPort == 0);
220+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyUserName == "");
221+
REQUIRE(mock_s3_request_sender_ptr->getClientConfig().proxyPassword == "");
222+
}
223+
213224
TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test access control setting", "[awsS3ACL]") {
214225
setRequiredProperties();
215226
plan->setDynamicProperty(update_attribute, "s3.permissions.full.users", "myuserid123, myuser@example.com");

0 commit comments

Comments
 (0)