exceptionfactory commented on a change in pull request #5206:
URL: https://github.com/apache/nifi/pull/5206#discussion_r671537819



##########
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/bootstrap.conf
##########
@@ -63,6 +63,20 @@ nifi.bootstrap.sensitive.key=
 # HashiCorp Vault Sensitive Property Providers
 
nifi.bootstrap.protection.hashicorp.vault.conf=./conf/bootstrap-hashicorp-vault.conf
 
+# Note: the following mapping properties only apply if a Sensitive Property 
Provider that uses property contexts
+# is configured.  Otherwise, these values are ignored.
+#
+# If no nifi.bootstrap.protection.xml.context.location.mapping.* properties 
are provided, the context for protected
+# properties uses their filename as a location prefix, e.g. 
"authorizers.xml||Manager Password".

Review comment:
       It looks like `getContextKey()` changed the separator from `||` to `/`.

##########
File path: 
nifi-commons/nifi-property-utils/src/main/java/org/apache/nifi/properties/ProtectedPropertyContext.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.properties;
+
+import java.util.Arrays;
+import java.util.Objects;
+
+/**
+ * A context for protected properties, encapsulating the name and location of 
a property.
+ * @see PropertyLocation#contextFor(String)
+ */
+public class ProtectedPropertyContext {
+    /**
+     * A property location, representing a specific configuration file.
+     */
+    public enum PropertyLocation {
+        LOGIN_IDENTITY_PROVIDERS("login-identity-providers.xml"),
+        AUTHORIZERS("authorizers.xml"),
+        NIFI_PROPERTIES("nifi.properties"),
+        NIFI_REGISTRY_PROPERTIES("nifi-registry.properties"),
+        CUSTOM("custom");

Review comment:
       Although this looks like it should cover existing configuration files, 
it seems somewhat difficult to maintain. Without making this too abstract, what 
do you think about other options for configuring the context?  Perhaps 
separating the sensitive properties provider configuration into its own file 
where these mappings could be configured?

##########
File path: 
nifi-commons/nifi-property-utils/src/main/java/org/apache/nifi/properties/BootstrapProperties.java
##########
@@ -32,13 +33,22 @@
 
     public enum BootstrapPropertyKey {
         SENSITIVE_KEY("bootstrap.sensitive.key"),
-        
HASHICORP_VAULT_SENSITIVE_PROPERTY_PROVIDER_CONF("bootstrap.protection.hashicorp.vault.conf");
+        
HASHICORP_VAULT_SENSITIVE_PROPERTY_PROVIDER_CONF("bootstrap.protection.hashicorp.vault.conf"),
+        
XML_CONTEXT_LOCATION_MAPPING("bootstrap.protection.xml.context.location.mapping.");

Review comment:
       Naming this property with `XML` seems to be more format-specific than 
necessary, what do you think about renaming this to `CONTEXT_LOCATION_MAPPING`?

##########
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/bootstrap.conf
##########
@@ -63,6 +63,20 @@ nifi.bootstrap.sensitive.key=
 # HashiCorp Vault Sensitive Property Providers
 
nifi.bootstrap.protection.hashicorp.vault.conf=./conf/bootstrap-hashicorp-vault.conf
 
+# Note: the following mapping properties only apply if a Sensitive Property 
Provider that uses property contexts
+# is configured.  Otherwise, these values are ignored.
+#
+# If no nifi.bootstrap.protection.xml.context.location.mapping.* properties 
are provided, the context for protected
+# properties uses their filename as a location prefix, e.g. 
"authorizers.xml||Manager Password".
+# This creates a separate context for each unique property name in each XML 
configuration file.
+#
+# However, to reuse the same context in a more logical fashion, context 
mappings may be provided, in the format:
+# 
nifi.bootstrap.protection.xml.context.location.mapping.<contextLocation>=<identifier
 matching regex>
+# With the following configuration, for example, any XML property named 
"Manager Password" located inside
+# an XML block whose <identifier> starts with "ldap-" will be mapped to the 
context named "ldap||Manager Password",
+# regardless of whether it resides in authorizers.xml or 
login-identity-providers.xml.
+nifi.bootstrap.protection.xml.context.location.mapping.ldap=ldap-.*

Review comment:
       This explanation is helpful. Perhaps instead of defaulting to include a 
context, what about defaulting to no context or an internal default context?  
That would allow values to be shared, but also allow differentiating them if 
necessary. Is there too much overlap in property naming for that to work?  
Alternatively, having the default set of context name to file mapping in 
`bootstrap.conf` could provide a standard configuration without having to 
hard-coded locations in a `PropertyLocation` enum.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to