nifi icon indicating copy to clipboard operation
nifi copied to clipboard

NIFI-10228: parse json flow using instanceIdentifier as the UUID

Open markobean opened this issue 3 years ago • 1 comments

Summary

Updates the FlowParser to use the "Instance Identifier" of the root process group versus "Identifier". When using flow.json.gz (versus flow.xml.gz), this nuance makes a difference in particular when establishing access policies on initial startup. Policies were being incorrectly defined when not using Instance Identifier.

For testing, NiFi was configured with the 'managed-authorizer' (nifi.properties) and a specific initial admin user (authorizers.xml). NiFi was started in order to generate an initial, empty flow.xml.gz/flow.json.gz. NiFi was shutdown and the authorizations.xml and users.xml files were removed. NiFi was started again.

Now, since both the flow and a root process group exist, component access policies for the group are created and the initial admin user is added to the policy. Prior to this fix, the component access policies would be populated incorrectly with the "Identifier" UUID. Now, they are populated correctly with the "Instance Identifier" UUID.

NIFI-10228

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • [X] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • [X] Pull Request based on current revision of the main branch
  • [X] Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • [X] Build completed using mvn clean install -P contrib-check
    • [x] JDK 8
    • [x] JDK 11
    • [x] JDK 17

Licensing

  • [ ] New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • [ ] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • [ ] Documentation formatting appears as expected in rendered files

markobean avatar Jul 18 '22 15:07 markobean

LGTM. Only comment is whether this is easily verified by a unit test. 🙂

mh013370 avatar Jul 19 '22 08:07 mh013370

@markap14 reports this same issue applies to the input/output ports as well. Will investigate/update.

markobean avatar Oct 27 '22 15:10 markobean

@markap14 There is no issue with Input/Output ports. The flow.json.gz is accurate which is where the Input/Output ports become relevant. Rather it is the FlowInfo which contains incorrect information. This object contains ports by reference, but not their ID values directly; the references and their ID values are correct. However, FlowInfo does contain the UUID for the root process group. This is the value which is in error and needs to be corrected; this PR corrects this error. The only change required is the root group UUID referenced when creating the FlowInfo object.

The PR was rebased to current main, but otherwise remains the same. No additional changes are required.

As noted in the original JIRA ticket description, you can observe the error by starting NiFi with an existing flow.json.gz (and flow.xml.gz) but without authorizations.xml or users.xml. Also, the managed-authorizer should be used, not single-user-authorizer, and an Initial Admin User should be specified. You will see that the root group UUID found in the Component Access Policies in the resulting authorizations.xml is incorrect. It references the root group identifier rather than instanceIdentifier. Repeat the above process with this PR and you will note the instanceIdentifier is correctly used in the policies (authorizations.xml.)

markobean avatar Oct 30 '22 15:10 markobean

Reviewing...

mattyb149 avatar Nov 04 '22 16:11 mattyb149

+1 LGTM, reproduced and verified expected behavior. Thanks for the fix! Merging to main

mattyb149 avatar Nov 04 '22 17:11 mattyb149

Appreciate the review @mattyb149 ! I know this one took a bit of setup to reproduce.

markobean avatar Nov 04 '22 17:11 markobean