NIFI-14381: Add flow analysis rule to identify unset SSL Context Service controller services

[mattyb149]

Summary

NIFI-14381 This PR introduces a Flow Analysis Rule that will examine specified (or all if the Component Type is left blank) components and if they have an SSL Context Service property that is unset will issue a violation.

Tracking

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

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

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

Pull Request Formatting

• Pull Request based on current revision of the main branch
• 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

• Build completed using mvn clean install -P contrib-check
  • JDK 21

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

[exceptionfactory]

Thanks for proposing this new rule @mattyb149.

I noted a few questions and concerns related to the rule name, as RequireSecureConnection implies that SSL Context Service is required, which is not always the case.

The fact that some components do not require an SSL Context Service raises the question as to the validity of this rule for general purposes. As mentioned in the detailed comments, InvokeHTTP defaults to the system trust store, as do some other components. The fact that the nature of the default behavior can be different raises a question about the usefulness of this rule. For example, it would not work in a flow that requires a mixture of standard trusted certificates and custom trusted certificates for internal services. That could be addressed by always requiring a custom SSL Context Service configuration.

At minimum, changing the name and adjusting the description is important to convey the actual behavior. Thoughts?

[pvillard31]

The JSON flow definitions in the tests contain vendor specific versions, can we have the same flows where vendor specific versions are replaced by Apache NiFi versions?

Also, just a suggestion: do we want to expose an optional property where an admin can provide additional processors to check? I'm thinking about custom extensions that are quite frequent. I remember numerous NiFi users with custom variations of ListenHTTP / HandleHTTPRequest processors.

[mattyb149]

@pvillard31 Yes I missed that, will replace with Apache NiFi versions. I'm in favor of the property to add components by the name of the type (that was in the original PR), what do you think about having the default value be the entire list and return to using the comma-separated list as a configurable property

[exceptionfactory]

Regarding custom class names, I think the problem is that the Property Name has to be SSL Context Service, and that isn't guaranteed with other third-party components. From that angle, I recommend using the full class name, not just the simple name, to make it clear that this applies only to Apache NiFi components.

[pvillard31]

Yep, ok, my bad, I thought we had the information and we could do something like:

if (component instanceof VersionedConfigurableComponent versionedConfigurableComponent) {
    final Map<String, VersionedPropertyDescriptor> props = versionedConfigurableComponent.getPropertyDescriptors();
    for (Entry<String, VersionedPropertyDescriptor> entry : props.entrySet()) {
        if (entry.getValue().getIdentifiesControllerService()) {
            // entry.getValue().getControllerServiceDefinition();
        }
    }
}

and have the information about the controller service interface that this property expects but this it not something available. That could be an interesting improvement I guess.

[mattyb149]

Regarding custom class names, I think the problem is that the Property Name has to be SSL Context Service, and that isn't guaranteed with other third-party components. From that angle, I recommend using the full class name, not just the simple name, to make it clear that this applies only to Apache NiFi components.

I can't get the class name for it (see Pierre's comments above), we'd need an API change to add a method to retrieve the identified controller service unless there's some other way to get it (but I couldn't find one).

[exceptionfactory]

Regarding custom class names, I think the problem is that the Property Name has to be SSL Context Service, and that isn't guaranteed with other third-party components. From that angle, I recommend using the full class name, not just the simple name, to make it clear that this applies only to Apache NiFi components.

I can't get the class name for it (see Pierre's comments above), we'd need an API change to add a method to retrieve the identified controller service unless there's some other way to get it (but I couldn't find one).

I was referring to the Processor class, such as org.apache.nifi.processors.standard.ListenHTTP, which is available from the getType() method on VersionedConfigurableExtension.

[mattyb149]

Regarding custom class names, I think the problem is that the Property Name has to be SSL Context Service, and that isn't guaranteed with other third-party components. From that angle, I recommend using the full class name, not just the simple name, to make it clear that this applies only to Apache NiFi components.

I can't get the class name for it (see Pierre's comments above), we'd need an API change to add a method to retrieve the identified controller service unless there's some other way to get it (but I couldn't find one).

I was referring to the Processor class, such as org.apache.nifi.processors.standard.ListenHTTP, which is available from the getType() method on VersionedConfigurableExtension.

Ah gotcha, will change

[exceptionfactory]

Thanks for the updates @mattyb149. This looks close to completion. Now that the scope is clear, I have one more recommendation regarding the rule name, then I think this is ready to proceed.

[exceptionfactory]

Thanks for renaming, on final review, I noticed that the logic to check the component type includes some unnecessary checking for the simple type, and for an empty collection, neither of which are necessary.

[exceptionfactory]

Suggested change

	private final List<String> componentTypes = List.of(
	private static final List<String> componentTypes = List.of(

[exceptionfactory]

This logic can be simplified since componentTypes is static:

Suggested change

	String encounteredSimpleComponentType = encounteredComponentType.substring(encounteredComponentType.lastIndexOf(".") + 1);
	if (componentTypes.isEmpty() || componentTypes.contains(encounteredComponentType) || componentTypes.contains(encounteredSimpleComponentType)) {
	if (componentTypes.contains(encounteredComponentType)) {

[exceptionfactory]

Thanks again @mattyb149, the latest version looks good!

Any remaining comments @pvillard31?