You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@ozone.apache.org by "Pochatkin (via GitHub)" <gi...@apache.org> on 2023/03/13 12:38:35 UTC

[GitHub] [ozone] Pochatkin opened a new pull request, #4389: HDDS-7814 Remote S3 secret store

Pochatkin opened a new pull request, #4389:
URL: https://github.com/apache/ozone/pull/4389

   ## What changes were proposed in this pull request?
   
   Implemented a remote S3 secret storage based on HashiCorp Vault. So, after this changes users may configure integration with Vault storage and local instanse of RocksDB will not contain S3 secrets.
   
   Default storage is still RocksDB, it means that backward compatibility doesn't break and all existed configuration and instances should works as is. 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7814
   
   ## How was this patch tested?
   It was tested mannually with HashiCorp Vault instanse but unfortenatly its impossible to run in memory instance of Vault in Java code, this is a main reasone why new integration tests doesn't added. So, is it possible to add hashicorp vault CLI to build agents, this CLI provide possibility to start in memory dev server.
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1147605031


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Thanks @Pochatkin for updating the patch.  Can you please fix checkstyle/findbugs/rat failures?
   
   https://github.com/Pochatkin/ozone/actions/runs/4499127644
   
   Also, I think the new module should be added as a dependency to `hadoop-ozone/dist`, so that it ends up in the release tarballs.  (With that I think the failed _dependency_ check will be passing again.)



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1512772200

   @myskov As I see we can add ozone-s3-secret-store.classpath to distribution as file. I think it will be enough


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai merged pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai merged PR #4389:
URL: https://github.com/apache/ozone/pull/4389


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1504049150

   > How do we deal with the fact that we do not receive a response from @duongkame ?
   
   I guess we can wait a bit, but please feel free to ask for help with review from others.
   
   I would be more confident in merging this if it was covered with docker-based acceptance tests as suggested.  If tests are deferred to another task, we'll need to have more pairs of eyes look at this.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1176055403


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/auth/AuthType.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE,
+  TOKEN;
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE).toUpperCase());
+    switch (authType) {
+    case TOKEN:
+      String token = conf.get(S3SecretRemoteStoreConfigurationKeys.TOKEN);

Review Comment:
   Its impossible to renew token in general case, because this is setting from Vault storage and its false by default.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] duongkame commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "duongkame (via GitHub)" <gi...@apache.org>.

duongkame commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1141227802


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -293,6 +295,8 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
   private Map<String, Table> tableMap = new HashMap<>();
   private List<TableCacheMetrics> tableCacheMetrics = new LinkedList<>();
 
+  private TableCache<CacheKey<String>, CacheValue<S3SecretValue>> s3SecretCache;

Review Comment:
   I don't think the secret cache needs to be coupled with the secret store implementation. Just using the `S3ImMemoryCache` for both cases is good enough. At the end of the day, they're just im-memory cache. 



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1512728415

   @adoroszlai In the progress of test implementation I faced with problem that I can't modify classpath for docker image that used in docker tests, but also as I understood we don't want to add s3-remote-secret-store classpath to ozone-manager.classpath, because it will affect production distribution. Can you please help me with this problem?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1535046667

   @ashishkumar50 @duongkame @kerneltime @Tejaskriya @vshipb Please take another look at the final patch.  If there are no further comments, I'll merge this tomorrow.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1478351024

   @adoroszlai @duongkame @vshipb Thanks for review. I have fixed your comment. Could you please take a look again?
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1473753172

   Guys, any progress? @kerneltime @tanvipenumudy?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1149154875


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Done



##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/S3RemoteSecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class S3RemoteSecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(S3RemoteSecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public S3RemoteSecretStore(String vaultAddress,

Review Comment:
   Done



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1144799951


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Thanks!



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1478387338

   > > > It was tested mannually with HashiCorp Vault instanse but unfortenatly its impossible to run in memory instance of Vault in Java code, this is a main reasone why new integration tests doesn't added.
   > > 
   > > 
   > > Hashicorp provides [official Docker image for Vault](https://hub.docker.com/r/hashicorp/vault), which can probably be integrated with the Docker-based acceptance tests.
   > 
   > Thank you for this information. May I do it in a separate ticket?
   
   Sure, it can be done in separate task to keep scope manageable.
   
   > Also, could you please provide some example how already used some docker images for testing?
   
   Ozone has a few docker-compose environments, defined in subdirectories of `hadoop-ozone/dist/src/main/compose`.  Most of these have `test.sh`, which is executed by CI.  Helper functions are defined in `hadoop-ozone/dist/src/main/compose/testlib.sh`.
   
   The actual tests are implemented in Robot Framework in `hadoop-ozone/dist/src/main/smoketest`.  But I don't think you need to change the Robot scripts, since the remote secret store feature should be transparent.
   
   As a start, we can:
   * copy `ozonesecure` (single OM and SCM) or `ozonesecure-ha` (OM and SCM HA) to a new `ozonesecure-vault` directory
   * add a container for Vault in `docker-compose.yaml`
   * set up Ozone to use it by adding the relevant config properties in `docker-config`
   * remove unnecessary tests from `test.sh`, only keeping the ones specific to S3 Gateway


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1144470526


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Ok, but how OzoneManger will initialize S3 secret store without dependency on it? You suggest to start using class loader for this interface implementation?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1144760515


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   We already have some configuration properties where the implementation is pluggable and is instantiated via reflection, see callers of `ConfigurationSource#getClass`.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1141124300


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/S3SecretStoreType.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.hadoop.ozone.om.s3http;
+
+import java.util.Objects;
+
+/**
+ * S3 secret storage type enum.
+ */
+public enum S3SecretStoreType {
+  ROCKSDB("rocksDB"),
+  HASHICORP_VAULT("vault");
+
+  private final String name;
+
+  S3SecretStoreType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static S3SecretStoreType fromName(String name) {
+    for (S3SecretStoreType value : values()) {
+      if (Objects.equals(value.name, name)) {
+        return value;
+      }
+    }
+    return null;
+  }

Review Comment:
   Can you please rename `String name` to avoid confusing with `Enum#name()`?



##########
hadoop-ozone/dist/src/main/license/bin/LICENSE.txt:
##########
@@ -455,6 +455,7 @@ Apache License 2.0
 MIT
 =====================
 
+   com.bettercloud:vault-java-drive

Review Comment:
   typo:
   
   ```suggestion
      com.bettercloud:vault-java-driver
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Tejaskriya commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.

Tejaskriya commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1152801223


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/auth/AuthType.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE("appRole"),
+  TOKEN("token");
+
+  private final String name;
+
+  AuthType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE));

Review Comment:
   While using enums it is better to convert to uppercase to handle case differences caused due to the configurations
   (To handle - java.lang.IllegalArgumentException: No enum constant)
   
   ```suggestion
       AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE).toUpperCase());```



##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class VaultS3SecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(VaultS3SecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public VaultS3SecretStore(String vaultAddress,
+                            String nameSpace,
+                            String secretPath,
+                            int engineVersion,
+                            Auth auth,
+                            SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath.endsWith("/")
+          ? secretPath.substring(0, secretPath.length() - 1)
+          : secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() throws VaultException {
+    vault = auth.auth(config);
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      checkAuth();
+      vault.logical().write(secretPath + '/' + kerberosId,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+    } catch (VaultException e) {
+      LOG.error("Failed to store secret", e);
+      throw new IOException("Failed to store secret", e);
+    }
+  }
+
+  @Override
+  public S3SecretValue getSecret(String kerberosID) throws IOException {
+    try {
+      checkAuth();
+      String s3Secret = vault.logical()
+          .read(secretPath + '/' + kerberosID).getData().get(kerberosID);
+      return new S3SecretValue(kerberosID, s3Secret);

Review Comment:
   When the command ozone s3 getsecret is run for the first time (i.e., when there is no secret existing for the username) a secret needs to be created if a flag createIfNotExist flag is set to true (by default, it is set to true) in `hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java`
   This works only is getSecret here returns a null. Hence, here before returning an object of s3SecretValue, a check needs to be present
   ```suggestion
         if(s3Secret==null)  
           return null;
         return new S3SecretValue(kerberosID, s3Secret);
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1152949126


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class VaultS3SecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(VaultS3SecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public VaultS3SecretStore(String vaultAddress,
+                            String nameSpace,
+                            String secretPath,
+                            int engineVersion,
+                            Auth auth,
+                            SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath.endsWith("/")
+          ? secretPath.substring(0, secretPath.length() - 1)
+          : secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() throws VaultException {
+    vault = auth.auth(config);
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      checkAuth();
+      vault.logical().write(secretPath + '/' + kerberosId,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+    } catch (VaultException e) {
+      LOG.error("Failed to store secret", e);
+      throw new IOException("Failed to store secret", e);
+    }
+  }
+
+  @Override
+  public S3SecretValue getSecret(String kerberosID) throws IOException {
+    try {
+      checkAuth();
+      String s3Secret = vault.logical()
+          .read(secretPath + '/' + kerberosID).getData().get(kerberosID);
+      return new S3SecretValue(kerberosID, s3Secret);

Review Comment:
   @adoroszlai @Tejaskriya Moreover, I think we should add tests to cover this contract.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1180048669


##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-compose.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+version: "3"
+services:
+  kdc:
+    image: ${OZONE_TESTKRB5_IMAGE}
+    hostname: kdc
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+    command: ["krb5kdc","-n"]
+  kms:
+    image: apache/hadoop:${HADOOP_VERSION}
+    ports:
+      - 9600:9600
+    env_file:
+      - ./docker-config
+    environment:
+      HADOOP_CONF_DIR: /opt/hadoop/etc/hadoop
+    volumes:
+      - ../../libexec/transformation.py:/opt/transformation.py
+    command: ["hadoop", "kms"]
+  datanode:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9864
+    command: ["/opt/hadoop/bin/ozone","datanode"]
+    env_file:
+      - docker-config
+    environment:
+      OZONE_OPTS:
+  om:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: om
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9874:9874
+      - 9862:9862
+    environment:
+      ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+      OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false
+    env_file:
+      - docker-config
+    command: ["/opt/hadoop/bin/ozone","om"]
+  httpfs:
+    image: apache/ozone-runner:${OZONE_RUNNER_VERSION}
+    hostname: httpfs
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 14000:14000
+    env_file:
+      - ./docker-config
+    command: [ "/opt/hadoop/bin/ozone","httpfs" ]
+    environment:
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: ${OZONE_SAFEMODE_MIN_DATANODES:-1}
+      OZONE_OPTS:
+  s3g:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: s3g
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9878:9878
+    env_file:
+      - ./docker-config
+    command: ["/opt/hadoop/bin/ozone","s3g"]
+    environment:
+      OZONE_OPTS:
+  recon:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: recon
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9888:9888
+    env_file:
+      - ./docker-config
+    environment:
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","recon"]
+  scm:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: scm
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9876:9876
+      - 9860:9860
+    env_file:
+      - docker-config
+    environment:
+      ENSURE_SCM_INITIALIZED: /data/metadata/scm/current/VERSION
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: "${OZONE_SAFEMODE_MIN_DATANODES:-1}"
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","scm"]
+  vault:
+    image: hashicorp/vault
+    hostname: vault
+    cap_add:
+      - IPC_LOCK
+    env_file:
+      - .env
+    ports:
+      - ${VAULT_PORT}:8200
+    volumes:
+      - ./data/vault:/vault/file
+      - ./logs/vault:/vault/logs

Review Comment:
   Why do we need to bind-mount these volumes?  After running the tests these local dirs have strange ownership and cannot be simply deleted.  If possible, please remove this.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1521563375

   @Pochatkin there are several test failures, some of them related to S3 auth, please check
   
   https://github.com/Pochatkin/ozone/actions/runs/4788544370


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1143787816


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/auth/AuthType.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE("appRole"),
+  TOKEN("token");
+
+  private final String name;
+
+  AuthType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE));
+    switch (authType) {
+    case TOKEN:
+      String token = conf.get(S3SecretStoreConfigurationKeys.TOKEN);

Review Comment:
   This method may be used in cases when token lifetime is infinite, of course if token renewal currently it will fail on next Vault access. I think I will create a follow-up ticket with ability to configure token provider in configuration.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1143810047


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   > Thanks for review. I have fixed your comment. Could you please take a look again?
   
   @Pochatkin Thanks for updating the patch.  It is much improved, resolving several previous comments.
   
   However, `ozone-manager` still depends on Vault, only transitively.
   ```
   [INFO] org.apache.ozone:ozone-manager:jar:1.4.0-SNAPSHOT
   ...
   [INFO] +- org.apache.ozone:ozone-s3-secret-store:jar:1.4.0-SNAPSHOT:compile
   [INFO] |  \- com.bettercloud:vault-java-driver:jar:5.1.0:compile
   ```
   
   We need an interface.  Originally I suggested this to be a separate module, but can also be in `ozone-manager` for simplicity.  Vault-based implementation should depend on the module that contains the interface (separate module or `ozone-manager` depending on your choice for the above), not the other way around.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1478359570

   > > It was tested mannually with HashiCorp Vault instanse but unfortenatly its impossible to run in memory instance of Vault in Java code, this is a main reasone why new integration tests doesn't added.
   > 
   > Hashicorp provides [official Docker image for Vault](https://hub.docker.com/r/hashicorp/vault), which can probably be integrated with the Docker-based acceptance tests.
   
   Thank you for this information. May I done it in separate ticket? Also, could you please provide some example how already used some docker images for testing?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] duongkame commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "duongkame (via GitHub)" <gi...@apache.org>.

duongkame commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1145199658


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SecretStoreProvider.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.ozone.om;
+
+import org.apache.hadoop.conf.Configuration;
+
+import java.io.IOException;
+
+/**
+ * S3 secret store provider.
+ */
+public interface SecretStoreProvider {

Review Comment:
   nit: I can't really justify the necessity of this interface. Will we need more than 1 implementation?



##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/S3RemoteSecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class S3RemoteSecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(S3RemoteSecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public S3RemoteSecretStore(String vaultAddress,

Review Comment:
   nit: maybe a simple unit test can help cover the data conversion and error handling in this class. 



##########
hadoop-ozone/s3-secret-store/pom.xml:
##########
@@ -0,0 +1,194 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.apache.ozone</groupId>
+    <artifactId>ozone</artifactId>
+    <version>1.4.0-SNAPSHOT</version>
+  </parent>
+  <artifactId>ozone-s3-secret-store</artifactId>
+  <name>Apache Ozone S3 Secret Store</name>
+  <packaging>jar</packaging>
+  <version>1.4.0-SNAPSHOT</version>
+  <properties>
+    <file.encoding>UTF-8</file.encoding>
+    <downloadSources>true</downloadSources>
+  </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-common</artifactId>
+      <scope>compile</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>com.bettercloud</groupId>
+      <artifactId>vault-java-driver</artifactId>
+    </dependency>
+<!--    <dependency>-->

Review Comment:
   nit: remove?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java:
##########
@@ -64,6 +69,10 @@ public String getSecretString(String awsAccessKey)
         "awsAccessKeyId cannot be null or empty.");
     LOG.trace("Get secret for awsAccessKey:{}", awsAccessKey);
 
+    S3SecretValue cacheValue = s3SecretCache.get(awsAccessKey);

Review Comment:
   Today, s3SecretCache is updated from `OMSetSecretRequest` and `S3GetSecretRequest` only. 
   It's not updated on normal S3 operations (key/bucket read/write). 
   Thus, there's a good chance that this cache doesn't help improve the performance or reduce the load to the external vault. 
   
   The Secret cache should be implemented as a read-through cache (or LoadingCache in Guava term).
   
   And IMHO, for simplicity, it can be just a `LoadingCache` directly put in `S3SecretManagerImpl`. I don't see it can be anything else (that needs the flexibility of a separate component/interface) in the future. 



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] kerneltime commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.

kerneltime commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1466443836

   cc @myskov 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1494367410

   @adoroszlai @duongkame Any comments guys?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1473976834

   > It was tested mannually with HashiCorp Vault instanse but unfortenatly its impossible to run in memory instance of Vault in Java code, this is a main reasone why new integration tests doesn't added.
   
   Hashicorp provides [official Docker image for Vault](https://hub.docker.com/r/hashicorp/vault), which can probably be integrated with the Docker-based acceptance tests.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1147526459


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java:
##########
@@ -64,6 +69,10 @@ public String getSecretString(String awsAccessKey)
         "awsAccessKeyId cannot be null or empty.");
     LOG.trace("Get secret for awsAccessKey:{}", awsAccessKey);
 
+    S3SecretValue cacheValue = s3SecretCache.get(awsAccessKey);

Review Comment:
   Sorry, I missunderstand your main point. 
   Why S3 secret cache should updating on key/bucket read/write operations? 
   Your suggestion is remove cache update operations from `SecretRequest` and create a LoadingCache directly in SecretManager? Load all values on start from storage and update cache manually on each write operation?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1512831512

   > adding jars to OM classpath is too difficult for an administrator: how can he know what jars to add to OM classpath?
   
   The same way as learning about required Vault-specific config properties: it can be documented.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] vshipb commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "vshipb (via GitHub)" <gi...@apache.org>.

vshipb commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1142463551


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/S3RemoteSecretStoreBuilder.java:
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault;
+
+import com.bettercloud.vault.SslConfig;
+import org.apache.hadoop.ozone.om.s3http.vault.auth.Auth;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+
+/**
+ * Builder for {@link S3RemoteSecretStore}.
+ */
+public class S3RemoteSecretStoreBuilder {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(S3RemoteSecretStoreBuilder.class);
+
+  private String address;
+  private String nameSpace;
+  private String secretPath;
+  private int engineVersion = 1;
+  private String keyStoreType;
+  private String keyStore;
+  private String keyStorePassword;
+  private String trustStoreType;
+  private String trustStore;
+  private String trustStorePassword;
+  private Auth auth;
+
+  public S3RemoteSecretStoreBuilder setAddress(String address) {
+    this.address = address;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setEngineVersion(int engineVersion) {
+    this.engineVersion = engineVersion;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setKeyStoreType(String keyStoreType) {
+    this.keyStoreType = keyStoreType;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setKeyStore(String keyStore) {
+    this.keyStore = keyStore;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setKeyStorePassword(String ksPassword) {
+    this.keyStorePassword = ksPassword;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setTrustStoreType(String trustStoreType) {
+    this.trustStoreType = trustStoreType;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setTrustStore(String trustStore) {
+    this.trustStore = trustStore;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setTrustStorePassword(String tsPassword) {
+    this.trustStorePassword = tsPassword;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setAuth(Auth auth) {
+    this.auth = auth;
+    return this;
+  }
+
+  public S3RemoteSecretStoreBuilder setSecretPath(String secretPath) {
+    this.secretPath = secretPath;
+    return this;
+  }
+
+
+  public S3RemoteSecretStoreBuilder setNameSpace(String nameSpace) {
+    this.nameSpace = nameSpace;
+    return this;
+  }
+
+  public S3RemoteSecretStore build() throws IOException {
+    SslConfig sslConfig = loadKeyStore(null);
+    sslConfig = loadTrustStore(sslConfig);
+    return new S3RemoteSecretStore(address, nameSpace,
+        secretPath, engineVersion, auth, sslConfig);
+  }
+
+  private SslConfig loadKeyStore(SslConfig sslConfig) {
+    if (sslConfig == null) {
+      sslConfig = new SslConfig();
+    }
+    if (keyStoreType != null) {
+      try {
+        KeyStore store = loadStore(keyStoreType,
+            keyStore, keyStorePassword);
+        return sslConfig.keyStore(store, keyStorePassword);
+      } catch (CertificateException
+          | KeyStoreException
+          | IOException
+          | NoSuchAlgorithmException e) {
+        LOG.error("Failed to load keystore for S3 remove secret store", e);

Review Comment:
   A typo in "remove"?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1485061518

   Thanks @Pochatkin for updating the patch.  The following failures still need to be fixed.
   
   Add new module in `jar-report.txt`:
   
   ```diff
   diff --git hadoop-ozone/dist/src/main/license/jar-report.txt hadoop-ozone/dist/src/main/license/jar-report.txt
   --- hadoop-ozone/dist/src/main/license/jar-report.txt
   +++ hadoop-ozone/dist/src/main/license/jar-report.txt
   @@ -214,6 +214,7 @@ share/ozone/lib/ozone-interface-storage.jar
    share/ozone/lib/ozone-manager.jar
    share/ozone/lib/ozone-reconcodegen.jar
    share/ozone/lib/ozone-recon.jar
   +share/ozone/lib/ozone-s3-secret-store.jar
    share/ozone/lib/ozone-s3gateway.jar
    share/ozone/lib/ozone-tools.jar
    share/ozone/lib/perfmark-api.jar
   ```
   
   Remove reference to non-existent exclude file for findbugs:
   
   ```diff
   diff --git hadoop-ozone/s3-secret-store/pom.xml hadoop-ozone/s3-secret-store/pom.xml
   --- hadoop-ozone/s3-secret-store/pom.xml
   +++ hadoop-ozone/s3-secret-store/pom.xml
   @@ -65,16 +65,4 @@
        </dependency>
      </dependencies>
    
   -  <build>
   -    <plugins>
   -      <plugin>
   -        <groupId>com.github.spotbugs</groupId>
   -        <artifactId>spotbugs-maven-plugin</artifactId>
   -        <configuration>
   -          <excludeFilterFile>${basedir}/dev-support/findbugsExcludeFile.xml
   -          </excludeFilterFile>
   -        </configuration>
   -      </plugin>
   -    </plugins>
   -  </build>
    </project>
   ```
   
   Fix findbugs failures:
   
   ```
   M D ST: Write to static field org.apache.hadoop.ozone.s3.remote.vault.VaultS3SecretStoreTest.failAuthCounter from instance method org.apache.hadoop.ozone.s3.remote.vault.VaultS3SecretStoreTest.testAuthFail()  At VaultS3SecretStoreTest.java:[line 111]
   M D ST: Write to static field org.apache.hadoop.ozone.s3.remote.vault.VaultS3SecretStoreTest.failAuthCounter from instance method org.apache.hadoop.ozone.s3.remote.vault.VaultS3SecretStoreTest.testReAuth()  At VaultS3SecretStoreTest.java:[line 97]
   ```


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1494373937

   > Any comments guys?
   
   I'm fine with the latest iteration of the patch, but would like @duongkame to also 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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1176054494


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java:
##########
@@ -64,6 +69,10 @@ public String getSecretString(String awsAccessKey)
         "awsAccessKeyId cannot be null or empty.");
     LOG.trace("Get secret for awsAccessKey:{}", awsAccessKey);
 
+    S3SecretValue cacheValue = s3SecretCache.get(awsAccessKey);
+    if (cacheValue != null) {
+      return cacheValue.getAwsSecret();
+    }
     S3SecretValue s3Secret = s3SecretStore.getSecret(awsAccessKey);

Review Comment:
   Fixed in last commit 



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1504818726

   > I'm already in the process of developing tests and have done about half of them. Maybe I'll just add them to this pull request?
   
   That would be great.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1531166118

   @adoroszlai All your comments fixed


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1502867897

   @adoroszlai How do we deal with the fact that we do not receive a response from @duongkame ?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1488241864

   @duongkame May you please take a look one more time?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1144470526


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Ok, but how OzoneManger will initialize S3 secret store without dependency on it? You suggest to start using class loader for this interface implementations?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Tejaskriya commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.

Tejaskriya commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1152898921


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class VaultS3SecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(VaultS3SecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public VaultS3SecretStore(String vaultAddress,
+                            String nameSpace,
+                            String secretPath,
+                            int engineVersion,
+                            Auth auth,
+                            SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath.endsWith("/")
+          ? secretPath.substring(0, secretPath.length() - 1)
+          : secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() throws VaultException {
+    vault = auth.auth(config);
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      checkAuth();
+      vault.logical().write(secretPath + '/' + kerberosId,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+    } catch (VaultException e) {
+      LOG.error("Failed to store secret", e);
+      throw new IOException("Failed to store secret", e);
+    }
+  }
+
+  @Override
+  public S3SecretValue getSecret(String kerberosID) throws IOException {
+    try {
+      checkAuth();
+      String s3Secret = vault.logical()
+          .read(secretPath + '/' + kerberosID).getData().get(kerberosID);
+      return new S3SecretValue(kerberosID, s3Secret);

Review Comment:
   @adoroszlai Yes I'm sure vault returns null. 



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ashishkumar50 commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.

ashishkumar50 commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1173721257


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java:
##########
@@ -64,6 +69,10 @@ public String getSecretString(String awsAccessKey)
         "awsAccessKeyId cannot be null or empty.");
     LOG.trace("Get secret for awsAccessKey:{}", awsAccessKey);
 
+    S3SecretValue cacheValue = s3SecretCache.get(awsAccessKey);
+    if (cacheValue != null) {
+      return cacheValue.getAwsSecret();
+    }
     S3SecretValue s3Secret = s3SecretStore.getSecret(awsAccessKey);

Review Comment:
   How does cache is helping here? Cache value will get inserted only during first time when getsecret is invoked or setsecret. Since cache will expire after sometime, always there will be cache miss and it will try to get secret from vault which will be more costly compare to current rocksdb retrieval. 



##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/auth/AuthType.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE,
+  TOKEN;
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE).toUpperCase());
+    switch (authType) {
+    case TOKEN:
+      String token = conf.get(S3SecretRemoteStoreConfigurationKeys.TOKEN);

Review Comment:
   Is there any validity of vault token? We should consider renewing the token if expires, otherwise vault communication will fail.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -754,8 +758,27 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
     }
     volumeManager = new VolumeManagerImpl(metadataManager);
     bucketManager = new BucketManagerImpl(metadataManager);
+
+    Class<? extends S3SecretStoreProvider> storeProviderClass =
+        configuration.getClass(
+            S3_SECRET_STORAGE_TYPE,
+            DEFAULT_SECRET_STORAGE_TYPE,
+            S3SecretStoreProvider.class);
+    S3SecretStore store;
+    try {
+      store = storeProviderClass == DEFAULT_SECRET_STORAGE_TYPE
+              ? metadataManagerImpl

Review Comment:
   Before migrating to store like vault, there might be secrets present in default rocksdb. In this case operations will fail if we directly switch to vault without considering this. Is there way to retrieve those and migrate those secrets to vault?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] myskov commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "myskov (via GitHub)" <gi...@apache.org>.

myskov commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1512769673

   @adoroszlai not adding vault libs to OM classpath makes enabling vault integration much harder.
   As an Ozone administrator I want to configure vault integration by modifying core-site.xml via cluster manager (Ambari for example), adding jars to OM classpath is too difficult for an administrator: how can he know what jars to add to OM classpath?
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] kerneltime commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.

kerneltime commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1466440402

   cc @tanvipenumudy can you please take a look.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1504813683

   @adoroszlai I'm already in the process of developing tests and have done about half of them. Maybe I'll just add them to this pull request?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] duongkame commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "duongkame (via GitHub)" <gi...@apache.org>.

duongkame commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1141226466


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -754,8 +776,48 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
     }
     volumeManager = new VolumeManagerImpl(metadataManager);
     bucketManager = new BucketManagerImpl(metadataManager);
+    S3SecretStoreType type = S3SecretStoreType.fromName(
+        configuration.get(S3_SECRET_STORAGE_TYPE, DEFAULT_SECRET_STORAGE_TYPE));
+    S3SecretStore store;
+    S3SecretCache cache;
+    switch (type) {
+    case HASHICORP_VAULT:
+      S3RemoteSecretStoreBuilder builder = S3RemoteSecretStore.builder()

Review Comment:
   The config extraction and construction of Vault Secret store and cache should be moved out of `OzoneManager`, which is too complex and already responsible for many things.
   e.g. `S3RemoteSecretStoreBuilder` should know how to build from config.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/S3RemoteSecretStore.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LogicalResponse;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.om.s3http.vault.auth.Auth;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class S3RemoteSecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(S3RemoteSecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final boolean reAuth = true;
+  private final Auth auth;
+
+  public S3RemoteSecretStore(String vaultAddress,
+                             String nameSpace,
+                             String secretPath,
+                             int engineVersion,
+                             Auth auth,
+                             SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() {
+    try {
+      vault = auth.auth(config);
+    } catch (VaultException e) {
+      LOG.error("Failed to s3 secret store auth", e);
+    }
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      LogicalResponse s3Secret = vault.logical()
+          .write(secretPath,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+      System.out.println(s3Secret.getData());

Review Comment:
   This should be removed. 



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/S3RemoteSecretStore.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LogicalResponse;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.om.s3http.vault.auth.Auth;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class S3RemoteSecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(S3RemoteSecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final boolean reAuth = true;
+  private final Auth auth;
+
+  public S3RemoteSecretStore(String vaultAddress,
+                             String nameSpace,
+                             String secretPath,
+                             int engineVersion,
+                             Auth auth,
+                             SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() {

Review Comment:
   Is this method used at all?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -293,6 +295,8 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
   private Map<String, Table> tableMap = new HashMap<>();
   private List<TableCacheMetrics> tableCacheMetrics = new LinkedList<>();
 
+  private TableCache<CacheKey<String>, CacheValue<S3SecretValue>> s3SecretCache;

Review Comment:
   I don't think the secret cache needs to be coupled with the secret store implementation. Just using the `S3ImMemoryCache` for both caches is good enough. At the end of the day, they're just im-memory cache. 



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/auth/AuthType.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE("appRole"),
+  TOKEN("token");
+
+  private final String name;
+
+  AuthType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE));
+    switch (authType) {
+    case TOKEN:
+      String token = conf.get(S3SecretStoreConfigurationKeys.TOKEN);

Review Comment:
   Just to clarify, in this mode, can we use a fixed token to talk to the vault? What does the token renewal process look like?
   I guess the use of `Supplier` in `DirectTokenAuth` indicate a future improvement in this area. If so, please put a comment with JIRA for the improvement.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on PR #4389:
URL: https://github.com/apache/ozone/pull/4389#issuecomment-1512746856

   > I can't modify classpath for docker image that used in docker tests
   
   You can add items to the classpath by setting the `OZONE_CLASSPATH` variable here:
   
   https://github.com/apache/ozone/blob/b74faeff9105f42dfd98184c5b76bc63db70e2c2/hadoop-ozone/dist/src/main/compose/ozonesecure/docker-compose.yaml#L60-L62
   
   You may use the `@project.version@` placeholder to reference Ozone version, like this:
   
   https://github.com/apache/ozone/blob/7506842d83ce802c5da80b05670fc7d36b3daf4b/hadoop-ozone/dist/src/main/compose/ozone-mr/hadoop33/docker-config#L21


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1152848657


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/auth/AuthType.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE("appRole"),
+  TOKEN("token");
+
+  private final String name;
+
+  AuthType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE));

Review Comment:
   I think it's better to call `conf.getEnum` instead, which centralises logic for enum lookup from config.
   
   Also please use `ConfigurationSource` instead of `Configuration`.
   
   It's fine to make these changes in a follow-up task, or in this PR if any other change is requested.  No need to update the patch only for these minor improvements.



##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class VaultS3SecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(VaultS3SecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public VaultS3SecretStore(String vaultAddress,
+                            String nameSpace,
+                            String secretPath,
+                            int engineVersion,
+                            Auth auth,
+                            SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath.endsWith("/")
+          ? secretPath.substring(0, secretPath.length() - 1)
+          : secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() throws VaultException {
+    vault = auth.auth(config);
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      checkAuth();
+      vault.logical().write(secretPath + '/' + kerberosId,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+    } catch (VaultException e) {
+      LOG.error("Failed to store secret", e);
+      throw new IOException("Failed to store secret", e);
+    }
+  }
+
+  @Override
+  public S3SecretValue getSecret(String kerberosID) throws IOException {
+    try {
+      checkAuth();
+      String s3Secret = vault.logical()
+          .read(secretPath + '/' + kerberosID).getData().get(kerberosID);
+      return new S3SecretValue(kerberosID, s3Secret);

Review Comment:
   Good point @Tejaskriya.  Are you sure Vault returns `null` in this case, or will it throw exception?
   
   (Note: the suggested code change would be rejected by checkstyle due to missing whitespace and braces)



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1152885090


##########
hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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.hadoop.ozone.s3.remote.vault;
+
+import com.bettercloud.vault.SslConfig;
+import com.bettercloud.vault.Vault;
+import com.bettercloud.vault.VaultConfig;
+import com.bettercloud.vault.VaultException;
+import com.bettercloud.vault.response.LookupResponse;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.S3Batcher;
+import org.apache.hadoop.ozone.om.S3SecretStore;
+import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.Auth;
+import org.apache.hadoop.ozone.s3.remote.vault.auth.AuthType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ADDRESS;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.ENGINE_VER;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.KEY_STORE_TYPE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.NAMESPACE;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.SECRET_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PASSWORD;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_PATH;
+import static org.apache.hadoop.ozone.s3.remote.S3SecretRemoteStoreConfigurationKeys.TRUST_STORE_TYPE;
+
+/**
+ * Based on HashiCorp Vault secret storage.
+ * Documentation link {@code https://developer.hashicorp.com/vault}.
+ */
+public class VaultS3SecretStore implements S3SecretStore {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(VaultS3SecretStore.class);
+
+  private final VaultConfig config;
+  private Vault vault;
+  private final String secretPath;
+  private final Auth auth;
+
+  public VaultS3SecretStore(String vaultAddress,
+                            String nameSpace,
+                            String secretPath,
+                            int engineVersion,
+                            Auth auth,
+                            SslConfig sslConfig) throws IOException {
+    try {
+      config = new VaultConfig()
+          .address(vaultAddress)
+          .engineVersion(engineVersion)
+          .nameSpace(nameSpace)
+          .sslConfig(sslConfig)
+          .build();
+
+      this.auth = auth;
+      vault = auth.auth(config);
+      this.secretPath = secretPath.endsWith("/")
+          ? secretPath.substring(0, secretPath.length() - 1)
+          : secretPath;
+    } catch (VaultException e) {
+      throw new IOException("Failed to initialize remote secret store", e);
+    }
+  }
+
+  private void auth() throws VaultException {
+    vault = auth.auth(config);
+  }
+
+  @Override
+  public void storeSecret(String kerberosId, S3SecretValue secret)
+      throws IOException {
+    try {
+      checkAuth();
+      vault.logical().write(secretPath + '/' + kerberosId,
+              Collections.singletonMap(kerberosId, secret.getAwsSecret()));
+    } catch (VaultException e) {
+      LOG.error("Failed to store secret", e);
+      throw new IOException("Failed to store secret", e);
+    }
+  }
+
+  @Override
+  public S3SecretValue getSecret(String kerberosID) throws IOException {
+    try {
+      checkAuth();
+      String s3Secret = vault.logical()
+          .read(secretPath + '/' + kerberosID).getData().get(kerberosID);
+      return new S3SecretValue(kerberosID, s3Secret);

Review Comment:
   I have fix it. Thanks!



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.

adoroszlai commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1180049216


##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-compose.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+version: "3"
+services:
+  kdc:
+    image: ${OZONE_TESTKRB5_IMAGE}
+    hostname: kdc
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+    command: ["krb5kdc","-n"]
+  kms:
+    image: apache/hadoop:${HADOOP_VERSION}
+    ports:
+      - 9600:9600
+    env_file:
+      - ./docker-config
+    environment:
+      HADOOP_CONF_DIR: /opt/hadoop/etc/hadoop
+    volumes:
+      - ../../libexec/transformation.py:/opt/transformation.py
+    command: ["hadoop", "kms"]
+  datanode:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9864
+    command: ["/opt/hadoop/bin/ozone","datanode"]
+    env_file:
+      - docker-config
+    environment:
+      OZONE_OPTS:
+  om:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: om
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9874:9874
+      - 9862:9862
+    environment:
+      ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+      OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false
+    env_file:
+      - docker-config
+    command: ["/opt/hadoop/bin/ozone","om"]
+  httpfs:
+    image: apache/ozone-runner:${OZONE_RUNNER_VERSION}
+    hostname: httpfs
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 14000:14000
+    env_file:
+      - ./docker-config
+    command: [ "/opt/hadoop/bin/ozone","httpfs" ]
+    environment:
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: ${OZONE_SAFEMODE_MIN_DATANODES:-1}
+      OZONE_OPTS:
+  s3g:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: s3g
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9878:9878
+    env_file:
+      - ./docker-config
+    command: ["/opt/hadoop/bin/ozone","s3g"]
+    environment:
+      OZONE_OPTS:
+  recon:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: recon
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9888:9888
+    env_file:
+      - ./docker-config
+    environment:
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","recon"]
+  scm:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: scm
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9876:9876
+      - 9860:9860
+    env_file:
+      - docker-config
+    environment:
+      ENSURE_SCM_INITIALIZED: /data/metadata/scm/current/VERSION
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: "${OZONE_SAFEMODE_MIN_DATANODES:-1}"
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","scm"]
+  vault:
+    image: hashicorp/vault
+    hostname: vault
+    cap_add:
+      - IPC_LOCK
+    env_file:
+      - .env

Review Comment:
   I think this is unnecessary, Docker Compose already reads `.env` for default values.



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-compose.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+version: "3"
+services:
+  kdc:
+    image: ${OZONE_TESTKRB5_IMAGE}
+    hostname: kdc
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+    command: ["krb5kdc","-n"]
+  kms:
+    image: apache/hadoop:${HADOOP_VERSION}
+    ports:
+      - 9600:9600
+    env_file:
+      - ./docker-config
+    environment:
+      HADOOP_CONF_DIR: /opt/hadoop/etc/hadoop
+    volumes:
+      - ../../libexec/transformation.py:/opt/transformation.py
+    command: ["hadoop", "kms"]
+  datanode:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9864
+    command: ["/opt/hadoop/bin/ozone","datanode"]
+    env_file:
+      - docker-config
+    environment:
+      OZONE_OPTS:
+  om:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: om
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9874:9874
+      - 9862:9862
+    environment:
+      ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+      OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false
+    env_file:
+      - docker-config
+    command: ["/opt/hadoop/bin/ozone","om"]
+  httpfs:
+    image: apache/ozone-runner:${OZONE_RUNNER_VERSION}
+    hostname: httpfs
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 14000:14000
+    env_file:
+      - ./docker-config
+    command: [ "/opt/hadoop/bin/ozone","httpfs" ]
+    environment:
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: ${OZONE_SAFEMODE_MIN_DATANODES:-1}
+      OZONE_OPTS:
+  s3g:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: s3g
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9878:9878
+    env_file:
+      - ./docker-config
+    command: ["/opt/hadoop/bin/ozone","s3g"]
+    environment:
+      OZONE_OPTS:
+  recon:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: recon
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9888:9888
+    env_file:
+      - ./docker-config
+    environment:
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","recon"]
+  scm:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: scm
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9876:9876
+      - 9860:9860
+    env_file:
+      - docker-config
+    environment:
+      ENSURE_SCM_INITIALIZED: /data/metadata/scm/current/VERSION
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: "${OZONE_SAFEMODE_MIN_DATANODES:-1}"
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","scm"]
+  vault:
+    image: hashicorp/vault
+    hostname: vault
+    cap_add:
+      - IPC_LOCK
+    env_file:
+      - .env
+    ports:
+      - ${VAULT_PORT}:8200
+    volumes:
+      - ./data/vault:/vault/file
+      - ./logs/vault:/vault/logs

Review Comment:
   Why do we need to bind-mount these volumes?  After running the tests these local dirs have strange ownership and cannot be simply removed.  If possible, please remove this.



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-compose.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+version: "3"
+services:
+  kdc:
+    image: ${OZONE_TESTKRB5_IMAGE}
+    hostname: kdc
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+    command: ["krb5kdc","-n"]
+  kms:
+    image: apache/hadoop:${HADOOP_VERSION}
+    ports:
+      - 9600:9600
+    env_file:
+      - ./docker-config
+    environment:
+      HADOOP_CONF_DIR: /opt/hadoop/etc/hadoop
+    volumes:
+      - ../../libexec/transformation.py:/opt/transformation.py
+    command: ["hadoop", "kms"]
+  datanode:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9864
+    command: ["/opt/hadoop/bin/ozone","datanode"]
+    env_file:
+      - docker-config
+    environment:
+      OZONE_OPTS:
+  om:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: om
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9874:9874
+      - 9862:9862
+    environment:
+      ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+      OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false

Review Comment:
   Add the new module and Vault to the classpath:
   
   ```suggestion
       environment:
         ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
         OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false
         OZONE_CLASSPATH: /opt/hadoop/share/ozone/lib/ozone-s3-secret-store-@project.version@.jar:/opt/hadoop/share/ozone/lib/vault-java-driver-5.1.0.jar
   ```



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/vault-config.json:
##########
@@ -0,0 +1,14 @@
+{
+  "stora  ge": {

Review Comment:
   Typo?
   
   ```suggestion
     "storage": {
   ```



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-compose.yaml:
##########
@@ -0,0 +1,142 @@
+# 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.
+
+version: "3"
+services:
+  kdc:
+    image: ${OZONE_TESTKRB5_IMAGE}
+    hostname: kdc
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+    command: ["krb5kdc","-n"]
+  kms:
+    image: apache/hadoop:${HADOOP_VERSION}
+    ports:
+      - 9600:9600
+    env_file:
+      - ./docker-config
+    environment:
+      HADOOP_CONF_DIR: /opt/hadoop/etc/hadoop
+    volumes:
+      - ../../libexec/transformation.py:/opt/transformation.py
+    command: ["hadoop", "kms"]
+  datanode:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9864
+    command: ["/opt/hadoop/bin/ozone","datanode"]
+    env_file:
+      - docker-config
+    environment:
+      OZONE_OPTS:
+  om:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: om
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9874:9874
+      - 9862:9862
+    environment:
+      ENSURE_OM_INITIALIZED: /data/metadata/om/current/VERSION
+      OZONE_OPTS: -Dcom.sun.net.ssl.checkRevocation=false
+    env_file:
+      - docker-config
+    command: ["/opt/hadoop/bin/ozone","om"]
+  httpfs:
+    image: apache/ozone-runner:${OZONE_RUNNER_VERSION}
+    hostname: httpfs
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 14000:14000
+    env_file:
+      - ./docker-config
+    command: [ "/opt/hadoop/bin/ozone","httpfs" ]
+    environment:
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: ${OZONE_SAFEMODE_MIN_DATANODES:-1}
+      OZONE_OPTS:
+  s3g:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: s3g
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9878:9878
+    env_file:
+      - ./docker-config
+    command: ["/opt/hadoop/bin/ozone","s3g"]
+    environment:
+      OZONE_OPTS:
+  recon:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: recon
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9888:9888
+    env_file:
+      - ./docker-config
+    environment:
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","recon"]
+  scm:
+    image: ${OZONE_RUNNER_IMAGE}:${OZONE_RUNNER_VERSION}
+    hostname: scm
+    volumes:
+      - ../..:/opt/hadoop
+      - ../_keytabs:/etc/security/keytabs
+      - ./krb5.conf:/etc/krb5.conf
+    ports:
+      - 9876:9876
+      - 9860:9860
+    env_file:
+      - docker-config
+    environment:
+      ENSURE_SCM_INITIALIZED: /data/metadata/scm/current/VERSION
+      OZONE-SITE.XML_hdds.scm.safemode.min.datanode: "${OZONE_SAFEMODE_MIN_DATANODES:-1}"
+      OZONE_OPTS:
+    command: ["/opt/hadoop/bin/ozone","scm"]
+  vault:
+    image: hashicorp/vault

Review Comment:
   Please specify image version, avoid `latest`.



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/docker-config:
##########
@@ -0,0 +1,192 @@
+# 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.
+
+CORE-SITE.XML_fs.defaultFS=ofs://om
+CORE-SITE.XML_fs.trash.interval=1
+# For HttpFS service it is required to enable proxying users.
+CORE-SITE.XML_hadoop.proxyuser.httpfs.hosts=*
+CORE-SITE.XML_hadoop.proxyuser.httpfs.groups=*
+
+OZONE-SITE.XML_ozone.om.address=om
+OZONE-SITE.XML_ozone.om.http-address=om:9874
+# TODO: HDDS-7495
+#   S3 security tests need object store layout from s3 gateway. Other tests on
+#   this cluster use `ozone fs`. The config to set an S3 gateway specific bucket
+#   layout will be added in HDDS-7495. Until then, the server side default must
+#   be changed for all tests on this cluster to work.
+OZONE-SITE.XML_ozone.default.bucket.layout=LEGACY
+OZONE-SITE.XML_ozone.scm.http-address=scm:9876
+OZONE-SITE.XML_ozone.scm.container.size=1GB
+OZONE-SITE.XML_ozone.scm.pipeline.creation.interval=30s
+OZONE-SITE.XML_ozone.scm.pipeline.owner.container.count=1
+OZONE-SITE.XML_ozone.scm.names=scm
+OZONE-SITE.XML_ozone.scm.datanode.id.dir=/data
+OZONE-SITE.XML_ozone.scm.block.client.address=scm
+OZONE-SITE.XML_ozone.metadata.dirs=/data/metadata
+OZONE-SITE.XML_ozone.handler.type=distributed
+OZONE-SITE.XML_ozone.scm.client.address=scm
+OZONE-SITE.XML_hdds.block.token.enabled=true
+OZONE-SITE.XML_hdds.container.token.enabled=true
+OZONE-SITE.XML_hdds.grpc.tls.enabled=true
+OZONE-SITE.XML_ozone.replication=3
+OZONE-SITE.XML_ozone.datanode.pipeline.limit=1
+OZONE-SITE.XML_hdds.scmclient.max.retry.timeout=30s
+
+OZONE-SITE.XML_ozone.recon.om.snapshot.task.interval.delay=1m
+OZONE-SITE.XML_ozone.recon.db.dir=/data/metadata/recon
+OZONE-SITE.XML_ozone.recon.om.snapshot.task.initial.delay=20s
+OZONE-SITE.XML_ozone.recon.address=recon:9891
+
+OZONE-SITE.XML_ozone.security.enabled=true
+OZONE-SITE.XML_ozone.acl.enabled=true
+OZONE-SITE.XML_ozone.acl.authorizer.class=org.apache.hadoop.ozone.security.acl.OzoneNativeAuthorizer
+OZONE-SITE.XML_ozone.administrators="testuser,recon"
+OZONE-SITE.XML_ozone.recon.administrators="testuser2"
+
+OZONE-SITE.XML_hdds.datanode.dir=/data/hdds
+HDFS-SITE.XML_dfs.datanode.address=0.0.0.0:1019
+HDFS-SITE.XML_dfs.datanode.http.address=0.0.0.0:1012
+CORE-SITE.XML_dfs.data.transfer.protection=authentication
+CORE-SITE.XML_hadoop.security.authentication=kerberos
+CORE-SITE.XML_hadoop.security.auth_to_local="DEFAULT"
+CORE-SITE.XML_hadoop.security.key.provider.path=kms://http@kms:9600/kms
+
+
+OZONE-SITE.XML_hdds.scm.kerberos.principal=scm/scm@EXAMPLE.COM
+OZONE-SITE.XML_hdds.scm.kerberos.keytab.file=/etc/security/keytabs/scm.keytab
+OZONE-SITE.XML_ozone.om.kerberos.principal=om/om@EXAMPLE.COM
+OZONE-SITE.XML_ozone.om.kerberos.keytab.file=/etc/security/keytabs/om.keytab
+OZONE-SITE.XML_ozone.recon.kerberos.keytab.file=/etc/security/keytabs/recon.keytab
+OZONE-SITE.XML_ozone.recon.kerberos.principal=recon/recon@EXAMPLE.COM
+
+OZONE-SITE.XML_ozone.s3g.kerberos.keytab.file=/etc/security/keytabs/s3g.keytab
+OZONE-SITE.XML_ozone.s3g.kerberos.principal=s3g/s3g@EXAMPLE.COM
+
+OZONE-SITE.XML_ozone.httpfs.kerberos.keytab.file=/etc/security/keytabs/httpfs.keytab
+OZONE-SITE.XML_ozone.httpfs.kerberos.principal=httpfs/httpfs@EXAMPLE.COM
+
+OZONE-SITE.XML_hdds.scm.replication.thread.interval=5s
+OZONE-SITE.XML_hdds.scm.replication.enable.legacy=false
+OZONE-SITE.XML_ozone.scm.stale.node.interval=30s
+OZONE-SITE.XML_ozone.scm.dead.node.interval=45s
+OZONE-SITE.XML_hdds.container.report.interval=60s
+OZONE-SITE.XML_ozone.om.s3.grpc.server_enabled=true
+
+OZONE-SITE.XML_dfs.container.ratis.datastream.enabled=true
+
+HDFS-SITE.XML_dfs.datanode.kerberos.principal=dn/dn@EXAMPLE.COM
+HDFS-SITE.XML_dfs.datanode.kerberos.keytab.file=/etc/security/keytabs/dn.keytab
+HDFS-SITE.XML_dfs.web.authentication.kerberos.principal=HTTP/ozone@EXAMPLE.COM
+HDFS-SITE.XML_dfs.web.authentication.kerberos.keytab=/etc/security/keytabs/HTTP.keytab
+
+
+OZONE-SITE.XML_ozone.security.http.kerberos.enabled=true
+OZONE-SITE.XML_ozone.http.filter.initializers=org.apache.hadoop.security.AuthenticationFilterInitializer
+
+OZONE-SITE.XML_ozone.om.http.auth.type=kerberos
+OZONE-SITE.XML_hdds.scm.http.auth.type=kerberos
+OZONE-SITE.XML_hdds.datanode.http.auth.type=kerberos
+OZONE-SITE.XML_ozone.s3g.http.auth.type=kerberos
+OZONE-SITE.XML_ozone.httpfs.http.auth.type=kerberos
+OZONE-SITE.XML_ozone.recon.http.auth.type=kerberos
+
+OZONE-SITE.XML_hdds.scm.http.auth.kerberos.principal=HTTP/scm@EXAMPLE.COM
+OZONE-SITE.XML_hdds.scm.http.auth.kerberos.keytab=/etc/security/keytabs/scm.keytab
+OZONE-SITE.XML_ozone.om.http.auth.kerberos.principal=HTTP/om@EXAMPLE.COM
+OZONE-SITE.XML_ozone.om.http.auth.kerberos.keytab=/etc/security/keytabs/om.keytab
+OZONE-SITE.XML_hdds.datanode.http.auth.kerberos.principal=HTTP/dn@EXAMPLE.COM
+OZONE-SITE.XML_hdds.datanode.http.auth.kerberos.keytab=/etc/security/keytabs/dn.keytab
+OZONE-SITE.XML_ozone.s3g.http.auth.kerberos.keytab=/etc/security/keytabs/s3g.keytab
+OZONE-SITE.XML_ozone.s3g.http.auth.kerberos.principal=HTTP/s3g@EXAMPLE.COM
+OZONE-SITE.XML_ozone.httpfs.http.auth.kerberos.keytab=/etc/security/keytabs/httpfs.keytab
+OZONE-SITE.XML_ozone.httpfs.http.auth.kerberos.principal=HTTP/httpfs@EXAMPLE.COM
+OZONE-SITE.XML_ozone.recon.http.auth.kerberos.principal=HTTP/recon@EXAMPLE.COM
+OZONE-SITE.XML_ozone.recon.http.auth.kerberos.keytab=/etc/security/keytabs/recon.keytab
+
+CORE-SITE.XML_hadoop.http.authentication.simple.anonymous.allowed=false
+CORE-SITE.XML_hadoop.http.authentication.signature.secret.file=/etc/security/http_secret
+CORE-SITE.XML_hadoop.http.authentication.type=kerberos
+CORE-SITE.XML_hadoop.http.authentication.kerberos.principal=HTTP/ozone@EXAMPLE.COM
+CORE-SITE.XML_hadoop.http.authentication.kerberos.keytab=/etc/security/keytabs/HTTP.keytab
+
+
+CORE-SITE.XML_hadoop.security.authorization=true
+HADOOP-POLICY.XML_ozone.om.security.client.protocol.acl=*
+HADOOP-POLICY.XML_hdds.security.client.datanode.container.protocol.acl=*
+HADOOP-POLICY.XML_hdds.security.client.scm.container.protocol.acl=*
+HADOOP-POLICY.XML_hdds.security.client.scm.block.protocol.acl=*
+HADOOP-POLICY.XML_hdds.security.client.scm.certificate.protocol.acl=*
+
+HDFS-SITE.XML_rpc.metrics.quantile.enable=true
+HDFS-SITE.XML_rpc.metrics.percentiles.intervals=60,300
+
+HTTPFS-SITE.XML_hadoop.http.authentication.type=kerberos
+HTTPFS-SITE.XML_hadoop.http.authentication.kerberos.keytab=/etc/security/keytabs/httpfs.keytab
+HTTPFS-SITE.XML_hadoop.http.authentication.kerberos.principal=HTTP/httpfs@EXAMPLE.COM
+HTTPFS-SITE.XML_httpfs.hadoop.authentication.type=kerberos
+HTTPFS-SITE.XML_httpfs.hadoop.authentication.kerberos.keytab=/etc/security/keytabs/httpfs.keytab
+HTTPFS-SITE.XML_httpfs.hadoop.authentication.kerberos.principal=httpfs/httpfs@EXAMPLE.COM
+KMS-SITE.XML_hadoop.kms.proxyuser.s3g.users=*
+KMS-SITE.XML_hadoop.kms.proxyuser.s3g.groups=*
+KMS-SITE.XML_hadoop.kms.proxyuser.s3g.hosts=*
+
+#Enable this variable to print out all hadoop rpc traffic to the stdout. See http://byteman.jboss.org/ to define your own instrumentation.
+#BYTEMAN_SCRIPT_URL=https://raw.githubusercontent.com/apache/hadoop/trunk/dev-support/byteman/hadooprpc.btm
+
+OZONE_DATANODE_SECURE_USER=root
+JAVA_HOME=/usr/lib/jvm/jre
+JSVC_HOME=/usr/bin
+
+OZONE_CONF_DIR=/etc/hadoop
+OZONE_LOG_DIR=/var/log/hadoop
+
+no_proxy=om,scm,recon,s3g,kdc,localhost,127.0.0.1
+
+# Multi-Tenancy configs
+OZONE-SITE.XML_ozone.om.multitenancy.enabled=true
+OZONE-SITE.XML_ozone.om.ranger.service=cm_ozone
+
+# Note: Ranger address and credentials here doesn't matter when OM uses
+# InMemoryMultiTenantAccessController (used when dev flag is set).
+# But the values can't be empty otherwise OM config check would report failure.
+OZONE-SITE.XML_ozone.om.ranger.https-address=https://ranger:6182
+OZONE-SITE.XML_ozone.om.ranger.https.admin.api.user=admin
+OZONE-SITE.XML_ozone.om.ranger.https.admin.api.passwd=Passwd1
+
+# ozone.om.kerberos.principal and ozone.om.kerberos.keytab.file
+# (can be used for the RangerClient) are already defined above.
+
+OZONE-SITE.XML_ozone.om.multitenancy.ranger.sync.interval=30s
+OZONE-SITE.XML_ozone.om.multitenancy.ranger.sync.timeout=10s
+
+# Use InMemoryMultiTenantAccessController as we don't have Ranger Admin Server here.
+# This is fine with one OM. But for OM HA, each OM would have its own in-memory
+# "Ranger" state as a result.
+# New OM leader's in-memory "Ranger" state won't be consistent with OM DB until
+# a BG sync run has successfully finished because only previously leader OM
+# pushes updates to Ranger.
+#
+# Potential TODO: We could trigger BG sync automatically during OM leadership
+# change or let all OMs write to AccessController if this dev flag is set.
+#
+OZONE-SITE.XML_ozone.om.tenant.dev.skip.ranger=true
+
+OZONE-SITE.XML_ozone.secret.s3.store.provider=org.apache.hadoop.ozone.s3.remote.vault.VaultS3SecretStorageProvider
+OZONE-SITE.XML_ozone.secret.s3.store.remote.vault.address=localhost:8200

Review Comment:
   1. need protocol (`http`)
   2. OM and Vault run in separate containers, Vault is not `localhost` from OM's point of view
   
   ```suggestion
   OZONE-SITE.XML_ozone.secret.s3.store.remote.vault.address=http://vault:8200
   ```



##########
hadoop-ozone/dist/src/main/compose/ozonesecure-vault/test.sh:
##########
@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+# 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.
+
+#suite:secure
+
+COMPOSE_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+export COMPOSE_DIR
+
+# shellcheck source=/dev/null
+source "$COMPOSE_DIR/../testlib.sh"
+
+export SECURITY_ENABLED=true
+
+: ${OZONE_BUCKET_KEY_NAME:=key1}
+
+start_docker_env
+
+execute_command_in_container kms hadoop key create ${OZONE_BUCKET_KEY_NAME}
+
+execute_robot_test scm kinit.robot
+
+execute_robot_test scm basic
+
+execute_robot_test scm security
+
+execute_robot_test scm -v SCHEME:ofs -v BUCKET_TYPE:bucket -N ozonefs-ofs-bucket ozonefs/ozonefs.robot
+
+#expects 4 pipelines, should be run before
+#admincli which creates STANDALONE pipeline
+execute_robot_test scm recon
+
+execute_robot_test scm admincli
+execute_robot_test scm spnego
+execute_robot_test scm snapshot/snapshot-acls.robot
+
+execute_robot_test scm httpfs
+
+# test replication
+docker-compose up -d --scale datanode=2
+execute_robot_test scm -v container:1 -v count:2 replication/wait.robot
+docker-compose up -d --scale datanode=3
+execute_robot_test scm -v container:1 -v count:3 replication/wait.robot

Review Comment:
   Let's limit tests to S3:
   
   ```
   execute_robot_test scm s3
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1176054302


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -754,8 +758,27 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
     }
     volumeManager = new VolumeManagerImpl(metadataManager);
     bucketManager = new BucketManagerImpl(metadataManager);
+
+    Class<? extends S3SecretStoreProvider> storeProviderClass =
+        configuration.getClass(
+            S3_SECRET_STORAGE_TYPE,
+            DEFAULT_SECRET_STORAGE_TYPE,
+            S3SecretStoreProvider.class);
+    S3SecretStore store;
+    try {
+      store = storeProviderClass == DEFAULT_SECRET_STORAGE_TYPE
+              ? metadataManagerImpl

Review Comment:
   I don't think it's our responsibility. Because in the general case, we simply cannot guarantee that this is possible. Let's imagine that in the future some 3 types of storage appear, the user configures it, but he also removes everything related to vault from the configuration and we simply lose access to the old storage.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] duongkame commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "duongkame (via GitHub)" <gi...@apache.org>.

duongkame commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1145189121


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/s3http/vault/auth/AuthType.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.hadoop.ozone.om.s3http.vault.auth;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys;
+
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_ID;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_PATH;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.APP_ROLE_SECRET;
+import static org.apache.hadoop.ozone.om.s3http.S3SecretStoreConfigurationKeys.AUTH_TYPE;
+
+/**
+ * Type of authentication method.
+ */
+public enum AuthType {
+  APP_ROLE("appRole"),
+  TOKEN("token");
+
+  private final String name;
+
+  AuthType(String name) {
+    this.name = name;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public static Auth fromConf(Configuration conf) {
+    AuthType authType = AuthType.valueOf(conf.get(AUTH_TYPE));
+    switch (authType) {
+    case TOKEN:
+      String token = conf.get(S3SecretStoreConfigurationKeys.TOKEN);

Review Comment:
   Yup, that's a good idea. Would be great if you create the ticket and put a link to that ticket in a code comment 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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1145921660


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SecretStoreProvider.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.hadoop.ozone.om;
+
+import org.apache.hadoop.conf.Configuration;
+
+import java.io.IOException;
+
+/**
+ * S3 secret store provider.
+ */
+public interface SecretStoreProvider {

Review Comment:
   After last patch yeah.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Pochatkin commented on a diff in pull request #4389: HDDS-7814. Remote S3 secret store

Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.

Pochatkin commented on code in PR #4389:
URL: https://github.com/apache/ozone/pull/4389#discussion_r1147594332


##########
hadoop-ozone/ozone-manager/pom.xml:
##########
@@ -69,6 +69,12 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>ozone-interface-storage</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.ozone</groupId>
+      <artifactId>ozone-s3-secret-store</artifactId>
+      <scope>compile</scope>
+    </dependency>

Review Comment:
   Done



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org