NIFI-10567 Corrected Sensitive Dynamic Property handling for flow.xml (#6524)

- Updated XmlFlowSynchronizer to filter parsed Sensitive Dynamic Property Names using dynamic status of component Property Descriptor
- Lack of access to the Component Property Descriptor when parsing DOM elements required subsequent dynamic status filtering

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/XmlFlowSynchronizer.java

@@ -33,7 +33,6 @@ import org.apache.nifi.connectable.Port;
 import org.apache.nifi.connectable.Position;
 import org.apache.nifi.connectable.Size;
 import org.apache.nifi.controller.flow.FlowManager;
-import org.apache.nifi.controller.flowrepository.FlowRepositoryClientInstantiationException;
 import org.apache.nifi.controller.inheritance.AuthorizerCheck;
 import org.apache.nifi.controller.inheritance.BundleCompatibilityCheck;
 import org.apache.nifi.controller.inheritance.ConnectionMissingCheck;
@@ -42,7 +41,6 @@ import org.apache.nifi.controller.inheritance.FlowInheritability;
 import org.apache.nifi.controller.inheritance.FlowInheritabilityCheck;
 import org.apache.nifi.controller.inheritance.MissingComponentsCheck;
 import org.apache.nifi.controller.label.Label;
-import org.apache.nifi.controller.parameter.ParameterProviderInstantiationException;
 import org.apache.nifi.controller.queue.LoadBalanceCompression;
 import org.apache.nifi.controller.queue.LoadBalanceStrategy;
 import org.apache.nifi.controller.reporting.ReportingTaskInstantiationException;
@@ -146,6 +144,7 @@ import java.util.stream.Collectors;
 import java.util.zip.GZIPInputStream;
 
 /**
+ * XML implementation of Flow Synchronizer for reading configuration using XML Document Object Model
  */
 public class XmlFlowSynchronizer implements FlowSynchronizer {
 
@@ -383,7 +382,7 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
     }
 
     private void updateFlow(final FlowController controller, final Document configuration, final DataFlow existingFlow, final boolean existingFlowEmpty)
-            throws ReportingTaskInstantiationException, ParameterProviderInstantiationException, FlowRepositoryClientInstantiationException {
+            throws ReportingTaskInstantiationException {
         final boolean flowAlreadySynchronized = controller.isFlowSynchronized();
         final FlowManager flowManager = controller.getFlowManager();
 
@@ -769,8 +768,7 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
     }
 
     private ParameterProviderNode getOrCreateParameterProvider(final FlowController controller, final ParameterProviderDTO dto,
-                                                               final boolean controllerInitialized, final boolean existingFlowEmpty)
+                                                               final boolean controllerInitialized, final boolean existingFlowEmpty) {
-            throws ParameterProviderInstantiationException {
         // create a new parameter provider node when the controller is not initialized or the flow is empty
         if (!controllerInitialized || existingFlowEmpty) {
             BundleCoordinate coordinate;
@@ -818,8 +816,8 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
             registryClient.setName(dto.getName());
             registryClient.setDescription(dto.getDescription());
             registryClient.setAnnotationData(dto.getAnnotationData());
-            final Set<String> sensitiveDynamicPropertyNames = dto.getSensitiveDynamicPropertyNames();
+            final Set<String> sensitiveDynamicPropertyNames = getSensitiveDynamicPropertyNames(dto.getSensitiveDynamicPropertyNames(), registryClient);
-            registryClient.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames == null ? Collections.emptySet() : sensitiveDynamicPropertyNames);
+            registryClient.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames);
             return registryClient;
         } else {
             // otherwise return the existing flow registry client node
@@ -850,8 +848,8 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
             reportingTask.setSchedulingStrategy(SchedulingStrategy.valueOf(dto.getSchedulingStrategy()));
 
             reportingTask.setAnnotationData(dto.getAnnotationData());
-            final Set<String> sensitiveDynamicPropertyNames = dto.getSensitiveDynamicPropertyNames();
+            final Set<String> sensitiveDynamicPropertyNames = getSensitiveDynamicPropertyNames(dto.getSensitiveDynamicPropertyNames(), reportingTask);
-            reportingTask.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames == null ? Collections.emptySet() : sensitiveDynamicPropertyNames);
+            reportingTask.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames);
             return reportingTask;
         } else {
             // otherwise return the existing reporting task node
@@ -1396,8 +1394,8 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
                 procNode.setAutoTerminatedRelationships(relationships);
             }
 
-            final Set<String> sensitiveDynamicPropertyNames = config.getSensitiveDynamicPropertyNames();
+            final Set<String> sensitiveDynamicPropertyNames = getSensitiveDynamicPropertyNames(config.getSensitiveDynamicPropertyNames(), procNode);
-            procNode.setProperties(config.getProperties(), false, sensitiveDynamicPropertyNames == null ? Collections.emptySet() : sensitiveDynamicPropertyNames);
+            procNode.setProperties(config.getProperties(), false, sensitiveDynamicPropertyNames);
 
             final ScheduledState scheduledState = ScheduledState.valueOf(processorDTO.getState());
             if (ScheduledState.RUNNING.equals(scheduledState)) {
@@ -1413,6 +1411,16 @@ public class XmlFlowSynchronizer implements FlowSynchronizer {
         }
     }
 
