-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIFI-11231 Stateless NiFi sensitive parameter context support #6993
NIFI-11231 Stateless NiFi sensitive parameter context support #6993
Conversation
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @slambrose!
I noted a handful a small naming issues, but the basic approach follows the example of other components.
Can you provide some additional background on the intended use case? The Stateless execution environment has a different approach for loading flow parameters using Parameter Value Providers. This is different than Parameter Providers in traditional NiFi, which is part of the reason that Parameter Providers are not supported right now.
@gresockj May also have some additional background from work on both Parameter Providers and Parameter Value Providers.
...mework-components/src/main/java/org/apache/nifi/logging/ParameterContextTaskLogObserver.java
Outdated
Show resolved
Hide resolved
...mework-components/src/main/java/org/apache/nifi/logging/ParameterContextTaskLogObserver.java
Outdated
Show resolved
Hide resolved
...mework-components/src/main/java/org/apache/nifi/logging/ParameterContextTaskLogObserver.java
Outdated
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Outdated
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Outdated
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Outdated
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Outdated
Show resolved
Hide resolved
...fi-stateless-engine/src/main/java/org/apache/nifi/stateless/engine/StatelessFlowManager.java
Outdated
Show resolved
Hide resolved
...mework-components/src/main/java/org/apache/nifi/logging/ParameterContextTaskLogObserver.java
Outdated
Show resolved
Hide resolved
…ork package where all of the other logging files are. This keep consistency and also makes the class available to stateless NiFi code without needed to add the nifi-core dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback/review! I implemented all of the requested changes. For ParameterProviderLogObserver, I found there was already an existing class by this name doing exactly what my class was doing, but in a strange location in the core package. I moved it to the framework->logging package where all of the other LogObserver classes are, and implemented that instead. I also removed my duplicate class.
Use case: |
Thanks for addressing the code comments and providing some background on the use case @slambrose, that is helpful. Have you or @Dye357 reviewed the Parameter Value Provider implementations for NiFi Stateless? The Parameter Value Providers support the use case of supplying sensitive parameter values, which should make the proposed changes unnecessary. If there is some feature gap in the Parameter Value Providers, we should evaluate that for improvement, as opposed to introducing Parameter Providers, intended for traditional NiFi deployments. |
Hmm.. I'm not sure Parameter Value Provider would work for us since these have to be passed in on runtime or in the properties file. We're going to be supporting a variety of flows stored in Registry. The goal is to run stateless NiFi with any given Registry url, bucket id, flow id, and flow version. So those flows that utilize sensitive parameter contexts need to be able to work under any running statless nifi pod/container. We wouldn't be changing the run command or properties files for each flow that's processed with stateless NiFi. Instead, they would have to be pulled from some secrets manager or k8s secrets. Let me chat some more with @Dye357 to see if there's anything I'm missing. |
Hi @slambrose, what you describe should be possible with the existing EnvironmentVariableParameterValueProvider. Let me know if you have any other questions about the setup. |
Closing based on discussion of recommended alternatives. If there are any additional questions on the current implementation, the discussion can be continued under the linked Jira issue: NIFI-11231 |
Summary
NIFI-11231
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
files - N/ADocumentation