Skip to content

NIFI-8964 Add Cluster Firewall Configuration to Admin Guide#5264

Merged
markap14 merged 2 commits intoapache:mainfrom
exceptionfactory:NIFI-8964
Aug 3, 2021
Merged

NIFI-8964 Add Cluster Firewall Configuration to Admin Guide#5264
markap14 merged 2 commits intoapache:mainfrom
exceptionfactory:NIFI-8964

Conversation

@exceptionfactory
Copy link
Contributor

Description of PR

NIFI-8964 Adds a Cluster Firewall Configuration section under the general Clustering Configuration heading in the NiFi System Adminstrator's Guide. The section includes an example configuration file showing supported file syntax.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically main)?

  • Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not squash or use --force when pushing to allow for clean monitoring of changes.

For code changes:

  • Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
  • Have you written or updated unit tests to verify your changes?
  • Have you verified that the full build is successful on JDK 8?
  • Have you verified that the full build is successful on JDK 11?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.

heartbeats and connection requests from potential cluster members.

The configuration file format expects one entry per line and ignores lines beginning with the `#` character. NiFi uses
standard hostname resolution to convert names to IP addresses. The configuration file supports IPv4 addresses or subnet
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add more info on what "standard hostname resolution" means? For example, I believe NiFi has no inherent ability to resolve an IP address. This capability relies on that of the host.

Copy link
Contributor Author

@exceptionfactory exceptionfactory Aug 1, 2021

Choose a reason for hiding this comment

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

What do you think about changing it to read as follows?

NiFi uses standard Java host name resolution to convert names to IP addresses. Java host name resolution leverages a combination of local machine configuration and network services, such as DNS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that looks good. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I appreciate the feedback!

192.168.0.2
192.168.0.3
# Cluster Subnet Address
192.168.0.0/29 # Address Range from 192.168.0.1 to 192.168.0.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Good examples, particularly showing how comments can be used

@markobean
Copy link
Contributor

It may require a separate JIRA ticket for tracking purposes, but I happen to notice just now that the Admin Guide states nifi.provenance.repository.rollover.time is 30 secs, but in reality it appears to be 10 mins (in the default nifi.properties file.)
Do you want to add it here while updating the guide? If not, let me know, and I'll create a separate JIRA ticket.

@exceptionfactory
Copy link
Contributor Author

It may require a separate JIRA ticket for tracking purposes, but I happen to notice just now that the Admin Guide states nifi.provenance.repository.rollover.time is 30 secs, but in reality it appears to be 10 mins (in the default nifi.properties file.)
Do you want to add it here while updating the guide? If not, let me know, and I'll create a separate JIRA ticket.

Thanks for catching that additional detail and for the feedback on this PR. Given the current scope, I think it would be better to address that in a separate Jira issue with additional context and focus.

@markobean
Copy link
Contributor

Built and installed this branch to confirm the Admin Guide renders as expected. Looks good.
The only nit-pick is not mentioning that text after a # character (even if not the first character of the line) are considered a comment and ignored. It is only mentioned that lines beginning with # are ignored. However, the example illustrates this behavior, so I won't belabor the point.
Other than that optional suggestion, +1

@markap14 markap14 merged commit 633cdab into apache:main Aug 3, 2021
@markap14
Copy link
Contributor

markap14 commented Aug 3, 2021

Thanks @exceptionfactory for the update to the admin guide! And thanks for reviewing @markobean. I'm a +1 as well. Merged to main.

krisztina-zsihovszki pushed a commit to krisztina-zsihovszki/nifi that referenced this pull request Jun 28, 2022
)

NIFI-8964 Added Cluster Firewall Configuration to Admin Guide
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.

3 participants