fapifta commented on code in PR #6770: URL: https://github.com/apache/ozone/pull/6770#discussion_r1637166886
########## hadoop-hdds/crypto-default/pom.xml: ########## @@ -0,0 +1,37 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed 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. See accompanying LICENSE file. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.ozone</groupId> + <artifactId>hdds</artifactId> + <version>1.5.0-SNAPSHOT</version> + </parent> + + <artifactId>hdds-crypto-default</artifactId> Review Comment: Thank you for bringing this up @adoroszlai, you are absolutely right, this conflict I tried to resolve in the PR description, but let me re-iterate that a bit. So, yeah... naming things is always hard, here is my thought process on this as of now: It is reasonable to use the value `unrestricted` as the `ozone.security.crypto.compliance.mode` property's default value, because this value aims to express that the default implementation is not restricted by any compliance mechanism in any jurisdiction. The unrestricted mode is usable in all the cases where the cluster is not falling under any regulation, and does not need to comply with any restrictions on crypto usage. On the other hand, it is reasonable to use `default` in the module name, as it expresses the fact that the module contains our default implementation to the crypto module, which is most likely non-compliant in any jurisdictions. All in all as I said, you are right, and thank you for pointing it out that the design doc explicitly suggests a different naming, but I came to this conclusion about the naming of the module later on when I got to really name these. In this sense I believe the design doc is the one that needs to be updated to conform with the approach I propose here. Also note that, the design doc at least in between the lines implies that the name is important, but actually it is not, as at the end of the day, the module will be just a jar on the classpath, and the proper implementation won't be selected based on the module/jar name, but based on the content. The Service Provider Interface based approach gives us the opportunity to register the `default` module (/provider) to the `unrestricted` property value without any significant complexity or sophisticated logic added. Please let me know if you can agree, however if you have any other reasonable naming I am happy to change my mind on this. At the moment I firmly believe we do not benefit from either using `unrestricted` in the module name or `default` in the property value, but both ways I think we loose the possibility of expressing our explicit intent within these two different contexts. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
