Copilot commented on code in PR #2269:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2269#discussion_r4154987476


##########
extensions/aws/controllerservices/AWSCredentialsService.cpp:
##########
@@ -28,25 +32,49 @@ void AWSCredentialsService::initialize() {
 }
 
 void AWSCredentialsService::onEnable() {
+  AWSCredentialsProviderSettings settings;
   if (const auto access_key = getProperty(AccessKey.name)) {
-    aws_credentials_provider_.setAccessKey(*access_key);
+    settings.access_key = *access_key;
   }
   if (const auto secret_key = getProperty(SecretKey.name)) {
-    aws_credentials_provider_.setSecretKey(*secret_key);
+    settings.secret_key = *secret_key;
   }
   if (const auto credentials_file = getProperty(CredentialsFile.name)) {
-    aws_credentials_provider_.setCredentialsFile(*credentials_file);
+    settings.credentials_file = *credentials_file;
+  }
+  if (const auto profile_name = getProperty(ProfileName.name)) {
+    settings.profile_name = *profile_name;
+  }
+  if (const auto sso_profile_name = getProperty(SSOProfileName.name)) {
+    settings.sso_profile_name = *sso_profile_name;
   }
-  if (const auto use_credentials = getProperty(UseDefaultCredentials.name) | 
minifi::utils::andThen(parsing::parseBool)) {
-    aws_credentials_provider_.setUseDefaultCredentials(*use_credentials);
+  if (const auto use_default_credentials = 
getProperty(UseDefaultCredentials.name) | 
minifi::utils::andThen(parsing::parseBool); use_default_credentials && 
*use_default_credentials) {
+    settings.credential_configuration_strategy = 
CredentialConfigurationStrategyOption::DefaultCredentials;

Review Comment:
   The deprecated flag previously tried the default chain and then fell back to 
this service's Access Key/Secret Key or Credentials File settings. Converting 
it to the strict `DefaultCredentials` strategy removes that fallback, breaking 
existing controller-service configurations whenever the default chain is 
unavailable. Keep the legacy flag distinguishable in the factory so its old 
fallback order remains intact; the new strategy can retain strict source 
selection.



##########
extensions/aws/processors/AwsProcessor.cpp:
##########
@@ -32,36 +32,35 @@
 
 namespace org::apache::nifi::minifi::aws::processors {
 
-std::optional<Aws::Auth::AWSCredentials> 
AwsProcessor::getAWSCredentialsFromControllerService(core::ProcessContext& 
context) const {
+std::shared_ptr<Aws::Auth::AWSCredentialsProvider> 
AwsProcessor::getAWSCredentialsProviderFromControllerService(core::ProcessContext&
 context) const {
   if (auto service = 
minifi::utils::parseOptionalControllerService<controllers::AWSCredentialsService>(context,
 AWSCredentialsProviderService, getUUID())) {
-    return service->getAWSCredentials();
+    return service->getAWSCredentialsProvider();
   }
   logger_->log_debug("AWS credentials service could not be found");
-  return std::nullopt;
+  return nullptr;
 }
 
-std::optional<Aws::Auth::AWSCredentials> 
AwsProcessor::getAWSCredentials(core::ProcessContext& context) {
-  auto service_cred = getAWSCredentialsFromControllerService(context);
-  if (service_cred) {
+std::shared_ptr<Aws::Auth::AWSCredentialsProvider> 
AwsProcessor::getAWSCredentialsProvider(core::ProcessContext& context) {
+  if (auto service_credentials_provider = 
getAWSCredentialsProviderFromControllerService(context)) {
     logger_->log_info("AWS Credentials successfully set from controller 
service");
-    return service_cred;
+    return service_credentials_provider;
   }
 
-  aws::AWSCredentialsProvider aws_credentials_provider;
+  aws::AWSCredentialsProviderSettings settings;
   if (const auto access_key = context.getProperty(AccessKey.name)) {
-    aws_credentials_provider.setAccessKey(*access_key);
+    settings.access_key = *access_key;
   }
   if (const auto secret_key = context.getProperty(SecretKey.name)) {
-    aws_credentials_provider.setSecretKey(*secret_key);
+    settings.secret_key = *secret_key;
   }
   if (const auto credentials_file = context.getProperty(CredentialsFile.name)) 
{
-    aws_credentials_provider.setCredentialsFile(*credentials_file);
+    settings.credentials_file = *credentials_file;
   }
-  if (const auto use_credentials = 
context.getProperty(UseDefaultCredentials.name) | 
minifi::utils::andThen(parsing::parseBool)) {
-    aws_credentials_provider.setUseDefaultCredentials(*use_credentials);
+  if (const auto use_default_credentials = 
context.getProperty(UseDefaultCredentials.name) | 
minifi::utils::andThen(parsing::parseBool); use_default_credentials && 
*use_default_credentials) {
+    settings.credential_configuration_strategy = 
CredentialConfigurationStrategyOption::DefaultCredentials;

Review Comment:
   This changes the deprecated `Use Default Credentials=true` behavior: the 
removed provider tried the default chain first and then fell back to the 
configured access/secret keys or credentials file when that chain returned 
nothing. Mapping the flag directly to the strict `DefaultCredentials` strategy 
skips those fallbacks, so existing processor configurations can now fail to 
schedule. Preserve the legacy fallback semantics separately from the new 
explicit strategy (and add a regression test for a missing default-chain 
credential with configured keys/file).



##########
core-framework/common/src/utils/Environment.cpp:
##########
@@ -107,6 +107,7 @@ bool Environment::unsetEnvironmentVariable(const char* 
name) {
 
   Environment::accessEnvironment([&success, name](){
 #ifdef WIN32
+    _putenv_s(name, "");
     success = SetEnvironmentVariableA(name, nullptr);

Review Comment:
   The CRT update result is discarded, so this function can return success when 
`SetEnvironmentVariableA` succeeds but `_putenv_s` fails, leaving `std::getenv` 
unsynchronized—the condition this change is intended to prevent. Capture both 
results without short-circuiting and report success only when both environment 
stores were cleared.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to