You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by ad...@apache.org on 2021/05/13 11:42:48 UTC

[nifi-minifi-cpp] branch main updated: MINIFICPP-1556 Fix partial credential setting in AWSCredentialsService

This is an automated email from the ASF dual-hosted git repository.

adebreceni pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git


The following commit(s) were added to refs/heads/main by this push:
     new 062fd37  MINIFICPP-1556 Fix partial credential setting in AWSCredentialsService
062fd37 is described below

commit 062fd37ab5aaa6ef0e809a64dc4122f681c02ae9
Author: Gabor Gyimesi <ga...@gmail.com>
AuthorDate: Thu May 13 13:39:00 2021 +0200

    MINIFICPP-1556 Fix partial credential setting in AWSCredentialsService
    
    Signed-off-by: Adam Debreceni <ad...@apache.org>
    
    This closes #1070
---
 .../controllerservices/AWSCredentialsService.cpp   | 33 ++++++++++------------
 .../aws/controllerservices/AWSCredentialsService.h |  7 ++---
 extensions/aws/processors/S3Processor.cpp          | 26 +++++++++--------
 .../test/aws-tests/AWSCredentialsServiceTest.cpp   | 13 +++++----
 libminifi/test/aws-tests/PutS3ObjectTests.cpp      | 19 ++++++++++---
 libminifi/test/aws-tests/S3TestsFixture.h          |  2 ++
 6 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/extensions/aws/controllerservices/AWSCredentialsService.cpp b/extensions/aws/controllerservices/AWSCredentialsService.cpp
