ctubbsii commented on a change in pull request #2197:
URL: https://github.com/apache/accumulo/pull/2197#discussion_r678459279
##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -176,22 +176,6 @@
"One-line configuration property controlling the network locations "
+ "(hostnames) that are allowed to impersonate other users",
"1.7.1"),
- // Crypto-related properties
- @Experimental
- INSTANCE_CRYPTO_PREFIX("instance.crypto.opts.", null, PropertyType.PREFIX,
- "Properties related to on-disk file encryption.", "2.0.0"),
- @Experimental
- @Sensitive
- INSTANCE_CRYPTO_SENSITIVE_PREFIX("instance.crypto.opts.sensitive.", null,
PropertyType.PREFIX,
- "Sensitive properties related to on-disk file encryption.", "2.0.0"),
- @Experimental
- INSTANCE_CRYPTO_SERVICE("instance.crypto.service",
- "org.apache.accumulo.core.spi.crypto.NoCryptoService",
PropertyType.CLASSNAME,
- "The class which executes on-disk file encryption. The default does
nothing. To enable "
- + "encryption, replace this classname with an implementation of the"
- + "org.apache.accumulo.core.spi.crypto.CryptoService interface.",
- "2.0.0"),
-
Review comment:
Part of the benefit of having crypto properties as `instance.`
properties ensures that all tservers are using the same config. The potential
for different tservers to have different crypto services means that data
written by one tserver might not be able to be read by another, as they could
have wildly different behaviors. This is one reason why I think we should keep
an `instance.` property to refer to the crypto service factory, which is the
overall module that supports all the different crypto that will be needed, and
that per-tserver and per-table properties should only modify the behavior of
the crypto services produced by this factory with some options.
##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -894,6 +893,21 @@
TABLE_COMPACTION_STRATEGY_PREFIX("table.majc.compaction.strategy.opts.",
null,
PropertyType.PREFIX,
"Properties in this category are used to configure the compaction
strategy.", "1.6.0"),
+ // Crypto-related properties
+ @Experimental
+ TABLE_CRYPTO_PREFIX("table.crypto.opts.", null, PropertyType.PREFIX,
+ "Properties related to on-disk file encryption.", "2.1.0"),
+ @Experimental
+ @Sensitive
+ TABLE_CRYPTO_SENSITIVE_PREFIX("table.crypto.opts.sensitive.", null,
PropertyType.PREFIX,
+ "Sensitive properties related to on-disk file encryption.", "2.1.0"),
+ @Experimental
+ TABLE_CRYPTO_SERVICE("table.crypto.service",
+ "org.apache.accumulo.core.spi.crypto.NoCryptoService",
PropertyType.CLASSNAME,
+ "The class which executes on-disk table encryption/decryption. The
default does "
+ + "nothing. This property must be a classname with an implementation
of the "
+ + "org.apache.accumulo.core.spi.crypto.CryptoService interface.",
+ "2.1.0"),
Review comment:
If there were a crypto service factory specified as an `instance.`
option, then there's no need to specify a specific crypto service here, as that
would be determined by the factory, given the TABLE scope and the options
(either the options in the config, in the case of writing a new file, or the
options read from the file in the case of reading a previously written file).
##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -474,6 +458,21 @@
PropertyType.TIMEDURATION,
"The maximum amount of time to wait after a failure to create or write a
write-ahead log.",
"1.7.1"),
+ // Crypto-related properties
+ @Experimental
+ TSERV_WAL_CRYPTO_PREFIX("tserver.wal.crypto.opts.", null,
PropertyType.PREFIX,
+ "Properties related to on-disk file encryption.", "2.1.0"),
+ @Experimental
+ @Sensitive
+ TSERV_WAL_CRYPTO_SENSITIVE_PREFIX("tserver.wal.crypto.opts.sensitive.",
null, PropertyType.PREFIX,
+ "Sensitive properties related to on-disk file encryption.", "2.1.0"),
+ @Experimental
+ TSERV_WAL_CRYPTO_SERVICE("tserver.wal.crypto.service",
+ "org.apache.accumulo.core.spi.crypto.NoCryptoService",
PropertyType.CLASSNAME,
+ "The class which executes on-disk write ahead log encryption/decryption.
The default does "
+ + "nothing. This property must be a classname with an implementation
of the "
+ + "org.apache.accumulo.core.spi.crypto.CryptoService interface.",
+ "2.1.0"),
Review comment:
I would expect all tservers to use the same WAL options, so that files
written by one tserver will be able to be read by another. This implies the use
of `instance.` properties for WAL crypto options, not `tserver.` Also, if a
crypto service factory is specified in an `instance.` property, then there's no
need to specify a specific crypto service here, as the factory would make that
decision based on the options and scope.
--
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]
