[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


szaszm commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535539954



##
File path: libminifi/src/utils/tls/DistinguishedName.cpp
##
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "utils/tls/DistinguishedName.h"
+
+#include 
+
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace tls {
+
+DistinguishedName::DistinguishedName(const std::vector& 
components) {
+  std::transform(components.begin(), components.end(), 
std::back_inserter(components_),
+  [](const std::string& component) { return 
utils::StringUtils::trim(component); });
+  std::sort(components_.begin(), components_.end());
+}
+
+DistinguishedName DistinguishedName::fromCommaSeparated(const std::string& 
comma_separated_components) {
+  return 
DistinguishedName{utils::StringUtils::split(comma_separated_components, ",")};
+}
+
+DistinguishedName DistinguishedName::fromSlashSeparated(const std::string 
_separated_components) {
+  return 
DistinguishedName{utils::StringUtils::split(slash_separated_components, "/")};
+}
+
+utils::optional DistinguishedName::getCN() const {
+  const auto it = std::find_if(components_.begin(), components_.end(),
+  [](const std::string& component) { return component.substr(0, 3) == 
"CN="; });

Review comment:
   Not sure how often this would be called but this is a way to do the same 
without a temporary allocation/deallocation:
   ```suggestion
 [](const std::string& component) { return component.compare(0, 3, 
"CN="); });
   ```

##
File path: libminifi/src/utils/tls/ExtendedKeyUsage.cpp
##
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifdef OPENSSL_SUPPORT
+
+#include "utils/tls/ExtendedKeyUsage.h"
+
+#include 
+
+#include 
+#include 
+#include 
+
+#include "core/logging/LoggerConfiguration.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace tls {
+
+namespace {
+
+struct KeyValuePair {
+  const char* key;
+  uint8_t value;
+};
+constexpr std::array EXT_KEY_USAGE_NAME_TO_BIT_POS{{
+KeyValuePair{"Server Authentication", 1},
+KeyValuePair{"Client Authentication", 2},
+KeyValuePair{"Code Signing", 3},
+KeyValuePair{"Secure Email", 4},
+KeyValuePair{"Time Stamping", 8},
+KeyValuePair{"OCSP Signing", 9}
+}};
+
+}  // namespace
+
+void EXTENDED_KEY_USAGE_deleter::operator()(EXTENDED_KEY_USAGE* key_usage) 
const { EXTENDED_KEY_USAGE_free(key_usage); }
+
+ExtendedKeyUsage::ExtendedKeyUsage() : 
logger_(core::logging::LoggerFactory::getLogger()) {}
+
+ExtendedKeyUsage::ExtendedKeyUsage(const EXTENDED_KEY_USAGE& key_usage_asn1) : 
ExtendedKeyUsage{} {
+  const int num_oids = sk_ASN1_OBJECT_num(_usage_asn1);
+  for (int i = 0; i < num_oids; ++i) {
+const ASN1_OBJECT* const oid = sk_ASN1_OBJECT_value(_usage_asn1, i);
+assert(oid && oid->length > 0);
+const unsigned char bit_pos = oid->data[oid->length - 1];
+if (bit_pos < CHAR_BIT * sizeof(bits_)) {
+  bits_ |= (1 << bit_pos);
+}
+  }
+}
+
+ExtendedKeyUsage::ExtendedKeyUsage(const std::string& key_usage_str) : 
ExtendedKeyUsage{} {
+  const std::vector key_usages = 
utils::StringUtils::split(key_usage_str, ",");
+  for (const auto& key_usage : key_usages) {
+const 

[GitHub] [nifi] MikeThomsen commented on a change in pull request #4702: NIFI-8063: Added profile (enabled) to include most NARs, can be disabled

2020-12-03 Thread GitBox


MikeThomsen commented on a change in pull request #4702:
URL: https://github.com/apache/nifi/pull/4702#discussion_r535737353



##
File path: nifi-assembly/pom.xml
##
@@ -451,356 +218,597 @@ language governing permissions and limitations under 
the License. -->
 
 
 org.apache.nifi
-nifi-azure-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-azure-services-api-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-scripting-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-groovyx-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-elasticsearch-nar
+nifi-registry-nar
 1.13.0-SNAPSHOT
 nar
 
 
 org.apache.nifi
-nifi-elasticsearch-client-service-api-nar
+nifi-record-serialization-services-nar
 1.13.0-SNAPSHOT
 nar
 
 
 org.apache.nifi
-nifi-elasticsearch-client-service-nar
+nifi-tcp-nar
 1.13.0-SNAPSHOT
 nar
 
 
 org.apache.nifi
-nifi-elasticsearch-restapi-nar
+nifi-kerberos-credentials-service-nar
 1.13.0-SNAPSHOT
 nar
 
 
 org.apache.nifi
-nifi-lumberjack-nar
+nifi-proxy-configuration-nar
 1.13.0-SNAPSHOT
 nar
 
+
+
 
-org.apache.nifi
-nifi-beats-nar
-1.13.0-SNAPSHOT
-nar
+javax.xml.bind
+jaxb-api
+2.3.0
 
 
-org.apache.nifi
-nifi-cybersecurity-nar
-1.13.0-SNAPSHOT
-nar
+com.sun.xml.bind
+jaxb-impl
+2.3.0
 
 
-org.apache.nifi
-nifi-email-nar
-1.13.0-SNAPSHOT
-nar
+com.sun.xml.bind
+jaxb-core
+2.3.0
 
 
-org.apache.nifi
-nifi-amqp-nar
-1.13.0-SNAPSHOT
-nar
+javax.annotation
+javax.annotation-api
+1.3.2
 
 
-org.apache.nifi
-nifi-splunk-nar
-1.13.0-SNAPSHOT
-nar
+javax.activation
+javax.activation-api
+1.2.0
 
+
 
-org.apache.nifi
-nifi-jms-cf-service-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-jms-processors-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-cassandra-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-cassandra-services-api-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-cassandra-services-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-spring-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-registry-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-hive-services-api-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-hive-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-site-to-site-reporting-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-record-serialization-services-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-mqtt-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-snmp-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-evtx-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-slack-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-smb-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-nifi-windows-event-log-nar
-1.13.0-SNAPSHOT
-nar
-
-
-org.apache.nifi
-

[GitHub] [nifi] markap14 commented on pull request #4708: NIFI-8070: Added coalesce function to RecordPath

2020-12-03 Thread GitBox


markap14 commented on pull request #4708:
URL: https://github.com/apache/nifi/pull/4708#issuecomment-738370638


   Thanks for reviewing @exceptionfactory. Will merge to main.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 merged pull request #4708: NIFI-8070: Added coalesce function to RecordPath

2020-12-03 Thread GitBox


markap14 merged pull request #4708:
URL: https://github.com/apache/nifi/pull/4708


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] turcsanyip commented on a change in pull request #4697: NIFI-7989: Add support to UpdateHiveTable for creating external tables

2020-12-03 Thread GitBox


turcsanyip commented on a change in pull request #4697:
URL: https://github.com/apache/nifi/pull/4697#discussion_r535663284



##
File path: 
nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/processors/hive/UpdateHive3Table.java
##
@@ -322,29 +411,55 @@ private synchronized void checkAndUpdateTableSchema(final 
ProcessSession session
 s.execute(createTableSql);
 }
 
-// Now that the table is created, describe it and determine 
its location (for placing the flowfile downstream)
-String describeTable = "DESC FORMATTED " + tableName;
-ResultSet tableInfo = s.executeQuery(describeTable);
-boolean moreRows = tableInfo.next();
-boolean locationFound = false;
-while (moreRows && !locationFound) {
-String line = tableInfo.getString(1);
-if (line.startsWith("Location:")) {
-locationFound = true;
-continue; // Don't do a next() here, need to get the 
second column value
-}
-moreRows = tableInfo.next();
-}
-outputPath = tableInfo.getString(2);
+tableCreated = true;
+}
 
-} else {
-List hiveColumns = new ArrayList<>();
+// Process the table (columns, partitions, location, etc.)
+List hiveColumns = new ArrayList<>();
 
-String describeTable = "DESC FORMATTED " + tableName;
-ResultSet tableInfo = s.executeQuery(describeTable);
-// Result is 3 columns, col_name, data_type, comment. Check 
the first row for a header and skip if so, otherwise add column name
+String describeTable = "DESC FORMATTED " + tableName;

Review comment:
   Backticks are missing around tablename which leads to an error in case 
of `_test_table` eg.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 merged pull request #4698: NIFI-1121: Use display name for dependent property documentation

2020-12-03 Thread GitBox


markap14 merged pull request #4698:
URL: https://github.com/apache/nifi/pull/4698


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] exceptionfactory commented on pull request #4708: NIFI-8070: Added coalesce function to RecordPath

2020-12-03 Thread GitBox


exceptionfactory commented on pull request #4708:
URL: https://github.com/apache/nifi/pull/4708#issuecomment-738331777


   LGTM. Ran unit test and observed all branches of Coalesce class appear to be 
covered.  Helpful documentation. +1 



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 opened a new pull request #4708: NIFI-8070: Added coalesce function to RecordPath

2020-12-03 Thread GitBox


markap14 opened a new pull request #4708:
URL: https://github.com/apache/nifi/pull/4708


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   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 **NIFI-** where  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? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 closed pull request #4707: NIFI-8058 Fixed a UI error to correctly delete dynamic properties whi…

2020-12-03 Thread GitBox


mattyb149 closed pull request #4707:
URL: https://github.com/apache/nifi/pull/4707


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 commented on pull request #4707: NIFI-8058 Fixed a UI error to correctly delete dynamic properties whi…

2020-12-03 Thread GitBox


mattyb149 commented on pull request #4707:
URL: https://github.com/apache/nifi/pull/4707#issuecomment-738297648


   +1 LGTM, tested on a live NiFi instance, verified the expected behavior. 
Thanks for the fix! Merging to main



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mtien-apache opened a new pull request #4707: NIFI-8058 Fixed a UI error to correctly delete dynamic properties whi…

2020-12-03 Thread GitBox


mtien-apache opened a new pull request #4707:
URL: https://github.com/apache/nifi/pull/4707


   …le configuring processors.
   
   Changed to check the length of all unfiltered properties instead of only 
filtered properties.
   Added additional check if descriptor is a dynamic property.
   
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Fixed a bug due to the recent NIFI-1121 of dependent properties. After 
deleting a dynamic property while configuring processor properties, it would 
return the deleted dynamic property back to the UI. Added an additional check 
for dynamic properties that are set for deletion. Also fixed an error that 
didn't allow adding dynamic properties by checking all unfiltered properties 
instead of only filtered properties._
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
in the commit message?
   
   - [x] Does your PR title start with **NIFI-** where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `main`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [x] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [x] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 merged pull request #4703: NIFI-8064 Convert TestSecureClientZooKeeperFactory to integration test

2020-12-03 Thread GitBox


markap14 merged pull request #4703:
URL: https://github.com/apache/nifi/pull/4703


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on pull request #4703: NIFI-8064 Convert TestSecureClientZooKeeperFactory to integration test

2020-12-03 Thread GitBox


markap14 commented on pull request #4703:
URL: https://github.com/apache/nifi/pull/4703#issuecomment-738258085


   Thanks @bbende  +1 merged to main



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 merged pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-03 Thread GitBox


markap14 merged pull request #4700:
URL: https://github.com/apache/nifi/pull/4700


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-03 Thread GitBox


markap14 commented on pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#issuecomment-738252857


   @bbende thanks for updating that. This is definitely a better approach than 
the route that I took, I believe. +1 merged to main.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 opened a new pull request #4706: NIFI-8068: Ensure that when we determine the best of multiple possibl…

2020-12-03 Thread GitBox


markap14 opened a new pull request #4706:
URL: https://github.com/apache/nifi/pull/4706


   …e types in a UNION that we handle Arrays of Records properly. Also 
refactored code to be a bit cleaner by extracting blocks of it into 
appropriately named methods
   
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   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 **NIFI-** where  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? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


fgerlits commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535467558



##
File path: libminifi/include/utils/tls/CertificateUtils.h
##
@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+#ifdef OPENSSL_SUPPORT
+
+#include 
+
+#ifdef WIN32
+#include 
+#include 
+#endif  // WIN32
+
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace tls {
+
+struct EVP_PKEY_deleter {
+  void operator()(EVP_PKEY* pkey) const { EVP_PKEY_free(pkey); }
+};
+using EVP_PKEY_unique_ptr = std::unique_ptr;
+
+struct X509_deleter {
+  void operator()(X509* cert) const { X509_free(cert); }
+};
+using X509_unique_ptr = std::unique_ptr;
+
+#ifdef WIN32
+// Returns nullptr on errors
+X509_unique_ptr convertWindowsCertificate(const PCCERT_CONTEXT certificate);
+
+// Returns nullptr if the certificate has no associated private key, or the 
private key could not be extracted
+EVP_PKEY_unique_ptr extractPrivateKey(const PCCERT_CONTEXT certificate);

Review comment:
   Thanks, fixed in 
https://github.com/apache/nifi-minifi-cpp/pull/947/commits/e63e9e6c64be860026b6756fd4ffce85ee3d7d8f.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


fgerlits commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535466913



##
File path: libminifi/src/controllers/SSLContextService.cpp
##
@@ -269,10 +505,8 @@ void SSLContextService::onEnable() {
 }
 passphrase_file.close();
   }
-  // load CA certificates
-  if (!getProperty(caCert.getName(), ca_certificate_)) {
-logger_->log_error("Can not load CA certificate.");

Review comment:
   It isn't an error any longer, because now there is a valid use case when 
the server certificate name is blank and 
`nifi.security.use.system.cert.store=true`.  In that case, we will try to find 
the server certificate in the system store.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


adamdebreceni commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535328333



##
File path: libminifi/src/controllers/SSLContextService.cpp
##
@@ -269,10 +505,8 @@ void SSLContextService::onEnable() {
 }
 passphrase_file.close();
   }
-  // load CA certificates
-  if (!getProperty(caCert.getName(), ca_certificate_)) {
-logger_->log_error("Can not load CA certificate.");

Review comment:
   is the log no longer necessary?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


adamdebreceni commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535295187



##
File path: libminifi/include/utils/tls/CertificateUtils.h
##
@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+#ifdef OPENSSL_SUPPORT
+
+#include 
+
+#ifdef WIN32
+#include 
+#include 
+#endif  // WIN32
+
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace tls {
+
+struct EVP_PKEY_deleter {
+  void operator()(EVP_PKEY* pkey) const { EVP_PKEY_free(pkey); }
+};
+using EVP_PKEY_unique_ptr = std::unique_ptr;
+
+struct X509_deleter {
+  void operator()(X509* cert) const { X509_free(cert); }
+};
+using X509_unique_ptr = std::unique_ptr;
+
+#ifdef WIN32
+// Returns nullptr on errors
+X509_unique_ptr convertWindowsCertificate(const PCCERT_CONTEXT certificate);
+
+// Returns nullptr if the certificate has no associated private key, or the 
private key could not be extracted
+EVP_PKEY_unique_ptr extractPrivateKey(const PCCERT_CONTEXT certificate);

Review comment:
   it seems like top-level `const` has no effect here, a parameter passed 
by value being `const` is an implementation detail IMO and should be only in 
the definition
   https://abseil.io/tips/109





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


adamdebreceni commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535295187



##
File path: libminifi/include/utils/tls/CertificateUtils.h
##
@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+#ifdef OPENSSL_SUPPORT
+
+#include 
+
+#ifdef WIN32
+#include 
+#include 
+#endif  // WIN32
+
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace tls {
+
+struct EVP_PKEY_deleter {
+  void operator()(EVP_PKEY* pkey) const { EVP_PKEY_free(pkey); }
+};
+using EVP_PKEY_unique_ptr = std::unique_ptr;
+
+struct X509_deleter {
+  void operator()(X509* cert) const { X509_free(cert); }
+};
+using X509_unique_ptr = std::unique_ptr;
+
+#ifdef WIN32
+// Returns nullptr on errors
+X509_unique_ptr convertWindowsCertificate(const PCCERT_CONTEXT certificate);
+
+// Returns nullptr if the certificate has no associated private key, or the 
private key could not be extracted
+EVP_PKEY_unique_ptr extractPrivateKey(const PCCERT_CONTEXT certificate);

Review comment:
   top-level `const` has no effect here, a parameter passed by value being 
`const` is an implementation detail IMO and should be only in the definition
   https://abseil.io/tips/109





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] asfgit closed pull request #4701: NIFI-7906 fixed windows build

2020-12-03 Thread GitBox


asfgit closed pull request #4701:
URL: https://github.com/apache/nifi/pull/4701


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


lordgamez commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535199561



##
File path: libminifi/src/utils/tls/WindowsCertStoreLocation.cpp
##
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifdef WIN32
+
+#include "utils/tls/WindowsCertStoreLocation.h"
+
+#include 
+
+#include 
+#include 
+
+#pragma comment(lib, "crypt32.lib")
+#pragma comment(lib, "Ws2_32.lib")
+
+namespace {
+
+constexpr std::array, 8> SYSTEM_STORE_LOCATIONS{{

Review comment:
   I see, thanks for the explanation :)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


fgerlits commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535196611



##
File path: libminifi/src/utils/tls/WindowsCertStoreLocation.cpp
##
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifdef WIN32
+
+#include "utils/tls/WindowsCertStoreLocation.h"
+
+#include 
+
+#include 
+#include 
+
+#pragma comment(lib, "crypt32.lib")
+#pragma comment(lib, "Ws2_32.lib")
+
+namespace {
+
+constexpr std::array, 8> SYSTEM_STORE_LOCATIONS{{

Review comment:
   There is a reason, but not a very good one.  :)
   
   Originally both were `std::pair`; I prefer that as it means less code, and 
it is more similar to `std::map`.  But the constructor of `std::pair` is not 
constexpr before C++14, and AppleClang is strict about that, so I had to change 
`std::pair` to a custom struct in code which is compiled on MacOS.  
`WindowsCertStoreLocation` is only compiled on Windows, which is on C++14 
already, so I could use `std::pair` here.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-03 Thread GitBox


lordgamez commented on a change in pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947#discussion_r535137971



##
File path: libminifi/src/utils/tls/WindowsCertStoreLocation.cpp
##
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifdef WIN32
+
+#include "utils/tls/WindowsCertStoreLocation.h"
+
+#include 
+
+#include 
+#include 
+
+#pragma comment(lib, "crypt32.lib")
+#pragma comment(lib, "Ws2_32.lib")
+
+namespace {
+
+constexpr std::array, 8> SYSTEM_STORE_LOCATIONS{{

Review comment:
   Is there any reason using std::pair here but using a custom struct 
KeyValuePair in ExtendedKeyUsage? It just tickles my OCD :)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534998855



##
File path: libminifi/include/utils/ProcessorConfigUtils.h
##
@@ -0,0 +1,76 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getRequiredPropertyOrThrow(const core::ProcessContext* context, 
const std::string& property_name) {

Review comment:
   Thanks!





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


lordgamez commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534990092



##
File path: extensions/librdkafka/tests/ConsumeKafkaTests.cpp
##
@@ -0,0 +1,595 @@
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include 
+#include 
+#include 
+#include 
+
+// #include "TestBase.h"
+#include "../../../libminifi/test/TestBase.h"
+
+#include "../ConsumeKafka.h"
+#include "../rdkafka_utils.h"
+#include "../../standard-processors/processors/ExtractText.h"
+#include "utils/file/FileUtils.h"
+#include "utils/OptionalUtils.h"
+#include "utils/RegexUtils.h"
+#include "utils/StringUtils.h"
+#include "utils/TestUtils.h"
+
+#include "utils/IntegrationTestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::optional;
+
+class KafkaTestProducer {
+ public:
+  enum class PublishEvent {
+PUBLISH,
+TRANSACTION_START,
+TRANSACTION_COMMIT,
+CANCEL
+  };
+  KafkaTestProducer(const std::string& kafka_brokers, const std::string& 
topic, const bool transactional) :
+  logger_(logging::LoggerFactory::getLogger()) {
+using utils::setKafkaConfigurationField;
+
+std::unique_ptr conf = { 
rd_kafka_conf_new(), utils::rd_kafka_conf_deleter() };
+
+setKafkaConfigurationField(conf.get(), "bootstrap.servers", kafka_brokers);
+// setKafkaConfigurationField(conf.get(), "client.id", 
PRODUCER_CLIENT_NAME);
+setKafkaConfigurationField(conf.get(), "compression.codec", "snappy");
+setKafkaConfigurationField(conf.get(), "batch.num.messages", "1");
+
+if (transactional) {
+  setKafkaConfigurationField(conf.get(), "transactional.id", 
"ConsumeKafkaTest_transaction_id");
+}
+
+static std::array errstr{};
+producer_ = { rd_kafka_new(RD_KAFKA_PRODUCER, conf.release(), 
errstr.data(), errstr.size()), utils::rd_kafka_producer_deleter() };
+if (producer_ == nullptr) {
+  auto error_msg = utils::StringUtils::join_pack("Failed to create Kafka 
producer %s", errstr.data());
+  throw std::runtime_error(error_msg);
+}
+
+// The last argument is a config here, but it is already owned by the 
consumer. I assume that this would mean an override on the original config if 
used
+topic_ = { rd_kafka_topic_new(producer_.get(), topic.c_str(), nullptr), 
utils::rd_kafka_topic_deleter() };
+
+if (transactional) {
+  rd_kafka_init_transactions(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+}
+  }
+
+  // Uses all the headers for every published message
+  void publish_messages_to_topic(
+  const std::vector& messages_on_topic, const std::string& 
message_key, std::vector events,
+  const std::vector>& message_headers, 
const optional& message_header_encoding) {
+auto next_message = messages_on_topic.cbegin();
+for (const PublishEvent event : events) {
+  switch (event) {
+case PublishEvent::PUBLISH:
+  REQUIRE(messages_on_topic.cend() != next_message);
+  publish_message(*next_message, message_key, message_headers, 
message_header_encoding);
+  std::advance(next_message, 1);
+  break;
+case PublishEvent::TRANSACTION_START:
+  logger_->log_debug("Starting new transaction...");
+  rd_kafka_begin_transaction(producer_.get());
+  break;
+case PublishEvent::TRANSACTION_COMMIT:
+  logger_->log_debug("Committing transaction...");
+  rd_kafka_commit_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  break;
+case PublishEvent::CANCEL:
+  logger_->log_debug("Cancelling transaction...");
+  rd_kafka_abort_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  }
+}
+  }
+
+ private:
+  void publish_message(
+  const std::string& message, const std::string& message_key, const 
std::vector>& message_headers, const 
optional& message_header_encoding) {
+logger_->log_debug("Producing: %s", message.c_str());
+std::unique_ptr 
headers(rd_kafka_headers_new(message_headers.size()), 
utils::rd_kafka_headers_deleter());
+if (!headers) {
+  throw std::runtime_error("Generating message headers failed.");
+}
+ 

[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


lordgamez commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534990092



##
File path: extensions/librdkafka/tests/ConsumeKafkaTests.cpp
##
@@ -0,0 +1,595 @@
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include 
+#include 
+#include 
+#include 
+
+// #include "TestBase.h"
+#include "../../../libminifi/test/TestBase.h"
+
+#include "../ConsumeKafka.h"
+#include "../rdkafka_utils.h"
+#include "../../standard-processors/processors/ExtractText.h"
+#include "utils/file/FileUtils.h"
+#include "utils/OptionalUtils.h"
+#include "utils/RegexUtils.h"
+#include "utils/StringUtils.h"
+#include "utils/TestUtils.h"
+
+#include "utils/IntegrationTestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::optional;
+
+class KafkaTestProducer {
+ public:
+  enum class PublishEvent {
+PUBLISH,
+TRANSACTION_START,
+TRANSACTION_COMMIT,
+CANCEL
+  };
+  KafkaTestProducer(const std::string& kafka_brokers, const std::string& 
topic, const bool transactional) :
+  logger_(logging::LoggerFactory::getLogger()) {
+using utils::setKafkaConfigurationField;
+
+std::unique_ptr conf = { 
rd_kafka_conf_new(), utils::rd_kafka_conf_deleter() };
+
+setKafkaConfigurationField(conf.get(), "bootstrap.servers", kafka_brokers);
+// setKafkaConfigurationField(conf.get(), "client.id", 
PRODUCER_CLIENT_NAME);
+setKafkaConfigurationField(conf.get(), "compression.codec", "snappy");
+setKafkaConfigurationField(conf.get(), "batch.num.messages", "1");
+
+if (transactional) {
+  setKafkaConfigurationField(conf.get(), "transactional.id", 
"ConsumeKafkaTest_transaction_id");
+}
+
+static std::array errstr{};
+producer_ = { rd_kafka_new(RD_KAFKA_PRODUCER, conf.release(), 
errstr.data(), errstr.size()), utils::rd_kafka_producer_deleter() };
+if (producer_ == nullptr) {
+  auto error_msg = utils::StringUtils::join_pack("Failed to create Kafka 
producer %s", errstr.data());
+  throw std::runtime_error(error_msg);
+}
+
+// The last argument is a config here, but it is already owned by the 
consumer. I assume that this would mean an override on the original config if 
used
+topic_ = { rd_kafka_topic_new(producer_.get(), topic.c_str(), nullptr), 
utils::rd_kafka_topic_deleter() };
+
+if (transactional) {
+  rd_kafka_init_transactions(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+}
+  }
+
+  // Uses all the headers for every published message
+  void publish_messages_to_topic(
+  const std::vector& messages_on_topic, const std::string& 
message_key, std::vector events,
+  const std::vector>& message_headers, 
const optional& message_header_encoding) {
+auto next_message = messages_on_topic.cbegin();
+for (const PublishEvent event : events) {
+  switch (event) {
+case PublishEvent::PUBLISH:
+  REQUIRE(messages_on_topic.cend() != next_message);
+  publish_message(*next_message, message_key, message_headers, 
message_header_encoding);
+  std::advance(next_message, 1);
+  break;
+case PublishEvent::TRANSACTION_START:
+  logger_->log_debug("Starting new transaction...");
+  rd_kafka_begin_transaction(producer_.get());
+  break;
+case PublishEvent::TRANSACTION_COMMIT:
+  logger_->log_debug("Committing transaction...");
+  rd_kafka_commit_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  break;
+case PublishEvent::CANCEL:
+  logger_->log_debug("Cancelling transaction...");
+  rd_kafka_abort_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  }
+}
+  }
+
+ private:
+  void publish_message(
+  const std::string& message, const std::string& message_key, const 
std::vector>& message_headers, const 
optional& message_header_encoding) {
+logger_->log_debug("Producing: %s", message.c_str());
+std::unique_ptr 
headers(rd_kafka_headers_new(message_headers.size()), 
utils::rd_kafka_headers_deleter());
+if (!headers) {
+  throw std::runtime_error("Generating message headers failed.");
+}
+ 

[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534973423



##
File path: libminifi/test/TestBase.cpp
##
@@ -247,45 +221,65 @@ void TestPlan::reset(bool reschedule) {
   }
 }
 
-bool TestPlan::runNextProcessor(std::function, const 
std::shared_ptr)> verify) {
-  if (!finalized) {
-finalize();
+std::vector>::iterator 
TestPlan::getProcessorItByUuid(const std::string& uuid) {
+  const auto processor_node_matches_processor = [] (const 
std::shared_ptr& processor) {
+return processor->getUUIDStr() == uuid;
+  };
+  auto processor_found_at = std::find_if(processor_queue_.begin(), 
processor_queue_.end(), processor_node_matches_processor);
+  if (processor_found_at == processor_queue_.end()) {
+throw std::runtime_error("Processor not found in test plan.");
   }
-  logger_->log_info("Running next processor %d, processor_queue_.size %d, 
processor_contexts_.size %d", location, processor_queue_.size(), 
processor_contexts_.size());
-  std::lock_guard guard(mutex);
-  location++;
-  std::shared_ptr processor = processor_queue_.at(location);
-  std::shared_ptr context = 
processor_contexts_.at(location);
-  std::shared_ptr factory = 
std::make_shared(context);
-  factories_.push_back(factory);
+  return processor_found_at;
+}
+
+std::shared_ptr 
TestPlan::getProcessContextForProcessor(const std::shared_ptr& 
processor) {
+  const auto contextMatchesProcessor = [] (const 
std::shared_ptr& context) {
+return context->getProcessorNode()->getUUIDStr() ==  
processor->getUUIDStr();
+  };
+  const auto context_found_at = std::find_if(processor_contexts_.begin(), 
processor_contexts_.end(), contextMatchesProcessor);
+  if (context_found_at == processor_contexts_.end()) {
+throw std::runtime_error("Context not found in test plan.");
+  }
+  return *context_found_at;
+}
+
+void TestPlan::schedule_processors() {
+  for(std::size_t target_location = 0; target_location < 
processor_queue_.size(); ++target_location) {
+std::shared_ptr processor = 
processor_queue_.at(target_location);
+std::shared_ptr context = 
processor_contexts_.at(target_location);
+schedule_processor(processor, context);
+  }
+}
+
+void TestPlan::schedule_processor(const std::shared_ptr& 
processor) {
+  schedule_processor(processor, getProcessContextForProcessor(processor));
+}
+
+void TestPlan::schedule_processor(const std::shared_ptr& 
processor, const std::shared_ptr& context) {

Review comment:
   Will change this to camelCase.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534960543



##
File path: extensions/librdkafka/tests/ConsumeKafkaTests.cpp
##
@@ -0,0 +1,595 @@
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include 
+#include 
+#include 
+#include 
+
+// #include "TestBase.h"
+#include "../../../libminifi/test/TestBase.h"
+
+#include "../ConsumeKafka.h"
+#include "../rdkafka_utils.h"
+#include "../../standard-processors/processors/ExtractText.h"
+#include "utils/file/FileUtils.h"
+#include "utils/OptionalUtils.h"
+#include "utils/RegexUtils.h"
+#include "utils/StringUtils.h"
+#include "utils/TestUtils.h"
+
+#include "utils/IntegrationTestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::optional;
+
+class KafkaTestProducer {
+ public:
+  enum class PublishEvent {
+PUBLISH,
+TRANSACTION_START,
+TRANSACTION_COMMIT,
+CANCEL
+  };
+  KafkaTestProducer(const std::string& kafka_brokers, const std::string& 
topic, const bool transactional) :
+  logger_(logging::LoggerFactory::getLogger()) {
+using utils::setKafkaConfigurationField;
+
+std::unique_ptr conf = { 
rd_kafka_conf_new(), utils::rd_kafka_conf_deleter() };
+
+setKafkaConfigurationField(conf.get(), "bootstrap.servers", kafka_brokers);
+// setKafkaConfigurationField(conf.get(), "client.id", 
PRODUCER_CLIENT_NAME);
+setKafkaConfigurationField(conf.get(), "compression.codec", "snappy");
+setKafkaConfigurationField(conf.get(), "batch.num.messages", "1");
+
+if (transactional) {
+  setKafkaConfigurationField(conf.get(), "transactional.id", 
"ConsumeKafkaTest_transaction_id");
+}
+
+static std::array errstr{};
+producer_ = { rd_kafka_new(RD_KAFKA_PRODUCER, conf.release(), 
errstr.data(), errstr.size()), utils::rd_kafka_producer_deleter() };
+if (producer_ == nullptr) {
+  auto error_msg = utils::StringUtils::join_pack("Failed to create Kafka 
producer %s", errstr.data());
+  throw std::runtime_error(error_msg);
+}
+
+// The last argument is a config here, but it is already owned by the 
consumer. I assume that this would mean an override on the original config if 
used
+topic_ = { rd_kafka_topic_new(producer_.get(), topic.c_str(), nullptr), 
utils::rd_kafka_topic_deleter() };
+
+if (transactional) {
+  rd_kafka_init_transactions(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+}
+  }
+
+  // Uses all the headers for every published message
+  void publish_messages_to_topic(
+  const std::vector& messages_on_topic, const std::string& 
message_key, std::vector events,
+  const std::vector>& message_headers, 
const optional& message_header_encoding) {
+auto next_message = messages_on_topic.cbegin();
+for (const PublishEvent event : events) {
+  switch (event) {
+case PublishEvent::PUBLISH:
+  REQUIRE(messages_on_topic.cend() != next_message);
+  publish_message(*next_message, message_key, message_headers, 
message_header_encoding);
+  std::advance(next_message, 1);
+  break;
+case PublishEvent::TRANSACTION_START:
+  logger_->log_debug("Starting new transaction...");
+  rd_kafka_begin_transaction(producer_.get());
+  break;
+case PublishEvent::TRANSACTION_COMMIT:
+  logger_->log_debug("Committing transaction...");
+  rd_kafka_commit_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  break;
+case PublishEvent::CANCEL:
+  logger_->log_debug("Cancelling transaction...");
+  rd_kafka_abort_transaction(producer_.get(), 
TRANSACTIONS_TIMEOUT_MS.count());
+  }
+}
+  }
+
+ private:
+  void publish_message(
+  const std::string& message, const std::string& message_key, const 
std::vector>& message_headers, const 
optional& message_header_encoding) {
+logger_->log_debug("Producing: %s", message.c_str());
+std::unique_ptr 
headers(rd_kafka_headers_new(message_headers.size()), 
utils::rd_kafka_headers_deleter());
+if (!headers) {
+  throw std::runtime_error("Generating message headers failed.");
+}

[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534946721



##
File path: extensions/librdkafka/tests/ConsumeKafkaTests.cpp
##
@@ -0,0 +1,595 @@
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include 
+#include 
+#include 
+#include 
+
+// #include "TestBase.h"
+#include "../../../libminifi/test/TestBase.h"
+
+#include "../ConsumeKafka.h"
+#include "../rdkafka_utils.h"
+#include "../../standard-processors/processors/ExtractText.h"
+#include "utils/file/FileUtils.h"
+#include "utils/OptionalUtils.h"
+#include "utils/RegexUtils.h"
+#include "utils/StringUtils.h"
+#include "utils/TestUtils.h"
+
+#include "utils/IntegrationTestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::optional;
+
+class KafkaTestProducer {
+ public:
+  enum class PublishEvent {
+PUBLISH,
+TRANSACTION_START,
+TRANSACTION_COMMIT,
+CANCEL
+  };
+  KafkaTestProducer(const std::string& kafka_brokers, const std::string& 
topic, const bool transactional) :
+  logger_(logging::LoggerFactory::getLogger()) {
+using utils::setKafkaConfigurationField;
+
+std::unique_ptr conf = { 
rd_kafka_conf_new(), utils::rd_kafka_conf_deleter() };
+
+setKafkaConfigurationField(conf.get(), "bootstrap.servers", kafka_brokers);
+// setKafkaConfigurationField(conf.get(), "client.id", 
PRODUCER_CLIENT_NAME);

Review comment:
   It was not obvious to me, so I left it commented out and forgotten about 
it. It is probably completely unneccessary as the testing went fine without it, 
will remove :)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534941010



##
File path: extensions/librdkafka/tests/ConsumeKafkaTests.cpp
##
@@ -0,0 +1,595 @@
+
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include 
+#include 
+#include 
+#include 
+
+// #include "TestBase.h"

Review comment:
   Ah, forgotten about this. I would prefer this as the way of including 
TestBase instead of the long one below. Will correct this by adding the 
`libminifi/test` include directory to the cmake file. 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534937049



##
File path: extensions/librdkafka/tests/CMakeLists.txt
##
@@ -29,8 +29,11 @@ FOREACH(testfile ${KAFKA_TESTS})
 createTests("${testfilename}")
 MATH(EXPR KAFKA_TEST_COUNT "${KAFKA_TEST_COUNT}+1")
 # The line below handles integration test
-add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
"${TEST_RESOURCES}/TestKafkaOnSchedule.yml"  "${TEST_RESOURCES}/")
+   target_include_directories(${testfilename} BEFORE PRIVATE 
"../../standard-processors/processors")

Review comment:
   Good catch!





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534936351



##
File path: extensions/librdkafka/rdkafka_utils.h
##
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core/logging/LoggerConfiguration.h"
+#include "utils/OptionalUtils.h"
+#include "rdkafka.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+enum class KafkaEncoding {
+  UTF8,
+  HEX
+};
+
+struct rd_kafka_conf_deleter {
+  void operator()(rd_kafka_conf_t* ptr) const noexcept { 
rd_kafka_conf_destroy(ptr); }
+};
+
+struct rd_kafka_producer_deleter {
+  void operator()(rd_kafka_t* ptr) const noexcept {
+rd_kafka_resp_err_t flush_ret = rd_kafka_flush(ptr, 1 /* ms */);  // 
Matching the wait time of KafkaConnection.cpp
+// If concerned, we could log potential errors here:
+// if (RD_KAFKA_RESP_ERR__TIMED_OUT == flush_ret) {
+//   std::cerr << "Deleting producer failed: time-out while trying to 
flush" << std::endl;
+// }

Review comment:
   I think as the error-enum returned is not obvious this is nice to have 
here. Another point is that one might not immediately think this could be a 
point of failure. I would have had this code in as debug log, but that would 
mean that any deleter would have to have an access to a logger.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534927466



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-03 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534926679



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi] r65535 opened a new pull request #4705: Added FIFO options to PutSQS

2020-12-03 Thread GitBox


r65535 opened a new pull request #4705:
URL: https://github.com/apache/nifi/pull/4705


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
in the commit message?
   
   - [x] Does your PR title start with **NIFI-** where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `main`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [x] Have you written or updated unit tests to verify your changes?
   - [x] Have you verified that the full build is successful on JDK 8?
   - [x] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 commented on a change in pull request #4204: NIFI-7355 Added Gremlin bytecode client service.

2020-12-02 Thread GitBox


mattyb149 commented on a change in pull request #4204:
URL: https://github.com/apache/nifi/pull/4204#discussion_r534549148



##
File path: 
nifi-nar-bundles/nifi-graph-bundle/nifi-other-graph-services/src/main/java/org/apache/nifi/graph/GremlinBytecodeClientService.java
##
@@ -0,0 +1,365 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.graph;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.annotation.lifecycle.OnDisabled;
+import org.apache.nifi.annotation.lifecycle.OnEnabled;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.graph.gremlin.SimpleEntry;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+import org.apache.nifi.util.StringUtils;
+import org.apache.tinkerpop.gremlin.driver.Cluster;
+import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
+import org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource;
+import 
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+
+import javax.script.Bindings;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
+import static org.apache.nifi.graph.GremlinClientService.NOT_SUPPORTED;
+
+@CapabilityDescription("A client service that provides a scriptable interface 
to open a remote connection/travseral " +
+"against a Gremlin Server and execute operations against it.")
+@Tags({"graph", "database", "gremlin", "tinkerpop"})
+public class GremlinBytecodeClientService extends 
AbstractTinkerpopClientService implements GraphClientService {
+private static final List NEW_DESCRIPTORS;
+
+public static final PropertyDescriptor REMOTE_OBJECTS_FILE = new 
PropertyDescriptor.Builder()

Review comment:
   Would this apply to GremlinClientService as well? If so we should have a 
base or util class with this property (and appropriate handling) that 
GremlinClientService and GremlinBytecodeClientService could both use. Also see 
my comment below about combining the processors if it makes it easier on the 
user.

##
File path: 
nifi-nar-bundles/nifi-graph-bundle/nifi-other-graph-services/src/main/resources/docs/org.apache.nifi.graph.GremlinBytecodeClientService/additionalDetails.html
##
@@ -0,0 +1,41 @@
+
+
+
+
+
+GremlinBytecodeClientservice
+
+
+
+
+
+Description:
+
+This client service configures a remote connection/traversal and allows a 
script to execute operations against it. The key
+difference between this and GremlinClientService is that this version does 
not run the script on the Gremlin Server, but rather on
+the client side (NiFi). For more details, see the Tinkerpop documentation

Review comment:
   Is it easier for the user to have two different processors, or a 
property that decides where to run the script (server or client)?

##
File path: 
nifi-nar-bundles/nifi-graph-bundle/nifi-other-graph-services/src/main/resources/docs/org.apache.nifi.graph.GremlinBytecodeClientService/additionalDetails.html
##
@@ -0,0 +1,41 @@
+
+
+
+
+
+GremlinBytecodeClientservice
+
+
+
+
+
+Description:
+
+This client service configures a remote connection/traversal and allows a 
script to execute operations against it. The key
+

[GitHub] [nifi] mattyb149 opened a new pull request #4704: NIFI-7906: Implemented RecordSetWriter support for ExecuteGraphQueryRecord

2020-12-02 Thread GitBox


mattyb149 opened a new pull request #4704:
URL: https://github.com/apache/nifi/pull/4704


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   ExecuteGraphQueryRecord specifies a Failed Record Writer property that is 
meant to write out any records that fail during processing of the graph query. 
However the current implementation doesn't use it and instead sends the entire 
input flowfile to failure, and can result in a flowfile that doesn't get 
transferred (causing an exception). This PR refactors the processor to use the 
writer to write any failed records to the failure relationship.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
in the commit message?
   
   - [x] Does your PR title start with **NIFI-** where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `main`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [x] Have you written or updated unit tests to verify your changes?
   - [x] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [x] 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-02 Thread GitBox


markap14 commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r534527090



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
##
@@ -602,8 +602,7 @@ public boolean isAllDataBufferedForEntireText() {
 @Override
 public FlowFile replace(FlowFile flowFile, final ProcessSession 
session, final ProcessContext context, final String evaluateMode, final Charset 
charset, final int maxBufferSize) {

Review comment:
   Yeah, I cannot argue that point with you. Like many processors, this one 
started pretty simple, once upon a time. And a new feature was added. And 
another. And it's become quite the beast. Definitely wouldn't hurt to updates 
with some docs. And probably would help to add some additionalDetails.html, 
too, to be honest, because there are a lot of options here.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] MikeThomsen commented on pull request #4701: NIFI-7906 fixed windows build

2020-12-02 Thread GitBox


MikeThomsen commented on pull request #4701:
URL: https://github.com/apache/nifi/pull/4701#issuecomment-737524052


   @joewitt looks gtg



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


fgerlits commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534274245



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+namespace internal {
+
+template
+struct find_in_range {
+  static auto call(const T& range, const Arg& arg) -> 
decltype(std::find(range.begin(), range.end(), arg)) {
+return std::find(range.begin(), range.end(), arg);
+  }
+};
+
+template
+struct find_in_range().find(std::declval()), void())> {
+  static auto call(const T& range, const Arg& arg) -> 
decltype(range.find(arg)) {
+return range.find(arg);
+  }
+};
+
+}  // namespace internal
+
+template
+bool haveCommonItem(const T& a, const U& b) {
+  using Item = typename T::value_type;
+  return std::any_of(a.begin(), a.end(), [&] (const Item& item) {
+return internal::find_in_range::call(b, item) != b.end();

Review comment:
   I am hesitant to comment as this PR already has a record number of 
comments, but why do we need the two versions?  If we always do `std::find`, 
we'll have 1/3 the amount of code.  Is that less efficient in some cases?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] bbende opened a new pull request #4703: NIFI-8064 Convert TestSecureClientZooKeeperFactory to integration test

2020-12-02 Thread GitBox


bbende opened a new pull request #4703:
URL: https://github.com/apache/nifi/pull/4703


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   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 **NIFI-** where  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? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] ottobackwards commented on pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-02 Thread GitBox


ottobackwards commented on pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#issuecomment-737511510


   +1 fwiw



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] ottobackwards commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-02 Thread GitBox


ottobackwards commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r534497643



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
##
@@ -602,8 +602,7 @@ public boolean isAllDataBufferedForEntireText() {
 @Override
 public FlowFile replace(FlowFile flowFile, final ProcessSession 
session, final ProcessContext context, final String evaluateMode, final Charset 
charset, final int maxBufferSize) {

Review comment:
   fair enough.  In general, I think better dev comments about what is 
going on in this class would help.  Point taken on this specific ask

##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
##
@@ -53,6 +54,66 @@ public TestRunner getRunner() {
 return runner;
 }
 

Review comment:
   Sorry, please disregard.  I literally skipped past the LITERAL part of 
this.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 opened a new pull request #4702: NIFI-8063: Added profile (enabled) to include most NARs, can be disabled

2020-12-02 Thread GitBox


mattyb149 opened a new pull request #4702:
URL: https://github.com/apache/nifi/pull/4702


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   Moves most NARs in the assembly POM to an enabled-by-default profile. It can 
be disabled from the command line by deactivating the profile:
   
   `-P\!most-nars`
   
   This results in an assembly package roughly 1/4 of the total size.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
in the commit message?
   
   - [x] Does your PR title start with **NIFI-** where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `main`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [x] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] joewitt commented on pull request #4701: NIFI-7906 fixed windows build

2020-12-02 Thread GitBox


joewitt commented on pull request #4701:
URL: https://github.com/apache/nifi/pull/4701#issuecomment-737483420


   watching to see if build gets happy.  if so i'll merge



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on a change in pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


markap14 commented on a change in pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#discussion_r534466759



##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/StatelessProvenanceRepository.java
##
@@ -0,0 +1,380 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import org.apache.nifi.provenance.search.SearchableField;
+import org.apache.nifi.util.RingBuffer;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class StatelessProvenanceRepository implements ProvenanceRepository {
+
+public static String CONTAINER_NAME = "in-memory";
+
+private final RingBuffer ringBuffer;
+private final int maxSize;
+
+private final AtomicLong idGenerator = new AtomicLong(0L);
+
+public StatelessProvenanceRepository(final int maxEvents) {
+maxSize = maxEvents;
+ringBuffer = new RingBuffer<>(maxSize);
+}
+
+@Override
+public void initialize(final EventReporter eventReporter, final Authorizer 
authorizer, final ProvenanceAuthorizableFactory resourceFactory,
+   final IdentifierLookup idLookup) throws IOException 
{
+
+}
+
+@Override
+public ProvenanceEventRepository getProvenanceEventRepository() {
+return this;
+}
+
+@Override
+public ProvenanceEventBuilder eventBuilder() {
+return new StandardProvenanceEventRecord.Builder();
+}
+
+@Override
+public void registerEvent(final ProvenanceEventRecord event) {
+final long id = idGenerator.getAndIncrement();
+ringBuffer.add(new IdEnrichedProvEvent(event, id));
+}
+
+@Override
+public void registerEvents(final Iterable events) {
+for (final ProvenanceEventRecord event : events) {
+registerEvent(event);
+}
+}
+
+@Override
+public List getEvents(final long firstRecordId, 
final int maxRecords) throws IOException {
+return getEvents(firstRecordId, maxRecords, null);
+}
+
+@Override
+public List getEvents(final long firstRecordId, 
final int maxRecords, final NiFiUser user) throws IOException {
+return ringBuffer.getSelectedElements(new 
RingBuffer.Filter() {
+@Override
+public boolean select(final ProvenanceEventRecord value) {
+return value.getEventId() >= firstRecordId;
+}
+}, maxRecords);
+}
+
+@Override
+public Long getMaxEventId() {
+final ProvenanceEventRecord newest = ringBuffer.getNewestElement();
+return (newest == null) ? null : newest.getEventId();
+}
+
+public ProvenanceEventRecord getEvent(final String identifier) throws 
IOException {
+final List records = 
ringBuffer.getSelectedElements(new RingBuffer.Filter() {
+@Override
+public boolean select(final ProvenanceEventRecord event) {
+return identifier.equals(event.getFlowFileUuid());
+}
+}, 1);
+return records.isEmpty() ? null : records.get(0);
+}
+
+@Override
+public ProvenanceEventRecord getEvent(final long 

[GitHub] [nifi] MikeThomsen opened a new pull request #4701: NIFI-7906 fixed windows build

2020-12-02 Thread GitBox


MikeThomsen opened a new pull request #4701:
URL: https://github.com/apache/nifi/pull/4701


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   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 **NIFI-** where  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? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-02 Thread GitBox


markap14 commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r534465100



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
##
@@ -53,6 +54,66 @@ public TestRunner getRunner() {
 return runner;
 }
 

Review comment:
   I'm not sure that I follow your suggestion. I feel it is quite heavily 
tested at this point. Can you provide an example configuration that you think 
would be worthwhile testing? As in, provide a set of:
   Search Value = ___
   Replacement Value = ___
   Replacement Strategy = ___
   Sample input = 
   Expected output = 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] bbende commented on pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


bbende commented on pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#issuecomment-737480941


   @markap14 I pushed a commit to address the comments, thanks!



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] bbende commented on a change in pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


bbende commented on a change in pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#discussion_r534464510



##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import org.apache.nifi.provenance.search.SearchableField;
+import org.apache.nifi.util.RingBuffer;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class VolatileProvenanceRepository implements ProvenanceRepository {

Review comment:
   Agree

##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import org.apache.nifi.provenance.search.SearchableField;
+import org.apache.nifi.util.RingBuffer;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class VolatileProvenanceRepository implements ProvenanceRepository {
+
+// default property values
+public static final int DEFAULT_BUFFER_SIZE = 1;
+
+public static String 

[GitHub] [nifi] bbende commented on a change in pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


bbende commented on a change in pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#discussion_r534464332



##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import org.apache.nifi.provenance.search.SearchableField;
+import org.apache.nifi.util.RingBuffer;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class VolatileProvenanceRepository implements ProvenanceRepository {
+
+// default property values
+public static final int DEFAULT_BUFFER_SIZE = 1;
+
+public static String CONTAINER_NAME = "in-memory";
+
+private final RingBuffer ringBuffer;
+private final int maxSize;
+
+private final AtomicLong idGenerator = new AtomicLong(0L);
+private final AtomicBoolean initialized = new AtomicBoolean(false);
+
+/**
+ * Default no args constructor for service loading only
+ */
+public VolatileProvenanceRepository() {

Review comment:
   Agree

##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import 

[GitHub] [nifi] markap14 commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-02 Thread GitBox


markap14 commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r534464347



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
##
@@ -602,8 +602,7 @@ public boolean isAllDataBufferedForEntireText() {
 @Override
 public FlowFile replace(FlowFile flowFile, final ProcessSession 
session, final ProcessContext context, final String evaluateMode, final Charset 
charset, final int maxBufferSize) {

Review comment:
   I can understand why the existence of it and the removal of it would be 
confusing. I don't think a comment in the code would be helpful though. It 
never should have been escaped. I think it was escaped as a result of 
refactoring the code, or perhaps because it is escaped when using a Regular 
Expression as the search value but if we added an inline comment about why 
we're not arbitrarily escaping something that doesn't need escaping, outside of 
the scope of this PR, I think it would result in more confusion than 
clarification :)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on a change in pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


markap14 commented on a change in pull request #4700:
URL: https://github.com/apache/nifi/pull/4700#discussion_r534455443



##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import org.apache.nifi.provenance.lineage.ComputeLineageSubmission;
+import org.apache.nifi.provenance.search.Query;
+import org.apache.nifi.provenance.search.QuerySubmission;
+import org.apache.nifi.provenance.search.SearchableField;
+import org.apache.nifi.util.RingBuffer;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class VolatileProvenanceRepository implements ProvenanceRepository {
+
+// default property values
+public static final int DEFAULT_BUFFER_SIZE = 1;
+
+public static String CONTAINER_NAME = "in-memory";
+
+private final RingBuffer ringBuffer;
+private final int maxSize;
+
+private final AtomicLong idGenerator = new AtomicLong(0L);
+private final AtomicBoolean initialized = new AtomicBoolean(false);
+
+/**
+ * Default no args constructor for service loading only
+ */
+public VolatileProvenanceRepository() {

Review comment:
   Given the usage pattern, that this will only be used in stateless, and 
it's not loaded via the service loader, I don't think we even need the no-arg 
constructor.

##
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-stateless-bundle/nifi-stateless-engine/src/main/java/org/apache/nifi/stateless/repository/VolatileProvenanceRepository.java
##
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.stateless.repository;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUser;
+import org.apache.nifi.events.EventReporter;
+import org.apache.nifi.provenance.AsyncLineageSubmission;
+import org.apache.nifi.provenance.IdentifierLookup;
+import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
+import org.apache.nifi.provenance.ProvenanceEventBuilder;
+import org.apache.nifi.provenance.ProvenanceEventRecord;
+import org.apache.nifi.provenance.ProvenanceEventRepository;
+import org.apache.nifi.provenance.ProvenanceEventType;
+import org.apache.nifi.provenance.ProvenanceRepository;
+import org.apache.nifi.provenance.StandardProvenanceEventRecord;
+import 

[GitHub] [nifi] bbende opened a new pull request #4700: NIFI-8060 Added minimal VolatileProvenanceRepository to nifi-stateles…

2020-12-02 Thread GitBox


bbende opened a new pull request #4700:
URL: https://github.com/apache/nifi/pull/4700


   …s-engine and remove dependency on nifi-volatile-provenance-repo module
   
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   _Enables X functionality; fixes bug NIFI-._
   
   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 **NIFI-** where  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? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 commented on pull request #4204: NIFI-7355 Added Gremlin bytecode client service.

2020-12-02 Thread GitBox


mattyb149 commented on pull request #4204:
URL: https://github.com/apache/nifi/pull/4204#issuecomment-737444733


   I'm getting an error when trying to send a Gremlin script with double-quoted 
strings (GStrings) to JanusGraph: 
   
   ```
   org.apache.tinkerpop.gremlin.driver.ser.SerializationException: 
java.lang.IllegalArgumentException: Class is not registered: 
org.codehaus.groovy.runtime.GStringImpl
   Note: To register this class use: 
kryo.register(org.codehaus.groovy.runtime.GStringImpl.class);
   ```
   
   We should probably register this automatically? Otherwise the user would 
have to point at the class and JAR just to use normal Groovy objects.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] dependabot[bot] opened a new pull request #4699: Bump jetty.version from 9.4.34.v20201102 to 9.4.35.v20201120

2020-12-02 Thread GitBox


dependabot[bot] opened a new pull request #4699:
URL: https://github.com/apache/nifi/pull/4699


   Bumps `jetty.version` from 9.4.34.v20201102 to 9.4.35.v20201120.
   Updates `jetty-server` from 9.4.34.v20201102 to 9.4.35.v20201120
   
   Release notes
   Sourced from https://github.com/eclipse/jetty.project/releases;>jetty-server's 
releases.
   
   9.4.35.v20201120
   Important Change
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 : java.io.IOException: unconsumed input during http request 
parsing
   
   Bugs
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/4711;>#4711
 : Reset trailers on recycled response
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5486;>#5486
 : PropertyFileLoginModule retains PropertyUserStores
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5562;>#5562
 : ArrayTernaryTrie consumes too much memory
   
   Enhancements
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5539;>#5539
 : StatisticsServlet output now available in json, xml, text, and html
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5575;>#5575
 : Add SEARCH as a known HttpMethod
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5633;>#5633
 : Allow to configure HttpClient request authority (even on HTTP/2)
   
   
   
   
   Commits
   
   https://github.com/eclipse/jetty.project/commit/bdc54f03a5e0a7e280fab27f55c3c75ee8da89fb;>bdc54f0
 Updating to version 9.4.35.v20201120
   https://github.com/eclipse/jetty.project/commit/41bf9534eba774b669a01aa3cc102e4690e3e6c9;>41bf953
 Issue https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5603;>#5603
 - Single page documentation (https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5636;>#5636)
   https://github.com/eclipse/jetty.project/commit/fcfa72ee400fec42c88d86454cdc09c0d1761b30;>fcfa72e
 Bump javax.servlet.jsp.jstl from 1.2.2 to 1.2.5 (https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5673;>#5673)
   https://github.com/eclipse/jetty.project/commit/901a17d197fd9ab7f0bbcf01faf9969668a8ee7f;>901a17d
 Issue https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 - Adding more comments
   https://github.com/eclipse/jetty.project/commit/a6d432e9e3031c5dec0712b9b44a80ff63d2da1c;>a6d432e
 Issue https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 - Adding more comments and fixing logging
   https://github.com/eclipse/jetty.project/commit/d4feb4f29dea48f70585a07af9a3ed06067f639f;>d4feb4f
 Removed unused code.
   https://github.com/eclipse/jetty.project/commit/248779e1958214a7d5ca842d1f88972aba7c5ca9;>248779e
 Bump grpc-core from 1.33.0 to 1.33.1 (https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5623;>#5623)
   https://github.com/eclipse/jetty.project/commit/5f6e72d2ed099ea72b4f665d7aac2c34282762a3;>5f6e72d
 Issue https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 - Adding more gzip consume all tests
   https://github.com/eclipse/jetty.project/commit/14f94f738df5adf68549749d01d4f9d97ea51452;>14f94f7
 Issue https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 unconsumed input on sendError (https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5637;>#5637)
   https://github.com/eclipse/jetty.project/commit/1448444c657e4aa49cdafd372e943679e945a581;>1448444
 Merge pull request https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5560;>#5560
 from eclipse/jetty-9.4.x-5539-statisticsservlet-output
   Additional commits viewable in https://github.com/eclipse/jetty.project/compare/jetty-9.4.34.v20201102...jetty-9.4.35.v20201120;>compare
 view
   
   
   
   
   Updates `jetty-servlet` from 9.4.34.v20201102 to 9.4.35.v20201120
   
   Release notes
   Sourced from https://github.com/eclipse/jetty.project/releases;>jetty-servlet's 
releases.
   
   9.4.35.v20201120
   Important Change
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5605;>#5605
 : java.io.IOException: unconsumed input during http request 
parsing
   
   Bugs
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/4711;>#4711
 : Reset trailers on recycled response
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5486;>#5486
 : PropertyFileLoginModule retains PropertyUserStores
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5562;>#5562
 : ArrayTernaryTrie consumes too much memory
   
   Enhancements
   
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5539;>#5539
 : StatisticsServlet output now available in json, xml, text, and html
   https://github-redirect.dependabot.com/eclipse/jetty.project/issues/5575;>#5575
 : Add SEARCH as a known HttpMethod
   

[GitHub] [nifi] mattyb149 commented on a change in pull request #4651: NIFI-7988 Prometheus Remote Write Processor

2020-12-02 Thread GitBox


mattyb149 commented on a change in pull request #4651:
URL: https://github.com/apache/nifi/pull/4651#discussion_r534304482



##
File path: 
nifi-nar-bundles/nifi-prometheus-bundle/nifi-prometheus-processors/pom.xml
##
@@ -0,0 +1,133 @@
+
+
+http://maven.apache.org/POM/4.0.0; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; 
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+
+org.apache.nifi
+nifi-prometheus-bundle
+1.13.0-SNAPSHOT
+
+
+nifi-prometheus-processors
+jar
+
+
+9.3.9.v20160517
+3.13.0
+1.1.7.1
+0.6.1
+1.6.2
+
+
+
+
+org.apache.nifi
+nifi-api
+1.13.0-SNAPSHOT
+
+
+org.apache.nifi
+nifi-utils
+1.13.0-SNAPSHOT
+
+
+org.apache.nifi
+nifi-mock
+1.13.0-SNAPSHOT
+test
+
+
+org.slf4j
+slf4j-simple
+test
+
+
+junit
+junit
+test
+
+
+com.google.protobuf
+protobuf-java
+${protobuf.version}
+
+
+com.google.protobuf
+protobuf-java-util
+${protobuf.version}
+
+
+org.xerial.snappy
+snappy-java

Review comment:
   This needs an entry in the NOTICE file for the NAR, see examples in 
other NARs (such as nifi-avro-nar)

##
File path: 
nifi-nar-bundles/nifi-prometheus-bundle/nifi-prometheus-processors/pom.xml
##
@@ -0,0 +1,133 @@
+
+
+http://maven.apache.org/POM/4.0.0; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; 
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+
+org.apache.nifi
+nifi-prometheus-bundle
+1.13.0-SNAPSHOT
+
+
+nifi-prometheus-processors
+jar
+
+
+9.3.9.v20160517
+3.13.0
+1.1.7.1
+0.6.1
+1.6.2
+
+
+
+
+org.apache.nifi
+nifi-api
+1.13.0-SNAPSHOT
+
+
+org.apache.nifi
+nifi-utils
+1.13.0-SNAPSHOT
+
+
+org.apache.nifi
+nifi-mock
+1.13.0-SNAPSHOT
+test
+
+
+org.slf4j
+slf4j-simple
+test
+
+
+junit
+junit
+test
+
+
+com.google.protobuf
+protobuf-java
+${protobuf.version}
+
+
+com.google.protobuf
+protobuf-java-util
+${protobuf.version}
+
+
+org.xerial.snappy
+snappy-java
+${snappy.version}
+
+
+org.eclipse.jetty
+jetty-server
+${jetty.version}
+
+
+org.eclipse.jetty
+jetty-servlet
+${jetty.version}
+
+   
+org.eclipse.jetty
+jetty-client
+${jetty.version}
+test
+
+
+
+
+
+  
+kr.motd.maven
+os-maven-plugin
+${maven.os.version}
+  
+
+
+
+org.xolstice.maven.plugins
+protobuf-maven-plugin
+${maven.protobuf.version}
+
+
com.google.protobuf:protoc:3.13.0:exe:${os.detected.classifier}
+
${basedir}/src/main/resources/proto
+
+
+
+
+compile
+test-compile
+
+
+
+
+
+org.apache.rat
+apache-rat-plugin
+
+
+
**/src/main/resources/proto/gogoproto/gogo.proto

Review comment:
   gogo.proto has a license header, does it not pass the Rat check?

##
File path: 
nifi-nar-bundles/nifi-prometheus-bundle/nifi-prometheus-processors/src/main/java/org/apache/nifi/processors/prometheus/PrometheusRemoteWrite.java
##
@@ -0,0 +1,318 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * 

[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-02 Thread GitBox


lordgamez commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534279245



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


fgerlits commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534274245



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+namespace internal {
+
+template
+struct find_in_range {
+  static auto call(const T& range, const Arg& arg) -> 
decltype(std::find(range.begin(), range.end(), arg)) {
+return std::find(range.begin(), range.end(), arg);
+  }
+};
+
+template
+struct find_in_range().find(std::declval()), void())> {
+  static auto call(const T& range, const Arg& arg) -> 
decltype(range.find(arg)) {
+return range.find(arg);
+  }
+};
+
+}  // namespace internal
+
+template
+bool haveCommonItem(const T& a, const U& b) {
+  using Item = typename T::value_type;
+  return std::any_of(a.begin(), a.end(), [&] (const Item& item) {
+return internal::find_in_range::call(b, item) != b.end();

Review comment:
   I am hesitant to comment as this PR already has a record number of 
comments, but why do we need the two versions?  If we always do `std::find`, 
we'll have 1/3 the amount of code.  Is that less efficient in some cases?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-02 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534272416



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-02 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534270988



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-02 Thread GitBox


hunyadi-dev commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r534256571



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")

Review comment:
   This is verbatim from nifi, but will rephrase.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534193427



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   so that was that sound!
   
   true, that calling a file `CollectionUtils` and then having a single 
function in it which only works for `std::set` is not great :) 
   
   made it generic, and added tests





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


arpadboda commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534161723



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   I think the filename itself screams for generalisation. :)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534140777



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   even though in my opinion "could be made generic" does not imply "should 
be made generic" (because nobody yet asked for it), since you and @lordgamez 
also think it is worth generalizing, I am turning this into a template





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


arpadboda commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534122907



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   This is specialised for set and string with no reason. 
   
   In case this just requries two iterables with the same underlying type and 
you use find instead of count, this is going to work with vectors, lists, etc, 
while keeping the log(N) efficiency for sets. 
   
   Not to mention unordered sets as well. 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


arpadboda commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534122907



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   This is specialised for set with no reason. 
   
   In case this just requries two iterables with the same underlying type and 
you use find instead of count, this is going to work with vectors, lists, etc, 
while keeping the log(N) efficiency for sets. 
   
   Not to mention unordered sets as well. 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


arpadboda commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534122907



##
File path: libminifi/include/utils/CollectionUtils.h
##
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   This is specialised for set with no reason. 
   
   In case this just requries two iterables with the same underlying type and 
you use find instead of count, this is going to work with vectors, lists, etc, 
while keeping the log(N) efficiency for sets. 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] arpadboda closed pull request #942: MINIFICPP-1410 Add permissions property support for Putfile processor

2020-12-02 Thread GitBox


arpadboda closed pull request #942:
URL: https://github.com/apache/nifi-minifi-cpp/pull/942


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534006825



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   moved it





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r534001123



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all 

[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


lordgamez commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533999239



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   I would move it now, just in case someone needs a similar utility 
function in the future would find it easier there. A wide generalization may 
not be needed at the moment, anyone using it later could handle that if needed.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


lordgamez commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533997043



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all 

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533983259



##
File path: libminifi/include/utils/file/FileSystem.h
##
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "utils/OptionalUtils.h"
+#include "utils/EncryptionProvider.h"
+#include "core/logging/LoggerConfiguration.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace file {
+
+class FileSystem {
+ public:
+  explicit FileSystem(bool should_encrypt = false, 
utils::optional encryptor = {});
+
+  FileSystem(const FileSystem&) = delete;
+  FileSystem(FileSystem&&) = delete;
+  FileSystem& operator=(const FileSystem&) = delete;
+  FileSystem& operator=(FileSystem&&) = delete;
+
+  utils::optional read(const std::string& file_name);
+
+  bool write(const std::string& file_name, const std::string& file_content);
+
+ private:
+  bool should_encrypt_on_write_;
+  utils::optional encryptor_;
+  std::shared_ptr 
logger_{logging::LoggerFactory::getLogger()};
+};

Review comment:
   how about `FileIO`?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533973565



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   I would wait with generalizing it, because it is currently used once, I 
would move it to the utils on an as-needed basis, should we move it now?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533971996



##
File path: libminifi/src/utils/StringUtils.cpp
##
@@ -31,7 +31,18 @@ bool StringUtils::StringToBool(std::string input, bool 
) {
   return output;
 }
 
-std::string StringUtils::trim(std::string s) {
+utils::optional StringUtils::toBool(const std::string& str) {

Review comment:
   there were tests for it before the "StringView massacre" but I threw the 
baby out with the bathwater, added test for it (yes it aims to replace the 
`StringToBool`, but they have slightly different semantics for now, and 
`StringToBool` is used in many places, so I would retire `StringToBool` 
separately)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533970246



##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  configuration_->setAgentIdentifier(identifier_str);
+
+  if (initialized_ && !flow_update_) {
+return;
+  }
+
+  {
+std::lock_guard guard(metrics_mutex_);
+component_metrics_.clear();
+// root_response_nodes_ was not cleared before, it is unclear if that was 
intentional
+  }
+
+  std::map> connections;
+  if (root_ != nullptr) {
+root_->getConnections(connections);
+  }
+
+  std::string class_csv;
+  if (configuration_->get("nifi.c2.root.classes", class_csv)) {
+std::vector classes = utils::StringUtils::split(class_csv, 
",");
+
+for (const std::string& clazz : classes) {
+  auto processor = 
std::dynamic_pointer_cast(core::ClassLoader::getDefaultClassLoader().instantiate(clazz,
 clazz));
+  if (nullptr == processor) {
+logger_->log_error("No metric defined for %s", clazz);
+continue;
+  }
+  auto identifier = 
std::dynamic_pointer_cast(processor);
+  if (identifier != nullptr) {
+identifier->setIdentifier(identifier_str);
+identifier->setAgentClass(class_str);
+  }
+  auto monitor = 
std::dynamic_pointer_cast(processor);
+  if (monitor != nullptr) {
+monitor->addRepository(provenance_repo_);
+monitor->addRepository(flow_file_repo_);
+monitor->setStateMonitor(update_sink);
+  }
+  auto flowMonitor = 
std::dynamic_pointer_cast(processor);
+  if (flowMonitor != nullptr) {
+for (auto  : connections) {
+  flowMonitor->addConnection(con.second);
+}
+flowMonitor->setStateMonitor(update_sink);
+flowMonitor->setFlowVersion(flow_configuration_->getFlowVersion());
+  }
+  std::lock_guard guard(metrics_mutex_);
+  root_response_nodes_[processor->getName()] = processor;
+}
+  }
+
+  // get all 

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-02 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533968416



##
File path: libminifi/include/utils/StringUtils.h
##
@@ -23,7 +23,7 @@
 #include 
 #include 
 #ifdef WIN32
-  #include 
+#include 

Review comment:
   indeed, it was unintentional, fixed

##
File path: libminifi/include/utils/StringUtils.h
##
@@ -143,6 +146,12 @@ class StringUtils {
 return std::equal(endString.rbegin(), endString.rend(), value.rbegin(), 
[](unsigned char lc, unsigned char rc) {return tolower(lc) == tolower(rc);});
   }
 
+  inline static bool startsWith(const std::string , const std::string & 
startString) {

Review comment:
   done, and for `endsWith` as well





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev opened a new pull request #948: MINIFICPP-1325 - Refactor and test YAML connection parsing

2020-12-01 Thread GitBox


hunyadi-dev opened a new pull request #948:
URL: https://github.com/apache/nifi-minifi-cpp/pull/948


   Update-after-rebase from #874.
   
   This is a refactor and test only PR - done as prework for name and UUID 
usage rework. It is probably simplest to **review this PR on a per commit 
basis**.
   
   This PR fixes faulty scenarios for parsing configuration values related to 
connections. 
   1. We agreed to fix this one:
   https://issues.apache.org/jira/browse/MINIFICPP-1331
   1. The others are documented here.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #874: MINIFICPP-1325 - Refactor and test YAML connection parsing

2020-12-01 Thread GitBox


hunyadi-dev commented on pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#issuecomment-736764240


   Closing this PR and opening a new one for the same content rebased.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] hunyadi-dev closed pull request #874: MINIFICPP-1325 - Refactor and test YAML connection parsing

2020-12-01 Thread GitBox


hunyadi-dev closed pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] markap14 commented on pull request #4698: NIFI-1121: Use display name for dependent property documentation

2020-12-01 Thread GitBox


markap14 commented on pull request #4698:
URL: https://github.com/apache/nifi/pull/4698#issuecomment-736702263


   Great catch @mattyb149 , thanks for fixing that. Am +1 on the change. Will 
wait until Github Actions completes but assuming that it passes I'll merge to 
main.



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] mattyb149 opened a new pull request #4698: NIFI-1121: Use display name for dependent property documentation

2020-12-01 Thread GitBox


mattyb149 opened a new pull request #4698:
URL: https://github.com/apache/nifi/pull/4698


   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
    Description of PR
   
   Before this PR, the machine-friendly name was being shown in the 
documentation for dependent properties, which can make it difficult to map to 
the other property's name/doc. This minor change shows the display name instead.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
in the commit message?
   
   - [x] Does your PR title start with **NIFI-** where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `main`)?
   
   - [x] Is your initial contribution a single, squashed commit? _Additional 
commits in response to PR reviewer feedback should be made on this branch and 
pushed to allow change tracking. Do not `squash` or use `--force` when pushing 
to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn 
-Pcontrib-check clean install` at the root `nifi` folder?
   - [x] Have you written or updated unit tests to verify your changes?
   - [x] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main 
`LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main 
`NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to 
.name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [x] 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 for 
build issues and submit an update to your PR as soon as possible.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #942: MINIFICPP-1410 Add permissions property support for Putfile processor

2020-12-01 Thread GitBox


lordgamez commented on a change in pull request #942:
URL: https://github.com/apache/nifi-minifi-cpp/pull/942#discussion_r533564990



##
File path: libminifi/include/utils/file/FileUtils.h
##
@@ -303,55 +322,74 @@ inline bool get_uid_gid(const std::string , uint64_t 
, uint64_t ) {
 #endif
 
 inline int is_directory(const char * path) {
-struct stat dir_stat;
-if (stat(path, _stat) < 0) {
-return 0;
-}
-return S_ISDIR(dir_stat.st_mode);
+  struct stat dir_stat;
+  if (stat(path, _stat) < 0) {
+  return 0;
+  }
+  return S_ISDIR(dir_stat.st_mode);
+}
+
+inline int exists(const std::string& path) {

Review comment:
   Fixed in 
[163bbea](https://github.com/apache/nifi-minifi-cpp/pull/942/commits/163bbeaa1f80756ff7d9b3f034e4e0bc71c3dcee)

##
File path: extensions/standard-processors/processors/PutFile.cpp
##
@@ -54,6 +54,19 @@ core::Property PutFile::CreateDirs("Create Missing 
Directories", "If true, then
 core::Property PutFile::MaxDestFiles(
 core::PropertyBuilder::createProperty("Maximum File 
Count")->withDescription("Specifies the maximum number of files that can exist 
in the output directory")->withDefaultValue(-1)->build());
 
+#ifndef WIN32
+core::Property PutFile::Permissions(
+core::PropertyBuilder::createProperty("Permissions")
+  ->withDescription("Sets the permissions on the output file to the value 
of this attribute. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")

Review comment:
   Fixed in 
[163bbea](https://github.com/apache/nifi-minifi-cpp/pull/942/commits/163bbeaa1f80756ff7d9b3f034e4e0bc71c3dcee)

##
File path: PROCESSORS.md
##
@@ -899,6 +899,8 @@ In the list below, the names of required properties appear 
in bold. Any other pr
 |**Create Missing Directories**|true||If true, then missing destination 
directories will be created. If false, flowfiles are penalized and sent to 
failure.|
 |Directory|.||The output directory to which to put files**Supports 
Expression Language: true**|
 |Maximum File Count|-1||Specifies the maximum number of files that can exist 
in the output directory|
+|Permissions|||Sets the permissions on the output file to the value of this 
attribute. Format must be in octal number (e.g. 644 or 0755). Not supported on 
Windows systems.|
+|Directory Permissions|||Sets the permissions on the directories being created 
if 'Create Missing Directories' property is set. Format must be format octal 
number (e.g. 644 or 0755). Not supported on Windows systems.|

Review comment:
   Fixed in 
[163bbea](https://github.com/apache/nifi-minifi-cpp/pull/942/commits/163bbeaa1f80756ff7d9b3f034e4e0bc71c3dcee)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #942: MINIFICPP-1410 Add permissions property support for Putfile processor

2020-12-01 Thread GitBox


lordgamez commented on a change in pull request #942:
URL: https://github.com/apache/nifi-minifi-cpp/pull/942#discussion_r533552705



##
File path: extensions/standard-processors/processors/PutFile.cpp
##
@@ -54,6 +54,19 @@ core::Property PutFile::CreateDirs("Create Missing 
Directories", "If true, then
 core::Property PutFile::MaxDestFiles(
 core::PropertyBuilder::createProperty("Maximum File 
Count")->withDescription("Specifies the maximum number of files that can exist 
in the output directory")->withDefaultValue(-1)->build());
 
+#ifndef WIN32
+core::Property PutFile::Permissions(
+core::PropertyBuilder::createProperty("Permissions")
+  ->withDescription("Sets the permissions on the output file to the value 
of this attribute. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")
+  ->build());
+core::Property PutFile::DirectoryPermissions(
+core::PropertyBuilder::createProperty("Directory Permissions")
+  ->withDescription("Sets the permissions on the directories being created 
if 'Create Missing Directories' property is set. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")
+  ->build());

Review comment:
   In that case it works the same way as 
`boost::filesystem::create_directory` which is creating the directory in mode 
`777` then applies the system's default umask.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox


lordgamez commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533408675



##
File path: encrypt-config/ArgParser.cpp
##
@@ -0,0 +1,178 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "ArgParser.h"
+#include "utils/OptionalUtils.h"
+#include "utils/StringUtils.h"
+#include "CommandException.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace encrypt_config {
+
+const std::vector Arguments::registered_args_{
+{std::set{"--minifi-home", "-m"},
+ true,
+ "minifi home",
+ "Specifies the home directory used by the minifi agent"}
+};
+
+const std::vector Arguments::registered_flags_{
+{std::set{"--help", "-h"},
+ "Prints this help message"},
+{std::set{"--encrypt-flow-config"},
+ "If set, the flow configuration file (as specified in minifi.properties) 
is also encrypted."}
+};
+
+bool haveCommonItem(const std::set& a, const 
std::set& b) {

Review comment:
   This could be moved to utils, also could be generalized as a template.

##
File path: libminifi/src/c2/C2Client.cpp
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include "c2/C2Client.h"
+#include "core/state/nodes/MetricsBase.h"
+#include "core/state/nodes/QueueMetrics.h"
+#include "core/state/nodes/AgentInformation.h"
+#include "core/state/nodes/RepositoryMetrics.h"
+#include "properties/Configure.h"
+#include "core/state/UpdateController.h"
+#include "core/controller/ControllerServiceProvider.h"
+#include "c2/C2Agent.h"
+#include "core/state/nodes/FlowInformation.h"
+#include "utils/file/FileSystem.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+C2Client::C2Client(
+std::shared_ptr configuration, 
std::shared_ptr provenance_repo,
+std::shared_ptr flow_file_repo, 
std::shared_ptr content_repo,
+std::unique_ptr flow_configuration, 
std::shared_ptr filesystem,
+std::shared_ptr logger)
+: core::Flow(std::move(provenance_repo), std::move(flow_file_repo), 
std::move(content_repo), std::move(flow_configuration)),
+  configuration_(std::move(configuration)),
+  filesystem_(std::move(filesystem)),
+  logger_(std::move(logger)) {}
+
+void C2Client::stopC2() {
+  if (c2_agent_) {
+c2_agent_->stop();
+  }
+}
+
+bool C2Client::isC2Enabled() const {
+  std::string c2_enable_str;
+  configuration_->get(Configure::nifi_c2_enable, "c2.enable", c2_enable_str);
+  return utils::StringUtils::toBool(c2_enable_str).value_or(false);
+}
+
+void C2Client::initialize(core::controller::ControllerServiceProvider 
*controller, const std::shared_ptr _sink) {
+  std::string class_str;
+  configuration_->get("nifi.c2.agent.class", "c2.agent.class", class_str);
+  configuration_->setAgentClass(class_str);
+
+  if (!isC2Enabled()) {
+return;
+  }
+
+  if (class_str.empty()) {
+logger_->log_error("Class name must be defined when C2 is enabled");
+throw std::runtime_error("Class name must be defined when C2 is enabled");
+  }
+
+  std::string identifier_str;
+  if (!configuration_->get("nifi.c2.agent.identifier", "c2.agent.identifier", 
identifier_str) || identifier_str.empty()) {
+// set to the flow controller's identifier
+identifier_str = getControllerUUID().to_string();
+  }
+  

[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #942: MINIFICPP-1410 Add permissions property support for Putfile processor

2020-12-01 Thread GitBox


fgerlits commented on a change in pull request #942:
URL: https://github.com/apache/nifi-minifi-cpp/pull/942#discussion_r533520945



##
File path: PROCESSORS.md
##
@@ -899,6 +899,8 @@ In the list below, the names of required properties appear 
in bold. Any other pr
 |**Create Missing Directories**|true||If true, then missing destination 
directories will be created. If false, flowfiles are penalized and sent to 
failure.|
 |Directory|.||The output directory to which to put files**Supports 
Expression Language: true**|
 |Maximum File Count|-1||Specifies the maximum number of files that can exist 
in the output directory|
+|Permissions|||Sets the permissions on the output file to the value of this 
attribute. Format must be in octal number (e.g. 644 or 0755). Not supported on 
Windows systems.|
+|Directory Permissions|||Sets the permissions on the directories being created 
if 'Create Missing Directories' property is set. Format must be format octal 
number (e.g. 644 or 0755). Not supported on Windows systems.|

Review comment:
   Typo: two "format"s.  I would get rid of all "format"s and write "Must 
be an octal number (e.g. 644 or 0755)." in both property descriptions.

##
File path: libminifi/include/utils/file/FileUtils.h
##
@@ -303,55 +322,74 @@ inline bool get_uid_gid(const std::string , uint64_t 
, uint64_t ) {
 #endif
 
 inline int is_directory(const char * path) {
-struct stat dir_stat;
-if (stat(path, _stat) < 0) {
-return 0;
-}
-return S_ISDIR(dir_stat.st_mode);
+  struct stat dir_stat;
+  if (stat(path, _stat) < 0) {
+  return 0;
+  }
+  return S_ISDIR(dir_stat.st_mode);
+}
+
+inline int exists(const std::string& path) {

Review comment:
   I can see that returning 0 (true) and -1 (false) fits into the existing 
pattern, but I think it would be better to return a `bool` at least from 
`is_directory()` and `exists()`.

##
File path: extensions/standard-processors/processors/PutFile.cpp
##
@@ -54,6 +54,19 @@ core::Property PutFile::CreateDirs("Create Missing 
Directories", "If true, then
 core::Property PutFile::MaxDestFiles(
 core::PropertyBuilder::createProperty("Maximum File 
Count")->withDescription("Specifies the maximum number of files that can exist 
in the output directory")->withDefaultValue(-1)->build());
 
+#ifndef WIN32
+core::Property PutFile::Permissions(
+core::PropertyBuilder::createProperty("Permissions")
+  ->withDescription("Sets the permissions on the output file to the value 
of this attribute. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")

Review comment:
   even more "format"s here :)
   same suggestion as above

##
File path: extensions/standard-processors/processors/PutFile.cpp
##
@@ -54,6 +54,19 @@ core::Property PutFile::CreateDirs("Create Missing 
Directories", "If true, then
 core::Property PutFile::MaxDestFiles(
 core::PropertyBuilder::createProperty("Maximum File 
Count")->withDescription("Specifies the maximum number of files that can exist 
in the output directory")->withDefaultValue(-1)->build());
 
+#ifndef WIN32
+core::Property PutFile::Permissions(
+core::PropertyBuilder::createProperty("Permissions")
+  ->withDescription("Sets the permissions on the output file to the value 
of this attribute. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")
+  ->build());
+core::Property PutFile::DirectoryPermissions(
+core::PropertyBuilder::createProperty("Directory Permissions")
+  ->withDescription("Sets the permissions on the directories being created 
if 'Create Missing Directories' property is set. "
+"Format must be format octal number (e.g. 644 or 
0755). Not supported on Windows systems.")
+  ->build());

Review comment:
   What happens if these are not set?  I would have expected a default 
value here.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] asfgit closed pull request #4638: NIFI-7906: parameterized graph query

2020-12-01 Thread GitBox


asfgit closed pull request #4638:
URL: https://github.com/apache/nifi/pull/4638


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] MikeThomsen commented on a change in pull request #4638: NIFI-7906: parameterized graph query

2020-12-01 Thread GitBox


MikeThomsen commented on a change in pull request #4638:
URL: https://github.com/apache/nifi/pull/4638#discussion_r533437833



##
File path: 
nifi-nar-bundles/nifi-graph-bundle/nifi-graph-processors/src/main/java/org/apache/nifi/processors/graph/ExecuteGraphQueryRecord.java
##
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.graph;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.nifi.annotation.behavior.DynamicProperty;
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.lifecycle.OnScheduled;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.graph.GraphClientService;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.util.StandardValidators;
+import org.apache.nifi.record.path.FieldValue;
+import org.apache.nifi.record.path.RecordPath;
+import org.apache.nifi.record.path.RecordPathResult;
+import org.apache.nifi.record.path.util.RecordPathCache;
+import org.apache.nifi.serialization.RecordReader;
+import org.apache.nifi.serialization.RecordReaderFactory;
+import org.apache.nifi.serialization.RecordSetWriterFactory;
+
+import org.apache.nifi.serialization.record.Record;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.stream.Collectors;
+
+@Tags({"graph, gremlin"})
+@CapabilityDescription("This uses a flowfile as input to perform graph 
mutations.")
+@WritesAttributes({
+@WritesAttribute(attribute = 
ExecuteGraphQueryRecord.GRAPH_OPERATION_TIME, description = "The amount of time 
it took to execute all of the graph operations."),
+@WritesAttribute(attribute = ExecuteGraphQueryRecord.RECORD_COUNT, 
description = "The amount of record processed")
+})
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@DynamicProperty(name = "A FlowFile property to be used as a parameter in the 
graph script",

Review comment:
   Wording's a little inaccurate, but I'll change this.

##
File path: 
nifi-nar-bundles/nifi-graph-bundle/nifi-graph-processors/src/main/java/org/apache/nifi/processors/graph/ExecuteGraphQueryRecord.java
##
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.graph;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.nifi.annotation.behavior.DynamicProperty;
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import 

[GitHub] [nifi] ottobackwards commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-01 Thread GitBox


ottobackwards commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r533514874



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
##
@@ -53,6 +54,66 @@ public TestRunner getRunner() {
 return runner;
 }
 

Review comment:
   I think the edge case is where there is a mix of expression language and 
other things like captures etc.
   Maybe a multi-case test would be appropriate?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] ottobackwards commented on a change in pull request #4685: NIFI-8042: Fixed bug that was escaping Expression Language references…

2020-12-01 Thread GitBox


ottobackwards commented on a change in pull request #4685:
URL: https://github.com/apache/nifi/pull/4685#discussion_r533513856



##
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
##
@@ -602,8 +602,7 @@ public boolean isAllDataBufferedForEntireText() {
 @Override
 public FlowFile replace(FlowFile flowFile, final ProcessSession 
session, final ProcessContext context, final String evaluateMode, final Charset 
charset, final int maxBufferSize) {

Review comment:
   I think one of the issues with maintaining this processor is that the 
reasons why things are quoted, escaped or not escaped are not documented in the 
code.
   
   Maybe a comment here would be helpful?
   





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi] asfgit closed pull request #4147: NIFI-7260: Fix error handling and re-evaluate Module Directory property on changed for scripted controller services

2020-12-01 Thread GitBox


asfgit closed pull request #4147:
URL: https://github.com/apache/nifi/pull/4147


   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request #947: MINIFICPP-1401 Read certificates from the Windows system store

2020-12-01 Thread GitBox


fgerlits opened a new pull request #947:
URL: https://github.com/apache/nifi-minifi-cpp/pull/947


   https://issues.apache.org/jira/browse/MINIFICPP-1401
   
   If the (new) `nifi.security.use.system.cert.store` property is set to true, 
then read client and server certificates from the Windows system store if no 
certificate file is given in the properties.  On Linux/Mac, only server 
certificates are supported, and even that is untested.
   
   By default, we use `LocalMachine/ROOT` ("Trusted Root Certification 
Authorities") as the server cert store and `LocalMachine/MY` ("Personal") as 
the client cert store, but these can be overridden by setting the
   `nifi.security.windows.cert.store.location` (default: `LocalMachine`)
   `nifi.security.windows.server.cert.store` (default: `ROOT`)
   `nifi.security.windows.client.cert.store` (default: `MY`)
   properties.
   
   If `nifi.security.windows.client.cert.cn` is given a non-empty value, then 
the client certificate will only be accepted if it has this CN (in the Subject).
   
   `nifi.security.windows.client.cert.key.usage` can contain a list of 
comma-separated Extended Key Usage strings; the default is "Client 
Authentication".  The client certificate will only be accepted if its Extended 
Key Usage contains all these key usages (it is allowed to contain more).  
   
   ---
   
   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:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
   
   - [x] Does your PR title start with MINIFICPP- where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically main)?
   
   - [x] 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](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] 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.
   



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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #940: MINIFICPP-1373 - Implement ConsumeKafka

2020-12-01 Thread GitBox


lordgamez commented on a change in pull request #940:
URL: https://github.com/apache/nifi-minifi-cpp/pull/940#discussion_r530919194



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -0,0 +1,522 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ConsumeKafka.h"
+
+#include 
+#include 
+
+#include "core/PropertyValidation.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/gsl.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace processors {
+
+constexpr const std::size_t ConsumeKafka::DEFAULT_MAX_POLL_RECORDS;
+constexpr char const* ConsumeKafka::DEFAULT_MAX_POLL_TIME;
+
+core::Property 
ConsumeKafka::KafkaBrokers(core::PropertyBuilder::createProperty("Kafka 
Brokers")
+  ->withDescription("A comma-separated list of known Kafka Brokers in the 
format :.")
+  ->withDefaultValue("localhost:9092", 
core::StandardValidators::get().NON_BLANK_VALIDATOR)
+  ->supportsExpressionLanguage(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::SecurityProtocol(core::PropertyBuilder::createProperty("Security 
Protocol")
+  ->withDescription("This property is currently not supported. Protocol used 
to communicate with brokers. Corresponds to Kafka's 'security.protocol' 
property.")
+  ->withAllowableValues({SECURITY_PROTOCOL_PLAINTEXT/*, 
SECURITY_PROTOCOL_SSL, SECURITY_PROTOCOL_SASL_PLAINTEXT, 
SECURITY_PROTOCOL_SASL_SSL*/ })
+  ->withDefaultValue(SECURITY_PROTOCOL_PLAINTEXT)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNames(core::PropertyBuilder::createProperty("Topic Names")
+  ->withDescription("The name of the Kafka Topic(s) to pull from. More than 
one can be supplied if comma separated.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::TopicNameFormat(core::PropertyBuilder::createProperty("Topic Name 
Format")
+  ->withDescription("Specifies whether the Topic(s) provided are a comma 
separated list of names or a single regular expression.")
+  ->withAllowableValues({TOPIC_FORMAT_NAMES, 
TOPIC_FORMAT_PATTERNS})
+  ->withDefaultValue(TOPIC_FORMAT_NAMES)
+  ->build());
+
+core::Property 
ConsumeKafka::HonorTransactions(core::PropertyBuilder::createProperty("Honor 
Transactions")
+  ->withDescription(
+  "Specifies whether or not NiFi should honor transactional guarantees 
when communicating with Kafka. If false, the Processor will use an \"isolation 
level\" of "
+  "read_uncomitted. This means that messages will be received as soon as 
they are written to Kafka but will be pulled, even if the producer cancels the 
transactions. "
+  "If this value is true, NiFi will not receive any messages for which the 
producer's transaction was canceled, but this can result in some latency since 
the consumer "
+  "must wait for the producer to finish its entire transaction instead of 
pulling as the messages become available.")
+  ->withDefaultValue(true)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::GroupID(core::PropertyBuilder::createProperty("Group ID")
+  ->withDescription("A Group ID is used to identify consumers that are within 
the same consumer group. Corresponds to Kafka's 'group.id' property.")
+  ->supportsExpressionLanguage(true)
+  ->build());
+
+core::Property 
ConsumeKafka::OffsetReset(core::PropertyBuilder::createProperty("Offset Reset")
+  ->withDescription("Allows you to manage the condition when there is no 
initial offset in Kafka or if the current offset does not exist any more on the 
server (e.g. because that "
+  "data has been deleted). Corresponds to Kafka's 'auto.offset.reset' 
property.")
+  ->withAllowableValues({OFFSET_RESET_EARLIEST, 
OFFSET_RESET_LATEST, OFFSET_RESET_NONE})
+  ->withDefaultValue(OFFSET_RESET_LATEST)
+  ->isRequired(true)
+  ->build());
+
+core::Property 
ConsumeKafka::KeyAttributeEncoding(core::PropertyBuilder::createProperty("Key 
Attribute Encoding")
+  ->withDescription("FlowFiles that are emitted have an attribute named 
'kafka.key'. This property dictates how the value of the attribute should be 
encoded.")
+  ->withAllowableValues({KEY_ATTR_ENCODING_UTF_8, 
KEY_ATTR_ENCODING_HEX})
+  

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533149990



##
File path: extensions/http-curl/tests/C2ConfigEncryption.cpp
##
@@ -0,0 +1,58 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "utils/EncryptionProvider.h"
+
+int main(int argc, char **argv) {
+  const cmd_args args = parse_cmdline_args(argc, argv, "update");
+  TestController controller;
+  // copy config file to temporary location as it will get overridden
+  char tmp_format[] = "/var/tmp/c2.XX";
+  std::string home_path = controller.createTempDirectory(tmp_format);
+  std::string live_config_file = 
utils::file::FileUtils::concat_path(home_path, "config.yml");
+  utils::file::FileUtils::copy_file(args.test_file, live_config_file);
+  // the C2 server will update the flow with the contents of args.test_file
+  // which will be encrypted and persisted to the temporary live_config_file
+  C2UpdateHandler handler(args.test_file);
+  VerifyC2Update harness(1);
+  
harness.getConfiguration()->set(minifi::Configure::nifi_flow_configuration_encrypt,
 "true");
+  harness.setKeyDir(args.key_dir);
+  harness.setUrl(args.url, );
+  handler.setC2RestResponse(harness.getC2RestUrl(), "configuration", "true");
+
+  const auto start = std::chrono::system_clock::now();

Review comment:
   removed





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533152594



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   (although the is-a relationship arising from the resulting 
inheritance-chain might be troublesome, we could go for protected inheritance 
or composition (generally I favor composition, but in this case I'd go with 
protected inheritance))





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533152594



##
File path: libminifi/include/core/Flow.h
##
@@ -0,0 +1,59 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include 
+#include 
+#include "core/ProcessGroup.h"
+#include "core/Repository.h"
+#include "core/ContentRepository.h"
+#include "core/FlowConfiguration.h"
+#include "utils/Id.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class Flow {

Review comment:
   (although the is-a relationship arising from the resulting 
inheritance-chain might be troublesome, we could go for protected inheritance 
or composition)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #937: MINIFICPP-1402 - Encrypt flow configuration and change encryption key

2020-12-01 Thread GitBox


adamdebreceni commented on a change in pull request #937:
URL: https://github.com/apache/nifi-minifi-cpp/pull/937#discussion_r533149990



##
File path: extensions/http-curl/tests/C2ConfigEncryption.cpp
##
@@ -0,0 +1,58 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "utils/EncryptionProvider.h"
+
+int main(int argc, char **argv) {
+  const cmd_args args = parse_cmdline_args(argc, argv, "update");
+  TestController controller;
+  // copy config file to temporary location as it will get overridden
+  char tmp_format[] = "/var/tmp/c2.XX";
+  std::string home_path = controller.createTempDirectory(tmp_format);
+  std::string live_config_file = 
utils::file::FileUtils::concat_path(home_path, "config.yml");
+  utils::file::FileUtils::copy_file(args.test_file, live_config_file);
+  // the C2 server will update the flow with the contents of args.test_file
+  // which will be encrypted and persisted to the temporary live_config_file
+  C2UpdateHandler handler(args.test_file);
+  VerifyC2Update harness(1);
+  
harness.getConfiguration()->set(minifi::Configure::nifi_flow_configuration_encrypt,
 "true");
+  harness.setKeyDir(args.key_dir);
+  harness.setUrl(args.url, );
+  handler.setC2RestResponse(harness.getC2RestUrl(), "configuration", "true");
+
+  const auto start = std::chrono::system_clock::now();

Review comment:
   remove

##
File path: libminifi/src/utils/EncryptionProvider.cpp
##
@@ -33,22 +40,19 @@ constexpr const char* CONFIG_ENCRYPTION_KEY_PROPERTY_NAME = 
"nifi.bootstrap.sens
 
 }  // namespace
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-
-utils::optional Decryptor::create(const std::string& 
minifi_home) {
+utils::optional EncryptionProvider::create(const 
std::string& home_path) {
   minifi::Properties bootstrap_conf;
-  bootstrap_conf.setHome(minifi_home);
+  bootstrap_conf.setHome(home_path);
   bootstrap_conf.loadConfigureFile(DEFAULT_NIFI_BOOTSTRAP_FILE);
   return bootstrap_conf.getString(CONFIG_ENCRYPTION_KEY_PROPERTY_NAME)
-  | utils::map([](const std::string& encryption_key_hex) { return 
utils::StringUtils::from_hex(encryption_key_hex); })
-  | utils::map(::crypto::stringToBytes)
-  | utils::map([](const utils::crypto::Bytes& encryption_key_bytes) { 
return minifi::Decryptor{encryption_key_bytes}; });
+ | utils::map([](const std::string _key_hex) { return 
utils::StringUtils::from_hex(encryption_key_hex); })
+ | utils::map(::crypto::stringToBytes)
+ | utils::map([](const utils::crypto::Bytes _key_bytes) { 
return EncryptionProvider{encryption_key_bytes}; });

Review comment:
   done





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




  1   2   3   4   5   6   7   8   9   10   >