From 590d2df40fdb02d8f7973152777ee8bf2775473a Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 6 Nov 2020 20:37:43 -0800 Subject: [PATCH 1/8] feat(artifacts): add connection timeout for artifacts download. --- .../spinnaker/config/ArtifactConfiguration.java | 15 +++++++++++++-- .../config/ArtifactProviderProperties.java | 13 +++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java index 43aa9dff12a..0b5d30b9377 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java @@ -17,6 +17,9 @@ package com.netflix.spinnaker.config; import com.squareup.okhttp.OkHttpClient; +import java.util.concurrent.TimeUnit; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; @@ -25,13 +28,21 @@ import org.springframework.stereotype.Component; @Configuration -@EnableConfigurationProperties +@EnableConfigurationProperties(ArtifactProviderProperties.class) @EnableScheduling +@RequiredArgsConstructor @Component @ComponentScan("com.netflix.spinnaker.clouddriver.artifacts") +@Slf4j public class ArtifactConfiguration { + private final ArtifactProviderProperties properties; + @Bean OkHttpClient okHttpClient() { - return new OkHttpClient(); + log.info("Initializing okHttpClient for Artifact provider"); + OkHttpClient client = new OkHttpClient(); + client.setConnectTimeout(properties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); + client.setRetryOnConnectionFailure(true); + return client; } } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java new file mode 100644 index 00000000000..c7580a381c5 --- /dev/null +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java @@ -0,0 +1,13 @@ +package com.netflix.spinnaker.config; + +import lombok.Data; +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.context.properties.ConfigurationProperties; + +@Data +@ConfigurationProperties("artifacts") +@Slf4j +class ArtifactProviderProperties { + long connectTimeoutMs; + long readTimeoutMs; +} From 0b8eb4d0a783e74e011de38341b5457c5d5f17a5 Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 6 Nov 2020 20:45:39 -0800 Subject: [PATCH 2/8] feat(artifacts): add retryOnConnectionFailure attribute for artifacts configuration . ##Clouddriver artifacts downloader settings. artifacts: connectTimeoutMs: 1200000 readTimeoutMs: 1200000 retryOnConnectionFailure: true --- .../com/netflix/spinnaker/config/ArtifactConfiguration.java | 2 +- .../netflix/spinnaker/config/ArtifactProviderProperties.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java index 0b5d30b9377..88ed2b866db 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java @@ -42,7 +42,7 @@ OkHttpClient okHttpClient() { log.info("Initializing okHttpClient for Artifact provider"); OkHttpClient client = new OkHttpClient(); client.setConnectTimeout(properties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); - client.setRetryOnConnectionFailure(true); + client.setRetryOnConnectionFailure(properties.isRetryOnConnectionFailure()); return client; } } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java index c7580a381c5..848fbafb65f 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java @@ -10,4 +10,5 @@ class ArtifactProviderProperties { long connectTimeoutMs; long readTimeoutMs; + boolean retryOnConnectionFailure; } From 3abb491aa1e5116ad2084ee1496ede31e8d43a1d Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Tue, 10 Nov 2020 14:59:55 -0800 Subject: [PATCH 3/8] feat(artifacts): Provide an option to override the timeout config for github artifacts. --- .../github/GitHubArtifactConfiguration.java | 14 +++++++++++++- .../github/GitHubArtifactProviderProperties.java | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java index d63a97d5f1a..f7fa1b17696 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java @@ -21,6 +21,7 @@ import com.squareup.okhttp.OkHttpClient; import java.util.List; import java.util.Objects; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -44,7 +45,18 @@ List gitHubArtifactCredentials(OkHttpClient .map( a -> { try { - return new GitHubArtifactCredentials(a, okHttpClient, objectMapper); + if (gitHubArtifactProviderProperties.getConnectTimeoutMs() != 0) { + OkHttpClient extendedTimeoutClient = okHttpClient.clone(); + extendedTimeoutClient.setConnectTimeout( + gitHubArtifactProviderProperties.getConnectTimeoutMs(), + TimeUnit.MILLISECONDS); + extendedTimeoutClient.setReadTimeout( + gitHubArtifactProviderProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); + return new GitHubArtifactCredentials(a, extendedTimeoutClient, objectMapper); + } else { + return new GitHubArtifactCredentials(a, okHttpClient, objectMapper); + } + } catch (Exception e) { log.warn("Failure instantiating GitHub artifact account {}: ", a, e); return null; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactProviderProperties.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactProviderProperties.java index bc6ef4a3ac6..8e2403f21bd 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactProviderProperties.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactProviderProperties.java @@ -27,5 +27,7 @@ @ConfigurationProperties("artifacts.github") final class GitHubArtifactProviderProperties implements ArtifactProvider { private boolean enabled; + long connectTimeoutMs; + long readTimeoutMs; private List accounts = new ArrayList<>(); } From ef4e5ed2f7c240994087d18e31c758b8cce7b611 Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Thu, 12 Nov 2020 16:57:54 -0800 Subject: [PATCH 4/8] fix(artifacts): read the okhttp client properties from kork OkHttpClientConfigurationProperties --- .../clouddriver-artifacts.gradle | 2 ++ .../github/GitHubArtifactConfiguration.java | 19 ++++++++++----- .../config/ArtifactConfiguration.java | 24 +++++++++++++++---- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/clouddriver-artifacts/clouddriver-artifacts.gradle b/clouddriver-artifacts/clouddriver-artifacts.gradle index 8dd08ec15d3..30087c7f55d 100644 --- a/clouddriver-artifacts/clouddriver-artifacts.gradle +++ b/clouddriver-artifacts/clouddriver-artifacts.gradle @@ -14,6 +14,8 @@ dependencies { implementation "com.google.apis:google-api-services-storage:v1-rev141-1.25.0" implementation 'com.google.auth:google-auth-library-oauth2-http' implementation "com.netflix.frigga:frigga" + implementation "com.netflix.spinnaker.kork:kork-core" + implementation "com.netflix.spinnaker.kork:kork-web" implementation "com.netflix.spinnaker.kork:kork-artifacts" implementation "com.netflix.spinnaker.kork:kork-annotations" implementation "com.netflix.spinnaker.kork:kork-exceptions" diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java index f7fa1b17696..a106f610a43 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java @@ -45,13 +45,20 @@ List gitHubArtifactCredentials(OkHttpClient .map( a -> { try { - if (gitHubArtifactProviderProperties.getConnectTimeoutMs() != 0) { + if (gitHubArtifactProviderProperties.getConnectTimeoutMs() > 0 + || gitHubArtifactProviderProperties.getReadTimeoutMs() > 0) { + OkHttpClient extendedTimeoutClient = okHttpClient.clone(); - extendedTimeoutClient.setConnectTimeout( - gitHubArtifactProviderProperties.getConnectTimeoutMs(), - TimeUnit.MILLISECONDS); - extendedTimeoutClient.setReadTimeout( - gitHubArtifactProviderProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); + if (gitHubArtifactProviderProperties.getConnectTimeoutMs() > 0) { + extendedTimeoutClient.setConnectTimeout( + gitHubArtifactProviderProperties.getConnectTimeoutMs(), + TimeUnit.MILLISECONDS); + } + + if (gitHubArtifactProviderProperties.getReadTimeoutMs() > 0) { + extendedTimeoutClient.setReadTimeout( + gitHubArtifactProviderProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); + } return new GitHubArtifactCredentials(a, extendedTimeoutClient, objectMapper); } else { return new GitHubArtifactCredentials(a, okHttpClient, objectMapper); diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java index 88ed2b866db..8493a1af69b 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java @@ -16,6 +16,7 @@ package com.netflix.spinnaker.config; +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties; import com.squareup.okhttp.OkHttpClient; import java.util.concurrent.TimeUnit; import lombok.RequiredArgsConstructor; @@ -35,14 +36,29 @@ @ComponentScan("com.netflix.spinnaker.clouddriver.artifacts") @Slf4j public class ArtifactConfiguration { - private final ArtifactProviderProperties properties; + private final ArtifactProviderProperties providerProperties; @Bean - OkHttpClient okHttpClient() { + OkHttpClient okHttpClient( + OkHttpClientConfigurationProperties okHttpClientConfigurationProperties) { log.info("Initializing okHttpClient for Artifact provider"); + long connectionTimeout = + providerProperties.getConnectTimeoutMs() > 0 + ? providerProperties.getConnectTimeoutMs() + : okHttpClientConfigurationProperties.getConnectTimeoutMs(); + long readTimeout = + providerProperties.getReadTimeoutMs() > 0 + ? providerProperties.getReadTimeoutMs() + : okHttpClientConfigurationProperties.getReadTimeoutMs(); + boolean retryOnConnectionFailure = + providerProperties.isRetryOnConnectionFailure() + || okHttpClientConfigurationProperties.isRetryOnConnectionFailure(); + OkHttpClient client = new OkHttpClient(); - client.setConnectTimeout(properties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); - client.setRetryOnConnectionFailure(properties.isRetryOnConnectionFailure()); + client.setConnectTimeout(connectionTimeout, TimeUnit.MILLISECONDS); + client.setReadTimeout(readTimeout, TimeUnit.MILLISECONDS); + client.setRetryOnConnectionFailure(retryOnConnectionFailure); + return client; } } From d07b720908e8f2a8eca1a5e47a693fb7d999e1ff Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 13 Nov 2020 14:15:52 -0800 Subject: [PATCH 5/8] feat(artifacts): Update okhttp client to okhttp3 provided by kork-web. ArtifactConfiguration now doesnt need to provide OkHttpClient bean . Adding it as dependency for now. --- .../BitbucketArtifactConfiguration.java | 2 +- .../BitbucketArtifactCredentials.java | 2 +- .../config/BaseHttpArtifactCredentials.java | 7 +++- .../config/SimpleHttpArtifactCredentials.java | 4 +- .../github/GitHubArtifactConfiguration.java | 10 ++--- .../github/GitHubArtifactCredentials.java | 12 +++--- .../gitlab/GitlabArtifactConfiguration.java | 2 +- .../gitlab/GitlabArtifactCredentials.java | 6 +-- .../helm/HelmArtifactConfiguration.java | 2 +- .../helm/HelmArtifactCredentials.java | 9 +++-- .../http/HttpArtifactConfiguration.java | 2 +- .../http/HttpArtifactCredentials.java | 2 +- .../jenkins/JenkinsArtifactConfiguration.java | 2 +- .../jenkins/JenkinsArtifactCredentials.java | 4 +- .../config/ArtifactConfiguration.java | 39 +++++-------------- .../config/ArtifactProviderProperties.java | 14 ------- .../BitbucketArtifactCredentialsTest.java | 2 +- .../github/GithubArtifactCredentialsTest.java | 2 +- .../gitlab/GitlabArtifactCredentialsTest.java | 2 +- .../helm/HelmArtifactCredentialsTest.java | 2 +- .../http/HttpArtifactCredentialsTest.java | 2 +- 21 files changed, 50 insertions(+), 79 deletions(-) delete mode 100644 clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java index 0b3ca76456e..b6f7313aa6e 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java @@ -17,7 +17,7 @@ package com.netflix.spinnaker.clouddriver.artifacts.bitbucket; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java index 73fbf5bb85e..16c6d69566a 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java @@ -21,7 +21,7 @@ import com.netflix.spinnaker.clouddriver.artifacts.config.ArtifactCredentials; import com.netflix.spinnaker.clouddriver.artifacts.config.SimpleHttpArtifactCredentials; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import lombok.Getter; import lombok.extern.slf4j.Slf4j; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java index 3402874e966..aeaaf0cc967 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java @@ -17,10 +17,15 @@ package com.netflix.spinnaker.clouddriver.artifacts.config; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.squareup.okhttp.*; import java.io.IOException; import java.util.Optional; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; +import okhttp3.Headers; +import okhttp3.HttpUrl; +import okhttp3.ResponseBody; +import okhttp3.Request; +import okhttp3.Response; @Slf4j public abstract class BaseHttpArtifactCredentials { diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java index d01fb22ee85..0557af5a757 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java @@ -19,8 +19,8 @@ import com.netflix.spinnaker.clouddriver.artifacts.exceptions.FailedDownloadException; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.HttpUrl; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.*; + import java.io.IOException; import java.io.InputStream; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java index a106f610a43..73d9cc95ed3 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java @@ -18,13 +18,13 @@ package com.netflix.spinnaker.clouddriver.artifacts.github; import com.fasterxml.jackson.databind.ObjectMapper; -import com.squareup.okhttp.OkHttpClient; import java.util.List; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -48,18 +48,18 @@ List gitHubArtifactCredentials(OkHttpClient if (gitHubArtifactProviderProperties.getConnectTimeoutMs() > 0 || gitHubArtifactProviderProperties.getReadTimeoutMs() > 0) { - OkHttpClient extendedTimeoutClient = okHttpClient.clone(); + OkHttpClient.Builder extendedTimeoutClientBuilder = okHttpClient.newBuilder(); if (gitHubArtifactProviderProperties.getConnectTimeoutMs() > 0) { - extendedTimeoutClient.setConnectTimeout( + extendedTimeoutClientBuilder.connectTimeout( gitHubArtifactProviderProperties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); } if (gitHubArtifactProviderProperties.getReadTimeoutMs() > 0) { - extendedTimeoutClient.setReadTimeout( + extendedTimeoutClientBuilder.readTimeout( gitHubArtifactProviderProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); } - return new GitHubArtifactCredentials(a, extendedTimeoutClient, objectMapper); + return new GitHubArtifactCredentials(a, extendedTimeoutClientBuilder.build(), objectMapper); } else { return new GitHubArtifactCredentials(a, okHttpClient, objectMapper); } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java index 793c4a6739b..b781450d02a 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java @@ -27,14 +27,14 @@ import com.netflix.spinnaker.clouddriver.artifacts.exceptions.FailedDownloadException; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.HttpUrl; -import com.squareup.okhttp.OkHttpClient; -import com.squareup.okhttp.ResponseBody; -import java.io.IOException; -import javax.annotation.Nullable; import lombok.Data; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; +import okhttp3.HttpUrl; +import okhttp3.ResponseBody; +import javax.annotation.Nullable; +import java.io.IOException; @NonnullByDefault @Slf4j @@ -46,7 +46,7 @@ final class GitHubArtifactCredentials extends SimpleHttpArtifactCredentials 0 - ? providerProperties.getConnectTimeoutMs() - : okHttpClientConfigurationProperties.getConnectTimeoutMs(); - long readTimeout = - providerProperties.getReadTimeoutMs() > 0 - ? providerProperties.getReadTimeoutMs() - : okHttpClientConfigurationProperties.getReadTimeoutMs(); - boolean retryOnConnectionFailure = - providerProperties.isRetryOnConnectionFailure() - || okHttpClientConfigurationProperties.isRetryOnConnectionFailure(); - - OkHttpClient client = new OkHttpClient(); - client.setConnectTimeout(connectionTimeout, TimeUnit.MILLISECONDS); - client.setReadTimeout(readTimeout, TimeUnit.MILLISECONDS); - client.setRetryOnConnectionFailure(retryOnConnectionFailure); - - return client; + this.okHttpClient = okHttpClient; } } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java deleted file mode 100644 index 848fbafb65f..00000000000 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.netflix.spinnaker.config; - -import lombok.Data; -import lombok.extern.slf4j.Slf4j; -import org.springframework.boot.context.properties.ConfigurationProperties; - -@Data -@ConfigurationProperties("artifacts") -@Slf4j -class ArtifactProviderProperties { - long connectTimeoutMs; - long readTimeoutMs; - boolean retryOnConnectionFailure; -} diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java index 3b627701f4d..2633d254b2f 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java @@ -23,7 +23,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java index 624ec732403..a46942e53db 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java @@ -23,7 +23,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java index 4821db0be06..afef3412291 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java @@ -23,7 +23,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java index 01f1994fd7c..85a42fde04c 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java @@ -23,7 +23,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java index 8bc8376d307..e68381a4181 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java @@ -23,7 +23,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import com.squareup.okhttp.OkHttpClient; +import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; From 430389951bc4d61ae56530ac43280b451d3758f1 Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 13 Nov 2020 14:22:07 -0800 Subject: [PATCH 6/8] fix(artifacts): fixed formatting issue which was causing build failure --- .../bitbucket/BitbucketArtifactConfiguration.java | 2 +- .../artifacts/bitbucket/BitbucketArtifactCredentials.java | 2 +- .../artifacts/config/BaseHttpArtifactCredentials.java | 4 ++-- .../artifacts/config/SimpleHttpArtifactCredentials.java | 3 +-- .../artifacts/github/GitHubArtifactConfiguration.java | 3 ++- .../artifacts/github/GitHubArtifactCredentials.java | 8 ++++---- .../artifacts/gitlab/GitlabArtifactConfiguration.java | 2 +- .../artifacts/gitlab/GitlabArtifactCredentials.java | 6 +++--- .../artifacts/helm/HelmArtifactConfiguration.java | 2 +- .../artifacts/helm/HelmArtifactCredentials.java | 7 +++---- .../artifacts/jenkins/JenkinsArtifactConfiguration.java | 2 +- .../netflix/spinnaker/config/ArtifactConfiguration.java | 7 ++----- .../bitbucket/BitbucketArtifactCredentialsTest.java | 2 +- .../artifacts/github/GithubArtifactCredentialsTest.java | 2 +- .../artifacts/gitlab/GitlabArtifactCredentialsTest.java | 2 +- .../artifacts/helm/HelmArtifactCredentialsTest.java | 2 +- .../artifacts/http/HttpArtifactCredentialsTest.java | 2 +- 17 files changed, 27 insertions(+), 31 deletions(-) diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java index b6f7313aa6e..35d52d72715 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactConfiguration.java @@ -17,12 +17,12 @@ package com.netflix.spinnaker.clouddriver.artifacts.bitbucket; -import okhttp3.OkHttpClient; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java index 16c6d69566a..69e1e1a7144 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentials.java @@ -21,9 +21,9 @@ import com.netflix.spinnaker.clouddriver.artifacts.config.ArtifactCredentials; import com.netflix.spinnaker.clouddriver.artifacts.config.SimpleHttpArtifactCredentials; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; -import okhttp3.OkHttpClient; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; @NonnullByDefault @Slf4j diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java index aeaaf0cc967..c7192ed43f6 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/BaseHttpArtifactCredentials.java @@ -20,12 +20,12 @@ import java.io.IOException; import java.util.Optional; import lombok.extern.slf4j.Slf4j; -import okhttp3.OkHttpClient; import okhttp3.Headers; import okhttp3.HttpUrl; -import okhttp3.ResponseBody; +import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; +import okhttp3.ResponseBody; @Slf4j public abstract class BaseHttpArtifactCredentials { diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java index 0557af5a757..4bfc629fc78 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/config/SimpleHttpArtifactCredentials.java @@ -19,10 +19,9 @@ import com.netflix.spinnaker.clouddriver.artifacts.exceptions.FailedDownloadException; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.*; - import java.io.IOException; import java.io.InputStream; +import okhttp3.*; @NonnullByDefault public abstract class SimpleHttpArtifactCredentials diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java index 73d9cc95ed3..ab0e0fdc2b8 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactConfiguration.java @@ -59,7 +59,8 @@ List gitHubArtifactCredentials(OkHttpClient extendedTimeoutClientBuilder.readTimeout( gitHubArtifactProviderProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); } - return new GitHubArtifactCredentials(a, extendedTimeoutClientBuilder.build(), objectMapper); + return new GitHubArtifactCredentials( + a, extendedTimeoutClientBuilder.build(), objectMapper); } else { return new GitHubArtifactCredentials(a, okHttpClient, objectMapper); } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java index b781450d02a..aafacf83ecc 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/github/GitHubArtifactCredentials.java @@ -27,14 +27,14 @@ import com.netflix.spinnaker.clouddriver.artifacts.exceptions.FailedDownloadException; import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import java.io.IOException; +import javax.annotation.Nullable; import lombok.Data; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import okhttp3.OkHttpClient; import okhttp3.HttpUrl; +import okhttp3.OkHttpClient; import okhttp3.ResponseBody; -import javax.annotation.Nullable; -import java.io.IOException; @NonnullByDefault @Slf4j @@ -46,7 +46,7 @@ final class GitHubArtifactCredentials extends SimpleHttpArtifactCredentials diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/jenkins/JenkinsArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/jenkins/JenkinsArtifactConfiguration.java index 239ac5f35e3..76b43cc9ecb 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/jenkins/JenkinsArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/jenkins/JenkinsArtifactConfiguration.java @@ -16,12 +16,12 @@ package com.netflix.spinnaker.clouddriver.artifacts.jenkins; -import okhttp3.OkHttpClient; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import okhttp3.OkHttpClient; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java index 0ed1e19e6d1..513b79154a2 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java @@ -18,10 +18,8 @@ package com.netflix.spinnaker.config; import com.fasterxml.jackson.annotation.JsonIgnore; -import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import okhttp3.OkHttpClient; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.annotation.EnableScheduling; @@ -33,10 +31,9 @@ @ComponentScan("com.netflix.spinnaker.clouddriver.artifacts") @Slf4j public class ArtifactConfiguration { - @JsonIgnore - private final OkHttpClient okHttpClient; + @JsonIgnore private final OkHttpClient okHttpClient; - public ArtifactConfiguration(OkHttpClient okHttpClient){ + public ArtifactConfiguration(OkHttpClient okHttpClient) { log.info("Initializing okHttpClient for Artifact provider"); this.okHttpClient = okHttpClient; } diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java index 2633d254b2f..3197a7c44c1 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/bitbucket/BitbucketArtifactCredentialsTest.java @@ -23,12 +23,12 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.function.Function; +import okhttp3.OkHttpClient; import org.apache.commons.io.Charsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java index a46942e53db..bf31178bb5c 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/github/GithubArtifactCredentialsTest.java @@ -23,12 +23,12 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.function.Function; +import okhttp3.OkHttpClient; import org.apache.commons.io.Charsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java index afef3412291..e8db60dc508 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/gitlab/GitlabArtifactCredentialsTest.java @@ -23,12 +23,12 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.function.Function; +import okhttp3.OkHttpClient; import org.apache.commons.io.Charsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java index 85a42fde04c..d4535d6a11f 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/helm/HelmArtifactCredentialsTest.java @@ -23,13 +23,13 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; import java.util.function.Function; +import okhttp3.OkHttpClient; import org.apache.commons.io.Charsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java index e68381a4181..f6d70b59669 100644 --- a/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java +++ b/clouddriver-artifacts/src/test/java/com/netflix/spinnaker/clouddriver/artifacts/http/HttpArtifactCredentialsTest.java @@ -23,12 +23,12 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.MappingBuilder; import com.netflix.spinnaker.kork.artifacts.model.Artifact; -import okhttp3.OkHttpClient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.function.Function; +import okhttp3.OkHttpClient; import org.apache.commons.io.Charsets; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; From c360e995f0d7a75837a5ff0b4392c1ff068a5d83 Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 20 Nov 2020 18:05:50 -0800 Subject: [PATCH 7/8] feat(artifacts): add artifact provide global properties for http --- .../config/ArtifactConfiguration.java | 19 +++++++++--- .../config/ArtifactProviderProperties.java | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java index 513b79154a2..d63a947852c 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactConfiguration.java @@ -17,9 +17,11 @@ package com.netflix.spinnaker.config; -import com.fasterxml.jackson.annotation.JsonIgnore; +import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; import okhttp3.OkHttpClient; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.annotation.EnableScheduling; @@ -28,13 +30,20 @@ @Configuration @EnableScheduling @Component +@EnableConfigurationProperties(ArtifactProviderProperties.class) @ComponentScan("com.netflix.spinnaker.clouddriver.artifacts") @Slf4j public class ArtifactConfiguration { - @JsonIgnore private final OkHttpClient okHttpClient; - public ArtifactConfiguration(OkHttpClient okHttpClient) { - log.info("Initializing okHttpClient for Artifact provider"); - this.okHttpClient = okHttpClient; + @Bean + OkHttpClient okHttpClient( + OkHttp3ClientConfiguration okHttp3ClientConfiguration, + ArtifactProviderProperties properties) { + log.info("Initializing for Artifact provider okhttp client"); + OkHttpClient.Builder builder = okHttp3ClientConfiguration.create(); + builder.readTimeout(properties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); + builder.connectTimeout(properties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); + builder.retryOnConnectionFailure(properties.isRetryOnConnectionFailure()); + return builder.build(); } } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java new file mode 100644 index 00000000000..717d924a098 --- /dev/null +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/config/ArtifactProviderProperties.java @@ -0,0 +1,31 @@ +/* + * Copyright 2020 Netflix, Inc. + * + * 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.netflix.spinnaker.config; + +import lombok.Data; +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.context.properties.ConfigurationProperties; + +@Data +@ConfigurationProperties("artifacts") +@Slf4j +class ArtifactProviderProperties { + long connectTimeoutMs; + long readTimeoutMs; + boolean retryOnConnectionFailure = true; +} From fa0be3ce99aa84619fd27d2f5d68bb9d3c6d630f Mon Sep 17 00:00:00 2001 From: Ganesh Sapkal Date: Fri, 20 Nov 2020 18:07:45 -0800 Subject: [PATCH 8/8] feat(artifacts): added connection timeout for gitrepo account config --- .../gitRepo/GitRepoArtifactAccount.java | 6 +++++- .../gitRepo/GitRepoArtifactCredentials.java | 17 ++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactAccount.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactAccount.java index 1649d0d6231..19fc50aa623 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactAccount.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactAccount.java @@ -21,6 +21,7 @@ import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import javax.annotation.ParametersAreNullableByDefault; import lombok.Builder; +import lombok.Setter; import lombok.Value; import org.springframework.boot.context.properties.ConstructorBinding; @@ -35,6 +36,7 @@ final class GitRepoArtifactAccount implements ArtifactAccount { private final String sshPrivateKeyPassphrase; private final String sshKnownHostsFilePath; private final boolean sshTrustUnknownHosts; + @Setter private final int timeout; @Builder @ConstructorBinding @@ -47,7 +49,8 @@ public GitRepoArtifactAccount( String sshPrivateKeyFilePath, String sshPrivateKeyPassphrase, String sshKnownHostsFilePath, - boolean sshTrustUnknownHosts) { + boolean sshTrustUnknownHosts, + int timeout) { this.name = Strings.nullToEmpty(name); this.username = Strings.nullToEmpty(username); this.password = Strings.nullToEmpty(password); @@ -56,5 +59,6 @@ public GitRepoArtifactAccount( this.sshPrivateKeyPassphrase = Strings.nullToEmpty(sshPrivateKeyPassphrase); this.sshKnownHostsFilePath = Strings.nullToEmpty(sshKnownHostsFilePath); this.sshTrustUnknownHosts = sshTrustUnknownHosts; + this.timeout = timeout; } } diff --git a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactCredentials.java b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactCredentials.java index 7a80f43497f..d6973104b72 100644 --- a/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactCredentials.java +++ b/clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/gitRepo/GitRepoArtifactCredentials.java @@ -63,6 +63,7 @@ final class GitRepoArtifactCredentials implements ArtifactCredentials { private final String sshKnownHostsFilePath; private final boolean sshTrustUnknownHosts; private final AuthType authType; + private final int timeout; private enum AuthType { HTTP, @@ -80,6 +81,7 @@ public GitRepoArtifactCredentials(GitRepoArtifactAccount account) { this.sshPrivateKeyPassphrase = account.getSshPrivateKeyPassphrase(); this.sshKnownHostsFilePath = account.getSshKnownHostsFilePath(); this.sshTrustUnknownHosts = account.isSshTrustUnknownHosts(); + this.timeout = account.getTimeout(); if (!username.isEmpty() && !password.isEmpty()) { authType = AuthType.HTTP; @@ -126,11 +128,16 @@ public InputStream download(Artifact artifact) throws IOException { private Git clone(Artifact artifact, Path stagingPath, String remoteRef) throws GitAPIException { // TODO(ethanfrogers): add support for clone history depth once jgit supports it - return addAuthentication(Git.cloneRepository()) - .setURI(artifact.getReference()) - .setDirectory(stagingPath.toFile()) - .setBranch(remoteRef) - .call(); + CloneCommand cloneCommand = + addAuthentication(Git.cloneRepository()) + .setURI(artifact.getReference()) + .setDirectory(stagingPath.toFile()) + .setBranch(remoteRef); + + if (timeout > 0) { + cloneCommand.setTimeout(timeout); + } + return cloneCommand.call(); } private void archiveToOutputStream(