index 439f57b..4525420 100644
--- a/extensions/aws/controllerservices/AWSCredentialsService.cpp
+++ b/extensions/aws/controllerservices/AWSCredentialsService.cpp
@@ -61,29 +61,26 @@ void AWSCredentialsService::initialize() {
 
 void AWSCredentialsService::onEnable() {
   std::string value;
-  getProperty(AccessKey.getName(), value);
-  aws_credentials_provider_.setAccessKey(value);
-  getProperty(SecretKey.getName(), value);
-  aws_credentials_provider_.setSecretKey(value);
-  getProperty(CredentialsFile.getName(), value);
-  aws_credentials_provider_.setCredentialsFile(value);
+  if (getProperty(AccessKey.getName(), value)) {
+    aws_credentials_provider_.setAccessKey(value);
+  }
+  if (getProperty(SecretKey.getName(), value)) {
+    aws_credentials_provider_.setSecretKey(value);
+  }
+  if (getProperty(CredentialsFile.getName(), value)) {
+    aws_credentials_provider_.setCredentialsFile(value);
+  }
   bool use_default_credentials = false;
-  getProperty(UseDefaultCredentials.getName(), use_default_credentials);
-  aws_credentials_provider_.setUseDefaultCredentials(use_default_credentials);
-}
-
-Aws::Auth::AWSCredentials AWSCredentialsService::getAWSCredentials() {
-  if (aws_credentials_.IsExpiredOrEmpty()) {
-    cacheCredentials();
+  if (getProperty(UseDefaultCredentials.getName(), use_default_credentials)) {
+    aws_credentials_provider_.setUseDefaultCredentials(use_default_credentials);
   }
-  return aws_credentials_;
 }
 
-void AWSCredentialsService::cacheCredentials() {
-  auto aws_credentials_result = aws_credentials_provider_.getAWSCredentials();
-  if (aws_credentials_result) {
-    aws_credentials_ = aws_credentials_result.value();
+minifi::utils::optional<Aws::Auth::AWSCredentials> AWSCredentialsService::getAWSCredentials() {
+  if (!aws_credentials_ || aws_credentials_->IsExpiredOrEmpty()) {
+    aws_credentials_ = aws_credentials_provider_.getAWSCredentials();
   }
+  return aws_credentials_;
 }
 
 }  // namespace controllers
diff --git a/extensions/aws/controllerservices/AWSCredentialsService.h b/extensions/aws/controllerservices/AWSCredentialsService.h
index 22d29fd..b38b54f 100644
--- a/extensions/aws/controllerservices/AWSCredentialsService.h
+++ b/extensions/aws/controllerservices/AWSCredentialsService.h
@@ -24,6 +24,7 @@
 #include "aws/core/auth/AWSCredentials.h"
 
 #include "utils/AWSInitializer.h"
+#include "utils/OptionalUtils.h"
 #include "core/Resource.h"
 #include "core/controller/ControllerService.h"
 #include "core/logging/LoggerConfiguration.h"
@@ -68,15 +69,13 @@ class AWSCredentialsService : public core::controller::ControllerService {
 
   void onEnable() override;
 
-  Aws::Auth::AWSCredentials getAWSCredentials();
+  minifi::utils::optional<Aws::Auth::AWSCredentials> getAWSCredentials();
 
  private:
   friend class ::AWSCredentialsServiceTestAccessor;
 
-  void cacheCredentials();
-
   const utils::AWSInitializer& AWS_INITIALIZER = utils::AWSInitializer::get();
-  Aws::Auth::AWSCredentials aws_credentials_;
+  minifi::utils::optional<Aws::Auth::AWSCredentials> aws_credentials_;
   AWSCredentialsProvider aws_credentials_provider_;
 };
 
diff --git a/extensions/aws/processors/S3Processor.cpp b/extensions/aws/processors/S3Processor.cpp
index 52bdc2f..d8cfa33 100644
--- a/extensions/aws/processors/S3Processor.cpp
+++ b/extensions/aws/processors/S3Processor.cpp
@@ -147,7 +147,7 @@ minifi::utils::optional<Aws::Auth::AWSCredentials> S3Processor::getAWSCredential
     return minifi::utils::nullopt;
   }
 
-  return minifi::utils::make_optional<Aws::Auth::AWSCredentials>(aws_credentials_service->getAWSCredentials());
+  return aws_credentials_service->getAWSCredentials();
 }
 
 minifi::utils::optional<Aws::Auth::AWSCredentials> S3Processor::getAWSCredentials(
@@ -159,19 +159,21 @@ minifi::utils::optional<Aws::Auth::AWSCredentials> S3Processor::getAWSCredential
     return service_cred.value();
   }
 
-  std::string access_key;
-  context->getProperty(AccessKey, access_key, flow_file);
   aws::AWSCredentialsProvider aws_credentials_provider;
-  aws_credentials_provider.setAccessKey(access_key);
-  std::string secret_key;
-  context->getProperty(SecretKey, secret_key, flow_file);
-  aws_credentials_provider.setSecretKey(secret_key);
-  std::string credential_file;
-  context->getProperty(CredentialsFile.getName(), credential_file);
-  aws_credentials_provider.setCredentialsFile(credential_file);
+  std::string value;
+  if (context->getProperty(AccessKey, value, flow_file)) {
+    aws_credentials_provider.setAccessKey(value);
+  }
+  if (context->getProperty(SecretKey, value, flow_file)) {
+    aws_credentials_provider.setSecretKey(value);
+  }
+  if (context->getProperty(CredentialsFile.getName(), value)) {
+    aws_credentials_provider.setCredentialsFile(value);
+  }
   bool use_default_credentials = false;
-  context->getProperty(UseDefaultCredentials.getName(), use_default_credentials);
-  aws_credentials_provider.setUseDefaultCredentials(use_default_credentials);
+  if (context->getProperty(UseDefaultCredentials.getName(), use_default_credentials)) {
+    aws_credentials_provider.setUseDefaultCredentials(use_default_credentials);
+  }
 
   return aws_credentials_provider.getAWSCredentials();
 }
diff --git a/libminifi/test/aws-tests/AWSCredentialsServiceTest.cpp b/libminifi/test/aws-tests/AWSCredentialsServiceTest.cpp
index 27e4353..e6d8aba 100644
--- a/libminifi/test/aws-tests/AWSCredentialsServiceTest.cpp
+++ b/libminifi/test/aws-tests/AWSCredentialsServiceTest.cpp
@@ -53,14 +53,15 @@ TEST_CASE_METHOD(AWSCredentialsServiceTestAccessor, "Test expired credentials ar
   auto aws_credentials_impl = std::static_pointer_cast<minifi::aws::controllers::AWSCredentialsService>(aws_credentials_service->getControllerServiceImplementation());
 
   // Check intial credentials
-  REQUIRE(aws_credentials_impl->getAWSCredentials().GetAWSAccessKeyId() == "key");
-  REQUIRE(aws_credentials_impl->getAWSCredentials().GetAWSSecretKey() == "secret");
-  REQUIRE(!aws_credentials_impl->getAWSCredentials().IsExpired());
+  REQUIRE(aws_credentials_impl->getAWSCredentials());
+  REQUIRE(aws_credentials_impl->getAWSCredentials()->GetAWSAccessKeyId() == "key");
+  REQUIRE(aws_credentials_impl->getAWSCredentials()->GetAWSSecretKey() == "secret");
+  REQUIRE_FALSE(aws_credentials_impl->getAWSCredentials()->IsExpired());
 
   // Expire credentials
-  get_aws_credentials_(*aws_credentials_impl).SetExpiration(Aws::Utils::DateTime(0.0));
-  REQUIRE(get_aws_credentials_(*aws_credentials_impl).IsExpired());
+  get_aws_credentials_(*aws_credentials_impl)->SetExpiration(Aws::Utils::DateTime(0.0));
+  REQUIRE(get_aws_credentials_(*aws_credentials_impl)->IsExpired());
 
   // Check for credential refresh
-  REQUIRE(!aws_credentials_impl->getAWSCredentials().IsExpired());
+  REQUIRE_FALSE(aws_credentials_impl->getAWSCredentials()->IsExpired());
 }
