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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
