Skip to content

Commit

Permalink
Feature - gkatzioura#38 - Deprecated the PublicReadProperty in favour…
Browse files Browse the repository at this point in the history
… of the CannedAccessControlListProperty (cannedAcl), which offers a superset of functionality to the PublicReadProperty. For backwards compatibility, CannedAccessControlListProperty is preferred, but falls back to PublicReadProperty if unset.
  • Loading branch information
dancockerill committed Mar 4, 2019
1 parent 490837c commit 319f035
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright 2018 Emmanouil Gkatziouras
*
* 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.
*/

package com.gkatzioura.maven.cloud.s3;

import java.util.logging.Logger;

import com.amazonaws.services.s3.model.CannedAccessControlList;

public class CannedAccessControlListProperty {

private static final Logger LOGGER = Logger.getLogger(CannedAccessControlListProperty.class.getName());

private static final String CANNED_ACL_PROP_TAG = "cannedAcl";
private static final String CANNED_ACL_ENV_TAG = "CANNED_ACL";

private static final CannedAccessControlList DEFAULT_CANNED_ACL = CannedAccessControlList.Private;

private final CannedAccessControlList cannedAccessControlList;
@Deprecated
private final PublicReadProperty publicReadProperty;

public CannedAccessControlListProperty() {
this((CannedAccessControlList) null, new PublicReadProperty(null));
}

/**
* @param cannedAccessControlList may be null
* @param publicReadProperty may not be null
*/
public CannedAccessControlListProperty(String cannedAccessControlList, PublicReadProperty publicReadProperty) {
this(lenientValueOf(cannedAccessControlList), publicReadProperty);
}

private CannedAccessControlListProperty(CannedAccessControlList cannedAccessControlList, PublicReadProperty publicReadProperty) {
this.cannedAccessControlList = cannedAccessControlList;
this.publicReadProperty = publicReadProperty;
}

/**
* return the cannedAccessControlList set in the constructor or the cannedAcl set using the system property or environment variable, else the now deprecated {@link PublicReadProperty}, or finally the default value
*/
public CannedAccessControlList get() {
if (this.cannedAccessControlList != null) {
return this.cannedAccessControlList;
}

String cannedAccessControlListProp = System.getProperty(CANNED_ACL_PROP_TAG);
if (cannedAccessControlListProp != null) {
return lenientValueOf(cannedAccessControlListProp);
}

String cannedAccessControlListEnv = System.getenv(CANNED_ACL_ENV_TAG);
if (cannedAccessControlListEnv != null) {
return lenientValueOf(cannedAccessControlListEnv);
}

if (publicReadProperty.get()) {
LOGGER.warning("Public read was set to true, so setting cannedAcl to PublicRead. This feature will be removed in future in favour of the cannedAcl property");
return CannedAccessControlList.PublicRead;
}

return DEFAULT_CANNED_ACL;
}

private static CannedAccessControlList lenientValueOf(String stringValue) {
if (stringValue == null) {
return null;
} else {
for (CannedAccessControlList cannedAcl : CannedAccessControlList.values()) {
if (cannedAcl.name().equalsIgnoreCase(stringValue) || cannedAcl.toString().equalsIgnoreCase(stringValue)) {
return cannedAcl;
}
}
}
LOGGER.warning("Could not find a cannedAcl value with name: " + stringValue + " so returning the default value of: " + DEFAULT_CANNED_ACL);
return DEFAULT_CANNED_ACL;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package com.gkatzioura.maven.cloud.s3;

/**
* Deprecated in favour of {@link CannedAccessControlListProperty}. This functionality wil be removed in a future version.
*/
@Deprecated
public class PublicReadProperty {

private static final String PUBLIC_REPOSITORY_PROP_TAG = "publicRepository";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,17 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import com.amazonaws.SdkClientException;
import com.amazonaws.client.builder.AwsClientBuilder;
import org.apache.commons.io.IOUtils;
import org.apache.maven.wagon.ResourceDoesNotExistException;
import org.apache.maven.wagon.TransferFailedException;
import org.apache.maven.wagon.authentication.AuthenticationException;
import org.apache.maven.wagon.authentication.AuthenticationInfo;

import com.amazonaws.SdkClientException;
import com.amazonaws.client.builder.AwsClientBuilder;
import com.amazonaws.services.s3.AmazonS3;
import com.amazonaws.services.s3.AmazonS3ClientBuilder;
import com.amazonaws.services.s3.model.AmazonS3Exception;
import com.amazonaws.services.s3.model.CannedAccessControlList;
import com.amazonaws.services.s3.model.ListObjectsRequest;
import com.amazonaws.services.s3.model.ObjectListing;
import com.amazonaws.services.s3.model.ObjectMetadata;
Expand All @@ -51,36 +50,30 @@ public class S3StorageRepository {

private final String bucket;
private final String baseDirectory;
private final CannedAccessControlListProperty cannedAclProperty;

private final KeyResolver keyResolver = new KeyResolver();

private AmazonS3 amazonS3;
private PublicReadProperty publicReadProperty;

private static final Logger LOGGER = Logger.getLogger(S3StorageRepository.class.getName());

public S3StorageRepository(String bucket) {
this.bucket = bucket;
this.baseDirectory = "";
this.publicReadProperty = new PublicReadProperty(false);
this(bucket, "", new CannedAccessControlListProperty());
}

public S3StorageRepository(String bucket, PublicReadProperty publicReadProperty) {
this.bucket = bucket;
this.baseDirectory = "";
this.publicReadProperty = publicReadProperty;
public S3StorageRepository(String bucket, CannedAccessControlListProperty cannedAclProperty) {
this(bucket, "", cannedAclProperty);
}

public S3StorageRepository(String bucket, String baseDirectory) {
this.bucket = bucket;
this.baseDirectory = baseDirectory;
this.publicReadProperty = new PublicReadProperty(false);
this(bucket, baseDirectory, new CannedAccessControlListProperty());
}

public S3StorageRepository(String bucket, String baseDirectory, PublicReadProperty publicReadProperty) {
public S3StorageRepository(String bucket, String baseDirectory, CannedAccessControlListProperty cannedAclProperty) {
this.bucket = bucket;
this.baseDirectory = baseDirectory;
this.publicReadProperty = publicReadProperty;
this.cannedAclProperty = cannedAclProperty;
}


Expand Down Expand Up @@ -154,7 +147,7 @@ public void put(File file, String destination,TransferProgress transferProgress)
try {
try(InputStream inputStream = new TransferProgressFileInputStream(file,transferProgress)) {
PutObjectRequest putObjectRequest = new PutObjectRequest(bucket,key,inputStream,new ObjectMetadata());
applyPublicRead(putObjectRequest);
putObjectRequest.withCannedAcl(cannedAclProperty.get());
amazonS3.putObject(putObjectRequest);
}
} catch (AmazonS3Exception | IOException e) {
Expand Down Expand Up @@ -193,13 +186,6 @@ public List<String> list(String path) {
return objects;
}

private void applyPublicRead(PutObjectRequest putObjectRequest) {
if(publicReadProperty.get()) {
LOGGER.info("Public read was set to true");
putObjectRequest.withCannedAcl(CannedAccessControlList.PublicRead);
}
}

private void retrieveAllObjects(ObjectListing objectListing, List<String> objects) {

objectListing.getObjectSummaries().forEach( os-> objects.add(os.getKey()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
package com.gkatzioura.maven.cloud.s3;

import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
Expand All @@ -28,7 +25,6 @@
import java.util.logging.Logger;
import java.util.stream.Collectors;

import com.gkatzioura.maven.cloud.resolver.KeyResolver;
import org.apache.commons.io.FileUtils;
import org.apache.maven.wagon.ConnectionException;
import org.apache.maven.wagon.PathUtils;
Expand All @@ -43,20 +39,23 @@
import org.apache.maven.wagon.resource.Resource;

import com.amazonaws.services.s3.model.AmazonS3Exception;
import com.gkatzioura.maven.cloud.resolver.KeyResolver;
import com.gkatzioura.maven.cloud.transfer.TransferProgress;
import com.gkatzioura.maven.cloud.transfer.TransferProgressImpl;
import com.gkatzioura.maven.cloud.wagon.AbstractStorageWagon;

public class S3StorageWagon extends AbstractStorageWagon {

private static final Logger LOGGER = Logger.getLogger(S3StorageWagon.class.getName());

private S3StorageRepository s3StorageRepository;
private final KeyResolver keyResolver = new KeyResolver();


private String region;
private String cannedAcl;
@Deprecated
private Boolean publicRepository;

private static final Logger LOGGER = Logger.getLogger(S3StorageWagon.class.getName());
private String endpoint;
private String pathStyleEnabled;

Expand Down Expand Up @@ -183,7 +182,7 @@ public void connect(Repository repository, AuthenticationInfo authenticationInfo
final String directory = containerResolver.resolve(repository);

LOGGER.log(Level.FINER,String.format("Opening connection for bucket %s and directory %s",bucket,directory));
s3StorageRepository = new S3StorageRepository(bucket, directory, new PublicReadProperty(publicRepository));
s3StorageRepository = new S3StorageRepository(bucket, directory, new CannedAccessControlListProperty(this.cannedAcl, new PublicReadProperty(publicRepository)));
s3StorageRepository.connect(authenticationInfo, new RegionProperty(region), new EndpointProperty(endpoint), new PathStyleEnabledProperty(pathStyleEnabled));

sessionListenerContainer.fireSessionLoggedIn();
Expand All @@ -206,14 +205,24 @@ public void setRegion(String region) {
this.region = region;
}

@Deprecated
public Boolean getPublicRepository() {
return publicRepository;
}

@Deprecated
public void setPublicRepository(Boolean publicRepository) {
this.publicRepository = publicRepository;
}

public String getCannedAcl() {
return cannedAcl;
}

public void setCannedAcl(String cannedAcl) {
this.cannedAcl = cannedAcl;
}

public String getEndpoint() {
return endpoint;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.gkatzioura.maven.cloud.s3;

import static com.amazonaws.services.s3.model.CannedAccessControlList.AuthenticatedRead;
import static com.amazonaws.services.s3.model.CannedAccessControlList.AwsExecRead;
import static com.amazonaws.services.s3.model.CannedAccessControlList.BucketOwnerFullControl;
import static com.amazonaws.services.s3.model.CannedAccessControlList.BucketOwnerRead;
import static com.amazonaws.services.s3.model.CannedAccessControlList.Private;
import static com.amazonaws.services.s3.model.CannedAccessControlList.PublicRead;
import static org.junit.Assert.assertEquals;

import org.junit.Ignore;
import org.junit.Test;

public class CannedAccessControlListPropertyTest {

private static final PublicReadProperty PUBLIC_READ_TRUE = new PublicReadProperty(true);

@Test
public void defaultConstructorReturnsDefaultValue() {
assertEquals(Private, new CannedAccessControlListProperty().get());
}

@Test
public void cannedAclNameValue() {
assertEquals(AuthenticatedRead, new CannedAccessControlListProperty("AuthenticatedRead", PUBLIC_READ_TRUE).get());
}

@Test
public void cannedAclHeaderValue() {
assertEquals(AwsExecRead, new CannedAccessControlListProperty("aws-exec-read", PUBLIC_READ_TRUE).get());
}

@Test
public void nullCannedAclReturnsSystemPropertyValue() {
System.setProperty("cannedAcl", "BucketOwnerRead");
assertEquals(BucketOwnerRead, new CannedAccessControlListProperty().get());
}

@Ignore
@Test
public void nullCannedAclReturnsEnvironmentVariableValue() {
// TODO: work out how to test environment variables for test
// System.setenv("CANNED_ACL", "BucketOwnerFullControl");
assertEquals(BucketOwnerFullControl, new CannedAccessControlListProperty().get());
}

@Test
public void nullCannedAclFallsBackToPublicReadProperty() {
assertEquals(PublicRead, new CannedAccessControlListProperty(null, PUBLIC_READ_TRUE).get());
}

@Test
public void nonsenseValueSetsToDefault() {
assertEquals(Private, new CannedAccessControlListProperty("nonsense", PUBLIC_READ_TRUE).get());
}
}

0 comments on commit 319f035

Please sign in to comment.