diff --git a/libminifi/test/aws-tests/PutS3ObjectTests.cpp b/libminifi/test/aws-tests/PutS3ObjectTests.cpp
index b091ac3..868dbcc 100644
--- a/libminifi/test/aws-tests/PutS3ObjectTests.cpp
+++ b/libminifi/test/aws-tests/PutS3ObjectTests.cpp
@@ -34,10 +34,10 @@ class PutS3ObjectTestsFixture : public FlowProcessorS3TestsFixture<minifi::aws::
   }
 
   void checkEmptyPutObjectResults() {
-    REQUIRE(!LogTestController::getInstance().contains("key:s3.version value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
-    REQUIRE(!LogTestController::getInstance().contains("key:s3.etag value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
-    REQUIRE(!LogTestController::getInstance().contains("key:s3.expiration value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
-    REQUIRE(!LogTestController::getInstance().contains("key:s3.sseAlgorithm value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
+    REQUIRE_FALSE(LogTestController::getInstance().contains("key:s3.version value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
+    REQUIRE_FALSE(LogTestController::getInstance().contains("key:s3.etag value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
+    REQUIRE_FALSE(LogTestController::getInstance().contains("key:s3.expiration value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
+    REQUIRE_FALSE(LogTestController::getInstance().contains("key:s3.sseAlgorithm value:", std::chrono::seconds(0), std::chrono::milliseconds(0)));
   }
 };
 
@@ -107,6 +107,17 @@ TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test required property not set", "[aw
   REQUIRE_THROWS_AS(test_controller.runSession(plan, true), minifi::Exception&);
 }
 
+TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Test incomplete credentials in credentials service", "[awsS3Config]") {
+  setBucket();
+  plan->setProperty(aws_credentials_service, "Secret Key", "secret");
+  setCredentialsService();
+  REQUIRE_THROWS_AS(test_controller.runSession(plan, true), minifi::Exception&);
+  REQUIRE(verifyLogLinePresenceInPollTime(std::chrono::seconds(3), "AWS Credentials have not been set!"));
+
+  // Test that no invalid credentials file was set from previous properties
+  REQUIRE_FALSE(LogTestController::getInstance().contains("load configure file failed", std::chrono::seconds(0), std::chrono::milliseconds(0)));
+}
+
 TEST_CASE_METHOD(PutS3ObjectTestsFixture, "Check default client configuration", "[awsS3ClientConfig]") {
   setRequiredProperties();
   test_controller.runSession(plan, true);
diff --git a/libminifi/test/aws-tests/S3TestsFixture.h b/libminifi/test/aws-tests/S3TestsFixture.h
index 262e0e5..d363c51 100644
--- a/libminifi/test/aws-tests/S3TestsFixture.h
+++ b/libminifi/test/aws-tests/S3TestsFixture.h
@@ -29,6 +29,7 @@
 #include "utils/file/FileUtils.h"
 #include "MockS3RequestSender.h"
 #include "utils/TestUtils.h"
+#include "AWSCredentialsProvider.h"
 
 using org::apache::nifi::minifi::utils::createTempDir;
 
@@ -52,6 +53,7 @@ class S3TestsFixture {
     LogTestController::getInstance().setTrace<minifi::core::ProcessSession>();
     LogTestController::getInstance().setDebug<processors::LogAttribute>();
     LogTestController::getInstance().setTrace<T>();
+    LogTestController::getInstance().setDebug<minifi::aws::AWSCredentialsProvider>();
 
     // Build MiNiFi processing graph
     plan = test_controller.createPlan();