[GitHub] [ratis] adoroszlai commented on a change in pull request #613: RATIS-1539. Refactor GrpcTlsConfig.

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #613:
URL: https://github.com/apache/ratis/pull/613#discussion_r816545345



##########
File path: ratis-common/src/main/java/org/apache/ratis/security/TlsConf.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.ratis.security;
+
+import org.apache.ratis.util.JavaUtils;
+
+import java.io.File;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * TLS configurations.
+ */
+public class TlsConf {
+  /**
+   * The value is either an actual object or a file containing the object.
+   * @param <V> The value type.
+   */
+  private static class FileBasedValue<V> {
+    private final V value;
+    private final File file;
+
+    FileBasedValue(V value) {
+      this.value = Objects.requireNonNull(value, () -> "value == null in " + getClass());
+      this.file = null;
+    }
+
+    FileBasedValue(File file) {
+      this.value = null;
+      this.file = Objects.requireNonNull(file, () -> "file == null in " + getClass());
+    }
+
+    public V get() {
+      return value;
+    }
+
+    public File getFile() {
+      return file;
+    }
+
+    public final boolean isFileBased() {
+      return getFile() != null;
+    }
+  }
+
+  /** Configuration for {@link X509Certificate}s. */
+  public static class CertificatesConf extends FileBasedValue<Iterable<X509Certificate>> {
+    public CertificatesConf(Iterable<X509Certificate> certificates) {
+      super(certificates);

Review comment:
       I think a check that `certificates` has at least one item would be useful.

##########
File path: ratis-common/src/main/java/org/apache/ratis/security/TlsConf.java
##########
@@ -0,0 +1,210 @@
+/*
+ * 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.ratis.security;
+
+import org.apache.ratis.util.JavaUtils;
+
+import java.io.File;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * TLS configurations.
+ */
+public class TlsConf {
+  /**
+   * The value is either an actual object or a file containing the object.
+   * @param <V> The value type.
+   */
+  private static class FileBasedValue<V> {
+    private final V value;
+    private final File file;
+
+    FileBasedValue(V value) {
+      this.value = Objects.requireNonNull(value, () -> "value == null in " + getClass());
+      this.file = null;
+    }
+
+    FileBasedValue(File file) {
+      this.value = null;
+      this.file = Objects.requireNonNull(file, () -> "file == null in " + getClass());
+    }
+
+    public V get() {
+      return value;
+    }
+
+    public File getFile() {
+      return file;
+    }
+
+    public final boolean isFileBased() {
+      return getFile() != null;
+    }
+  }
+
+  /** Configuration for {@link X509Certificate}s. */
+  public static class CertificatesConf extends FileBasedValue<Iterable<X509Certificate>> {
+    public CertificatesConf(Iterable<X509Certificate> certificates) {
+      super(certificates);
+    }
+    public CertificatesConf(X509Certificate... certificates) {
+      this(Arrays.asList(certificates));
+    }
+    public CertificatesConf(File certificates) {
+      super(certificates);
+    }
+  }
+
+  /** Configuration for a {@link PrivateKey}. */
+  public static class PrivateKeyConf extends FileBasedValue<PrivateKey> {
+    public PrivateKeyConf(PrivateKey privateKey) {
+      super(privateKey);
+    }
+    public PrivateKeyConf(File privateKeyFile) {
+      super(privateKeyFile);
+    }
+  }
+
+  /** Configurations for a trust manager. */
+  public static final class TrustManagerConf {
+    /** Trust certificates. */
+    private final CertificatesConf trustCertificates;
+
+    private TrustManagerConf(CertificatesConf trustCertificates) {
+      this.trustCertificates = trustCertificates;
+    }
+
+    /** @return the trust certificates. */
+    public CertificatesConf getTrustCertificates() {
+      return trustCertificates;
+    }
+  }
+
+  /** Configurations for a key manager. */
+  public static final class KeyManagerConf {
+    /** A {@link PrivateKey}. */
+    private final PrivateKeyConf privateKey;
+    /** Certificates for the private key. */
+    private final CertificatesConf keyCertificates;
+
+    private KeyManagerConf(PrivateKeyConf privateKey, CertificatesConf keyCertificates) {
+      this.privateKey = Objects.requireNonNull(privateKey, "privateKey == null");
+      this.keyCertificates = Objects.requireNonNull(keyCertificates, "keyCertificates == null");
+    }
+
+    /** @return the private key. */
+    public PrivateKeyConf getPrivateKey() {
+      return privateKey;
+    }
+
+    /** @return the certificates for the private key. */
+    public CertificatesConf getKeyCertificates() {
+      return keyCertificates;
+    }
+  }
+
+  static final AtomicInteger COUNT = new AtomicInteger();

Review comment:
       Nit: `COUNT` can be private.




-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org