+    private Set<String> getSensitiveDynamicPropertyNames(final Set<String> parsedSensitivePropertyNames, final ComponentNode componentNode) {
+        final Set<String> sensitivePropertyNames = parsedSensitivePropertyNames == null ? Collections.emptySet() : parsedSensitivePropertyNames;
+        return sensitivePropertyNames.stream().filter(
+                propertyName -> {
+                    final PropertyDescriptor propertyDescriptor = componentNode.getPropertyDescriptor(propertyName);
+                    return propertyDescriptor.isDynamic();
+                }
+        ).collect(Collectors.toSet());
+    }
+
     private void updateNonFingerprintedProcessorSettings(final ProcessorNode procNode, final ProcessorDTO processorDTO) {
         procNode.setName(processorDTO.getName());
         procNode.setPosition(toPosition(processorDTO.getPosition()));

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/ControllerServiceLoader.java

@@ -24,25 +24,14 @@ import org.apache.nifi.controller.serialization.FlowFromDOMFactory;
 import org.apache.nifi.encrypt.PropertyEncryptor;
 import org.apache.nifi.groups.ProcessGroup;
 import org.apache.nifi.logging.LogLevel;
-import org.apache.nifi.reporting.BulletinRepository;
 import org.apache.nifi.util.BundleUtils;
-import org.apache.nifi.util.DomUtils;
 import org.apache.nifi.web.api.dto.BundleDTO;
 import org.apache.nifi.web.api.dto.ControllerServiceDTO;
-import org.apache.nifi.xml.processing.ProcessingException;
-import org.apache.nifi.xml.processing.parsers.StandardDocumentProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.w3c.dom.Document;
 import org.w3c.dom.Element;
-import org.xml.sax.SAXException;
-import org.xml.sax.SAXParseException;
 
-import java.io.BufferedInputStream;
-import java.io.IOException;
-import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -51,59 +40,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.stream.Collectors;
 
 public class ControllerServiceLoader {
 
     private static final Logger logger = LoggerFactory.getLogger(ControllerServiceLoader.class);
 
-    public static List<ControllerServiceNode> loadControllerServices(final FlowController controller, final InputStream serializedStream, final ProcessGroup parentGroup,
-        final PropertyEncryptor encryptor, final BulletinRepository bulletinRepo, final boolean autoResumeState, final FlowEncodingVersion encodingVersion) throws IOException {
-
-        try (final InputStream in = new BufferedInputStream(serializedStream)) {
-            final StandardDocumentProvider documentProvider = new StandardDocumentProvider();
-
-            documentProvider.setErrorHandler(new org.xml.sax.ErrorHandler() {
-
-                @Override
-                public void fatalError(final SAXParseException err) throws SAXException {
-                    logger.error("Config file line " + err.getLineNumber() + ", col " + err.getColumnNumber() + ", uri " + err.getSystemId() + " :message: " + err.getMessage());
-                    if (logger.isDebugEnabled()) {
-                        logger.error("Error Stack Dump", err);
-                    }
-                    throw err;
-                }
-
-                @Override
-                public void error(final SAXParseException err) throws SAXParseException {
-                    logger.error("Config file line " + err.getLineNumber() + ", col " + err.getColumnNumber() + ", uri " + err.getSystemId() + " :message: " + err.getMessage());
-                    if (logger.isDebugEnabled()) {
-                        logger.error("Error Stack Dump", err);
-                    }
-                    throw err;
-                }
-
-                @Override
-                public void warning(final SAXParseException err) throws SAXParseException {
-                    logger.warn(" Config file line " + err.getLineNumber() + ", uri " + err.getSystemId() + " : message : " + err.getMessage());
-                    if (logger.isDebugEnabled()) {
-                        logger.warn("Warning stack dump", err);
-                    }
-                    throw err;
-                }
-            });
-
-            final Document document = documentProvider.parse(in);
-            final Element controllerServices = document.getDocumentElement();
-            final List<Element> serviceElements = DomUtils.getChildElementsByTagName(controllerServices, "controllerService");
-
-            final Map<ControllerServiceNode, Element> controllerServiceMap = ControllerServiceLoader.loadControllerServices(serviceElements, controller, parentGroup, encryptor, encodingVersion);
-            enableControllerServices(controllerServiceMap, controller, encryptor, autoResumeState, encodingVersion);
-            return new ArrayList<>(controllerServiceMap.keySet());
-        } catch (final ProcessingException e) {
-            throw new IOException("Parsing Controller Services failed", e);
-        }
-    }
-
     public static Map<ControllerServiceNode, Element> loadControllerServices(final List<Element> serviceElements, final FlowController controller,
                                                                              final ProcessGroup parentGroup, final PropertyEncryptor encryptor, final FlowEncodingVersion encodingVersion) {
 
@@ -227,11 +169,20 @@ public class ControllerServiceLoader {
         node.pauseValidationTrigger();
         try {
             node.setAnnotationData(dto.getAnnotationData());
-            final Set<String> sensitiveDynamicPropertyNames = dto.getSensitiveDynamicPropertyNames();
+            final Set<String> sensitiveDynamicPropertyNames = getSensitiveDynamicPropertyNames(dto.getSensitiveDynamicPropertyNames(), node);
-            node.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames == null ? Collections.emptySet() : sensitiveDynamicPropertyNames);
+            node.setProperties(dto.getProperties(), false, sensitiveDynamicPropertyNames);
         } finally {
             node.resumeValidationTrigger();
         }
     }
 
+    private static Set<String> getSensitiveDynamicPropertyNames(final Set<String> parsedSensitivePropertyNames, final ControllerServiceNode controllerServiceNode) {
+        final Set<String> sensitivePropertyNames = parsedSensitivePropertyNames == null ? Collections.emptySet() : parsedSensitivePropertyNames;
+        return sensitivePropertyNames.stream().filter(
+                propertyName -> {
+                    final PropertyDescriptor propertyDescriptor = controllerServiceNode.getPropertyDescriptor(propertyName);
+                    return propertyDescriptor.isDynamic();
+                }
+        ).collect(Collectors.toSet());
+    }
 }