From 8a7e165214fa4e97b881de0e0e90edf57d2c86ab Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 24 Jun 2016 16:03:59 -0600 Subject: [PATCH] Split ProxyRequestHelper to function without Actuator. Previously ProxyRequestHelper contained a TraceRepository field. This caused zuul apps to fail with a class not found exception if they excluded actuator. This splits TraceRepository functionality into a new TraceProxyRequestHelper that extends ProxyRequestHelper. Auto configuration creates the appropriate ProxyRequestHelper based on the existence or not of actuator classes. fixes gh-1135 --- .../netflix/zuul/ZuulProxyConfiguration.java | 43 ++++--- .../zuul/filters/ProxyRequestHelper.java | 74 +---------- .../zuul/filters/TraceProxyRequestHelper.java | 117 ++++++++++++++++++ .../zuul/filters/ProxyRequestHelperTests.java | 4 +- 4 files changed, 153 insertions(+), 85 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 9a6c43a3..e9a2a3ad 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -21,6 +21,7 @@ import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.trace.TraceRepository; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.discovery.DiscoveryClient; @@ -31,6 +32,7 @@ import org.springframework.cloud.client.discovery.event.ParentHeartbeatEvent; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; +import org.springframework.cloud.netflix.zuul.filters.TraceProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; import org.springframework.cloud.netflix.zuul.filters.discovery.ServiceRouteMapper; @@ -55,9 +57,6 @@ import org.springframework.context.annotation.Configuration; @Configuration public class ZuulProxyConfiguration extends ZuulConfiguration { - @Autowired(required = false) - private TraceRepository traces; - @Autowired private DiscoveryClient discovery; @@ -132,17 +131,6 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { return new SimpleHostRoutingFilter(helper, zuulProperties); } - @Bean - public ProxyRequestHelper proxyRequestHelper() { - ProxyRequestHelper helper = new ProxyRequestHelper(); - if (this.traces != null) { - helper.setTraces(this.traces); - } - helper.setIgnoredHeaders(this.zuulProperties.getIgnoredHeaders()); - helper.setTraceRequestBody(this.zuulProperties.isTraceRequestBody()); - return helper; - } - @Bean public ApplicationListener zuulDiscoveryRefreshRoutesListener() { return new ZuulDiscoveryRefreshListener(); @@ -154,15 +142,42 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { return new SimpleServiceRouteMapper(); } + @Configuration + @ConditionalOnMissingClass("org.springframework.boot.actuate.endpoint.Endpoint") + protected static class NoActuatorConfiguration { + + @Bean + public ProxyRequestHelper proxyRequestHelper(ZuulProperties zuulProperties) { + ProxyRequestHelper helper = new ProxyRequestHelper(); + helper.setIgnoredHeaders(zuulProperties.getIgnoredHeaders()); + helper.setTraceRequestBody(zuulProperties.isTraceRequestBody()); + return helper; + } + + } + @Configuration @ConditionalOnClass(Endpoint.class) protected static class RoutesEndpointConfiguration { + @Autowired(required = false) + private TraceRepository traces; + @Bean public RoutesEndpoint zuulEndpoint(RouteLocator routeLocator) { return new RoutesEndpoint(routeLocator); } + @Bean + public ProxyRequestHelper proxyRequestHelper(ZuulProperties zuulProperties) { + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); + if (this.traces != null) { + helper.setTraces(this.traces); + } + helper.setIgnoredHeaders(zuulProperties.getIgnoredHeaders()); + helper.setTraceRequestBody(zuulProperties.isTraceRequestBody()); + return helper; + } } private static class ZuulDiscoveryRefreshListener diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index 7e5e10d9..0f99a99e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -16,10 +16,11 @@ package org.springframework.cloud.netflix.zuul.filters; +import static org.springframework.http.HttpHeaders.CONTENT_ENCODING; +import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; + import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.Charset; import java.util.Collection; import java.util.Enumeration; import java.util.HashMap; @@ -33,7 +34,6 @@ import java.util.Set; import javax.servlet.http.HttpServletRequest; -import org.springframework.boot.actuate.trace.TraceRepository; import org.springframework.cloud.netflix.zuul.util.RequestUtils; import org.springframework.http.HttpHeaders; import org.springframework.util.LinkedMultiValueMap; @@ -45,14 +45,12 @@ import org.springframework.web.util.WebUtils; import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.util.HTTPRequestUtils; -import static org.springframework.http.HttpHeaders.CONTENT_ENCODING; -import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; - import lombok.extern.apachecommons.CommonsLog; /** * @author Dave Syer * @author Marcos Barbero + * @author Spencer Gibb */ @CommonsLog public class ProxyRequestHelper { @@ -63,8 +61,6 @@ public class ProxyRequestHelper { */ public static final String IGNORED_HEADERS = "ignoredHeaders"; - private TraceRepository traces; - private Set ignoredHeaders = new LinkedHashSet<>(); private Set sensitiveHeaders = new LinkedHashSet<>(); @@ -85,10 +81,6 @@ public class ProxyRequestHelper { this.ignoredHeaders.addAll(ignoredHeaders); } - public void setTraces(TraceRepository traces) { - this.traces = traces; - } - public void setTraceRequestBody(boolean traceRequestBody) { this.traceRequestBody = traceRequestBody; } @@ -238,32 +230,10 @@ public class ProxyRequestHelper { MultiValueMap headers, MultiValueMap params, InputStream requestEntity) throws IOException { Map info = new LinkedHashMap<>(); - if (this.traces != null) { - RequestContext context = RequestContext.getCurrentContext(); - info.put("method", verb); - info.put("path", uri); - info.put("query", getQueryString(params)); - info.put("remote", true); - info.put("proxy", context.get("proxy")); - Map trace = new LinkedHashMap<>(); - Map input = new LinkedHashMap<>(); - trace.put("request", input); - info.put("headers", trace); - transformHeaders(headers, input); - RequestContext ctx = RequestContext.getCurrentContext(); - if (shouldDebugBody(ctx)) { - // Prevent input stream from being read if it needs to go downstream - if (requestEntity != null) { - debugRequestEntity(info, ctx.getRequest().getInputStream()); - } - } - this.traces.add(info); - return info; - } return info; } - /* for tests */ boolean shouldDebugBody(RequestContext ctx) { + protected boolean shouldDebugBody(RequestContext ctx) { HttpServletRequest request = ctx.getRequest(); if (!this.traceRequestBody || ctx.isChunkedRequestBody() || RequestUtils.isZuulServletRequest()) { @@ -277,40 +247,6 @@ public class ProxyRequestHelper { public void appendDebug(Map info, int status, MultiValueMap headers) { - if (this.traces != null) { - @SuppressWarnings("unchecked") - Map trace = (Map) info.get("headers"); - Map output = new LinkedHashMap<>(); - trace.put("response", output); - transformHeaders(headers, output); - output.put("status", "" + status); - } - } - - void transformHeaders(MultiValueMap headers, Map output) { - for (Entry> key : headers.entrySet()) { - Collection collection = key.getValue(); - Object value = collection; - if (collection.size() < 2) { - value = collection.isEmpty() ? "" : collection.iterator().next(); - } - output.put(key.getKey(), value); - } - } - - private void debugRequestEntity(Map info, InputStream inputStream) - throws IOException { - if (RequestContext.getCurrentContext().isChunkedRequestBody()) { - info.put("body", ""); - return; - } - char[] buffer = new char[4096]; - int count = new InputStreamReader(inputStream, Charset.forName("UTF-8")) - .read(buffer, 0, buffer.length); - if (count > 0) { - String entity = new String(buffer).substring(0, count); - info.put("body", entity.length() < 4096 ? entity : entity + ""); - } } public String getQueryString(MultiValueMap params) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java new file mode 100644 index 00000000..e5b4895f --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java @@ -0,0 +1,117 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * 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 org.springframework.cloud.netflix.zuul.filters; + +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +import org.springframework.boot.actuate.trace.TraceRepository; +import org.springframework.util.MultiValueMap; + +import com.netflix.zuul.context.RequestContext; + +import lombok.extern.apachecommons.CommonsLog; + +/** + * @author Spencer Gibb + */ +@CommonsLog +public class TraceProxyRequestHelper extends ProxyRequestHelper { + + private TraceRepository traces; + + public void setTraces(TraceRepository traces) { + this.traces = traces; + } + + @Override + public Map debug(String verb, String uri, + MultiValueMap headers, MultiValueMap params, + InputStream requestEntity) throws IOException { + Map info = new LinkedHashMap<>(); + if (this.traces != null) { + RequestContext context = RequestContext.getCurrentContext(); + info.put("method", verb); + info.put("path", uri); + info.put("query", getQueryString(params)); + info.put("remote", true); + info.put("proxy", context.get("proxy")); + Map trace = new LinkedHashMap<>(); + Map input = new LinkedHashMap<>(); + trace.put("request", input); + info.put("headers", trace); + debugHeaders(headers, input); + RequestContext ctx = RequestContext.getCurrentContext(); + if (shouldDebugBody(ctx)) { + // Prevent input stream from being read if it needs to go downstream + if (requestEntity != null) { + debugRequestEntity(info, ctx.getRequest().getInputStream()); + } + } + this.traces.add(info); + return info; + } + return info; + } + + void debugHeaders(MultiValueMap headers, Map map) { + for (Entry> entry : headers.entrySet()) { + Collection collection = entry.getValue(); + Object value = collection; + if (collection.size() < 2) { + value = collection.isEmpty() ? "" : collection.iterator().next(); + } + map.put(entry.getKey(), value); + } + } + + public void appendDebug(Map info, int status, + MultiValueMap headers) { + if (this.traces != null) { + @SuppressWarnings("unchecked") + Map trace = (Map) info.get("headers"); + Map output = new LinkedHashMap(); + trace.put("response", output); + debugHeaders(headers, output); + output.put("status", "" + status); + } + } + + private void debugRequestEntity(Map info, InputStream inputStream) + throws IOException { + if (RequestContext.getCurrentContext().isChunkedRequestBody()) { + info.put("body", ""); + return; + } + char[] buffer = new char[4096]; + int count = new InputStreamReader(inputStream, Charset.forName("UTF-8")) + .read(buffer, 0, buffer.length); + if (count > 0) { + String entity = new String(buffer).substring(0, count); + info.put("body", entity.length() < 4096 ? entity : entity + ""); + } + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java index e2b4fb23..a3f6f6ab 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java @@ -72,7 +72,7 @@ public class ProxyRequestHelperTests { request.addHeader("multiName", "multiValue2"); RequestContext.getCurrentContext().setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); this.traceRepository = new InMemoryTraceRepository(); helper.setTraces(this.traceRepository); @@ -172,7 +172,7 @@ public class ProxyRequestHelperTests { request.addHeader("multiName", "multiValue1"); request.addHeader("multiName", "multiValue2"); - ProxyRequestHelper helper = new ProxyRequestHelper(); + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); helper.setTraces(this.traceRepository); MultiValueMap headers = helper.buildZuulRequestHeaders(request);