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/10/20 16:59:50 UTC

[GitHub] [nifi] mattyb149 opened a new pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

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


   <!--
     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.
   -->
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
   #### Description of PR
   
   Validation of contexts/properties was sometimes happening out of the intended lifecycle order. For InvokeScriptedProcessor 
   
   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-XXXX** where XXXX 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?
   - [ ] 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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



[GitHub] [nifi] markap14 merged pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
markap14 merged pull request #5415:
URL: https://github.com/apache/nifi/pull/5415


   


-- 
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



[GitHub] [nifi] gresockj commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [nifi] mattyb149 closed pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #5415:
URL: https://github.com/apache/nifi/pull/5415


   


-- 
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



[GitHub] [nifi] gresockj commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
gresockj commented on a change in pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#discussion_r735825183



##########
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:
       I think `parameterDescriptor` could still be `final` 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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



[GitHub] [nifi] gresockj commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [nifi] mattyb149 closed pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #5415:
URL: https://github.com/apache/nifi/pull/5415


   


-- 
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



[GitHub] [nifi] mattyb149 commented on pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#issuecomment-951154314


   @gresockj and @markap14 I believe I incorporated the requested changes, can you review this again when you have cycles? Please and 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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



[GitHub] [nifi] gresockj commented on pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
gresockj commented on pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#issuecomment-932915074


   I also verified the runtime behavior 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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



[GitHub] [nifi] mattyb149 commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#discussion_r717175855



##########
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:
       Good catch, I think so, will look into it

##########
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:
       They will eventually need one, but I think not right now because of [NIFI-5547](https://issues.apache.org/jira/browse/NIFI-5547)




-- 
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



[GitHub] [nifi] markap14 commented on pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
markap14 commented on pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#issuecomment-953208458


   Thanks @mattyb149 and @gresockj changes look good to me. +1 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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



[GitHub] [nifi] mattyb149 closed pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 closed pull request #5415:
URL: https://github.com/apache/nifi/pull/5415


   


-- 
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



[GitHub] [nifi] mattyb149 commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#discussion_r717176273



##########
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:
       They will eventually need one, but I think not right now because of [NIFI-5547](https://issues.apache.org/jira/browse/NIFI-5547)




-- 
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



[GitHub] [nifi] mattyb149 commented on a change in pull request #5415: NIFI-7012: Refactored OnConfigurationRestored to support sensitive property validation

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on a change in pull request #5415:
URL: https://github.com/apache/nifi/pull/5415#discussion_r717175855



##########
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:
       Good catch, I think so, will look into 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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

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