NIFI-7012: Refactored OnConfigurationRestored to support sensitive pr…

…operty validation (apache#5415)

nifi-api/src/main/java/org/apache/nifi/annotation/lifecycle/OnConfigurationRestored.java

@@ -17,6 +17,9 @@
 
 package org.apache.nifi.annotation.lifecycle;
 
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.processor.ProcessContext;
+
 import java.lang.annotation.Documented;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Inherited;
@@ -32,7 +35,11 @@
  * </p>
  *
  * <p>
- * Methods with this annotation must take zero arguments.
+ * Methods with this annotation are permitted to take no arguments or to take a
+ * single argument. If using a single argument, that argument must be of type
+ * {@link ConfigurationContext} if the component is a ReportingTask or a
+ * ControllerService. If the component is a Processor, then the argument must be
+ * of type {@link ProcessContext}.
  * </p>
  *
  * <p>

nifi-mock/src/main/java/org/apache/nifi/util/StandardProcessorTestRunner.java

@@ -135,8 +135,6 @@ public class StandardProcessorTestRunner implements TestRunner {
         }
 
         triggerSerially = null != processor.getClass().getAnnotation(TriggerSerially.class);
-
-        ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, processor);
     }
 
     @Override
@@ -195,6 +193,10 @@ public void run(final int iterations, final boolean stopOnFinish, final boolean
 
         context.assertValid();
         context.enableExpressionValidation();
+
+        // Call onConfigurationRestored here, right before the test run, as all properties should have been set byt this point.
+        ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, processor, this.context);
+
         try {
             if (initialize) {
                 try {

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java

@@ -26,6 +26,7 @@
 import org.apache.nifi.annotation.configuration.DefaultSchedule;
 import org.apache.nifi.annotation.documentation.CapabilityDescription;
 import org.apache.nifi.annotation.documentation.DeprecationNotice;
+import org.apache.nifi.annotation.lifecycle.OnConfigurationRestored;
 import org.apache.nifi.annotation.lifecycle.OnScheduled;
 import org.apache.nifi.annotation.lifecycle.OnStopped;
 import org.apache.nifi.annotation.lifecycle.OnUnscheduled;
@@ -1912,4 +1913,12 @@ public void setVersionedComponentId(final String versionedComponentId) {
             }
         }
     }
+
+    @Override
+    public void onConfigurationRestored(final ProcessContext context) {
+        try (final NarCloseable nc = NarCloseable.withComponentNarLoader(getExtensionManager(), getProcessor().getClass(), getProcessor().getIdentifier())) {
+            ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, getProcessor(), context);
+        }
+    }
+
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java

@@ -4989,6 +4989,10 @@ private ProcessorNode addProcessor(final ProcessGroup destination, final Version
 
         destination.addProcessor(procNode);
         updateProcessor(procNode, proposed);
+        // Notify the processor node that the configuration (properties, e.g.) has been restored
+        final StandardProcessContext processContext = new StandardProcessContext(procNode, controllerServiceProvider, encryptor,
+                stateManagerProvider.getStateManager(procNode.getProcessor().getIdentifier()), () -> false, nodeTypeProvider);
+        procNode.onConfigurationRestored(processContext);
 
         return procNode;
     }

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

@@ -80,6 +80,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -330,17 +331,6 @@ public void verifyCanUpdateProperties(final Map<String, String> properties) {
                         }
                     }
                 }
-            } else {
-                final ParameterContext parameterContext = getParameterContext();
-                if (parameterContext != null) {
-                    for (final ParameterReference reference : referenceList) {
-                        final Optional<Parameter> parameter = parameterContext.getParameter(reference.getParameterName());
-                        if (parameter.isPresent() && parameter.get().getDescriptor().isSensitive()) {
-                            throw new IllegalArgumentException("The property '" + descriptor.getDisplayName() + "' cannot reference Parameter '" + parameter.get().getDescriptor().getName()
-                                + "' because Sensitive Parameters may only be referenced by Sensitive Properties.");
-                        }
-                    }
-                }
             }
 
             if (descriptor.getControllerServiceDefinition() != null) {
@@ -550,15 +540,15 @@ public Map<PropertyDescriptor, PropertyConfiguration> getProperties() {
 
     @Override
     public Map<PropertyDescriptor, String> getRawPropertyValues() {
-        return getPropertyValues(PropertyConfiguration::getRawValue);
+        return getPropertyValues((descriptor, config) -> config.getRawValue());
     }
 
     @Override
     public Map<PropertyDescriptor, String> getEffectivePropertyValues() {
-        return getPropertyValues(config -> config.getEffectiveValue(getParameterContext()));
+        return getPropertyValues((descriptor, config) -> getConfigValue(config, isResolveParameter(descriptor, config)));
     }
 
-    private Map<PropertyDescriptor, String> getPropertyValues(final Function<PropertyConfiguration, String> valueFunction) {
+    private Map<PropertyDescriptor, String> getPropertyValues(final BiFunction<PropertyDescriptor, PropertyConfiguration, String> valueFunction) {
         try (final NarCloseable narCloseable = NarCloseable.withComponentNarLoader(extensionManager, getComponent().getClass(), getIdentifier())) {
             final List<PropertyDescriptor> supported = getComponent().getPropertyDescriptors();
 
@@ -569,7 +559,7 @@ private Map<PropertyDescriptor, String> getPropertyValues(final Function<Propert
                 }
             }
 
-            properties.forEach((descriptor, config) -> props.put(descriptor, valueFunction.apply(config)));
+            properties.forEach((descriptor, config) -> props.put(descriptor, valueFunction.apply(descriptor, config)));
             return props;
         }
     }
@@ -777,10 +767,22 @@ private List<ValidationResult> validateParameterReferences(final ValidationConte
             for (final String paramName : referencedParameters) {
                 if (!validationContext.isParameterDefined(paramName)) {
                     results.add(new ValidationResult.Builder()
-                        .subject(propertyDescriptor.getDisplayName())
-                        .valid(false)
-                        .explanation("Property references Parameter '" + paramName + "' but the currently selected Parameter Context does not have a Parameter with that name")
-                        .build());
+                            .subject(propertyDescriptor.getDisplayName())
+                            .valid(false)
+                            .explanation("Property references Parameter '" + paramName + "' but the currently selected Parameter Context does not have a Parameter with that name")
+                            .build());
+                }
+                final Optional<Parameter> parameterRef = parameterContext.getParameter(paramName);
+                if (parameterRef.isPresent()) {
+                    final ParameterDescriptor parameterDescriptor = parameterRef.get().getDescriptor();
+                    if (parameterDescriptor.isSensitive() != propertyDescriptor.isSensitive()) {
+                        results.add(new ValidationResult.Builder()
+                                .subject(propertyDescriptor.getDisplayName())
+                                .valid(false)
+                                .explanation("The property '" + propertyDescriptor.getDisplayName() + "' cannot reference Parameter '" + parameterDescriptor.getName()
+                                        + "' because the Sensitivity of the parameter does not match the Sensitivity of the property.")
+                                .build());
+                    }
                 }
             }
         }
@@ -1243,6 +1245,40 @@ protected void setAdditionalResourcesFingerprint(String additionalResourcesFinge
         this.additionalResourcesFingerprint = additionalResourcesFingerprint;
     }
 
+    // Determine whether the property value should be evaluated in terms of the parameter context or not.
+    // If the sensitivity of the property does not match the sensitivity of the parameter, the literal value will be returned
+    //
+    // Examples when SensitiveParam value = 'abc' and MY_PROP is non-sensitive:
+    // SensitiveProp            --> 'abc'
+    // NonSensitiveProp         --> '#{SensitiveParam}'
+    // context.getProperty(MY_PROP).getValue(); '#{SensitiveParam}'
+    private boolean isResolveParameter(final PropertyDescriptor descriptor, final PropertyConfiguration config) {
+        boolean okToResolve = true;
+
+        final ParameterContext context = getParameterContext();
+        if (context == null) {
+            return false;
+        }
+        for (final ParameterReference reference : config.getParameterReferences()) {
+            final String parameterName = reference.getParameterName();
+            final Optional<Parameter> optionalParameter = context.getParameter(parameterName);
+            if (optionalParameter.isPresent()) {
+                final boolean paramIsSensitive = optionalParameter.get().getDescriptor().isSensitive();
+                if (paramIsSensitive != descriptor.isSensitive()) {
+                    okToResolve = false;
+                    break;
+                }
+            }
+        }
+        return okToResolve;
+    }
+
+    // Evaluates the parameter value if it is ok to do so, otherwise return the raw "${param}" literal.
+    // This is done to prevent evaluation of a sensitive parameter when setting a non-sensitive property.
+    private String getConfigValue(final PropertyConfiguration config, final boolean okToResolve) {
+        return okToResolve ? config.getEffectiveValue(getParameterContext()) : config.getRawValue();
+    }
+
     protected abstract ParameterContext getParameterContext();
 
 }

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

