Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1842,7 +1842,7 @@ public void synchronize(final ParameterContext parameterContext, final Versioned
contextManager.removeParameterContext(parameterContext.getIdentifier());
LOG.info("Successfully synchronized {} by removing it from the flow", parameterContext);
} else {
final Map<String, Parameter> updatedParameters = createParameterMap(proposed.getParameters());
final Map<String, Parameter> updatedParameters = createParameterMap(proposed.getParameters(), proposed.getParameterProvider() != null);

// If any parameters are removed, need to add a null value to the map in order to make sure that the parameter is removed.
for (final ParameterDescriptor existingParameterDescriptor : parameterContext.getParameters().keySet()) {
Expand Down Expand Up @@ -2324,7 +2324,8 @@ private ParameterContext createParameterContextWithoutReferences(final Versioned
continue;
}

final Parameter parameter = createParameter(null, versionedParameter);
// Created without a Parameter Provider configuration, so parameters keep their serialized provided flag.
final Parameter parameter = createParameter(null, versionedParameter, false);
parameters.put(versionedParameter.getName(), parameter);
}

Expand All @@ -2341,7 +2342,7 @@ private ParameterContext createParameterContext(final VersionedParameterContext
final Map<String, VersionedParameterContext> versionedParameterContexts,
final Map<String, ParameterProviderReference> parameterProviderReferences, final ComponentIdGenerator componentIdGenerator) {

final Map<String, Parameter> parameters = createParameterMap(versionedParameterContext.getParameters());
final Map<String, Parameter> parameters = createParameterMap(versionedParameterContext.getParameters(), versionedParameterContext.getParameterProvider() != null);

final List<String> parameterContextRefs = new ArrayList<>();
if (versionedParameterContext.getInheritedParameterContexts() != null) {
Expand All @@ -2361,10 +2362,10 @@ private ParameterContext createParameterContext(final VersionedParameterContext
return contextReference.get();
}

private Map<String, Parameter> createParameterMap(final Collection<VersionedParameter> versionedParameters) {
private Map<String, Parameter> createParameterMap(final Collection<VersionedParameter> versionedParameters, final boolean providerBacked) {
final Map<String, Parameter> parameters = new HashMap<>();
for (final VersionedParameter versionedParameter : versionedParameters) {
final Parameter parameter = createParameter(null, versionedParameter);
final Parameter parameter = createParameter(null, versionedParameter, providerBacked);
parameters.put(versionedParameter.getName(), parameter);
}

Expand Down Expand Up @@ -2426,7 +2427,7 @@ private void addMissingConfiguration(final VersionedParameterContext versionedPa
continue;
}

final Parameter parameter = createParameter(currentParameterContext.getIdentifier(), versionedParameter);
final Parameter parameter = createParameter(currentParameterContext.getIdentifier(), versionedParameter, versionedParameterContext.getParameterProvider() != null);
parameters.put(versionedParameter.getName(), parameter);
}

Expand Down Expand Up @@ -2476,7 +2477,7 @@ private void addMissingConfiguration(final VersionedParameterContext versionedPa
}
}

private Parameter createParameter(final String contextId, final VersionedParameter versionedParameter) {
private Parameter createParameter(final String contextId, final VersionedParameter versionedParameter, final boolean providerBacked) {
final List<VersionedAsset> referencedAssets = versionedParameter.getReferencedAssets();

final List<Asset> assets;
Expand All @@ -2498,7 +2499,7 @@ private Parameter createParameter(final String contextId, final VersionedParamet
.sensitive(versionedParameter.isSensitive())
.value(versionedParameter.getValue())
.referencedAssets(assets)
.provided(versionedParameter.isProvided())
.provided(providerBacked || versionedParameter.isProvided())
.parameterContextId(contextId)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package org.apache.nifi.parameter;

import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.controller.ParameterProviderNode;
import org.apache.nifi.controller.ProcessorNode;
import org.apache.nifi.controller.PropertyConfiguration;
import org.apache.nifi.controller.parameter.ParameterProviderLookup;
import org.apache.nifi.controller.service.ControllerServiceNode;
import org.apache.nifi.controller.service.ControllerServiceState;
import org.apache.nifi.groups.ProcessGroup;
Expand Down Expand Up @@ -649,6 +651,43 @@ public void testChangingParameterForEnabledControllerService() {
}
}

@Test
public void testSetParametersRejectedWhenProviderBackedAndParameterNotProvided() {
final String providerId = "provider-1";

final ParameterProvider parameterProvider = mock(ParameterProvider.class);
when(parameterProvider.getIdentifier()).thenReturn(providerId);

final ParameterProviderNode parameterProviderNode = mock(ParameterProviderNode.class);
when(parameterProviderNode.getParameterProvider()).thenReturn(parameterProvider);

final ParameterProviderLookup parameterProviderLookup = mock(ParameterProviderLookup.class);
when(parameterProviderLookup.getParameterProvider(providerId)).thenReturn(parameterProviderNode);

// A Parameter Context whose parameters are supplied by a Parameter Provider.
final ParameterContext context = new StandardParameterContext.Builder()
.id("provider-backed-context")
.name("Provider Backed Context")
.parameterReferenceManager(new HashMapParameterReferenceManager())
.parameterProviderLookup(parameterProviderLookup)
.parameterProviderConfiguration(new StandardParameterProviderConfiguration(providerId, "Group", true))
.build();

// Setting a user-entered (provided=false) parameter on a provider-backed context must be rejected.
// This is the guard that, during cluster flow synchronization, surfaces as a FlowSynchronizationException
// when the proposed (cluster) flow carries a non-provided parameter for a provider-backed context.
final ParameterDescriptor descriptor = new ParameterDescriptor.Builder().name("db.host").build();
final Parameter userEnteredParameter = new Parameter.Builder()
.descriptor(descriptor)
.value("localhost")
.provided(false)
.build();

final IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
() -> context.setParameters(Collections.singletonMap("db.host", userEnteredParameter)));
assertTrue(e.getMessage().contains("cannot be manually updated because they are provided by Parameter Provider"));
}

private ParameterContext createStandardParameterContext(final ParameterReferenceManager referenceManager) {
return new StandardParameterContext.Builder()
.id("unit-test-context")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,13 +944,20 @@ private Map<String, Parameter> createParameterMap(
) {
final Map<String, Parameter> providedParameters = getProvidedParameters(flowManager, versionedParameterContext);

// A Parameter Context bound to a Parameter Provider owns all of its Parameters through that Provider; the
// context-level binding is authoritative, not the per-Parameter provided flag in the serialized flow. Treat
// every Parameter of such a context as provided so reconciliation does not attempt a manual (user-entered)
// update, which the Parameter Context rejects.
final boolean providerBacked = versionedParameterContext.getParameterProvider() != null;

final Map<String, Parameter> parameters = new HashMap<>();
for (final VersionedParameter versioned : versionedParameterContext.getParameters()) {
final boolean provided = providerBacked || versioned.isProvided();
final String parameterValue;
final String rawValue = versioned.getValue();
if (rawValue == null) {
parameterValue = null;
} else if (versioned.isProvided()) {
} else if (provided) {
final String name = versioned.getName();
final Parameter providedParameter = providedParameters.get(name);
if (providedParameter == null) {
Expand All @@ -973,7 +980,7 @@ private Map<String, Parameter> createParameterMap(
.sensitive(versioned.isSensitive())
.value(referencedAssets.isEmpty() ? parameterValue : null)
.referencedAssets(referencedAssets)
.provided(versioned.isProvided())
.provided(provided)
.build();

parameters.put(versioned.getName(), parameter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
import org.apache.nifi.components.connector.ConnectorState;
import org.apache.nifi.components.connector.ConnectorSyncMode;
import org.apache.nifi.components.connector.ConnectorSyncResult;
import org.apache.nifi.components.validation.ValidationStatus;
import org.apache.nifi.controller.FlowController;
import org.apache.nifi.controller.ParameterProviderNode;
import org.apache.nifi.controller.ReportingTaskNode;
import org.apache.nifi.controller.SnippetManager;
import org.apache.nifi.controller.UninheritableFlowException;
import org.apache.nifi.controller.flow.FlowManager;
import org.apache.nifi.controller.flow.VersionedDataflow;
import org.apache.nifi.controller.parameter.ParameterProviderLookup;
import org.apache.nifi.controller.service.ControllerServiceNode;
import org.apache.nifi.controller.service.ControllerServiceProvider;
import org.apache.nifi.encrypt.PropertyEncryptor;
Expand All @@ -37,11 +40,21 @@
import org.apache.nifi.flow.VersionedConnector;
import org.apache.nifi.flow.VersionedConnectorState;
import org.apache.nifi.flow.VersionedControllerService;
import org.apache.nifi.flow.VersionedParameter;
import org.apache.nifi.flow.VersionedParameterContext;
import org.apache.nifi.flow.VersionedProcessGroup;
import org.apache.nifi.flow.VersionedReportingTask;
import org.apache.nifi.groups.BundleUpdateStrategy;
import org.apache.nifi.groups.ProcessGroup;
import org.apache.nifi.nar.ExtensionManager;
import org.apache.nifi.parameter.Parameter;
import org.apache.nifi.parameter.ParameterDescriptor;
import org.apache.nifi.parameter.ParameterGroup;
import org.apache.nifi.parameter.ParameterProvider;
import org.apache.nifi.parameter.ParameterReferenceManager;
import org.apache.nifi.parameter.StandardParameterContext;
import org.apache.nifi.parameter.StandardParameterContextManager;
import org.apache.nifi.parameter.StandardParameterProviderConfiguration;
import org.apache.nifi.persistence.FlowConfigurationArchiveManager;
import org.apache.nifi.registry.flow.mapping.VersionedComponentStateLookup;
import org.apache.nifi.services.FlowService;
Expand All @@ -63,13 +76,16 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -276,6 +292,81 @@ private VersionedControllerService getVersionedControllerService() {
return controllerService;
}

@Test
void testSyncReconcilesProviderBackedContextWithNonProvidedParameter() {
setRootGroup();
setFlowController();

final String providerId = "provider-1";
final String contextName = "openflow-rds-ingest";
final String paramName = "db.host";

// Parameter Provider infrastructure
final ParameterProvider parameterProvider = mock(ParameterProvider.class);
when(parameterProvider.getIdentifier()).thenReturn(providerId);
final ParameterProviderNode parameterProviderNode = mock(ParameterProviderNode.class);
Comment thread
pvillard31 marked this conversation as resolved.
when(parameterProviderNode.getParameterProvider()).thenReturn(parameterProvider);
final ParameterProviderLookup parameterProviderLookup = mock(ParameterProviderLookup.class);
when(parameterProviderLookup.getParameterProvider(providerId)).thenReturn(parameterProviderNode);

// The node's existing, self-consistent flow: a provider-backed context whose parameter is provider-supplied.
final StandardParameterContext existingContext = new StandardParameterContext.Builder()
.id("provider-backed-context")
.name(contextName)
.parameterReferenceManager(ParameterReferenceManager.EMPTY)
.parameterProviderLookup(parameterProviderLookup)
.parameterProviderConfiguration(new StandardParameterProviderConfiguration(providerId, "Group", true))
.build();
final ParameterDescriptor descriptor = new ParameterDescriptor.Builder().name(paramName).build();
existingContext.setParameters(Collections.singletonMap(paramName,
new Parameter.Builder().descriptor(descriptor).value("localhost").provided(true).build()));

// Stub the Parameter Provider as VALID and supplying the parameter, so reconciliation sources the value
// from the Provider (exercising createParameterMap's provider value-sourcing branch) rather than the
// "provided parameter not found" fallback that yields a null value.
when(parameterProviderNode.getIdentifier()).thenReturn(providerId);
when(parameterProviderNode.getValidationStatus()).thenReturn(ValidationStatus.VALID);
final ParameterGroup fetchedGroup = new ParameterGroup("Group", List.of(
new Parameter.Builder().descriptor(descriptor).value("provider-host").provided(true).build()));
when(parameterProviderNode.findFetchedParameterGroup("Group")).thenReturn(Optional.of(fetchedGroup));

final StandardParameterContextManager contextManager = new StandardParameterContextManager();
contextManager.addParameterContext(existingContext);
when(flowManager.getParameterContextManager()).thenReturn(contextManager);
when(flowManager.getParameterProvider(providerId)).thenReturn(parameterProviderNode);
doAnswer(invocation -> {
invocation.getArgument(0, Runnable.class).run();
return null;
}).when(flowManager).withParameterContextResolution(any());

// The proposed (elected cluster) flow: same provider-backed context, but the parameter is flagged
// provided=false with a divergent value. This is the serialized-flow contradiction seen in production.
final VersionedParameter versionedParameter = new VersionedParameter();
versionedParameter.setName(paramName);
versionedParameter.setValue("different-host");
versionedParameter.setSensitive(false);
versionedParameter.setProvided(false);

final VersionedParameterContext versionedParameterContext = new VersionedParameterContext();
versionedParameterContext.setName(contextName);
versionedParameterContext.setParameterProvider(providerId);
versionedParameterContext.setParameterGroupName("Group");
versionedParameterContext.setParameters(Collections.singleton(versionedParameter));
when(versionedDataflow.getParameterContexts()).thenReturn(List.of(versionedParameterContext));

// With the fix, the synchronizer recognizes the context is provider-backed and reconciles it as
// provider-managed (provided=true, value sourced from the provider) rather than attempting a manual
// update, so synchronization succeeds instead of throwing FlowSynchronizationException.
assertDoesNotThrow(() ->
versionedFlowSynchronizer.sync(flowController, dataFlow, flowService, BundleUpdateStrategy.USE_SPECIFIED_OR_GHOST));

final Optional<Parameter> reconciled = existingContext.getParameter(paramName);
assertTrue(reconciled.isPresent(), "Parameter should still exist after reconciliation");
assertTrue(reconciled.get().isProvided(), "Parameter must be reconciled as provider-supplied (provided=true)");
assertEquals("provider-host", reconciled.get().getValue(),
"Parameter value must be re-sourced from the Parameter Provider, not the corrupted serialized value or null");
}

private void setRootGroup() {
when(flowController.getFlowManager()).thenReturn(flowManager);
when(flowManager.getRootGroup()).thenReturn(rootGroup);
Expand Down
Loading
Loading