From 24b46853d4997cc734793a23cb025f93b84a179c Mon Sep 17 00:00:00 2001 From: kkasa Date: Wed, 31 Jan 2018 13:04:08 +0100 Subject: [PATCH 1/2] AMBARI-22886 Infra Manager: store s3 credentials in Hadoop credential store --- .../conf/security/CompositePasswordStore.java | 42 +++++++++ .../conf/security/HadoopCredentialStore.java | 53 +++++++++++ .../security/InfraManagerSecurityConfig.java | 38 ++++++++ .../infra/conf/security/PasswordStore.java | 23 +++++ .../conf/security/SecurityEnvironment.java | 26 ++++++ .../infra/job/AbstractJobsConfiguration.java | 18 ++-- .../ambari/infra/job/JobProperties.java | 2 +- .../DocumentArchivingConfiguration.java | 8 +- .../archive/DocumentArchivingProperties.java | 54 ++--------- .../ambari/infra/job/archive/S3AccessCsv.java | 93 +++++++++++++++++++ .../archive/S3AccessCsvFormatException.java | 25 +++++ .../infra/job/archive/S3AccessKeyNames.java | 40 ++++++++ .../infra/job/archive/S3Properties.java | 28 ++---- .../ambari/infra/job/archive/S3Uploader.java | 14 ++- .../security/CompositePasswordStoreTest.java | 47 ++++++++++ .../infra/job/archive/S3AccessCsvTest.java | 71 ++++++++++++++ 16 files changed, 504 insertions(+), 78 deletions(-) create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/InfraManagerSecurityConfig.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsvFormatException.java create mode 100644 ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessKeyNames.java create mode 100644 ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java create mode 100644 ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java new file mode 100644 index 00000000000..6fcd49ee0ad --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java @@ -0,0 +1,42 @@ +/* + * 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.ambari.infra.conf.security; + +import java.util.ArrayList; +import java.util.List; + +import static java.util.Arrays.asList; + +public class CompositePasswordStore implements PasswordStore { + private List passwordStores; + + public CompositePasswordStore(PasswordStore... passwordStores) { + this.passwordStores = new ArrayList<>(asList(passwordStores)); + } + + @Override + public String getPassword(String propertyName) { + for (PasswordStore passwordStore : passwordStores) { + String password = passwordStore.getPassword(propertyName); + if (password != null) + return password; + } + return null; + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java new file mode 100644 index 00000000000..0eaca4d416c --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java @@ -0,0 +1,53 @@ +/* + * 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.ambari.infra.conf.security; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.commons.lang.StringUtils.isBlank; +import static org.apache.commons.lang3.ArrayUtils.isNotEmpty; + +public class HadoopCredentialStore implements PasswordStore { + private static final Logger LOG = LoggerFactory.getLogger(InfraManagerSecurityConfig.class); + public static final String CREDENTIAL_STORE_PROVIDER_PATH_PROPERTY = "hadoop.security.credential.provider.path"; + + private final String credentialStoreProviderPath; + + public HadoopCredentialStore(String credentialStoreProviderPath) { + this.credentialStoreProviderPath = credentialStoreProviderPath; + } + + @Override + public String getPassword(String propertyName) { + try { + if (isBlank(credentialStoreProviderPath)) { + return null; + } + + org.apache.hadoop.conf.Configuration config = new org.apache.hadoop.conf.Configuration(); + config.set(CREDENTIAL_STORE_PROVIDER_PATH_PROPERTY, credentialStoreProviderPath); + char[] passwordChars = config.getPassword(propertyName); + return (isNotEmpty(passwordChars)) ? new String(passwordChars) : null; + } catch (Exception e) { + LOG.warn("Could not load password {} from credential store.", propertyName); + return null; + } + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/InfraManagerSecurityConfig.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/InfraManagerSecurityConfig.java new file mode 100644 index 00000000000..45b79b36695 --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/InfraManagerSecurityConfig.java @@ -0,0 +1,38 @@ +/* + * 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.ambari.infra.conf.security; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import static org.apache.ambari.infra.conf.security.HadoopCredentialStore.CREDENTIAL_STORE_PROVIDER_PATH_PROPERTY; + +@Configuration +public class InfraManagerSecurityConfig { + + @Value("${"+ CREDENTIAL_STORE_PROVIDER_PATH_PROPERTY + ":}") + private String credentialStoreProviderPath; + + + @Bean + public PasswordStore passwords() { + return new CompositePasswordStore(new HadoopCredentialStore(credentialStoreProviderPath), new SecurityEnvironment()); + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java new file mode 100644 index 00000000000..e0b65f6adf7 --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java @@ -0,0 +1,23 @@ +/* + * 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.ambari.infra.conf.security; + +public interface PasswordStore { + String getPassword(String propertyName); +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java new file mode 100644 index 00000000000..7e57a78af25 --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java @@ -0,0 +1,26 @@ +/* + * 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.ambari.infra.conf.security; + +public class SecurityEnvironment implements PasswordStore { + @Override + public String getPassword(String propertyName) { + return System.getenv(propertyName); + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/AbstractJobsConfiguration.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/AbstractJobsConfiguration.java index a57d0e01a7c..02a688560db 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/AbstractJobsConfiguration.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/AbstractJobsConfiguration.java @@ -50,16 +50,20 @@ public void registerJobs() { if (propertyMap == null) return; - for (String jobName : propertyMap.keySet()) - propertyMap.get(jobName).validate(jobName); - propertyMap.keySet().stream() .filter(key -> propertyMap.get(key).isEnabled()) .forEach(jobName -> { - LOG.info("Registering job {}", jobName); - JobBuilder jobBuilder = jobs.get(jobName).listener(new JobsPropertyMap<>(propertyMap)); - Job job = buildJob(jobBuilder); - jobRegistryBeanPostProcessor.postProcessAfterInitialization(job, jobName); + try { + propertyMap.get(jobName).validate(jobName); + LOG.info("Registering job {}", jobName); + JobBuilder jobBuilder = jobs.get(jobName).listener(new JobsPropertyMap<>(propertyMap)); + Job job = buildJob(jobBuilder); + jobRegistryBeanPostProcessor.postProcessAfterInitialization(job, jobName); + } + catch (Exception e) { + LOG.warn("Unable to register job " + jobName, e); + propertyMap.get(jobName).setEnabled(false); + } }); } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/JobProperties.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/JobProperties.java index 53909aee1ea..79406d017e5 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/JobProperties.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/JobProperties.java @@ -68,7 +68,7 @@ public void validate(String jobName) { validate(); } catch (Exception ex) { - throw new JobConfigurationException(String.format("Configuration of job %s is invalid!", jobName), ex); + throw new JobConfigurationException(String.format("Configuration of job %s is invalid: %s!", jobName, ex.getMessage()), ex); } } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingConfiguration.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingConfiguration.java index 89f94bdfe37..8358dd08a54 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingConfiguration.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingConfiguration.java @@ -19,6 +19,7 @@ package org.apache.ambari.infra.job.archive; import org.apache.ambari.infra.conf.InfraManagerDataConfig; +import org.apache.ambari.infra.conf.security.PasswordStore; import org.apache.ambari.infra.job.AbstractJobsConfiguration; import org.apache.ambari.infra.job.JobContextRepository; import org.apache.ambari.infra.job.JobScheduler; @@ -86,13 +87,16 @@ public DocumentExporter documentExporter(DocumentItemReader documentItemReader, InfraManagerDataConfig infraManagerDataConfig, @Value("#{jobParameters[end]}") String intervalEnd, DocumentWiper documentWiper, - JobContextRepository jobContextRepository) { + JobContextRepository jobContextRepository, + PasswordStore passwordStore) { File baseDir = new File(infraManagerDataConfig.getDataFolder(), "exporting"); CompositeFileAction fileAction = new CompositeFileAction(new TarGzCompressor()); switch (properties.getDestination()) { case S3: - fileAction.add(new S3Uploader(properties.s3Properties().orElseThrow(() -> new IllegalStateException("S3 properties are not provided!")))); + fileAction.add(new S3Uploader( + properties.s3Properties().orElseThrow(() -> new IllegalStateException("S3 properties are not provided!")), + passwordStore)); break; case HDFS: org.apache.hadoop.conf.Configuration conf = new org.apache.hadoop.conf.Configuration(); diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingProperties.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingProperties.java index f8fa33b641f..b26da3656c8 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingProperties.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/DocumentArchivingProperties.java @@ -19,23 +19,14 @@ package org.apache.ambari.infra.job.archive; import org.apache.ambari.infra.job.JobProperties; -import org.apache.commons.csv.CSVParser; -import org.apache.commons.csv.CSVRecord; import org.springframework.batch.core.JobParameters; -import java.io.FileReader; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.Iterator; -import java.util.Map; import java.util.Optional; -import java.util.function.Supplier; import static java.util.Objects.requireNonNull; import static org.apache.ambari.infra.job.archive.ExportDestination.HDFS; import static org.apache.ambari.infra.job.archive.ExportDestination.LOCAL; import static org.apache.ambari.infra.job.archive.ExportDestination.S3; -import static org.apache.commons.csv.CSVFormat.DEFAULT; import static org.apache.commons.lang.StringUtils.isBlank; public class DocumentArchivingProperties extends JobProperties { @@ -50,47 +41,12 @@ public class DocumentArchivingProperties extends JobProperties> s3Properties; private String hdfsEndpoint; private String hdfsDestinationDirectory; public DocumentArchivingProperties() { super(DocumentArchivingProperties.class); - s3Properties = this::loadS3Properties; - } - - private Optional loadS3Properties() { - if (isBlank(s3BucketName)) - return Optional.empty(); - - String accessKey = System.getenv("AWS_ACCESS_KEY_ID"); - String secretKey = System.getenv("AWS_SECRET_ACCESS_KEY"); - - if (isBlank(accessKey) || isBlank(secretKey)) { - if (isBlank(s3AccessFile)) - return Optional.empty(); - try (CSVParser csvParser = CSVParser.parse(new FileReader(s3AccessFile), DEFAULT.withHeader("Access key ID", "Secret access key"))) { - Iterator iterator = csvParser.iterator(); - if (!iterator.hasNext()) { - return Optional.empty(); - } - - CSVRecord record = csvParser.iterator().next(); - Map header = csvParser.getHeaderMap(); - accessKey = record.get(header.get("Access key ID")); - secretKey = record.get(header.get("Secret access key")); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - return Optional.of(new S3Properties( - accessKey, - secretKey, - s3KeyPrefix, - s3BucketName, - s3Endpoint)); } public int getReadBlockSize() { @@ -155,7 +111,6 @@ public String getS3AccessFile() { public void setS3AccessFile(String s3AccessFile) { this.s3AccessFile = s3AccessFile; - s3Properties = this::loadS3Properties; } public String getS3KeyPrefix() { @@ -183,7 +138,14 @@ public void setS3Endpoint(String s3Endpoint) { } public Optional s3Properties() { - return s3Properties.get(); + if (isBlank(s3BucketName)) + return Optional.empty(); + + return Optional.of(new S3Properties( + s3AccessFile, + s3KeyPrefix, + s3BucketName, + s3Endpoint)); } public String getHdfsEndpoint() { diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java new file mode 100644 index 00000000000..4017c6e96ba --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java @@ -0,0 +1,93 @@ +/* + * 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.ambari.infra.job.archive; + +import org.apache.ambari.infra.conf.security.PasswordStore; +import org.apache.commons.csv.CSVParser; +import org.apache.commons.csv.CSVRecord; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.FileNotFoundException; +import java.io.FileReader; +import java.io.IOException; +import java.io.Reader; +import java.io.UncheckedIOException; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + +import static org.apache.commons.csv.CSVFormat.DEFAULT; + +public class S3AccessCsv implements PasswordStore { + private static final Logger LOG = LoggerFactory.getLogger(S3AccessCsv.class); + + public static S3AccessCsv file(String path) { + try { + return new S3AccessCsv(new FileReader(path)); + } catch (FileNotFoundException e) { + throw new UncheckedIOException(e); + } + } + + private Map passwordMap = new HashMap<>(); + + public S3AccessCsv(Reader reader) { + try (CSVParser csvParser = CSVParser.parse(reader, DEFAULT.withHeader( + S3AccessKeyNames.AccessKeyId.getCsvName(), S3AccessKeyNames.SecretAccessKey.getCsvName()))) { + Iterator iterator = csvParser.iterator(); + if (!iterator.hasNext()) { + throw new S3AccessCsvFormatException("Csv file is empty!"); + } + + CSVRecord record = iterator.next(); + if (record.size() < 2) { + throw new S3AccessCsvFormatException("Csv file contains less than 2 columns!"); + } + + checkColumnExists(record, S3AccessKeyNames.AccessKeyId); + checkColumnExists(record, S3AccessKeyNames.SecretAccessKey); + + if (!iterator.hasNext()) { + throw new S3AccessCsvFormatException("Csv file contains header only!"); + } + + record = iterator.next(); + + Map header = csvParser.getHeaderMap(); + for (S3AccessKeyNames keyNames : S3AccessKeyNames.values()) + passwordMap.put(keyNames.getEnvVariableName(), record.get(header.get(keyNames.getCsvName()))); + } catch (IOException e) { + throw new UncheckedIOException(e); + } catch (S3AccessCsvFormatException e) { + LOG.warn("Unable to parse csv file: {}", e.getMessage()); + } + } + + private void checkColumnExists(CSVRecord record, S3AccessKeyNames s3AccessKeyName) { + if (!s3AccessKeyName.getCsvName().equals(record.get(s3AccessKeyName.getCsvName()))) { + throw new S3AccessCsvFormatException(String.format("Csv file does not contain the required column: '%s'", s3AccessKeyName.getCsvName())); + } + } + + @Override + public String getPassword(String propertyName) { + return passwordMap.get(propertyName); + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsvFormatException.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsvFormatException.java new file mode 100644 index 00000000000..ef9d53918fb --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsvFormatException.java @@ -0,0 +1,25 @@ +/* + * 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.ambari.infra.job.archive; + +public class S3AccessCsvFormatException extends RuntimeException { + public S3AccessCsvFormatException(String message) { + super(message); + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessKeyNames.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessKeyNames.java new file mode 100644 index 00000000000..e840d3b329b --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessKeyNames.java @@ -0,0 +1,40 @@ +/* + * 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.ambari.infra.job.archive; + +public enum S3AccessKeyNames { + AccessKeyId("AWS_ACCESS_KEY_ID", "Access key ID"), + SecretAccessKey("AWS_SECRET_ACCESS_KEY", "Secret access key"); + + private final String envVariableName; + private final String csvName; + + S3AccessKeyNames(String envVariableName, String csvName) { + this.envVariableName = envVariableName; + this.csvName = csvName; + } + + public String getEnvVariableName() { + return envVariableName; + } + + public String getCsvName() { + return csvName; + } +} diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Properties.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Properties.java index 88b71cf5294..59a4469e945 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Properties.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Properties.java @@ -21,28 +21,18 @@ import static org.apache.commons.lang.StringUtils.isBlank; public class S3Properties { - private final String s3AccessKey; - private final String s3SecretKey; + private final String s3AccessFile; private final String s3KeyPrefix; private final String s3BucketName; private final String s3EndPoint; - public S3Properties(String s3AccessKey, String s3SecretKey, String s3KeyPrefix, String s3BucketName, String s3EndPoint) { - this.s3AccessKey = s3AccessKey; - this.s3SecretKey = s3SecretKey; + public S3Properties(String s3AccessFile, String s3KeyPrefix, String s3BucketName, String s3EndPoint) { + this.s3AccessFile = s3AccessFile; this.s3KeyPrefix = s3KeyPrefix; this.s3BucketName = s3BucketName; this.s3EndPoint = s3EndPoint; } - public String getS3AccessKey() { - return s3AccessKey; - } - - public String getS3SecretKey() { - return s3SecretKey; - } - public String getS3KeyPrefix() { return s3KeyPrefix; } @@ -55,10 +45,14 @@ public String getS3EndPoint() { return s3EndPoint; } + public String getS3AccessFile() { + return s3AccessFile; + } + @Override public String toString() { return "S3Properties{" + - "s3AccessKey='" + s3AccessKey + '\'' + + "s3AccessFile='" + s3AccessFile + '\'' + ", s3KeyPrefix='" + s3KeyPrefix + '\'' + ", s3BucketName='" + s3BucketName + '\'' + ", s3EndPoint='" + s3EndPoint + '\'' + @@ -66,12 +60,6 @@ public String toString() { } public void validate() { - if (isBlank(s3AccessKey)) - throw new IllegalArgumentException("The property s3AccessKey can not be null or empty string!"); - - if (isBlank(s3SecretKey)) - throw new IllegalArgumentException("The property s3SecretKey can not be null or empty string!"); - if (isBlank(s3BucketName)) throw new IllegalArgumentException("The property s3BucketName can not be null or empty string!"); } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java index 0ab68edaeae..ef303ec9b2d 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java @@ -2,12 +2,15 @@ import com.amazonaws.auth.BasicAWSCredentials; import com.amazonaws.services.s3.AmazonS3Client; +import org.apache.ambari.infra.conf.security.CompositePasswordStore; +import org.apache.ambari.infra.conf.security.PasswordStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.File; import static org.apache.commons.lang.StringUtils.isBlank; +import static org.apache.commons.lang.StringUtils.isNotBlank; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -35,12 +38,19 @@ public class S3Uploader extends AbstractFileAction { private final String keyPrefix; private final String bucketName; - public S3Uploader(S3Properties s3Properties) { + public S3Uploader(S3Properties s3Properties, PasswordStore passwordStore) { LOG.info("Initializing S3 client with " + s3Properties); this.keyPrefix = s3Properties.getS3KeyPrefix(); this.bucketName = s3Properties.getS3BucketName(); - BasicAWSCredentials credentials = new BasicAWSCredentials(s3Properties.getS3AccessKey(), s3Properties.getS3SecretKey()); + + PasswordStore compositePasswordStore = passwordStore; + if (isNotBlank((s3Properties.getS3AccessFile()))) + compositePasswordStore = new CompositePasswordStore(passwordStore, S3AccessCsv.file(s3Properties.getS3AccessFile())); + + BasicAWSCredentials credentials = new BasicAWSCredentials( + compositePasswordStore.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), + compositePasswordStore.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName())); client = new AmazonS3Client(credentials); if (!isBlank(s3Properties.getS3EndPoint())) client.setEndpoint(s3Properties.getS3EndPoint()); diff --git a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java new file mode 100644 index 00000000000..b4d8100b2f9 --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java @@ -0,0 +1,47 @@ +package org.apache.ambari.infra.conf.security; + +import org.junit.Test; + +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertThat; + +/* + * 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. + */ +public class CompositePasswordStoreTest { + @Test + public void testGetPasswordReturnNullIfNoPasswordStoresWereAdded() { + assertThat(new CompositePasswordStore().getPassword("any"), is(nullValue())); + } + + @Test + public void testGetPasswordReturnNullIfPasswordNotFoundInAnyStore() { + assertThat(new CompositePasswordStore((prop) -> null, (prop) -> null).getPassword("any"), is(nullValue())); + } + + @Test + public void testGetPasswordReturnPasswordFromFistStoreIfExists() { + assertThat(new CompositePasswordStore((prop) -> "Pass", (prop) -> null).getPassword("any"), is("Pass")); + } + + @Test + public void testGetPasswordReturnPasswordFromSecondStoreIfNotExistsInFirst() { + assertThat(new CompositePasswordStore((prop) -> null, (prop) -> "Pass").getPassword("any"), is("Pass")); + } +} \ No newline at end of file diff --git a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java new file mode 100644 index 00000000000..8091a6e717a --- /dev/null +++ b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java @@ -0,0 +1,71 @@ +package org.apache.ambari.infra.job.archive; + +import org.junit.Test; + +import java.io.StringReader; + +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertThat; + +/* + * 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. + */ +public class S3AccessCsvTest { + + private static final String VALID_ACCESS_FILE = "Access key ID,Secret access key\n" + + "someKey,someSecret\n"; + + private static final String ANY_CSV_FILE = "Column1,Column2\n" + + "Foo,Bar\n"; + + @Test + public void testGetPasswordReturnsNullIfInputIsEmpty() { + S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + } + + @Test + public void testGetPasswordReturnsAccessAndSecretKeyIfInputIsAValidS3AccessFile() { + S3AccessCsv accessCsv = new S3AccessCsv(new StringReader(VALID_ACCESS_FILE)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is("someKey")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is("someSecret")); + } + + @Test + public void testGetPasswordReturnsNullIfNotAValidS3AccessFileProvided() { + S3AccessCsv accessCsv = new S3AccessCsv(new StringReader(ANY_CSV_FILE)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + } + + @Test + public void testGetPasswordReturnsNullIfAHeaderOnlyS3AccessFileProvided() { + S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("Access key ID,Secret access key\n")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + } + + @Test + public void testGetPasswordReturnsNullIfOnlyOneValidColumnProvided() { + S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("Access key ID,Column\n")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + } +} \ No newline at end of file From 501b8ae2749df1e875a5eae5445346d988ef2f30 Mon Sep 17 00:00:00 2001 From: kkasa Date: Tue, 6 Feb 2018 09:36:56 +0100 Subject: [PATCH 2/2] AMBARI-22886 Infra Manager: store s3 credentials in Hadoop credential store - addendum: use Optional as a return type of getPassword, remove unnecessary ArrayList --- .../conf/security/CompositePasswordStore.java | 19 +++++++---------- .../conf/security/HadoopCredentialStore.java | 10 +++++---- .../infra/conf/security/PasswordStore.java | 4 +++- .../conf/security/SecurityEnvironment.java | 6 ++++-- .../ambari/infra/job/archive/S3AccessCsv.java | 5 +++-- .../ambari/infra/job/archive/S3Uploader.java | 6 ++++-- .../security/CompositePasswordStoreTest.java | 13 ++++++------ .../infra/job/archive/S3AccessCsvTest.java | 21 +++++++++---------- 8 files changed, 45 insertions(+), 39 deletions(-) diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java index 6fcd49ee0ad..6d32963ecc3 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/CompositePasswordStore.java @@ -18,25 +18,22 @@ */ package org.apache.ambari.infra.conf.security; -import java.util.ArrayList; -import java.util.List; - -import static java.util.Arrays.asList; +import java.util.Optional; public class CompositePasswordStore implements PasswordStore { - private List passwordStores; + private PasswordStore[] passwordStores; public CompositePasswordStore(PasswordStore... passwordStores) { - this.passwordStores = new ArrayList<>(asList(passwordStores)); + this.passwordStores = passwordStores; } @Override - public String getPassword(String propertyName) { + public Optional getPassword(String propertyName) { for (PasswordStore passwordStore : passwordStores) { - String password = passwordStore.getPassword(propertyName); - if (password != null) - return password; + Optional optionalPassword = passwordStore.getPassword(propertyName); + if (optionalPassword.isPresent()) + return optionalPassword; } - return null; + return Optional.empty(); } } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java index 0eaca4d416c..9e1a17f8a06 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/HadoopCredentialStore.java @@ -21,6 +21,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Optional; + import static org.apache.commons.lang.StringUtils.isBlank; import static org.apache.commons.lang3.ArrayUtils.isNotEmpty; @@ -35,19 +37,19 @@ public HadoopCredentialStore(String credentialStoreProviderPath) { } @Override - public String getPassword(String propertyName) { + public Optional getPassword(String propertyName) { try { if (isBlank(credentialStoreProviderPath)) { - return null; + return Optional.empty(); } org.apache.hadoop.conf.Configuration config = new org.apache.hadoop.conf.Configuration(); config.set(CREDENTIAL_STORE_PROVIDER_PATH_PROPERTY, credentialStoreProviderPath); char[] passwordChars = config.getPassword(propertyName); - return (isNotEmpty(passwordChars)) ? new String(passwordChars) : null; + return (isNotEmpty(passwordChars)) ? Optional.of(new String(passwordChars)) : Optional.empty(); } catch (Exception e) { LOG.warn("Could not load password {} from credential store.", propertyName); - return null; + return Optional.empty(); } } } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java index e0b65f6adf7..19848feac86 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/PasswordStore.java @@ -18,6 +18,8 @@ */ package org.apache.ambari.infra.conf.security; +import java.util.Optional; + public interface PasswordStore { - String getPassword(String propertyName); + Optional getPassword(String propertyName); } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java index 7e57a78af25..8e3387b4f51 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/conf/security/SecurityEnvironment.java @@ -18,9 +18,11 @@ */ package org.apache.ambari.infra.conf.security; +import java.util.Optional; + public class SecurityEnvironment implements PasswordStore { @Override - public String getPassword(String propertyName) { - return System.getenv(propertyName); + public Optional getPassword(String propertyName) { + return Optional.ofNullable(System.getenv(propertyName)); } } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java index 4017c6e96ba..879b58bc8f2 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3AccessCsv.java @@ -32,6 +32,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.Optional; import static org.apache.commons.csv.CSVFormat.DEFAULT; @@ -87,7 +88,7 @@ private void checkColumnExists(CSVRecord record, S3AccessKeyNames s3AccessKeyNam } @Override - public String getPassword(String propertyName) { - return passwordMap.get(propertyName); + public Optional getPassword(String propertyName) { + return Optional.ofNullable(passwordMap.get(propertyName)); } } diff --git a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java index ef303ec9b2d..2536cb5dfac 100644 --- a/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java +++ b/ambari-infra/ambari-infra-manager/src/main/java/org/apache/ambari/infra/job/archive/S3Uploader.java @@ -49,8 +49,10 @@ public S3Uploader(S3Properties s3Properties, PasswordStore passwordStore) { compositePasswordStore = new CompositePasswordStore(passwordStore, S3AccessCsv.file(s3Properties.getS3AccessFile())); BasicAWSCredentials credentials = new BasicAWSCredentials( - compositePasswordStore.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), - compositePasswordStore.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName())); + compositePasswordStore.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()) + .orElseThrow(() -> new IllegalArgumentException("Access key Id is not present!")), + compositePasswordStore.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()) + .orElseThrow(() -> new IllegalArgumentException("Secret Access Key is not present!"))); client = new AmazonS3Client(credentials); if (!isBlank(s3Properties.getS3EndPoint())) client.setEndpoint(s3Properties.getS3EndPoint()); diff --git a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java index b4d8100b2f9..26a6953d54a 100644 --- a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java +++ b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/conf/security/CompositePasswordStoreTest.java @@ -2,7 +2,8 @@ import org.junit.Test; -import static org.hamcrest.Matchers.nullValue; +import java.util.Optional; + import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; @@ -27,21 +28,21 @@ public class CompositePasswordStoreTest { @Test public void testGetPasswordReturnNullIfNoPasswordStoresWereAdded() { - assertThat(new CompositePasswordStore().getPassword("any"), is(nullValue())); + assertThat(new CompositePasswordStore().getPassword("any").isPresent(), is(false)); } @Test public void testGetPasswordReturnNullIfPasswordNotFoundInAnyStore() { - assertThat(new CompositePasswordStore((prop) -> null, (prop) -> null).getPassword("any"), is(nullValue())); + assertThat(new CompositePasswordStore((prop) -> Optional.empty(), (prop) -> Optional.empty()).getPassword("any").isPresent(), is(false)); } @Test - public void testGetPasswordReturnPasswordFromFistStoreIfExists() { - assertThat(new CompositePasswordStore((prop) -> "Pass", (prop) -> null).getPassword("any"), is("Pass")); + public void testGetPasswordReturnPasswordFromFirstStoreIfExists() { + assertThat(new CompositePasswordStore((prop) -> Optional.of("Pass"), (prop) -> Optional.empty()).getPassword("any").get(), is("Pass")); } @Test public void testGetPasswordReturnPasswordFromSecondStoreIfNotExistsInFirst() { - assertThat(new CompositePasswordStore((prop) -> null, (prop) -> "Pass").getPassword("any"), is("Pass")); + assertThat(new CompositePasswordStore((prop) -> Optional.empty(), (prop) -> Optional.of("Pass")).getPassword("any").get(), is("Pass")); } } \ No newline at end of file diff --git a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java index 8091a6e717a..e34a222cd70 100644 --- a/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java +++ b/ambari-infra/ambari-infra-manager/src/test/java/org/apache/ambari/infra/job/archive/S3AccessCsvTest.java @@ -4,7 +4,6 @@ import java.io.StringReader; -import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; @@ -37,35 +36,35 @@ public class S3AccessCsvTest { @Test public void testGetPasswordReturnsNullIfInputIsEmpty() { S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("")); - assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); - assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()).isPresent(), is(false)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()).isPresent(), is(false)); } @Test public void testGetPasswordReturnsAccessAndSecretKeyIfInputIsAValidS3AccessFile() { S3AccessCsv accessCsv = new S3AccessCsv(new StringReader(VALID_ACCESS_FILE)); - assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is("someKey")); - assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is("someSecret")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()).get(), is("someKey")); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()).get(), is("someSecret")); } @Test public void testGetPasswordReturnsNullIfNotAValidS3AccessFileProvided() { S3AccessCsv accessCsv = new S3AccessCsv(new StringReader(ANY_CSV_FILE)); - assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); - assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()).isPresent(), is(false)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()).isPresent(), is(false)); } @Test public void testGetPasswordReturnsNullIfAHeaderOnlyS3AccessFileProvided() { S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("Access key ID,Secret access key\n")); - assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); - assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()).isPresent(), is(false)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()).isPresent(), is(false)); } @Test public void testGetPasswordReturnsNullIfOnlyOneValidColumnProvided() { S3AccessCsv accessCsv = new S3AccessCsv(new StringReader("Access key ID,Column\n")); - assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()), is(nullValue())); - assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()), is(nullValue())); + assertThat(accessCsv.getPassword(S3AccessKeyNames.AccessKeyId.getEnvVariableName()).isPresent(), is(false)); + assertThat(accessCsv.getPassword(S3AccessKeyNames.SecretAccessKey.getEnvVariableName()).isPresent(), is(false)); } } \ No newline at end of file