Skip to content

Commit

Permalink
feat(expressions): add option to disable retry for fromUrl expression…
Browse files Browse the repository at this point in the history 
… function (#4214)

Co-authored-by: Cameron Motevasselani <[email protected]>
  • Loading branch information
@cristhian-castaneda @link108
cristhian-castaneda and link108 authored Feb 25, 2022
1 parent 48aeecc commit 9586ec7
Show file tree
Hide file tree
Showing 8 changed files with 262 additions and 75 deletions.
1 change: 1 addition & 0 deletions orca-core/orca-core.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ dependencies {

testImplementation(project(":orca-test"))
testImplementation(project(":orca-test-groovy"))
testImplementation("com.github.tomakehurst:wiremock:2.15.0")
testImplementation("org.junit.jupiter:junit-jupiter-api")
testImplementation("org.junit.platform:junit-platform-runner")
testImplementation("org.assertj:assertj-core")
Expand Down
37 changes: 35 additions & 2 deletions orca-core/src/main/java/com/netflix/spinnaker/orca/config/UserConfiguredUrlRestrictions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;
import org.apache.commons.lang3.StringUtils;
import org.springframework.security.web.util.matcher.IpAddressMatcher;

public class UserConfiguredUrlRestrictions {
Expand All @@ -38,6 +41,7 @@ public static class Builder {
private List<String> allowedSchemes = new ArrayList<>(Arrays.asList("http", "https"));
private boolean rejectLocalhost = true;
private boolean rejectLinkLocal = true;
private HttpClientProperties httpClientProperties = new HttpClientProperties();
private List<String> rejectedIps =
new ArrayList<>(); // can contain IP addresses and/or IP ranges (CIDR block)

Expand Down Expand Up @@ -66,13 +70,19 @@ public Builder withRejectedIps(List<String> rejectedIpRanges) {
return this;
}

public Builder withHttpClientProperties(HttpClientProperties httpClientProperties) {
setHttpClientProperties(httpClientProperties);
return this;
}

public UserConfiguredUrlRestrictions build() {
return new UserConfiguredUrlRestrictions(
Pattern.compile(allowedHostnamesRegex),
allowedSchemes,
rejectLocalhost,
rejectLinkLocal,
rejectedIps);
rejectedIps,
httpClientProperties);
}
}

Expand All @@ -81,13 +91,15 @@ public UserConfiguredUrlRestrictions build() {
private final boolean rejectLocalhost;
private final boolean rejectLinkLocal;
private final Set<String> rejectedIps;
private final HttpClientProperties clientProperties;

public UserConfiguredUrlRestrictions(
Pattern allowedHostnames,
Collection<String> allowedSchemes,
boolean rejectLocalhost,
boolean rejectLinkLocal,
Collection<String> rejectedIps) {
Collection<String> rejectedIps,
HttpClientProperties clientProperties) {
this.allowedHostnames = allowedHostnames;
this.allowedSchemes =
allowedSchemes == null
Expand All @@ -99,6 +111,7 @@ public UserConfiguredUrlRestrictions(
rejectedIps == null
? Collections.emptySet()
: Collections.unmodifiableSet(new HashSet<>(rejectedIps));
this.clientProperties = clientProperties;
}

public URI validateURI(String url) throws IllegalArgumentException {
Expand Down Expand Up @@ -126,6 +139,11 @@ public URI validateURI(String url) throws IllegalArgumentException {
throw new IllegalArgumentException("Unable to determine host for the url provided " + url);
}

if (StringUtils.isBlank(allowedHostnames.pattern())) {
throw new IllegalArgumentException(
"Allowed Hostnames are not set, external HTTP requests are not enabled. Please configure 'user-configured-url-restrictions.allowedHostnamesRegex' in your orca config.");
}

if (!allowedHostnames.matcher(host).matches()) {
throw new IllegalArgumentException(
"Host not allowed " + host + ". Host much match " + allowedHostnames.toString() + ".");
Expand Down Expand Up @@ -175,4 +193,19 @@ public boolean isRejectLocalhost() {
public boolean isRejectLinkLocal() {
return rejectLinkLocal;
}

public HttpClientProperties getHttpClientProperties() {
return clientProperties;
}

@Data
@lombok.Builder
@NoArgsConstructor
@AllArgsConstructor
public static class HttpClientProperties {
@lombok.Builder.Default private boolean enableRetry = true;
@lombok.Builder.Default private int maxRetryAttempts = 1;
@lombok.Builder.Default private int retryInterval = 5000;
@lombok.Builder.Default private int timeoutMillis = 30000;
}
}
7 changes: 5 additions & 2 deletions .../netflix/spinnaker/orca/pipeline/expressions/functions/UrlExpressionFunctionProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@
@SuppressWarnings("unused")
@Component
public class UrlExpressionFunctionProvider implements ExpressionFunctionProvider {
private static AtomicReference<HttpClientUtils> httpClientUtils = new AtomicReference<>();
private static AtomicReference<UserConfiguredUrlRestrictions> helperFunctionUrlRestrictions =
new AtomicReference<>();
private static final ObjectMapper mapper = OrcaObjectMapper.getInstance();

public UrlExpressionFunctionProvider(UserConfiguredUrlRestrictions urlRestrictions) {
public UrlExpressionFunctionProvider(
UserConfiguredUrlRestrictions urlRestrictions, HttpClientUtils httpClient) {
helperFunctionUrlRestrictions.set(urlRestrictions);
httpClientUtils.set(httpClient);
}

@Nullable
Expand Down Expand Up @@ -195,7 +198,7 @@ static Map readProperties(String text) throws IOException {
public static String fromUrl(String url) {
try {
URL u = helperFunctionUrlRestrictions.get().validateURI(url).toURL();
return HttpClientUtils.httpGetAsString(u.toString());
return httpClientUtils.get().httpGetAsString(u.toString());
} catch (Exception e) {
throw new SpelHelperFunctionException(format("#from(%s) failed", url), e);
}
Expand Down
4 changes: 3 additions & 1 deletion ...ore/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ public ContextParameterProcessor() {
new DeployedServerGroupsExpressionFunctionProvider(),
new ManifestLabelValueExpressionFunctionProvider(),
new StageExpressionFunctionProvider(),
new UrlExpressionFunctionProvider(new UserConfiguredUrlRestrictions.Builder().build())),
new UrlExpressionFunctionProvider(
new UserConfiguredUrlRestrictions.Builder().build(),
new HttpClientUtils(new UserConfiguredUrlRestrictions.Builder().build()))),
new DefaultPluginManager(),
DynamicConfigService.NOOP);
}
Expand Down
150 changes: 81 additions & 69 deletions orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/HttpClientUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.io.CharStreams;
import com.google.common.util.concurrent.Uninterruptibles;
import com.netflix.spinnaker.orca.config.UserConfiguredUrlRestrictions;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
Expand All @@ -39,12 +40,13 @@
import org.apache.http.protocol.HttpCoreContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;

@Component
public class HttpClientUtils {

private CloseableHttpClient httpClient;
private static final Logger LOGGER = LoggerFactory.getLogger(HttpClientUtils.class);
private static final int MAX_RETRIES = 5;
private static final int RETRY_INTERVAL = 5000;
private static final int TIMEOUT_MILLIS = 30000;
private static final String JVM_HTTP_PROXY_HOST = "http.proxyHost";
private static final String JVM_HTTP_PROXY_PORT = "http.proxyPort";
private static List<Integer> RETRYABLE_500_HTTP_STATUS_CODES =
Expand All @@ -53,68 +55,78 @@ public class HttpClientUtils {
HttpStatus.SC_INTERNAL_SERVER_ERROR,
HttpStatus.SC_GATEWAY_TIMEOUT);

private static CloseableHttpClient httpClient = httpClientWithServiceUnavailableRetryStrategy();

private static CloseableHttpClient httpClientWithServiceUnavailableRetryStrategy() {
HttpClientBuilder httpClientBuilder =
HttpClients.custom()
.setServiceUnavailableRetryStrategy(
new ServiceUnavailableRetryStrategy() {
@Override
public boolean retryRequest(
HttpResponse response, int executionCount, HttpContext context) {
int statusCode = response.getStatusLine().getStatusCode();
HttpUriRequest currentReq =
(HttpUriRequest) context.getAttribute(HttpCoreContext.HTTP_REQUEST);
LOGGER.info("Response code {} for {}", statusCode, currentReq.getURI());

if (statusCode >= HttpStatus.SC_OK && statusCode <= 299) {
return false;
}

boolean shouldRetry =
(statusCode == 429 || RETRYABLE_500_HTTP_STATUS_CODES.contains(statusCode))
&& executionCount <= MAX_RETRIES;

if ((statusCode >= 300) && (statusCode <= 399)) {
throw new RetryRequestException(
String.format(
"Attempted redirect from %s to %s which is not supported",
currentReq.getURI(), response.getFirstHeader("LOCATION").getValue()));
}
if (!shouldRetry) {
throw new RetryRequestException(
String.format(
"Not retrying %s. Count %d, Max %d",
currentReq.getURI(), executionCount, MAX_RETRIES));
}

LOGGER.error(
"Retrying request on response status {}. Count {} Max is {}",
statusCode,
public HttpClientUtils(UserConfiguredUrlRestrictions userConfiguredUrlRestrictions) {
this.httpClient = create(userConfiguredUrlRestrictions.getHttpClientProperties());
}

private CloseableHttpClient create(
UserConfiguredUrlRestrictions.HttpClientProperties httpClientProperties) {
HttpClientBuilder httpClientBuilder = HttpClients.custom();

if (httpClientProperties.isEnableRetry()) {
httpClientBuilder.setServiceUnavailableRetryStrategy(
new ServiceUnavailableRetryStrategy() {
@Override
public boolean retryRequest(
HttpResponse response, int executionCount, HttpContext context) {
int statusCode = response.getStatusLine().getStatusCode();
HttpUriRequest currentReq =
(HttpUriRequest) context.getAttribute(HttpCoreContext.HTTP_REQUEST);
LOGGER.info("Response code {} for {}", statusCode, currentReq.getURI());

if (statusCode >= HttpStatus.SC_OK && statusCode <= 299) {
return false;
}

boolean shouldRetry =
(statusCode == 429 || RETRYABLE_500_HTTP_STATUS_CODES.contains(statusCode))
&& executionCount <= httpClientProperties.getMaxRetryAttempts();

if ((statusCode >= 300) && (statusCode <= 399)) {
throw new RetryRequestException(
String.format(
"Attempted redirect from %s to %s which is not supported",
currentReq.getURI(), response.getFirstHeader("LOCATION").getValue()));
}
if (!shouldRetry) {
throw new RetryRequestException(
String.format(
"Not retrying %s. Count %d, Max %d",
currentReq.getURI(),
executionCount,
MAX_RETRIES);
return true;
}

@Override
public long getRetryInterval() {
return RETRY_INTERVAL;
}
});

httpClientBuilder.setRetryHandler(
(exception, executionCount, context) -> {
HttpUriRequest currentReq =
(HttpUriRequest) context.getAttribute(HttpCoreContext.HTTP_REQUEST);
Uninterruptibles.sleepUninterruptibly(RETRY_INTERVAL, TimeUnit.MILLISECONDS);
LOGGER.info(
"Encountered network error. Retrying request {}, Count {} Max is {}",
currentReq.getURI(),
executionCount,
MAX_RETRIES);
return executionCount <= MAX_RETRIES;
});
httpClientProperties.getMaxRetryAttempts()));
}

LOGGER.error(
"Retrying request on response status {}. Count {} Max is {}",
statusCode,
executionCount,
httpClientProperties.getMaxRetryAttempts());
return true;
}

@Override
public long getRetryInterval() {
return httpClientProperties.getRetryInterval();
}
});

httpClientBuilder.setRetryHandler(
(exception, executionCount, context) -> {
HttpUriRequest currentReq =
(HttpUriRequest) context.getAttribute(HttpCoreContext.HTTP_REQUEST);
Uninterruptibles.sleepUninterruptibly(
httpClientProperties.getRetryInterval(), TimeUnit.MILLISECONDS);
LOGGER.info(
"Encountered network error. Retrying request {}, Count {} Max is {}",
currentReq.getURI(),
executionCount,
httpClientProperties.getMaxRetryAttempts());
return executionCount <= httpClientProperties.getMaxRetryAttempts();
});
} else {
httpClientBuilder.disableAutomaticRetries();
}

String proxyHostname = System.getProperty(JVM_HTTP_PROXY_HOST);
if (proxyHostname != null) {
Expand All @@ -132,23 +144,23 @@ public long getRetryInterval() {

httpClientBuilder.setDefaultRequestConfig(
RequestConfig.custom()
.setConnectionRequestTimeout(TIMEOUT_MILLIS)
.setConnectTimeout(TIMEOUT_MILLIS)
.setSocketTimeout(TIMEOUT_MILLIS)
.setConnectionRequestTimeout(httpClientProperties.getTimeoutMillis())
.setConnectTimeout(httpClientProperties.getTimeoutMillis())
.setSocketTimeout(httpClientProperties.getTimeoutMillis())
.build());

return httpClientBuilder.build();
}

public static String httpGetAsString(String url) throws IOException {
public String httpGetAsString(String url) throws IOException {
try (CloseableHttpResponse response = httpClient.execute(new HttpGet(url))) {
try (final Reader reader = new InputStreamReader(response.getEntity().getContent())) {
return CharStreams.toString(reader);
}
}
}

static class RetryRequestException extends RuntimeException {
class RetryRequestException extends RuntimeException {
RetryRequestException(String cause) {
super(cause);
}
Expand Down
21 changes: 21 additions & 0 deletions ...rc/test/groovy/com/netflix/spinnaker/orca/config/UserConfiguredUrlRestrictionsSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,25 @@ class UserConfiguredUrlRestrictionsSpec extends Specification {
uri << ['https://www.test.com', 'https://201.152.178.212', 'https://www.test_underscore.com']
}

@Unroll
def 'should verify allowedHostnamesRegex is set'() {
given:
UserConfiguredUrlRestrictions config = new UserConfiguredUrlRestrictions.Builder()
.withAllowedHostnamesRegex("")
.withRejectedIps([])
.withAllowedSchemes(['https'])
.withRejectLocalhost(false)
.withRejectLinkLocal(false)
.build()

when:
config.validateURI(uri)

then:
thrown(IllegalArgumentException.class)

where:
uri << ['https://www.test.com', 'https://201.152.178.212', 'https://www.test_underscore.com']
}

}
Loading

0 comments on commit 9586ec7

Please sign in to comment.