ncover21 commented on PR #11240:
URL: https://github.com/apache/nifi/pull/11240#issuecomment-5025002574
While testing the MIGRATE action I ran into a case where a freshly-created
Connector is reported as modified, which blocks migration.
`createMigrateAction` gates MIGRATE on `isModified()`, and
`StandardConnectorNode.configurationDiffersFromDefaults(...)` treats any
`ConnectorValueReference` that isn't a `StringLiteralValue` as a modification:
```java
if (valueReference instanceof final StringLiteralValue stringLiteralValue) {
if (!Objects.equals(stringLiteralValue.getValue(),
propertyDefaults.get(propertyName))) {
return true;
}
} else {
// any SecretReference / AssetReference returns true
return true;
}
```
The issue is the `else` branch: a `SecretReference` or `AssetReference` that
is structurally empty (null fully-qualified/secret name, or null/empty asset
identifiers) represents an *unset* property, but it still returns `true`. So
any Connector that declares a Secret- or Asset-typed property and has never
been configured is reported as modified, and `createMigrateAction` disallows
migration with "Connector has been modified since it was created."
This stays hidden when the configuration is assembled in-memory from
`StringLiteralValue`s with unset secret/asset properties simply omitted from
the map - the `else` branch is never reached. It surfaces once the working
configuration is populated from a serialized/round-tripped configuration in
which an unset secret/asset property is represented as a typed reference with
null contents rather than being omitted; deserialized, that becomes an empty
`SecretReference`/`AssetReference` and trips the `else`.
The framework already handles this exact shape in
`resolvePropertyReferences`, which skips structurally-empty secret references
via `isEmptySecretReference(...)` ("A typed-but-empty SECRET_REFERENCE acts
like an unset property"). `configurationDiffersFromDefaults` should apply the
same rule:
- treat a structurally-empty `SecretReference` (null FQN and null secret
name) or `AssetReference` (null/empty asset identifiers) as unset, i.e. equal
to the default and not a modification;
- only treat a *populated* Secret/Asset reference as a modification.
The existing `TestStandardConnectorNode` cases only exercise
`StringLiteralValue`s (plus one populated `SecretReference`), so the
empty-typed-reference path isn't covered. A regression test that seeds the
working configuration with an empty `SecretReference`/`AssetReference` and
asserts `isModified()` is `false` (and a populated one asserts `true`) would
lock it down.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
