You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/09/27 19:41:45 UTC
[GitHub] [nifi] gresockj commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation
gresockj commented on a change in pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#discussion_r716972076
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
##########
@@ -1915,4 +1916,12 @@ public void setVersionedComponentId(final String versionedComponentId) {
}
}
}
+
+ @Override
+ public void onConfigurationRestored(ProcessContext context) {
Review comment:
Could be `final`
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java
##########
@@ -1239,6 +1241,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(PropertyDescriptor descriptor, PropertyConfiguration config) {
Review comment:
`final`
##########
File path: 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 void runOnce(ScheduledExecutorService scheduler, long administra
* @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);
Review comment:
Curious, why does `ProcessorNode` get an `onConfigurationRestored` method, but not `ControllerServiceNode` or `ReportingTaskNode`?
##########
File path: 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);
Review comment:
Should this also be added to `StatelessReloadComponent`?
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/FlowSnippet.java
##########
@@ -42,6 +42,6 @@
* @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;
Review comment:
Javadoc param needed for `flowController`
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java
##########
@@ -1239,6 +1241,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(PropertyDescriptor descriptor, 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(PropertyConfiguration config, boolean okToResolve) {
Review comment:
`final`
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java
##########
@@ -773,10 +763,22 @@ public final ValidationStatus performValidation() {
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()) {
+ ParameterDescriptor parameterDescriptor = parameterRef.get().getDescriptor();
Review comment:
`final`
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org