You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/05/18 21:37:57 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request, #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
exceptionfactory opened a new pull request, #6057:
URL: https://github.com/apache/nifi/pull/6057
# Summary
[NIFI-9958](https://issues.apache.org/jira/browse/NIFI-9958) Adds framework support for Sensitive Dynamic Properties in Processors, Controller Services and Reporting Tasks.
The implementation adds a `SupportsSensitiveDynamicProperties` behavior annotation, which must be applied to component classes to indicate support for Sensitive Dynamic Properties. No other changes are necessary at the component implementation level.
Changes include updates to REST Resource methods and models to indicate component support for sensitive dynamic properties, as well as the ability for a client to indicate names of dynamic properties that should be marked sensitive. The Property Descriptor REST Resource methods for components include an new optional `sensitive` query parameter to support subsequent user interface integration.
New system tests include basic component creation and updating with sensitive dynamic property names and values. Component auditor changes ensure the Flow Configuration History masks sensitive dynamic property values.
The flow persistence approach infers sensitive property value status based on whether the property value matches the standard encrypted pattern. This strategy avoids the need to introduce structural changes to the flow.xml.gz or flow.json.gz files.
This implementation provides the basic capabilities, and adds the `SupportsSensitiveDynamicProperties` annotation to `InvokeHTTP` for the purpose of testing REST Resource methods. Subsequent pull requests will incorporate user interface and documentation updates.
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### 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
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132962681
> But now it's also exporting "SecretHeader" = "SecretValue". So it didn't strip out the sensitive dynamic property.
Thanks for the additional feedback @markap14, I will take another look at the export handling.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877399549
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java:
##########
@@ -616,9 +616,10 @@ Set<DocumentedTypeDTO> getControllerServiceTypes(final String serviceType, final
*
* @param id id
* @param property property
+ * @param sensitive Sensitive Status
* @return descriptor
*/
- PropertyDescriptorDTO getProcessorPropertyDescriptor(String id, String property);
+ PropertyDescriptorDTO getProcessorPropertyDescriptor(String id, String property, boolean sensitive);
Review Comment:
Rather than passing the `sensitive` flag all the way down in these methods, what do you think of leaving the methods as they are, and then in the Resource class that calls this method, creating a PropertyDescriptor, if necessary, that sets the sensitive flag to `true`? Going with that approach, it keeps the `get<type>PropertyDescriptor` method a little cleaner, as it could be potentially used elsewhere if need be. Adding the `sensitive` flag seems to sort of dictate the use case where it would be applicable.
##########
nifi-api/src/main/java/org/apache/nifi/documentation/AbstractDocumentationWriter.java:
##########
@@ -141,6 +142,7 @@ protected void writeBody(final ConfigurableComponent component, Map<String,Servi
writeTriggerWhenEmpty(processor.getClass().getAnnotation(TriggerWhenEmpty.class));
writeTriggerWhenAnyDestinationAvailable(processor.getClass().getAnnotation(TriggerWhenAnyDestinationAvailable.class));
writeSupportsBatching(processor.getClass().getAnnotation(SupportsBatching.class));
+ writeSupportsSensitiveDynamicProperties(processor.getClass().getAnnotation(SupportsSensitiveDynamicProperties.class));
Review Comment:
This should be done for all `ConfigurableComponent`s, not just `Processor`s.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/NiFiClientUtil.java:
##########
@@ -768,6 +752,48 @@ public void waitForControllerServiceState(final String groupId, final String des
}
}
+ public void waitForControllerServiceValidationStatus(final String controllerServiceId, final String validationStatus) throws NiFiClientException, IOException {
+ while (true) {
+ final ControllerServiceEntity controllerServiceEntity = nifiClient.getControllerServicesClient().getControllerService(controllerServiceId);
+ final String currentValidationStatus = controllerServiceEntity.getStatus().getValidationStatus();
+ if (validationStatus.contentEquals(currentValidationStatus)) {
Review Comment:
Any reason for using `contentEquals` here instead of `equals`? Both are `String`s so equals() should work.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/controllerservice/ControllerServiceApiValidationIT.java:
##########
@@ -150,4 +167,59 @@ public void testMatchingGenericDynamicPropertyControllerService() throws NiFiCli
assertEquals("ENABLED", controllerStatus);
assertEquals("Stopped", processorStatus);
}
+
+ @Test
Review Comment:
This is very minor but i wonder if we should put these tests into a different class? This one is really intended to verify that Controller Service API's work properly. I.e., processors can reference service by its interface, etc., whereas the new ones are around sensitive property validation
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/ComponentNode.java:
##########
@@ -190,6 +191,15 @@ public default void setProperties(Map<String, String> properties) {
*/
boolean isValidationNecessary();
+ /**
+ * Indicates whether the Component supports sensitive dynamic properties
+ *
+ * @return Support status for Sensitive Dynamic Properties
+ */
+ default boolean isSupportsSensitiveDynamicProperties() {
+ return false;
+ }
+
Review Comment:
I don't think I'd put a default implementation here that returns `false`. I'd recommend either defining the method in the interface with no implementation, or maybe implementing with:
```
return getComponent().getClass().isAnnotationPresent(SupportsSensitiveDynamicProperties.class);
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/ReportingTaskAuditor.java:
##########
@@ -138,13 +142,13 @@ public Object updateReportingTaskAdvice(ProceedingJoinPoint proceedingJoinPoint,
// create a configuration action accordingly
if (operation != null) {
// clear the value if this property is sensitive
- final PropertyDescriptor propertyDescriptor = reportingTask.getReportingTask().getPropertyDescriptor(property);
- if (propertyDescriptor != null && propertyDescriptor.isSensitive()) {
+ final PropertyDescriptor propertyDescriptor = reportingTask.getPropertyDescriptor(property);
+ if (propertyDescriptor != null && (propertyDescriptor.isSensitive() || sensitiveDynamicPropertyNames.contains(property))) {
Review Comment:
Would recommend same thing here - adding a comment as to why we're not just trusting the PropertyDescriptor's isSenstiive() method
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/ControllerServiceAuditor.java:
##########
@@ -144,13 +148,13 @@ public Object updateControllerServiceAdvice(ProceedingJoinPoint proceedingJoinPo
// create a configuration action accordingly
if (operation != null) {
// clear the value if this property is sensitive
- final PropertyDescriptor propertyDescriptor = controllerService.getControllerServiceImplementation().getPropertyDescriptor(property);
- if (propertyDescriptor != null && propertyDescriptor.isSensitive()) {
+ final PropertyDescriptor propertyDescriptor = controllerService.getPropertyDescriptor(property);
+ if (propertyDescriptor != null && (propertyDescriptor.isSensitive() || sensitiveDynamicPropertyNames.contains(property))) {
Review Comment:
Might be worth adding a comment here that we have to check if isSensitive() or sensitiveDynamicPropertyNames.contains() because at this point in the stack, the changes will not have been applied yet. Otherwise, I can see someone changing this in the future, thinking oh this would be simpler....
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1135148207
@exceptionfactory I'm not able to reproduce either. So I think it must be that I didn't properly mark it as secret.
OK so this one has been pretty difficult with a lot of corner cases. But I think all is looking good. I'm having trouble causing any more issues. So many thanks sticking with this! Once we get the UI part in place, I think this is going to be a big help for a lot of folks. +1 will merge to main.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r879643327
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessorResource.java:
##########
@@ -423,6 +429,11 @@ public Response getPropertyDescriptor(
// get the property descriptor
final PropertyDescriptorDTO descriptor = serviceFacade.getProcessorPropertyDescriptor(id, propertyName);
+ // Adjust sensitive status for dynamic properties based on requested status
+ if (descriptor.isDynamic()) {
+ descriptor.setSensitive(sensitive);
+ }
Review Comment:
I don't think this is correct. The sensitive flag is, by default, false. There may be processors that already have set a particular dynamic property to sensitive, and if the UI requests that property but doesn't include a value for the 'sensitive' flag, even though the processor itself says it's sensitive, this would mark it non-sensitive. I think we need to change this to:
```
if (descriptor.isDynamic() && sensitive) {
descriptor.setSensitive(true);
}
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1135112094
@exceptionfactory thanks for checking. I think that I messed up my test. Because I was trying to run this in cluster mode and there's no UI, I was updating DTO fields manually and I think I accidentally left the property as non-sensitive. Sorry for the confusion!
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132104446
Thanks for the initial feedback @markap14! Pushed an update implementing the changes.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1135100035
> @exceptionfactory I think I ran into one more issue. I created an InvokeHTTP processor and added a new sensitive property. Did NOT use a parameter. Pushed to registry. Then checked it out. The sensitive property was gone, which we expected (of note, we should describe this behavior clearly in the user guide!). But then I added the property back into my flow, and it still shows as a local modification. I think we need to filter that out as a local modification.
I walked through this scenario with pushing a flow to Registry and then importing it again. It appeared to work as expected without showing any local modifications.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877417322
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/NiFiClientUtil.java:
##########
@@ -768,6 +752,48 @@ public void waitForControllerServiceState(final String groupId, final String des
}
}
+ public void waitForControllerServiceValidationStatus(final String controllerServiceId, final String validationStatus) throws NiFiClientException, IOException {
+ while (true) {
+ final ControllerServiceEntity controllerServiceEntity = nifiClient.getControllerServicesClient().getControllerService(controllerServiceId);
+ final String currentValidationStatus = controllerServiceEntity.getStatus().getValidationStatus();
+ if (validationStatus.contentEquals(currentValidationStatus)) {
Review Comment:
No strong reason, I was making some adjustments and ended up with this version, but will change it `equals()` for simplicity.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877418237
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/controllerservice/ControllerServiceApiValidationIT.java:
##########
@@ -150,4 +167,59 @@ public void testMatchingGenericDynamicPropertyControllerService() throws NiFiCli
assertEquals("ENABLED", controllerStatus);
assertEquals("Stopped", processorStatus);
}
+
+ @Test
Review Comment:
That's a good point, I considered a separate class, as in the case of Processors. Will move these methods to new a test class.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132298273
Thanks for catching the snippet and sensitive parameter handling issues @markap14! I updated `StandardSnippetDAO` and `DtoFactory` to maintain the sensitive dynamic property names when copying and pasting. I also updated the synchronizer to consider the Versioned Property Descriptor sensitive status to ensure that status will be preserved when loading a versioned flow.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132147795
@exceptionfactory I created an InvokeHTTP processor and set a breakpoint so that I could update the DTO to indicate that my newly added property was sensitive. This worked well!
But then I copied & pasted the processor and I found that the newly created Processor had the property value but didn't have the property marked as sensitive. Looks like there needs to be an update to DtoFactory.copy(ProcessorConfigDTO), .copy(ControllerServiceDTO) and .copy(ReportingTaskDTO) to ensure that any copies of the components have the sensitive property names copied as well.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132168528
Also ran into an issue where I pushed that same flow into a nifi registry (built on the same branch) and then changed the sensitive property to use a sensitive parameter. When I checked out the flow again, the value was NOT marked sensitive.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 merged pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 merged PR #6057:
URL: https://github.com/apache/nifi/pull/6057
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877414967
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/ControllerServiceAuditor.java:
##########
@@ -144,13 +148,13 @@ public Object updateControllerServiceAdvice(ProceedingJoinPoint proceedingJoinPo
// create a configuration action accordingly
if (operation != null) {
// clear the value if this property is sensitive
- final PropertyDescriptor propertyDescriptor = controllerService.getControllerServiceImplementation().getPropertyDescriptor(property);
- if (propertyDescriptor != null && propertyDescriptor.isSensitive()) {
+ final PropertyDescriptor propertyDescriptor = controllerService.getPropertyDescriptor(property);
+ if (propertyDescriptor != null && (propertyDescriptor.isSensitive() || sensitiveDynamicPropertyNames.contains(property))) {
Review Comment:
Good point, will add a comment on the reasoning.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877415864
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java:
##########
@@ -616,9 +616,10 @@ Set<DocumentedTypeDTO> getControllerServiceTypes(final String serviceType, final
*
* @param id id
* @param property property
+ * @param sensitive Sensitive Status
* @return descriptor
*/
- PropertyDescriptorDTO getProcessorPropertyDescriptor(String id, String property);
+ PropertyDescriptorDTO getProcessorPropertyDescriptor(String id, String property, boolean sensitive);
Review Comment:
I considered that approach, it is a tradeoff, I'm fine with implementing the evaluation in the REST Resource methods as opposed to the Service Facade.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877414577
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/ComponentNode.java:
##########
@@ -190,6 +191,15 @@ public default void setProperties(Map<String, String> properties) {
*/
boolean isValidationNecessary();
+ /**
+ * Indicates whether the Component supports sensitive dynamic properties
+ *
+ * @return Support status for Sensitive Dynamic Properties
+ */
+ default boolean isSupportsSensitiveDynamicProperties() {
+ return false;
+ }
+
Review Comment:
Adjusting the default implementation to your suggestion sounds good.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1134949397
@exceptionfactory I think I ran into one more issue. I created an InvokeHTTP processor and added a new sensitive property. Did NOT use a parameter. Pushed to registry.
Then checked it out. The sensitive property was gone, which we expected (of note, we should describe this behavior clearly in the user guide!). But then I added the property back into my flow, and it still shows as a local modification. I think we need to filter that out as a local modification.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6057:
URL: https://github.com/apache/nifi/pull/6057#issuecomment-1132957901
Thanks for the update @exceptionfactory. I think we've still got something not quite right, though. I With the latest updated, I have an InvokeHTTP processor with 2 sensitive dynamic properties. One is named "SecretHeader" with a value of "SecretValue" and the other is "SecretParamHeader" with a value of "#{SecretParam}". When I push to registry or download, we are now preserving the "SecretParamHeader" property as we want. But now it's also exporting "SecretHeader" = "SecretValue". So it didn't strip out the sensitive dynamic property.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r877413525
##########
nifi-api/src/main/java/org/apache/nifi/documentation/AbstractDocumentationWriter.java:
##########
@@ -141,6 +142,7 @@ protected void writeBody(final ConfigurableComponent component, Map<String,Servi
writeTriggerWhenEmpty(processor.getClass().getAnnotation(TriggerWhenEmpty.class));
writeTriggerWhenAnyDestinationAvailable(processor.getClass().getAnnotation(TriggerWhenAnyDestinationAvailable.class));
writeSupportsBatching(processor.getClass().getAnnotation(SupportsBatching.class));
+ writeSupportsSensitiveDynamicProperties(processor.getClass().getAnnotation(SupportsSensitiveDynamicProperties.class));
Review Comment:
Good catch, thanks, will make the correction.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
markap14 commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r879644699
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ReportingTaskResource.java:
##########
@@ -263,6 +269,11 @@ public Response getPropertyDescriptor(
// get the property descriptor
final PropertyDescriptorDTO descriptor = serviceFacade.getReportingTaskPropertyDescriptor(id, propertyName);
+ // Adjust sensitive status for dynamic properties based on requested status
+ if (descriptor.isDynamic()) {
+ descriptor.setSensitive(sensitive);
+ }
Review Comment:
Same as for processors, we shouldn't override to set the descriptor to non-sensitive as a result of the sensitivity flag not being set.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6057: NIFI-9958 Add Framework Support for Sensitive Dynamic Properties
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6057:
URL: https://github.com/apache/nifi/pull/6057#discussion_r879674931
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessorResource.java:
##########
@@ -423,6 +429,11 @@ public Response getPropertyDescriptor(
// get the property descriptor
final PropertyDescriptorDTO descriptor = serviceFacade.getProcessorPropertyDescriptor(id, propertyName);
+ // Adjust sensitive status for dynamic properties based on requested status
+ if (descriptor.isDynamic()) {
+ descriptor.setSensitive(sensitive);
+ }
Review Comment:
Thanks for catching that @markap14, that's a good point. Since there are a couple components that mark dynamic properties as sensitive based on the property name, this approach would override that setting. Will make the adjustment as suggested.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org