exceptionfactory commented on code in PR #7661:
URL: https://github.com/apache/nifi/pull/7661#discussion_r1332246606
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/resources/conf/nifi.properties:
##########
@@ -69,6 +69,10 @@ nifi.web.https.host=
nifi.web.https.port=
nifi.web.jetty.working.directory=./target/work/jetty
+# security properties #
+nifi.sensitive.props.key=key
+nifi.sensitive.props.algorithm=PBEWITHMD5AND256BITAES-CBC-OPENSSL
Review Comment:
This algorithm is deprecated and no longer supported on the main branch, so
these two properties can be removed.
##########
nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy:
##########
@@ -248,8 +228,8 @@ class ConfigEncryptionTool {
options.addOption(Option.builder("i").longOpt(OUTPUT_LOGIN_IDENTITY_PROVIDERS_ARG).hasArg(true).argName("file").desc("The
destination login-identity-providers.xml file containing protected config
values (will not modify input login-identity-providers.xml)").build())
options.addOption(Option.builder("a").longOpt(AUTHORIZERS_ARG).hasArg(true).argName("file").desc("The
authorizers.xml file containing unprotected config values (will be overwritten
unless -u is specified)").build())
options.addOption(Option.builder("u").longOpt(OUTPUT_AUTHORIZERS_ARG).hasArg(true).argName("file").desc("The
destination authorizers.xml file containing protected config values (will not
modify input authorizers.xml)").build())
-
options.addOption(Option.builder("f").longOpt(FLOW_XML_ARG).hasArg(true).argName("file").desc("The
flow.xml.gz file currently protected with old password (will be overwritten
unless -g is specified)").build())
-
options.addOption(Option.builder("g").longOpt(OUTPUT_FLOW_XML_ARG).hasArg(true).argName("file").desc("The
destination flow.xml.gz file containing protected config values (will not
modify input flow.xml.gz)").build())
+//
options.addOption(Option.builder("f").longOpt(FLOW_XML_ARG).hasArg(true).argName("file").desc("The
flow.xml.gz file currently protected with old password (will be overwritten
unless -g is specified)").build())
+//
options.addOption(Option.builder("g").longOpt(OUTPUT_FLOW_XML_ARG).hasArg(true).argName("file").desc("The
destination flow.xml.gz file containing protected config values (will not
modify input flow.xml.gz)").build())
Review Comment:
These commented lines should be removed.
##########
nifi-commons/nifi-flow-encryptor/src/main/java/org/apache/nifi/flow/encryptor/JsonFlowEncryptor.java:
##########
@@ -33,9 +34,13 @@ public class JsonFlowEncryptor extends AbstractFlowEncryptor
{
@Override
public void processFlow(final InputStream inputStream, final OutputStream
outputStream,
final PropertyEncryptor inputEncryptor, final
PropertyEncryptor outputEncryptor) {
+
+ final BufferedInputStream bufferedInputStream = new
BufferedInputStream(inputStream);
+ bufferedInputStream.mark(1);
Review Comment:
This addition does not appear necessary. The only reason for it in the
standard class was to support content type detection.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/JoinClusterWithDifferentFlow.java:
##########
@@ -106,16 +97,16 @@ public void testStartupWithDifferentFlow() throws
IOException, NiFiClientExcepti
node2.stop();
final File node2ConfDir = new File(node2.getInstanceDirectory(),
"conf");
- final File flowXmlFile = new File(node2ConfDir, "flow.xml.gz");
+ final File flowXmlFile = new File(node2ConfDir, "flow.json.gz");
Files.deleteIfExists(flowXmlFile.toPath());
-
Files.copy(Paths.get("src/test/resources/flows/mismatched-flows/flow2.xml.gz"),
flowXmlFile.toPath());
+
Files.copy(Paths.get("src/test/resources/flows/mismatched-flows/flow2.json.gz"),
flowXmlFile.toPath());
node2.start(true);
final File backupFile = getBackupFile(node2ConfDir);
final NodeDTO node2Dto = getNodeDtoByApiPort(5672);
- verifyFlowContentsOnDisk(backupFile);
+// verifyFlowContentsOnDisk(backupFile);
Review Comment:
Is this verify method not working?
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/resources/FlowConfiguration.xsd:
##########
@@ -410,8 +410,6 @@
<xs:extension base="PortType">
<xs:sequence>
<xs:element name="maxConcurrentTasks"
type="xs:positiveInteger" minOccurs="0" ></xs:element>
Review Comment:
This entire file can be deleted.
##########
minifi/minifi-integration-tests/src/test/resources/conf/nifi.properties:
##########
@@ -14,7 +14,6 @@
# limitations under the License.
# Core Properties #
-nifi.flow.configuration.file=./target/flow.xml.gz
Review Comment:
Should this be flow.json.gz?
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/resources/properties/conf/duplicates.nifi.properties:
##########
@@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-nifi.flow.configuration.file=./conf/flow.xml.gz
nifi.flow.configuration.file=./conf/flow.json.gz
+nifi.flow.configuration.file=./conf/flow2.json.gz
Review Comment:
Is this change correct?
##########
nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy:
##########
@@ -715,20 +695,20 @@ class ConfigEncryptionTool {
* @param existingProvider the {@link java.security.Provider} to use
(defaults to BC)
* @return the encrypted XML content as an InputStream
*/
- private InputStream migrateFlowXmlContent(InputStream flowXmlContent,
String existingFlowPassword, String newFlowPassword, String existingAlgorithm =
DEFAULT_FLOW_ALGORITHM, String newAlgorithm = DEFAULT_FLOW_ALGORITHM) {
- File tempFlowXmlFile = new
File(getTemporaryFlowXmlFile(outputFlowXmlPath).toString())
- final OutputStream flowOutputStream =
getFlowOutputStream(tempFlowXmlFile, flowXmlContent instanceof GZIPInputStream)
-
- final PropertyEncryptor inputEncryptor = new
PropertyEncryptorBuilder(existingFlowPassword).setAlgorithm(existingAlgorithm).build()
- final PropertyEncryptor outputEncryptor = new
PropertyEncryptorBuilder(newFlowPassword).setAlgorithm(newAlgorithm).build()
-
- final FlowEncryptor flowEncryptor = new StandardFlowEncryptor()
- flowEncryptor.processFlow(flowXmlContent, flowOutputStream,
inputEncryptor, outputEncryptor)
-
- // Overwrite the original flow file with the migrated flow file
- Files.move(tempFlowXmlFile.toPath(), Paths.get(outputFlowXmlPath),
StandardCopyOption.ATOMIC_MOVE)
- loadFlowXml(outputFlowXmlPath)
- }
+// private InputStream migrateFlowXmlContent(InputStream flowXmlContent,
String existingFlowPassword, String newFlowPassword, String existingAlgorithm =
DEFAULT_FLOW_ALGORITHM, String newAlgorithm = DEFAULT_FLOW_ALGORITHM) {
Review Comment:
This needs to be uncommented and working before going forward.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/FlowSynchronizationIT.java:
##########
@@ -546,7 +546,7 @@ private void restartWithOnlySingleFlowPersistenceFile(final
String filenameToDel
node2.stop();
final File confDir = new File(node2.getInstanceDirectory(), "conf");
- assertEquals(1, confDir.listFiles(file ->
file.getName().equals("flow.xml.gz")).length);
+ //assertEquals(1, confDir.listFiles(file ->
file.getName().equals("flow.xml.gz")).length);
Review Comment:
This commented line should be removed.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/JoinClusterWithDifferentFlow.java:
##########
@@ -124,61 +115,62 @@ public void testStartupWithDifferentFlow() throws
IOException, NiFiClientExcepti
}
- private List<File> getFlowXmlFiles(final File confDir) {
- final File[] flowXmlFileArray = confDir.listFiles(file ->
file.getName().startsWith("flow") && file.getName().endsWith(".xml.gz"));
- final List<File> flowXmlFiles = new
ArrayList<>(Arrays.asList(flowXmlFileArray));
- return flowXmlFiles;
+ private List<File> getFlowJsonFiles(final File confDir) {
+ final File[] flowJsonFileArray = confDir.listFiles(file ->
file.getName().startsWith("flow") && file.getName().endsWith(".json.gz"));
+ final List<File> flowJsonFiles = new
ArrayList<>(Arrays.asList(flowJsonFileArray));
+ return flowJsonFiles;
}
private File getBackupFile(final File confDir) throws InterruptedException
{
- waitFor(() -> getFlowXmlFiles(confDir).size() == 2);
+ waitFor(() -> getFlowJsonFiles(confDir).size() == 2);
- final List<File> flowXmlFiles = getFlowXmlFiles(confDir);
+ final List<File> flowXmlFiles = getFlowJsonFiles(confDir);
assertEquals(2, flowXmlFiles.size());
- flowXmlFiles.removeIf(file -> file.getName().equals("flow.xml.gz"));
+ flowXmlFiles.removeIf(file -> file.getName().equals("flow.json.gz"));
assertEquals(1, flowXmlFiles.size());
final File backupFile = flowXmlFiles.get(0);
return backupFile;
}
- private void verifyFlowContentsOnDisk(final File backupFile) throws
IOException {
- // Read the flow and make sure that the backup looks the same as the
original. We don't just do a byte comparison because the compression may result
in different
- // gzipped bytes and because if the two flows do differ, we want to
have the String representation so that we can compare to see how they are
different.
- final String flowXml = readFlow(backupFile);
- final String expectedFlow = readFlow(new
File("src/test/resources/flows/mismatched-flows/flow1.xml.gz"));
-
- assertEquals(expectedFlow, flowXml);
-
- // Verify some of the values that were persisted to disk
- final File confDir = backupFile.getParentFile();
- final String loadedFlow = readFlow(new File(confDir, "flow.xml.gz"));
-
- final StandardDocumentProvider documentProvider = new
StandardDocumentProvider();
- final Document document = documentProvider.parse(new
ByteArrayInputStream(loadedFlow.getBytes(StandardCharsets.UTF_8)));
- final Element rootElement = (Element)
document.getElementsByTagName("flowController").item(0);
- final FlowEncodingVersion encodingVersion =
FlowEncodingVersion.parse(rootElement);
-
- final NiFiInstance node2 = getNiFiInstance().getNodeInstance(2);
- final PropertyEncryptor encryptor =
createEncryptorFromProperties(node2.getProperties());
- final Element rootGroupElement = (Element)
rootElement.getElementsByTagName("rootGroup").item(0);
-
- final ProcessGroupDTO groupDto =
FlowFromDOMFactory.getProcessGroup(null, rootGroupElement, encryptor,
encodingVersion);
- final Set<ProcessGroupDTO> childGroupDtos =
groupDto.getContents().getProcessGroups();
- assertEquals(1, childGroupDtos.size());
-
- final ProcessGroupDTO childGroup = childGroupDtos.iterator().next();
- assertFalse(childGroup.getId().endsWith("00"));
- final FlowSnippetDTO childContents = childGroup.getContents();
-
- final Set<ProcessorDTO> childProcessors =
childContents.getProcessors();
- assertEquals(1, childProcessors.size());
-
- final ProcessorDTO procDto = childProcessors.iterator().next();
- assertFalse(procDto.getId().endsWith("00"));
- assertFalse(procDto.getName().endsWith("00"));
- }
+ // TODO we need a JSON analogue for this
Review Comment:
We should remove this method and create a new Jira issue instead of
commenting out the method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
