Skip to content

NIFI-15954 Honor property overrides when verifying Kafka Topics step#11264

Open
rfellows wants to merge 1 commit into
apache:mainfrom
rfellows:NIFI-15954
Open

NIFI-15954 Honor property overrides when verifying Kafka Topics step#11264
rfellows wants to merge 1 commit into
apache:mainfrom
rfellows:NIFI-15954

Conversation

@rfellows
Copy link
Copy Markdown
Contributor

KafkaToS3.verifyTopicsExists now reads the specified Topic Names list from the overridden ConnectorConfigurationContext built from the verify request, rather than from the persisted FlowContext, so a verification request that narrows the selection is no longer evaluated against stale persisted state.

NIFI-15954 Honor property overrides when verifying Kafka Topics step

Summary

NIFI-15954

This is a bug fix for the KafkaToS3 connector's Verify Kafka topics exist step.

Motivation

KafkaToS3.verifyConfigurationStep(...) (nifi-connectors/nifi-kafka-to-s3-bundle/nifi-kafka-to-s3-connector/src/main/java/org/apache/nifi/connectors/kafkas3/KafkaToS3.java) already constructs an overridden ConnectorConfigurationContext from the verification request's property overrides via workingFlowContext.getConfigurationContext().createWithOverrides(stepName, propertyValueOverrides), and feeds that overridden context into the VersionedExternalFlow it builds for verification.

The verifyTopicsExists(...) helper, however, was reading the Topic Names list directly from workingFlowContext.getConfigurationContext() — i.e. from the persisted FlowContext rather than from the overridden context. As a result, a verify request that narrowed (or otherwise altered) the topic selection was evaluated against stale persisted state instead of the request's own values. The companion verifyKafkaParsability(...) was already correct because it operates on the flow argument, which is built from the overridden context.

Changes

nifi-connectors/nifi-kafka-to-s3-bundle/nifi-kafka-to-s3-connector/src/main/java/org/apache/nifi/connectors/kafkas3/KafkaToS3.java

  • verifyTopicsExists(...) now accepts the overridden ConnectorConfigurationContext as a second argument and reads KafkaTopicsStep.TOPIC_NAMES from it, so the list of specified topics matches what the verification request asked to verify.
  • The single call site in verifyConfigurationStep(...) was updated to pass the already-built overridden configurationContext through to verifyTopicsExists(...).
  • Visibility of verifyTopicsExists(...) and getAvailableTopics(...) was relaxed from private to package-private to allow unit-test subclasses to stub the broker lookup. No external visibility changed.
  • Inside verifyTopicsExists(...), the if/else order was flipped so the success branch is handled first and the failure branch second. This is purely stylistic; the produced ConfigVerificationResult values and outcomes are unchanged.

nifi-connectors/nifi-kafka-to-s3-bundle/nifi-kafka-to-s3-connector/src/test/java/org/apache/nifi/connectors/kafkas3/KafkaToS3Test.java (new)

  • New unit test class targeting verifyTopicsExists(...). Uses Mockito to stub FlowContext, ConnectorConfigurationContext, and ConnectorPropertyValue, and uses two small private subclasses of KafkaToS3 to stub getAvailableTopics(FlowContext) so the test can drive both the success path and a broker-lookup failure without standing up a real connection service.
  • The class JavaDoc notes that the heavier StandardConnectorTestRunner harness is intentionally not used here because it requires a packaged NAR and a running broker, which is the responsibility of the nifi-kafka-to-s3-integration-tests module.

Verification

Three unit tests were added in KafkaToS3Test:

  1. verifyTopicsExistsUsesOverriddenTopicList — the specified-topic list read by the step comes from the overridden ConnectorConfigurationContext, and a topic present in the broker passes (Outcome.SUCCESSFUL).
  2. verifyTopicsExistsReportsTopicMissingFromBroker — a topic supplied via the override that is absent from the broker is reported as Outcome.FAILED, and the explanation references the missing topic.
  3. verifyTopicsExistsReturnsSkippedWhenAvailableTopicsLookupFails — when the broker lookup throws, the step is reported as Outcome.SKIPPED and the explanation propagates the underlying failure message.

Risk & Scope

  • Scope is limited to the nifi-kafka-to-s3-connector module; no public API surfaces outside this module are touched.
  • The behavioral change is restricted to which ConnectorConfigurationContext verifyTopicsExists(...) reads Topic Names from. Outcome values, step name, and explanation strings on the success and failure paths are unchanged.
  • The visibility relaxations (private → package-private) are confined to two internal helpers used only within the connector package; no new constructors, fields, or types are exposed.

KafkaToS3.verifyTopicsExists now reads the specified Topic Names list
from the overridden ConnectorConfigurationContext built from the verify
request, rather than from the persisted FlowContext, so a verification
request that narrows the selection is no longer evaluated against stale
persisted state.
Copy link
Copy Markdown
Contributor

@markap14 markap14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rfellows definitely agree, we should be looking at the ConnectorConfigurationContext that has the overrides. +1 will merge on successful Github Actions completions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants