Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>5.2102.v5f5fe09fccf1</version>
<version>6.2116.v7501b_67dc517</version>
<relativePath />
</parent>
<groupId>io.jenkins.plugins</groupId>
Expand All @@ -22,6 +22,7 @@
<spotbugs.threshold>Low</spotbugs.threshold>
<hpi.strictBundledArtifacts>true</hpi.strictBundledArtifacts>
<hpi.bundledArtifacts>aws-s3,guice-assistedinject,jakarta.ws.rs-api,jclouds-blobstore,jclouds-core,s3,sts,tika-core</hpi.bundledArtifacts>
<ban-commons-lang-2.skip>false</ban-commons-lang-2.skip>
</properties>

<name>Artifact Manager on S3 plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import edu.umd.cs.findbugs.annotations.NonNull;

import jenkins.security.FIPS140;
import org.apache.commons.lang.StringUtils;
import org.jclouds.ContextBuilder;
import org.jclouds.aws.domain.SessionCredentials;
import org.jclouds.aws.s3.AWSS3ProviderMetadata;
Expand Down Expand Up @@ -124,9 +123,9 @@
ProviderRegistry.registerProvider(AWSS3ProviderMetadata.builder().build());
try {
Properties props = new Properties();
boolean hasCustomEndpoint = StringUtils.isNotBlank(getConfiguration().getResolvedCustomEndpoint());
boolean hasCustomEndpoint = !(getConfiguration().getResolvedCustomEndpoint() != null && getConfiguration().getResolvedCustomEndpoint().isBlank());

Check warning on line 126 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 126 is only partially covered, 2 branches are missing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, what? This is equivalent to

Suggested change
boolean hasCustomEndpoint = !(getConfiguration().getResolvedCustomEndpoint() != null && getConfiguration().getResolvedCustomEndpoint().isBlank());
boolean hasCustomEndpoint = getConfiguration().getResolvedCustomEndpoint() == null || !getConfiguration().getResolvedCustomEndpoint().isBlank();

A null value was clearly supposed to be no endpoint.


if(StringUtils.isNotBlank(getRegion())) {
if(!getRegion().isBlank()) {

Check warning on line 128 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 128 is only partially covered, one branch is missing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also an incorrect translation of the original code, though in this case the original code does not seem to have made any sense; from my reading, it could never have been null or blank.

props.setProperty(LocationConstants.PROPERTY_REGIONS, getRegion());
}
if (hasCustomEndpoint) {
Expand Down Expand Up @@ -219,15 +218,15 @@
.region(getConfiguration().getRegion())
.credentialsProvider(CredentialsAwsGlobalConfiguration.get().getCredentials())
.s3Client(s3Client);
if (StringUtils.isNotBlank(customEndpoint)) {
if (customEndpoint != null && !customEndpoint.isBlank()) {

Check warning on line 221 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 221 is only partially covered, 2 branches are missing
presignerBuilder.endpointOverride(URI.create(customEndpoint));
}

String customRegion = getConfiguration().getCustomSigningRegion();
if(StringUtils.isBlank(customRegion)) {
if(customRegion.isBlank()) {

Check warning on line 226 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 226 is only partially covered, one branch is missing
customRegion = getConfiguration().getRegion().id();
}
if(StringUtils.isNotBlank(customRegion)) {
if(!customRegion.isBlank()) {

Check warning on line 229 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 229 is only partially covered, one branch is missing
presignerBuilder.region(Region.of(customRegion));
}
Comment on lines 226 to 232

Expand Down Expand Up @@ -313,7 +312,7 @@
* @return true if a container is configured.
*/
public boolean isConfigured(){
return StringUtils.isNotBlank(S3BlobStoreConfig.get().getContainer());
return !S3BlobStoreConfig.get().getContainer().isBlank();

Check warning on line 315 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 315 is only partially covered, one branch is missing
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.regex.Pattern;

import jenkins.util.SystemProperties;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;
Expand Down Expand Up @@ -254,7 +253,7 @@
}

public String getResolvedCustomEndpoint() {
if(StringUtils.isNotBlank(customEndpoint)) {
if(!customEndpoint.isBlank()) {

Check warning on line 256 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 256 is only partially covered, one branch is missing
String protocol;
if(getUseHttp()) {
protocol = "http";
Expand Down Expand Up @@ -295,13 +294,14 @@
S3ClientBuilder getAmazonS3ClientBuilder() throws URISyntaxException {
S3ClientBuilder ret = S3Client.builder();

if (StringUtils.isNotBlank(getResolvedCustomEndpoint())) {
String resolvedCustomEndpoint = getResolvedCustomEndpoint();
if (resolvedCustomEndpoint != null && !resolvedCustomEndpoint.isBlank()) {

Check warning on line 298 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 298 is only partially covered, 2 branches are missing
String resolvedCustomSigningRegion = customSigningRegion;
if (StringUtils.isBlank(resolvedCustomSigningRegion)) {
if (resolvedCustomSigningRegion.isBlank()) {

Check warning on line 300 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 300 is only partially covered, one branch is missing
// we must revert to a region if no custom defined
resolvedCustomSigningRegion = getRegion().id();
}
ret = ret.endpointOverride(new URI(getResolvedCustomEndpoint())).region(Region.of(resolvedCustomSigningRegion));
ret = ret.endpointOverride(new URI(resolvedCustomEndpoint)).region(Region.of(resolvedCustomSigningRegion));
} else {
// not really sure of why this was used.. this should have a dedicated parameter
//ret = ret.useArnRegion(true);
Expand Down Expand Up @@ -358,7 +358,7 @@

public FormValidation doCheckContainer(@QueryParameter String container){
FormValidation ret = FormValidation.ok();
if (StringUtils.isBlank(container)){
if (container.isBlank()){
ret = FormValidation.warning("The container name cannot be empty");
} else if (!bucketPattern.matcher(container).matches()){
ret = FormValidation.error("The S3 Bucket name does not match S3 bucket rules");
Expand All @@ -368,7 +368,7 @@

public FormValidation doCheckPrefix(@QueryParameter String prefix){
FormValidation ret;
if (StringUtils.isBlank(prefix)) {
if (prefix.isBlank()) {
ret = FormValidation.ok("Artifacts will be stored in the root folder of the S3 Bucket.");
} else if (prefix.endsWith("/")) {
ret = FormValidation.ok();
Expand All @@ -380,7 +380,7 @@

public FormValidation doCheckCustomSigningRegion(@QueryParameter String customSigningRegion) {
FormValidation ret;
if (StringUtils.isBlank(customSigningRegion) && StringUtils.isNotBlank(customEndpoint)) {
if (customSigningRegion != null && customSigningRegion.isBlank() && (customEndpoint == null || !customEndpoint.isBlank())) {

Check warning on line 383 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 383 is only partially covered, 2 branches are missing
ret = FormValidation.ok("'us-east-1' will be used when a custom endpoint is configured and custom signing region is blank.");
} else {
ret = FormValidation.ok();
Expand All @@ -390,7 +390,7 @@

public FormValidation doCheckCustomEndpoint(@QueryParameter String customEndpoint) {
FormValidation ret = FormValidation.ok();
if (!StringUtils.isBlank(customEndpoint) && !endPointPattern.matcher(customEndpoint).matches()) {
if (!customEndpoint.isBlank() && !endPointPattern.matcher(customEndpoint).matches()) {
ret = FormValidation.error("Custom Endpoint may not be valid.");
}
return ret;
Expand Down Expand Up @@ -441,44 +441,44 @@
createS3Bucket(container, disableSessionToken);
} catch (Throwable t){
String msg = processExceptionMessage(t);
ret = FormValidation.error(StringUtils.abbreviate(msg, 200));
ret = FormValidation.error(msg == null || msg.length() <= 200 ? msg : msg.substring(0, 200 - 3) + "...");
}
return ret;
}

void checkGetBucketLocation(String container, boolean disableSessionToken) throws IOException, URISyntaxException {
S3ClientBuilder builder = getAmazonS3ClientBuilderWithCredentials(disableSessionToken);
try (S3Client client = builder.build()) {
client.getBucketLocation(GetBucketLocationRequest.builder().bucket(container).build());
}
}

@RequirePOST
public FormValidation doValidateS3BucketConfig(
@QueryParameter String container,
@QueryParameter String prefix,
@QueryParameter boolean useHttp,
@QueryParameter boolean useTransferAcceleration,
@QueryParameter boolean usePathStyleUrl,
@QueryParameter boolean disableSessionToken,
@QueryParameter String customEndpoint,
@QueryParameter String customSigningRegion) {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);

if (FIPS140.useCompliantAlgorithms() && useHttp) {
return FormValidation.warning("Validation failed as \"use Insecure Http\" flag is enabled while in FIPS mode");
}
FormValidation ret = FormValidation.ok("success");
S3BlobStore provider = new S3BlobStoreTester(container, prefix,
useHttp, useTransferAcceleration,usePathStyleUrl,
disableSessionToken, customEndpoint, customSigningRegion);

try {
JCloudsVirtualFile jc = new JCloudsVirtualFile(provider, container, prefix.replaceFirst("/$", ""));
jc.list();
} catch (Throwable t){
String msg = processExceptionMessage(t);
ret = FormValidation.error(t, StringUtils.abbreviate(msg, 200));
ret = FormValidation.error(t, msg == null || msg.length() <= 200 ? msg : msg.substring(0, 200 - 3) + "...");

Check warning on line 481 in src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 444-481 are not covered by tests
}
try {
provider.getConfiguration().checkGetBucketLocation(container, disableSessionToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.cloudbees.plugins.credentials.domains.Domain;
import io.jenkins.plugins.aws.global_configuration.CredentialsAwsGlobalConfiguration;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.jclouds.aws.domain.Region;
import org.junit.AfterClass;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -84,7 +83,7 @@ public static void shutDownClass() {
config.setUseHttp(true);
config.setUsePathStyleUrl(true);
config.setDisableSessionToken(true);
config.setCustomSigningRegion(StringUtils.isBlank(region) ? Region.US_EAST_1.toLowerCase(Locale.US) : region);
config.setCustomSigningRegion(region == null || region.isBlank() ? Region.US_EAST_1.toLowerCase(Locale.US) : region);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@

import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.UUID;
import java.util.logging.Level;

import org.apache.commons.lang.RandomStringUtils;
import org.jclouds.blobstore.BlobStore;
import org.jclouds.blobstore.BlobStoreContext;
import org.junit.After;
Expand Down Expand Up @@ -92,7 +92,7 @@ public static String getContainer() {
*/
public static String generateUniquePrefix() {
return String.format("%s%s-%s/", S3_DIR, ZonedDateTime.now().format(DateTimeFormatter.ISO_INSTANT),
RandomStringUtils.randomAlphabetic(4).toLowerCase());
UUID.randomUUID().toString().substring(0, 4).toLowerCase());
}

protected String getPrefix() {
Expand Down
Loading