@@ -290,4 +290,11 @@ public abstract CompletableFuture<Void> stop(ProcessScheduler processScheduler,
      * @return the desired state for this Processor
      */
     public abstract ScheduledState getDesiredState();
+
+    /**
+     * This method will be called once the processor's configuration has been restored (on startup, reload, e.g.)
+     *
+     * @param context The ProcessContext associated with the Processor configuration
+     */
+    public abstract void onConfigurationRestored(ProcessContext context);
 }

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

@@ -113,6 +113,7 @@
 import org.apache.nifi.controller.serialization.ScheduledStateLookup;
 import org.apache.nifi.controller.service.ControllerServiceNode;
 import org.apache.nifi.controller.service.ControllerServiceProvider;
+import org.apache.nifi.controller.service.StandardConfigurationContext;
 import org.apache.nifi.controller.service.StandardControllerServiceProvider;
 import org.apache.nifi.controller.state.manager.StandardStateManagerProvider;
 import org.apache.nifi.controller.state.server.ZooKeeperStateServer;
@@ -152,6 +153,7 @@
 import org.apache.nifi.parameter.StandardParameterContextManager;
 import org.apache.nifi.processor.Processor;
 import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.StandardProcessContext;
 import org.apache.nifi.provenance.ComponentIdentifierLookup;
 import org.apache.nifi.provenance.IdentifierLookup;
 import org.apache.nifi.provenance.ProvenanceAuthorizableFactory;
@@ -985,23 +987,26 @@ private void notifyComponentsConfigurationRestored() {
         for (final ProcessorNode procNode : flowManager.getRootGroup().findAllProcessors()) {
             final Processor processor = procNode.getProcessor();
             try (final NarCloseable nc = NarCloseable.withComponentNarLoader(extensionManager, processor.getClass(), processor.getIdentifier())) {
-                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, processor);
+                final StandardProcessContext processContext = new StandardProcessContext(procNode, controllerServiceProvider, encryptor,
+                        getStateManagerProvider().getStateManager(processor.getIdentifier()), () -> false, this);
+                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, processor, processContext);
             }
         }
 
         for (final ControllerServiceNode serviceNode : flowManager.getAllControllerServices()) {
             final ControllerService service = serviceNode.getControllerServiceImplementation();
 
             try (final NarCloseable nc = NarCloseable.withComponentNarLoader(extensionManager, service.getClass(), service.getIdentifier())) {
-                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, service);
+                final ConfigurationContext configurationContext = new StandardConfigurationContext(serviceNode, controllerServiceProvider, null, variableRegistry);
+                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, service, configurationContext);
             }
         }
 
         for (final ReportingTaskNode taskNode : getAllReportingTasks()) {
             final ReportingTask task = taskNode.getReportingTask();
 
             try (final NarCloseable nc = NarCloseable.withComponentNarLoader(extensionManager, task.getClass(), task.getIdentifier())) {
-                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, task);
+                ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnConfigurationRestored.class, task, taskNode.getConfigurationContext());
             }
         }
     }

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

