NIFI-13653 Fixed Sensitive Dynamic Property Update Handling (#9175)

- Avoided removing Sensitive Dynamic Property Names unless the property itself is removed This closes #9175
apache · Aug 13, 2024 · 265ef59 · 265ef59
1 parent e349b7e
commit 265ef59
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git a/...fi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java b/...fi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java
@@ -261,18 +261,17 @@ protected void overwriteProperties(final Map<String, String> properties) {
      *
      * @param properties Property Names and Values to be updated
      * @param allowRemovalOfRequiredProperties Allow Removal of Required Properties
-     * @param updatedSensitiveDynamicPropertyNames Requested Sensitive Dynamic Property Names replaces current configuration
+     * @param requestedSensitiveDynamicPropertyNames Requested Sensitive Dynamic Property Names adds to current configuration
      */
     @Override
-    public void setProperties(final Map<String, String> properties, final boolean allowRemovalOfRequiredProperties, final Set<String> updatedSensitiveDynamicPropertyNames) {
+    public void setProperties(final Map<String, String> properties, final boolean allowRemovalOfRequiredProperties, final Set<String> requestedSensitiveDynamicPropertyNames) {
         if (properties == null) {
             return;
         }
 
         lock.lock();
         try {
-            Objects.requireNonNull(updatedSensitiveDynamicPropertyNames, "Sensitive Dynamic Property Names required");
-            sensitiveDynamicPropertyNames.getAndSet(updatedSensitiveDynamicPropertyNames);
+            Objects.requireNonNull(requestedSensitiveDynamicPropertyNames, "Sensitive Dynamic Property Names required");
 
             verifyCanUpdateProperties(properties);
 
@@ -282,14 +281,16 @@ public void setProperties(final Map<String, String> properties, final boolean al
             final PropertyConfigurationMapper configurationMapper = new PropertyConfigurationMapper();
             final Map<String, PropertyConfiguration> configurationMap = configurationMapper.mapRawPropertyValuesToPropertyConfiguration(this, properties);
 
+            final Set<String> removedPropertyNames = new LinkedHashSet<>();
+
             try (final NarCloseable narCloseable = NarCloseable.withComponentNarLoader(extensionManager, getComponent().getClass(), id)) {
                 boolean classpathChanged = false;
                 for (final Map.Entry<String, String> entry : properties.entrySet()) {
                     final String propertyName = entry.getKey();
 
                     // Set sensitive status on dynamic properties after getting canonical representation of Property Descriptor
                     final PropertyDescriptor componentDescriptor = getComponent().getPropertyDescriptor(propertyName);
-                    final PropertyDescriptor descriptor = componentDescriptor.isDynamic() && updatedSensitiveDynamicPropertyNames.contains(propertyName)
+                    final PropertyDescriptor descriptor = componentDescriptor.isDynamic() && requestedSensitiveDynamicPropertyNames.contains(propertyName)
                             ? new PropertyDescriptor.Builder().fromPropertyDescriptor(componentDescriptor).sensitive(true).build()
                             : componentDescriptor;
 
@@ -306,7 +307,10 @@ public void setProperties(final Map<String, String> properties, final boolean al
                     }
 
                     if (propertyName != null && entry.getValue() == null) {
-                        removeProperty(propertyName, allowRemovalOfRequiredProperties);
+                        final boolean removed = removeProperty(propertyName, allowRemovalOfRequiredProperties);
+                        if (removed) {
+                            removedPropertyNames.add(propertyName);
+                        }
                     } else if (propertyName != null) {
                         // Use the EL-Agnostic Parameter Parser to gather the list of referenced Parameters. We do this because we want to keep track of which parameters
                         // are referenced, regardless of whether or not they are referenced from within an EL Expression. However, we also will need to derive a different ParameterTokenList
@@ -323,6 +327,13 @@ public void setProperties(final Map<String, String> properties, final boolean al
                     }
                 }
 
+                // Update Sensitive Dynamic Property Names
+                final Set<String> updatedSensitiveDynamicPropertyNames = new LinkedHashSet<>(sensitiveDynamicPropertyNames.get());
+                updatedSensitiveDynamicPropertyNames.addAll(requestedSensitiveDynamicPropertyNames);
+                // Remove Sensitive Dynamic Property Names for removed properties
+                updatedSensitiveDynamicPropertyNames.removeAll(removedPropertyNames);
+                sensitiveDynamicPropertyNames.getAndSet(updatedSensitiveDynamicPropertyNames);
+
                 // Determine the updated Classloader Isolation Key, if applicable.
                 final String updatedIsolationKey = determineClasloaderIsolationKey();
                 final boolean classloaderIsolationKeyChanged = !Objects.equals(initialIsolationKey, updatedIsolationKey);

diff --git a/...ramework-core-api/src/test/java/org/apache/nifi/controller/TestAbstractComponentNode.java b/...ramework-core-api/src/test/java/org/apache/nifi/controller/TestAbstractComponentNode.java
@@ -343,11 +343,12 @@ public void testSetPropertiesSensitiveDynamicPropertyNamesAddedRemoved() {
         assertTrue(sensitivePropertyDescriptor.isDynamic());
         assertTrue(sensitivePropertyDescriptor.isSensitive());
 
+        // Set Properties without updating Sensitive Dynamic Property Names
         node.setProperties(properties, false, Collections.emptySet());
 
         final PropertyDescriptor updatedPropertyDescriptor = node.getPropertyDescriptor(PROPERTY_NAME);
         assertTrue(updatedPropertyDescriptor.isDynamic());
-        assertFalse(updatedPropertyDescriptor.isSensitive());
+        assertTrue(updatedPropertyDescriptor.isSensitive());
 
         final Map<PropertyDescriptor, PropertyConfiguration> configuredProperties = node.getProperties();
         final PropertyDescriptor configuredPropertyDescriptor = configuredProperties.keySet()
@@ -357,6 +358,14 @@ public void testSetPropertiesSensitiveDynamicPropertyNamesAddedRemoved() {
                 .orElseThrow(() -> new IllegalStateException("Property Name not found"));
         assertTrue(configuredPropertyDescriptor.isDynamic());
         assertFalse(configuredPropertyDescriptor.isSensitive());
+
+        // Set Properties with value removed to update Sensitive Dynamic Property Names
+        final Map<String, String> removedProperties = Collections.singletonMap(PROPERTY_NAME, null);
+        node.setProperties(removedProperties, false, Collections.emptySet());
+
+        final PropertyDescriptor removedPropertyDescriptor = node.getPropertyDescriptor(PROPERTY_NAME);
+        assertTrue(removedPropertyDescriptor.isDynamic());
+        assertFalse(removedPropertyDescriptor.isSensitive());
     }
 
     private ValidationContext getServiceValidationContext(final ControllerServiceState serviceState, final ControllerServiceProvider serviceProvider) {