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/04/12 18:18:21 UTC
[GitHub] [nifi] tpalfy opened a new pull request, #5958: NIFI-9895 Allow parameters to reference controller services
tpalfy opened a new pull request, #5958:
URL: https://github.com/apache/nifi/pull/5958
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
https://issues.apache.org/jira/browse/NIFI-9895
### 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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] 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.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850518699
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java:
##########
@@ -460,18 +455,14 @@ private void setProperty(final String name, final PropertyConfiguration property
// If it previously referenced a Controller Service, we need to also remove that reference.
// It is okay if the new & old values are the same - we just unregister the component/descriptor and re-register it.
if (descriptor.getControllerServiceDefinition() != null) {
- if (oldConfiguration != null) {
- final String oldEffectiveValue = oldConfiguration.getEffectiveValue(getParameterContext());
- final ControllerServiceNode oldNode = serviceProvider.getControllerServiceNode(oldEffectiveValue);
- if (oldNode != null) {
- oldNode.removeReference(this, descriptor);
- }
- }
-
- final ControllerServiceNode newNode = serviceProvider.getControllerServiceNode(effectiveValue);
- if (newNode != null) {
- newNode.addReference(this, descriptor);
- }
+ Optional.ofNullable(oldConfiguration)
+ .map(_oldConfiguration -> _oldConfiguration.getEffectiveValue(getParameterContext()))
+ .map(oldEffectiveValue -> serviceProvider.getControllerServiceNode(oldEffectiveValue))
+ .ifPresent(oldNode -> oldNode.removeReference(this, descriptor));
+
+ Optional.ofNullable(effectiveValue)
+ .map(_effectiveValue -> serviceProvider.getControllerServiceNode(_effectiveValue))
+ .ifPresent(newNode -> newNode.addReference(this, descriptor));
Review Comment:
Please refer to my previous comment. In this case the introduction of necessary null-checks were the main reason to change the style.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850621671
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeControllerServiceReference.java:
##########
@@ -88,9 +91,17 @@ public static void authorizeControllerServiceReferences(final Map<String, String
// if this descriptor identifies a controller service
if (propertyDescriptor.getControllerServiceDefinition() != null) {
- final String currentValue = authorizable.getValue(propertyDescriptor);
final String proposedValue = entry.getValue();
+ ComponentNode componentNode = (ComponentNode) authorizable.getAuthorizable();
Review Comment:
Ah, sorry, I overcomplicated things. There's a trivial solution.
I can simply do
```java
String proposedEffectiveValue = new PropertyConfigurationMapper()
.mapRawPropertyValuesToPropertyConfiguration(propertyDescriptor, proposedValue)
.getEffectiveValue(authorizable.getParameterContext());
```
--
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 #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #5958:
URL: https://github.com/apache/nifi/pull/5958#issuecomment-1098016608
Thanks @tpalfy , will review
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850607689
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
Review Comment:
I could but I can also use `Optional.ofNullable` just the same.
The only advantage of the `Optional.of` would be to force a `NullPointerException` if the passed value happens to be null.
That is not the case here so (neither do we want a `NullPointerException` nor is it possible for it to be null). The two approaches are equally correct.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850533889
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeControllerServiceReference.java:
##########
@@ -109,17 +120,54 @@ public static void authorizeControllerServiceReferences(final Map<String, String
final ParameterTokenList tokenList = parser.parseTokens(proposedValue);
final boolean referencesParameter = !tokenList.toReferenceList().isEmpty();
if (referencesParameter) {
- throw new IllegalArgumentException("The property '" + propertyDescriptor.getDisplayName() + "' cannot reference a Parameter because the property is a " +
- "Controller Service reference. Allowing Controller Service references to make use of Parameters could result in security issues and a poor user experience. " +
- "As a result, this is not allowed.");
+ authorizeControllerServiceReference(authorizable, authorizer, lookup, user, propertyDescriptor, proposedEffectiveValue);
+ } else {
+ final Authorizable newServiceAuthorizable = lookup.getControllerService(proposedValue).getAuthorizable();
+ newServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
}
-
- final Authorizable newServiceAuthorizable = lookup.getControllerService(proposedValue).getAuthorizable();
- newServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
}
}
}
}
}
}
+
+ /**
+ * Authorizes a proposed new controller service reference.
+ *
+ * @param authorizable authorizable that may reference a controller service
+ * @param authorizer authorizer
+ * @param lookup lookup
+ * @param user user
+ * @param propertyDescriptor the propertyDescriptor referencing a controller service
+ * @param proposedEffectiveValue the new proposed value (id of the controller service) to use as a reference
+ */
+ public static void authorizeControllerServiceReference(
+ ComponentAuthorizable authorizable,
+ Authorizer authorizer,
+ AuthorizableLookup lookup,
+ NiFiUser user,
+ PropertyDescriptor propertyDescriptor,
+ String proposedEffectiveValue
+ ) {
+ final String currentValue = authorizable.getValue(propertyDescriptor);
+
+ if (authorizable.getAuthorizable() instanceof ComponentNode) {
+ authorize(authorizer, lookup, user, currentValue);
+ authorize(authorizer, lookup, user, proposedEffectiveValue);
+ } else {
+ throw new IllegalArgumentException("The property '" + propertyDescriptor.getDisplayName() + "' cannot reference a Parameter because the property is a " +
+ "Controller Service reference. Allowing Controller Service references to make use of Parameters could result in security issues and a poor user experience. " +
+ "As a result, this is not allowed.");
Review Comment:
To be honest it should be impossible to reach this clause but I guess in case it happens for some reason there should be a better error message.
I'll change it.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850517804
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getName)
+ .flatMap(parameterContext::getParameter)
+ .map(Parameter::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+ });
+
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+
+ if (
+ !referencedControllerServiceTypes
+ .stream()
+ .findFirst()
+ .get()
+ .isAssignableFrom(
+ ((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
+ )
+ ) {
+ throw new IllegalArgumentException("New Parameter value attempts to reference an incompatible controller service");
+ }
+ });
+ }
+ });
Review Comment:
The Guava's recommendations are for Guava's own functional API _specifically_.
The very reason is the lack of language-level support. This makes the use of Guava functional API bloated with boilerplate code. So the advantages of the functional approach is counteracted with the disadvantages of the cumbersome API.
That is not the case for the java 8 API. It was deliberately designed to be much more user(dev)-friendly and is very readable. This point is supported by the fact that in the Guava documentation the provided examples showing bad readability in certain circumstances would not hold when used the java 8 API.
Effective Java mostly focuses on streams when addressing this issue. My cases are very different as it concerns optionals. And while I do agree that the overuse of stream (and stream-like) API can be to the detriment of understandability in many cases, I'd argue it isn't the case _every time_.
In my cases each transformation logic is a one-step value lookup and the steps follow a direct outer-scope -> inner-scope flow. Can't go much simpler than that in my opinion.
Also using stream-like api for optionals saves on the multiple annoying and not at all reader-friendly null-checks. The alternative would be several layered if-else statements with half the code being boilerplate.
--
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 #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
markap14 commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r849445707
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeControllerServiceReference.java:
##########
@@ -88,9 +91,17 @@ public static void authorizeControllerServiceReferences(final Map<String, String
// if this descriptor identifies a controller service
if (propertyDescriptor.getControllerServiceDefinition() != null) {
- final String currentValue = authorizable.getValue(propertyDescriptor);
final String proposedValue = entry.getValue();
+ ComponentNode componentNode = (ComponentNode) authorizable.getAuthorizable();
Review Comment:
This is not a safe cast. If we are sure that `ComponentAuthorizable#getAuthorizable` will always return an `ComponentNode`, we should update the interface to return a `ComponentNode` instead of an `Authorizable`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getName)
+ .flatMap(parameterContext::getParameter)
+ .map(Parameter::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+ });
+
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+
+ if (
+ !referencedControllerServiceTypes
+ .stream()
+ .findFirst()
+ .get()
+ .isAssignableFrom(
+ ((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
+ )
+ ) {
+ throw new IllegalArgumentException("New Parameter value attempts to reference an incompatible controller service");
+ }
+ });
+ }
+ });
Review Comment:
We should be preferring procedural/imperative coding over functional coding where it makes sense. As a general rule of thumb, a lambda should not exceed 3-5 lines of code.
See Google's Guava's recommendations/explanations here as to why: https://github.com/google/guava/wiki/FunctionalExplained
As well as Effective Java by Joshua Bloch, where he discusses the some reasons for using functional style judiciously and keeping lambdas to only a few lines.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java:
##########
@@ -460,18 +455,14 @@ private void setProperty(final String name, final PropertyConfiguration property
// If it previously referenced a Controller Service, we need to also remove that reference.
// It is okay if the new & old values are the same - we just unregister the component/descriptor and re-register it.
if (descriptor.getControllerServiceDefinition() != null) {
- if (oldConfiguration != null) {
- final String oldEffectiveValue = oldConfiguration.getEffectiveValue(getParameterContext());
- final ControllerServiceNode oldNode = serviceProvider.getControllerServiceNode(oldEffectiveValue);
- if (oldNode != null) {
- oldNode.removeReference(this, descriptor);
- }
- }
-
- final ControllerServiceNode newNode = serviceProvider.getControllerServiceNode(effectiveValue);
- if (newNode != null) {
- newNode.addReference(this, descriptor);
- }
+ Optional.ofNullable(oldConfiguration)
+ .map(_oldConfiguration -> _oldConfiguration.getEffectiveValue(getParameterContext()))
+ .map(oldEffectiveValue -> serviceProvider.getControllerServiceNode(oldEffectiveValue))
+ .ifPresent(oldNode -> oldNode.removeReference(this, descriptor));
+
+ Optional.ofNullable(effectiveValue)
+ .map(_effectiveValue -> serviceProvider.getControllerServiceNode(_effectiveValue))
+ .ifPresent(newNode -> newNode.addReference(this, descriptor));
Review Comment:
Why the change here from imperative to functional style? We should be preferring imperative coding style unless there's a very significant reason to use functional.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeControllerServiceReference.java:
##########
@@ -109,17 +120,54 @@ public static void authorizeControllerServiceReferences(final Map<String, String
final ParameterTokenList tokenList = parser.parseTokens(proposedValue);
final boolean referencesParameter = !tokenList.toReferenceList().isEmpty();
if (referencesParameter) {
- throw new IllegalArgumentException("The property '" + propertyDescriptor.getDisplayName() + "' cannot reference a Parameter because the property is a " +
- "Controller Service reference. Allowing Controller Service references to make use of Parameters could result in security issues and a poor user experience. " +
- "As a result, this is not allowed.");
+ authorizeControllerServiceReference(authorizable, authorizer, lookup, user, propertyDescriptor, proposedEffectiveValue);
+ } else {
+ final Authorizable newServiceAuthorizable = lookup.getControllerService(proposedValue).getAuthorizable();
+ newServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
}
-
- final Authorizable newServiceAuthorizable = lookup.getControllerService(proposedValue).getAuthorizable();
- newServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
}
}
}
}
}
}
+
+ /**
+ * Authorizes a proposed new controller service reference.
+ *
+ * @param authorizable authorizable that may reference a controller service
+ * @param authorizer authorizer
+ * @param lookup lookup
+ * @param user user
+ * @param propertyDescriptor the propertyDescriptor referencing a controller service
+ * @param proposedEffectiveValue the new proposed value (id of the controller service) to use as a reference
+ */
+ public static void authorizeControllerServiceReference(
+ ComponentAuthorizable authorizable,
+ Authorizer authorizer,
+ AuthorizableLookup lookup,
+ NiFiUser user,
+ PropertyDescriptor propertyDescriptor,
+ String proposedEffectiveValue
+ ) {
+ final String currentValue = authorizable.getValue(propertyDescriptor);
+
+ if (authorizable.getAuthorizable() instanceof ComponentNode) {
+ authorize(authorizer, lookup, user, currentValue);
+ authorize(authorizer, lookup, user, proposedEffectiveValue);
+ } else {
+ throw new IllegalArgumentException("The property '" + propertyDescriptor.getDisplayName() + "' cannot reference a Parameter because the property is a " +
+ "Controller Service reference. Allowing Controller Service references to make use of Parameters could result in security issues and a poor user experience. " +
+ "As a result, this is not allowed.");
Review Comment:
This is no longer an accurate message for the Exception, as we're now allowing this and providing sufficient authorization in order to avoid any security issues.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/registry/flow/mapping/NiFiRegistryFlowMapper.java:
##########
@@ -822,7 +817,42 @@ private void mapParameterContext(final ParameterContext parameterContext, final
parameterContexts.put(versionedContext.getName(), versionedContext);
}
+ private Set<VersionedParameter> mapParameters(ParameterContext parameterContext) {
+ final Set<VersionedParameter> parameters = parameterContext.getParameters().entrySet().stream()
+ .map(descriptorAndParameter -> {
+ VersionedParameter versionedParameter;
+
+ ParameterDescriptor parameterDescriptor = descriptorAndParameter.getKey();
+ Parameter parameter = descriptorAndParameter.getValue();
+
+ if (this.flowMappingOptions.isMapControllerServiceReferencesToVersionedId()) {
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceData = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterDescriptor.getName());
+
+ if (referencedControllerServiceData.isEmpty()) {
+ versionedParameter = mapParameter(parameter);
+ } else {
+ versionedParameter = mapParameter(
+ parameter,
+ getId(Optional.ofNullable(referencedControllerServiceData.get(0).getVersionedServiceId()), parameter.getValue())
+ );
+ }
+ } else {
+ versionedParameter = mapParameter(parameter);
+ }
+
+ return versionedParameter;
+ })
Review Comment:
We should avoid large lambdas in favor or imperative/procedural coding. See additional comments below on this. It's simple enough to create a method that takes in a ParameterDescriptor and a Parameter and returns a Versioned Parameter and then call that from the lambda so that the code is more readable and IDE/debugger friendly. Or to simply use a `for-each` loop here to iterate over the entry 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] Lehel44 commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850562198
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
Review Comment:
As `parameterEntity` is nonnullable, you can use `Optional.of` here.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getName)
+ .flatMap(parameterContext::getParameter)
+ .map(Parameter::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+ });
+
+ Optional.ofNullable(parameterEntity)
Review Comment:
Here 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 a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
markap14 commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850659898
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getName)
+ .flatMap(parameterContext::getParameter)
+ .map(Parameter::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+ });
+
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+
+ if (
+ !referencedControllerServiceTypes
+ .stream()
+ .findFirst()
+ .get()
+ .isAssignableFrom(
+ ((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
+ )
+ ) {
+ throw new IllegalArgumentException("New Parameter value attempts to reference an incompatible controller service");
+ }
+ });
+ }
+ });
Review Comment:
@tpalfy there are comments in the linked doc that are specific to Guava's own functional API and the boilerplate necessary pre-Java 8. However, the caveats there are not specific to Guava. They largely discuss imperative vs. functional programming in Java, generally. With Java 8, it's often less of a concern than it was previously with the Guava API, but the same caveats still apply.
Likewise, the guidance spelled out in Effective Java is not specific to streams. One could argue that it is even more applicable within the context of streams. But it's not specific to streams. Introducing huge lambdas, and use of functional programming, in general, results in code that is less readable.
Take for example the following snippets of code above:
```
if (
!referencedControllerServiceTypes
.stream()
.findFirst()
.get()
.isAssignableFrom(
((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
)
) {
```
This is more simply and more clearly written as:
```
if (!referencedControllerServiceTypes.iterator().next().isAssignableFrom(
((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
)) {
```
Also:
```
Optional.ofNullable(parameterEntity)
.map(ParameterEntity::getParameter)
.map(ParameterDTO::getValue)
.map(lookup::getControllerService)
.map(ComponentAuthorizable::getAuthorizable)
.ifPresent(controllerServiceAuthorizable -> {
controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
...
```
Is simpler as:
```
if (parameterEntity != null) {
Authorizable controllerServiceAuthorizable = lookup.getControllerService(parameterEntity.getParameter().getValue());
if (controllerServiceAuthorizable != null) {
controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
...
}
}
```
And even when needing some null checks, the following:
```
Optional.ofNullable(parameterEntity)
.map(ParameterEntity::getParameter)
.map(ParameterDTO::getName)
.flatMap(parameterContext::getParameter)
.map(Parameter::getValue)
.map(lookup::getControllerService)
.map(ComponentAuthorizable::getAuthorizable)
.ifPresent(controllerServiceAuthorizable -> {
controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
});
```
Is still much simpler and more understandable in the imperative style, as:
```
if (parameterEntity != null) {
final Parameter parameter = parameterContext.getParameter(parameterEntity.getParameter().getName());
if (parameter != null) {
ControllerService service = lookup.getControllerService(parameter.getValue());
if (service != null) {
final Authorizable controllerServiceAuthorizable = service.getAuthorizable();
controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
}
}
}
```
And all of this is purely stylistic. Another very important consideration is that it also results in code that's harder to step through in a debugger. With imperative code, you can simply Step Over, Step Over, Step Over. With this functional method you end up having to constantly step breakpoints in order to step within the lambdas.
--
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] Lehel44 commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850555720
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java:
##########
@@ -460,18 +455,14 @@ private void setProperty(final String name, final PropertyConfiguration property
// If it previously referenced a Controller Service, we need to also remove that reference.
// It is okay if the new & old values are the same - we just unregister the component/descriptor and re-register it.
if (descriptor.getControllerServiceDefinition() != null) {
- if (oldConfiguration != null) {
- final String oldEffectiveValue = oldConfiguration.getEffectiveValue(getParameterContext());
- final ControllerServiceNode oldNode = serviceProvider.getControllerServiceNode(oldEffectiveValue);
- if (oldNode != null) {
- oldNode.removeReference(this, descriptor);
- }
- }
-
- final ControllerServiceNode newNode = serviceProvider.getControllerServiceNode(effectiveValue);
- if (newNode != null) {
- newNode.addReference(this, descriptor);
- }
+ Optional.ofNullable(oldConfiguration)
+ .map(_oldConfiguration -> _oldConfiguration.getEffectiveValue(getParameterContext()))
+ .map(oldEffectiveValue -> serviceProvider.getControllerServiceNode(oldEffectiveValue))
+ .ifPresent(oldNode -> oldNode.removeReference(this, descriptor));
+
+ Optional.ofNullable(effectiveValue)
+ .map(_effectiveValue -> serviceProvider.getControllerServiceNode(_effectiveValue))
+ .ifPresent(newNode -> newNode.addReference(this, descriptor));
Review Comment:
What's the purpose of underscores in the naming? The `_effectiveValue -> serviceProvider.getControllerServiceNode(_effectiveValue)` can be replaced with `serviceProvider::getControllerServiceNode` which makes it easier to read.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850560128
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractComponentNode.java:
##########
@@ -460,18 +455,14 @@ private void setProperty(final String name, final PropertyConfiguration property
// If it previously referenced a Controller Service, we need to also remove that reference.
// It is okay if the new & old values are the same - we just unregister the component/descriptor and re-register it.
if (descriptor.getControllerServiceDefinition() != null) {
- if (oldConfiguration != null) {
- final String oldEffectiveValue = oldConfiguration.getEffectiveValue(getParameterContext());
- final ControllerServiceNode oldNode = serviceProvider.getControllerServiceNode(oldEffectiveValue);
- if (oldNode != null) {
- oldNode.removeReference(this, descriptor);
- }
- }
-
- final ControllerServiceNode newNode = serviceProvider.getControllerServiceNode(effectiveValue);
- if (newNode != null) {
- newNode.addReference(this, descriptor);
- }
+ Optional.ofNullable(oldConfiguration)
+ .map(_oldConfiguration -> _oldConfiguration.getEffectiveValue(getParameterContext()))
+ .map(oldEffectiveValue -> serviceProvider.getControllerServiceNode(oldEffectiveValue))
+ .ifPresent(oldNode -> oldNode.removeReference(this, descriptor));
+
+ Optional.ofNullable(effectiveValue)
+ .map(_effectiveValue -> serviceProvider.getControllerServiceNode(_effectiveValue))
+ .ifPresent(newNode -> newNode.addReference(this, descriptor));
Review Comment:
Good point, I should use parameter-reference indeed.
--
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 #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
markap14 merged PR #5958:
URL: https://github.com/apache/nifi/pull/5958
--
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 #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #5958:
URL: https://github.com/apache/nifi/pull/5958#issuecomment-1128175669
Thanks @tpalfy. All looks good to me at this point. Did some testing and everything seems to work as expected. +1 merged 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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850523972
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/registry/flow/mapping/NiFiRegistryFlowMapper.java:
##########
@@ -822,7 +817,42 @@ private void mapParameterContext(final ParameterContext parameterContext, final
parameterContexts.put(versionedContext.getName(), versionedContext);
}
+ private Set<VersionedParameter> mapParameters(ParameterContext parameterContext) {
+ final Set<VersionedParameter> parameters = parameterContext.getParameters().entrySet().stream()
+ .map(descriptorAndParameter -> {
+ VersionedParameter versionedParameter;
+
+ ParameterDescriptor parameterDescriptor = descriptorAndParameter.getKey();
+ Parameter parameter = descriptorAndParameter.getValue();
+
+ if (this.flowMappingOptions.isMapControllerServiceReferencesToVersionedId()) {
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceData = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterDescriptor.getName());
+
+ if (referencedControllerServiceData.isEmpty()) {
+ versionedParameter = mapParameter(parameter);
+ } else {
+ versionedParameter = mapParameter(
+ parameter,
+ getId(Optional.ofNullable(referencedControllerServiceData.get(0).getVersionedServiceId()), parameter.getValue())
+ );
+ }
+ } else {
+ versionedParameter = mapParameter(parameter);
+ }
+
+ return versionedParameter;
+ })
Review Comment:
Yeah, I can extract the mapping logic.
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850548196
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeControllerServiceReference.java:
##########
@@ -88,9 +91,17 @@ public static void authorizeControllerServiceReferences(final Map<String, String
// if this descriptor identifies a controller service
if (propertyDescriptor.getControllerServiceDefinition() != null) {
- final String currentValue = authorizable.getValue(propertyDescriptor);
final String proposedValue = entry.getValue();
+ ComponentNode componentNode = (ComponentNode) authorizable.getAuthorizable();
Review Comment:
This is true and was bothering me but I see no good solution to be honest.
I assumed that the authorizable here must be a ComponentNode because we are dealing with its properties. But as far as the code goes, authorizable can be a _wrapper_ for a ComponentNode. And maybe sometimes it can be something unrelated to ComponentNodes. Just not sure if those objects can arrive here.
In fact we only need `PropertyDescriptor getPropertyDescriptor(String name);` in `PropertyConfigurationMapper().mapRawPropertyValuesToPropertyConfiguration`
which is actually available on the `ComponentAuthorizable`. But that interface is not awailable in `PropertyConfigurationMapper`. An idea would be to pull up a `PropertyHolder` interface from the `ComponentAuthorizable` into the `nifi-framework-core-api` but that might be too messy.
Do you have a suggestion?
--
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] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services
Posted by GitBox <gi...@apache.org>.
tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850607932
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
requestRevision,
lookup -> {
// Verify READ and WRITE permissions for user, for the Parameter Context itself
- final Authorizable parameterContext = lookup.getParameterContext(contextId);
+ final ParameterContext parameterContext = lookup.getParameterContext(contextId);
parameterContext.authorize(authorizer, RequestAction.READ, user);
parameterContext.authorize(authorizer, RequestAction.WRITE, user);
// Verify READ and WRITE permissions for user, for every component that is affected
affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+ requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+ String parameterName = parameterEntity.getParameter().getName();
+ List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+ .getParameterReferenceManager()
+ .getReferencedControllerServiceData(parameterContext, parameterName);
+
+ Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+ .stream()
+ .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+ .collect(Collectors.toSet());
+
+ if (referencedControllerServiceTypes.size() > 1) {
+ throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+ } else if (!referencedControllerServiceTypes.isEmpty()) {
+ Optional.ofNullable(parameterEntity)
+ .map(ParameterEntity::getParameter)
+ .map(ParameterDTO::getName)
+ .flatMap(parameterContext::getParameter)
+ .map(Parameter::getValue)
+ .map(lookup::getControllerService)
+ .map(ComponentAuthorizable::getAuthorizable)
+ .ifPresent(controllerServiceAuthorizable -> {
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+ controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+ });
+
+ Optional.ofNullable(parameterEntity)
Review Comment:
Same here.
--
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