NIFI-15541 - Flow version upgrade binds new process groups to incorrect parameter context #10844
Conversation
…ct parameter context
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for addressing this issue @pvillard31. The basic approach looks good, though it does depend on the Parameter Context naming convention, it seems like a reasonable best effort match. I noted a few recommendations.
| * @return true if contextName is versionedName with a suffix like " (n)" | ||
| */ | ||
| private boolean isParameterContextNameWithSuffix(final String contextName, final String versionedName) { | ||
| if (!contextName.startsWith(versionedName + " (")) { |
There was a problem hiding this comment.
It looks like the logic in this method could be replaced with a regular expression pattern.
| if (parentParameterContext != null) { | ||
| selectedParameterContext = parentParameterContext; | ||
| // Ensure the parent's context has all the parameters from the versioned context | ||
| addMissingConfiguration(versionedParameterContext, selectedParameterContext, versionedParameterContexts, parameterProviderReferences, componentIdGenerator); | ||
| } else { |
There was a problem hiding this comment.
I recommend adjusting the logic so that the first block is if (parentParameterContext == null) { ... } else {
| // Save as v1 | ||
| final VersionControlInformationEntity vciV1 = util.startVersionControl(groupA, clientEntity, TEST_FLOWS_BUCKET, "FlowWithParameterContext"); | ||
| final String flowId = vciV1.getVersionControlInformation().getFlowId(); | ||
| logger.info("Saved v1: Process Group A with Processor X"); |
There was a problem hiding this comment.
I recommend removing the log statements from the test method, the integration test logs for nifi-app.log generally capture changes, and otherwise these additional logs are not a common approach in the system tests.
| final ParameterContextEntity paramContextP = util.createParameterContext(paramContextName, params); | ||
|
|
||
| // Step 2: Create v1 - Process Group A with just a Processor X using param1 | ||
| final ProcessGroupEntity groupA = util.createProcessGroup("A", "root"); |
There was a problem hiding this comment.
There are a number of string names and relationship values that could be changed to static final member variables and reused for consistency in the test method.
| final Map<String, String> params = new HashMap<>(); | ||
| params.put("param1", "value1"); |
There was a problem hiding this comment.
This can be simplified with Map.of()
| public class ParameterContextPreservationIT extends NiFiSystemIT { | ||
| private static final Logger logger = LoggerFactory.getLogger(ParameterContextPreservationIT.class); | ||
| public static final String TEST_FLOWS_BUCKET = "test-flows"; | ||
|
|
||
| @Test | ||
| public void testNewProcessGroupUsesCorrectParameterContextDuringUpgrade() throws NiFiClientException, IOException, InterruptedException { |
There was a problem hiding this comment.
| public class ParameterContextPreservationIT extends NiFiSystemIT { | |
| private static final Logger logger = LoggerFactory.getLogger(ParameterContextPreservationIT.class); | |
| public static final String TEST_FLOWS_BUCKET = "test-flows"; | |
| @Test | |
| public void testNewProcessGroupUsesCorrectParameterContextDuringUpgrade() throws NiFiClientException, IOException, InterruptedException { | |
| class ParameterContextPreservationIT extends NiFiSystemIT { | |
| private static final Logger logger = LoggerFactory.getLogger(ParameterContextPreservationIT.class); | |
| public static final String TEST_FLOWS_BUCKET = "test-flows"; | |
| @Test | |
| void testNewProcessGroupUsesCorrectParameterContextDuringUpgrade() throws NiFiClientException, IOException, InterruptedException { |
|
Thanks for the review @exceptionfactory - I pushed a commit to address your comments and also consolidate the code where this logic of parameter context naming (when using keepExisting=false) is defined. |
2 participants
Summary
NIFI-15541 - Flow version upgrade binds new process groups to incorrect parameter context
When upgrading a versioned flow that introduces a new child process group, the new process group is incorrectly bound to a parameter context matched by name rather than using the same parameter context as its parent.
Steps to reproduce:
Current behavior:
The newly added Process Group B in A2 gets bound to P instead of P (1)
Expected behavior:
Process Group B should use P (1), the same parameter context as its parent A2
Root cause:
In StandardVersionedComponentSynchronizer.updateParameterContext(), when a new child process group is added during synchronization, it looks up parameter contexts by name globally rather than inheriting from the parent group.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation