diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/ServiceStateTransition.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/ServiceStateTransition.java index 21dccaa67e..90f6fa0925 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/ServiceStateTransition.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/ServiceStateTransition.java @@ -76,7 +76,7 @@ public class ServiceStateTransition { } state = ControllerServiceState.ENABLED; - logger.debug("{} transitioned to ENABLED", controllerServiceNode); + logger.debug("{} is now fully ENABLED", controllerServiceNode); enabledFutures.forEach(future -> future.complete(null)); } finally { @@ -124,7 +124,7 @@ public class ServiceStateTransition { writeLock.lock(); try { state = ControllerServiceState.DISABLED; - logger.debug("{} transitioned to DISABLED", controllerServiceNode); + logger.info("{} is now fully DISABLED", controllerServiceNode); stateChangeCondition.signalAll(); disabledFutures.forEach(future -> future.complete(null)); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java index 02189a1579..934d738a90 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java @@ -304,8 +304,16 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme @Override public void verifyModifiable() throws IllegalStateException { - if (getState() != ControllerServiceState.DISABLED) { - throw new IllegalStateException("Cannot modify Controller Service configuration because it is currently enabled. Please disable the Controller Service first."); + final ControllerServiceState state = getState(); + + if (state == ControllerServiceState.DISABLING) { + // Provide precise/accurate error message for DISABLING case + throw new IllegalStateException("Cannot modify Controller Service configuration because it is currently still disabling. " + + "Please wait for the service to fully disable before attempting to modify it."); + } + if (state != ControllerServiceState.DISABLED) { + throw new IllegalStateException("Cannot modify Controller Service configuration because it is currently not disabled - it has a state of " + state + + ". Please disable the Controller Service first."); } } @@ -654,6 +662,11 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme } final CompletableFuture future = new CompletableFuture<>(); + final boolean transitioned = this.stateTransition.transitionToDisabling(ControllerServiceState.ENABLING, future); + if (transitioned) { + return future; + } + if (this.stateTransition.transitionToDisabling(ControllerServiceState.ENABLED, future)) { final ConfigurationContext configContext = new StandardConfigurationContext(this, this.serviceProvider, null, getVariableRegistry()); scheduler.execute(new Runnable() { @@ -672,8 +685,6 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme } } }); - } else { - this.stateTransition.transitionToDisabling(ControllerServiceState.ENABLING, future); } return future; diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizer.java index 48c69824aa..61b91ca362 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/serialization/VersionedFlowSynchronizer.java @@ -179,6 +179,15 @@ public class VersionedFlowSynchronizer implements FlowSynchronizer { // Stop the active components, and then wait for all components to be stopped. logger.info("In order to inherit proposed dataflow, will stop any components that will be affected by the update"); + if (logger.isDebugEnabled()) { + logger.debug("Will stop the following components:"); + logger.debug(activeSet.toString()); + final String differencesToString = flowDifferences.stream() + .map(FlowDifference::toString) + .collect(Collectors.joining("\n")); + logger.debug("This Active Set was determined from the following Flow Differences:\n{}", differencesToString); + } + activeSet.stop(); try { diff --git a/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java b/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java index f8da42e777..08c7aa9072 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java @@ -23,6 +23,8 @@ import org.apache.nifi.flow.VersionedControllerService; import org.apache.nifi.flow.VersionedFlowCoordinates; import org.apache.nifi.flow.VersionedFunnel; import org.apache.nifi.flow.VersionedLabel; +import org.apache.nifi.flow.VersionedParameter; +import org.apache.nifi.flow.VersionedParameterContext; import org.apache.nifi.flow.VersionedPort; import org.apache.nifi.flow.VersionedProcessGroup; import org.apache.nifi.flow.VersionedProcessor; @@ -30,8 +32,8 @@ import org.apache.nifi.flow.VersionedPropertyDescriptor; import org.apache.nifi.flow.VersionedRemoteGroupPort; import org.apache.nifi.flow.VersionedRemoteProcessGroup; import org.apache.nifi.flow.VersionedReportingTask; -import org.apache.nifi.flow.VersionedParameter; -import org.apache.nifi.flow.VersionedParameterContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Collection; import java.util.Collections; @@ -44,6 +46,8 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; public class StandardFlowComparator implements FlowComparator { + private static final Logger logger = LoggerFactory.getLogger(StandardFlowComparator.class); + private static final String ENCRYPTED_VALUE_PREFIX = "enc{"; private static final String ENCRYPTED_VALUE_SUFFIX = "}"; @@ -86,7 +90,6 @@ public class StandardFlowComparator implements FlowComparator { } - private Set compare(final VersionedProcessGroup groupA, final VersionedProcessGroup groupB) { final Set differences = new HashSet<>(); // Note that we do not compare the names, because when we import a Flow into NiFi, we may well give it a new name. @@ -95,14 +98,6 @@ public class StandardFlowComparator implements FlowComparator { return differences; } - private boolean allHaveInstanceId(Set components) { - if (components == null) { - return false; - } - - return components.stream().allMatch(component -> component.getInstanceIdentifier() != null); - } - private Set compareComponents(final Set componentsA, final Set componentsB, final ComponentComparator comparator) { final Map componentMapA = byId(componentsA == null ? Collections.emptySet() : componentsA); final Map componentMapB = byId(componentsB == null ? Collections.emptySet() : componentsB); @@ -198,7 +193,7 @@ public class StandardFlowComparator implements FlowComparator { compareProperties(taskA, taskB, taskA.getProperties(), taskB.getProperties(), taskA.getPropertyDescriptors(), taskB.getPropertyDescriptors(), differences); } - private void compare(final VersionedParameterContext contextA, final VersionedParameterContext contextB, final Set differences) { + void compare(final VersionedParameterContext contextA, final VersionedParameterContext contextB, final Set differences) { if (compareComponents(contextA, contextB, differences)) { return; } @@ -218,7 +213,9 @@ public class StandardFlowComparator implements FlowComparator { continue; } - if (!Objects.equals(parameterA.getValue(), parameterB.getValue())) { + final String decryptedValueA = decryptValue(parameterA); + final String decryptedValueB = decryptValue(parameterB); + if (!Objects.equals(decryptedValueA, decryptedValueB)) { final String valueA = parameterA.isSensitive() ? "" : parameterA.getValue(); final String valueB = parameterB.isSensitive() ? "" : parameterB.getValue(); final String description = differenceDescriptor.describeDifference(DifferenceType.PARAMETER_VALUE_CHANGED, flowA.getName(), flowB.getName(), contextA, contextB, name, valueA, valueB); @@ -282,6 +279,21 @@ public class StandardFlowComparator implements FlowComparator { return propertyDecryptor.apply(value.substring(ENCRYPTED_VALUE_PREFIX.length(), value.length() - ENCRYPTED_VALUE_SUFFIX.length())); } + private String decryptValue(final VersionedParameter parameter) { + final String rawValue = parameter.getValue(); + if (rawValue == null) { + return null; + } + + final boolean sensitive = parameter.isSensitive() && rawValue.startsWith(ENCRYPTED_VALUE_PREFIX) && rawValue.endsWith(ENCRYPTED_VALUE_SUFFIX); + if (!sensitive) { + logger.debug("Will not decrypt value for parameter {} because it is not encrypted", parameter.getName()); + return rawValue; + } + + return propertyDecryptor.apply(rawValue.substring(ENCRYPTED_VALUE_PREFIX.length(), rawValue.length() - ENCRYPTED_VALUE_SUFFIX.length())); + } + private void compareProperties(final VersionedComponent componentA, final VersionedComponent componentB, final Map propertiesA, final Map propertiesB, final Map descriptorsA, final Map descriptorsB, diff --git a/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/test/java/org/apache/nifi/registry/flow/diff/TestStandardFlowComparator.java b/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/test/java/org/apache/nifi/registry/flow/diff/TestStandardFlowComparator.java new file mode 100644 index 0000000000..6baf46a44f --- /dev/null +++ b/nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/test/java/org/apache/nifi/registry/flow/diff/TestStandardFlowComparator.java @@ -0,0 +1,120 @@ +/* + * 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. + */ + +package org.apache.nifi.registry.flow.diff; + +import org.apache.nifi.flow.VersionedComponent; +import org.apache.nifi.flow.VersionedParameter; +import org.apache.nifi.flow.VersionedParameterContext; +import org.apache.nifi.flow.VersionedProcessGroup; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; + +public class TestStandardFlowComparator { + private Map decryptedToEncrypted; + private Map encryptedToDecrypted; + private StandardFlowComparator comparator; + + @BeforeEach + public void setup() { + decryptedToEncrypted = new HashMap<>(); + decryptedToEncrypted.put("XYZ", "Hello"); + decryptedToEncrypted.put("xyz", "hola"); + + encryptedToDecrypted = new HashMap<>(); + encryptedToDecrypted.put("Hello", "XYZ"); + encryptedToDecrypted.put("hola", "xyz"); + + final Function decryptor = encryptedToDecrypted::get; + final ComparableDataFlow flowA = new StandardComparableDataFlow("Flow A", new VersionedProcessGroup()); + final ComparableDataFlow flowB = new StandardComparableDataFlow("Flow B", new VersionedProcessGroup()); + comparator = new StandardFlowComparator(flowA, flowB, Collections.emptySet(), + new StaticDifferenceDescriptor(), decryptor, VersionedComponent::getInstanceIdentifier); + } + + // Ensure that when we are comparing parameter values that we compare the decrypted values, but we don't include any + // decrypted values in the descriptions of the Flow Difference. + @Test + public void testSensitiveParametersDecryptedBeforeCompare() { + final Set differences = new HashSet<>(); + + final Set parametersA = new HashSet<>(); + parametersA.add(createParameter("Param 1", "xyz", false)); + parametersA.add(createParameter("Param 2", "XYZ", false)); + parametersA.add(createParameter("Param 3", "Hi there", false)); + parametersA.add(createParameter("Param 4", "xyz", true)); + parametersA.add(createParameter("Param 5", "XYZ", true)); + + // Now that we've created the parameters, change the mapping of decrypted to encrypted so that we encrypt the values + // differently in each context but have the same decrypted value + decryptedToEncrypted.put("xyz", "bonjour"); + encryptedToDecrypted.put("bonjour", "xyz"); + + final Set parametersB = new HashSet<>(); + parametersB.add(createParameter("Param 1", "xyz", false)); + parametersB.add(createParameter("Param 2", "XYZ", false)); + parametersB.add(createParameter("Param 3", "Hey", false)); + parametersB.add(createParameter("Param 4", "xyz", true)); + parametersB.add(createParameter("Param 5", "xyz", true)); + + final VersionedParameterContext contextA = new VersionedParameterContext(); + contextA.setIdentifier("id"); + contextA.setInstanceIdentifier("instanceId"); + contextA.setParameters(parametersA); + + final VersionedParameterContext contextB = new VersionedParameterContext(); + contextB.setIdentifier("id"); + contextB.setInstanceIdentifier("instanceId"); + contextB.setParameters(parametersB); + + comparator.compare(contextA, contextB, differences); + + assertEquals(2, differences.size()); + for (final FlowDifference difference : differences) { + assertSame(DifferenceType.PARAMETER_VALUE_CHANGED, difference.getDifferenceType()); + + // Ensure that the sensitive values are not contained in the description + assertFalse(difference.getDescription().contains("Hello")); + assertFalse(difference.getDescription().contains("Hola")); + assertFalse(difference.getDescription().contains("bonjour")); + } + + final long numContainingValue = differences.stream() + .filter(diff -> diff.getDescription().contains("Hey") && diff.getDescription().contains("Hi there")) + .count(); + assertEquals(1, numContainingValue); + } + + private VersionedParameter createParameter(final String name, final String value, final boolean sensitive) { + final VersionedParameter parameter = new VersionedParameter(); + parameter.setName(name); + parameter.setValue(sensitive ? "enc{" + decryptedToEncrypted.get(value) + "}" : value); + parameter.setSensitive(sensitive); + return parameter; + } +}