@@ -38,10 +38,11 @@ public interface FlowSnippet {
      * Instantiates this snippet, adding it to the given Process Group
      *
      * @param flowManager the FlowManager
+     * @param flowController the FlowController
      * @param group the group to add the snippet to
      * @throws ProcessorInstantiationException if unable to instantiate any of the Processors within the snippet
      * @throws org.apache.nifi.controller.exception.ControllerServiceInstantiationException if unable to instantiate any of the Controller Services within the snippet
      */
-    void instantiate(FlowManager flowManager, ProcessGroup group) throws ProcessorInstantiationException;
+    void instantiate(FlowManager flowManager, FlowController flowController, ProcessGroup group) throws ProcessorInstantiationException;
 }
 

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

@@ -42,6 +42,7 @@
 import org.apache.nifi.parameter.ParameterContext;
 import org.apache.nifi.processor.Processor;
 import org.apache.nifi.processor.Relationship;
+import org.apache.nifi.processor.StandardProcessContext;
 import org.apache.nifi.registry.flow.StandardVersionControlInformation;
 import org.apache.nifi.registry.flow.VersionControlInformation;
 import org.apache.nifi.remote.PublicPort;
@@ -151,8 +152,8 @@ public void verifyComponentTypesInSnippet() {
         }
     }
 
-    public void instantiate(final FlowManager flowManager, final ProcessGroup group) throws ProcessorInstantiationException {
-        instantiate(flowManager, group, true);
+    public void instantiate(final FlowManager flowManager, final FlowController flowController, final ProcessGroup group) throws ProcessorInstantiationException {
+        instantiate(flowManager, flowController, group, true);
     }
 
 
@@ -221,7 +222,7 @@ private void verifyProcessorsInSnippet(final FlowSnippetDTO templateContents, fi
     }
 
 
-    public void instantiate(final FlowManager flowManager, final ProcessGroup group, final boolean topLevel) {
+    public void instantiate(final FlowManager flowManager, final FlowController flowController, final ProcessGroup group, final boolean topLevel) {
         //
         // Instantiate Controller Services
         //
@@ -406,6 +407,11 @@ public void instantiate(final FlowManager flowManager, final ProcessGroup group,
                     procNode.setProperties(config.getProperties());
                 }
 
+                // Notify the processor node that the configuration (properties, e.g.) has been restored
+                final StandardProcessContext processContext = new StandardProcessContext(procNode, flowController.getControllerServiceProvider(), flowController.getEncryptor(),
+                        flowController.getStateManagerProvider().getStateManager(procNode.getProcessor().getIdentifier()), () -> false, flowController);
+                procNode.onConfigurationRestored(processContext);
+
                 group.addProcessor(procNode);
             } finally {
                 procNode.resumeValidationTrigger();
@@ -526,7 +532,7 @@ public void instantiate(final FlowManager flowManager, final ProcessGroup group,
             childTemplateDTO.setControllerServices(contents.getControllerServices());
 
             final StandardFlowSnippet childSnippet = new StandardFlowSnippet(childTemplateDTO, extensionManager);
-            childSnippet.instantiate(flowManager, childGroup, false);
+            childSnippet.instantiate(flowManager, flowController, childGroup, false);
 
             if (groupDTO.getVersionControlInformation() != null) {
                 final VersionControlInformation vci = StandardVersionControlInformation.Builder

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

@@ -97,6 +97,11 @@ public void reload(final ProcessorNode existingNode, final String newType, final
         // need to refresh the properties in case we are changing from ghost component to real component
         existingNode.refreshProperties();
 
+        // Notify the processor node that the configuration (properties, e.g.) has been restored
+        final StandardProcessContext processContext = new StandardProcessContext(existingNode, flowController.getControllerServiceProvider(), flowController.getEncryptor(),
+                flowController.getStateManagerProvider().getStateManager(existingNode.getProcessor().getIdentifier()), () -> false, flowController);
+        existingNode.onConfigurationRestored(processContext);
+
         logger.debug("Triggering async validation of {} due to processor reload", existingNode);
         flowController.getValidationTrigger().trigger(existingNode);
     }