redirect: add support to specify response code (#2030)

Signed-off-by: Constance Caramanolis <ccaramanolis@lyft.com>

bazel/repositories.bzl

@@ -113,7 +113,7 @@ def envoy_api_deps(skip_targets):
     native.git_repository(
         name = "envoy_api",
         remote = REPO_LOCATIONS["data-plane-api"],
-        commit = "4d18e6d236a6476782076b217cd62d43c30a7dfe",
+        commit = "971fb1b70f419348a1ac2273508237b7ebd08cf5",
     )
 
     api_bind_targets = [

include/envoy/router/BUILD

@@ -38,6 +38,7 @@ envoy_cc_library(
         "//include/envoy/access_log:access_log_interface",
         "//include/envoy/common:optional",
         "//include/envoy/http:codec_interface",
+        "//include/envoy/http:codes_interface",
         "//include/envoy/http:header_map_interface",
         "//include/envoy/tracing:http_tracer_interface",
         "//include/envoy/upstream:resource_manager_interface",

include/envoy/router/router.h

@@ -11,6 +11,7 @@
 #include "envoy/access_log/access_log.h"
 #include "envoy/common/optional.h"
 #include "envoy/http/codec.h"
+#include "envoy/http/codes.h"
 #include "envoy/http/header_map.h"
 #include "envoy/tracing/http_tracer.h"
 #include "envoy/upstream/resource_manager.h"
@@ -34,6 +35,12 @@ class RedirectEntry {
    * @return std::string the redirect URL.
    */
   virtual std::string newPath(const Http::HeaderMap& headers) const PURE;
+
+  /**
+   * Returns the HTTP status code to use when redirecting a request.
+   * @return Http::Code the redirect response Code.
+   */
+  virtual Http::Code redirectResponseCode() const PURE;
 };
 
 /**

source/common/http/utility.cc

@@ -243,9 +243,10 @@ void Utility::sendLocalReply(
   }
 }
 
-void Utility::sendRedirect(StreamDecoderFilterCallbacks& callbacks, const std::string& new_path) {
+void Utility::sendRedirect(StreamDecoderFilterCallbacks& callbacks, const std::string& new_path,
+                           Code response_code) {
   HeaderMapPtr response_headers{
-      new HeaderMapImpl{{Headers::get().Status, std::to_string(enumToInt(Code::MovedPermanently))},
+      new HeaderMapImpl{{Headers::get().Status, std::to_string(enumToInt(response_code))},
                         {Headers::get().Location, new_path}}};
 
   callbacks.encodeHeaders(std::move(response_headers), true);

source/common/http/utility.h

@@ -141,11 +141,13 @@ class Utility {
                  const bool& is_reset, Code response_code, const std::string& body_text);
 
   /**
-   * Send a redirect response (301).
+   * Send a redirect response.
    * @param callbacks supplies the filter callbacks to use.
    * @param new_path supplies the redirect target.
+   * @param response_code supplies the response code to use.
    */
-  static void sendRedirect(StreamDecoderFilterCallbacks& callbacks, const std::string& new_path);
+  static void sendRedirect(StreamDecoderFilterCallbacks& callbacks, const std::string& new_path,
+                           Code response_code);
 
   /**
    * Retrieves the last address in x-forwarded-for header. If it isn't set, returns empty string.

source/common/router/BUILD

@@ -42,6 +42,7 @@ envoy_cc_library(
     hdrs = ["config_utility.h"],
     external_deps = ["envoy_rds"],
     deps = [
+        "//include/envoy/http:codes_interface",
         "//include/envoy/upstream:resource_manager_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:empty_string",

source/common/router/config_impl.cc

@@ -232,7 +232,9 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
       rate_limit_policy_(route.route().rate_limits()), shadow_policy_(route.route()),
       priority_(ConfigUtility::parsePriority(route.route().priority())),
       request_headers_parser_(RequestHeaderParser::parse(route.route().request_headers_to_add())),
-      opaque_config_(parseOpaqueConfig(route)), decorator_(parseDecorator(route)) {
+      opaque_config_(parseOpaqueConfig(route)), decorator_(parseDecorator(route)),
+      redirect_response_code_(
+          ConfigUtility::parseRedirectResponseCode(route.redirect().response_code())) {
   if (route.route().has_metadata_match()) {
     const auto filter_it = route.route().metadata_match().filter_metadata().find(
         Envoy::Config::MetadataFilters::get().ENVOY_LB);
@@ -686,7 +688,7 @@ RouteMatcher::RouteMatcher(const envoy::api::v2::RouteConfiguration& route_confi
     for (const std::string& domain : virtual_host_config.domains()) {
       if ("*" == domain) {
         if (default_virtual_host_) {
-          throw EnvoyException(fmt::format("Only a single single wildcard domain is permitted"));
+          throw EnvoyException(fmt::format("Only a single wildcard domain is permitted"));
         }
         default_virtual_host_ = virtual_host;
       } else if (domain.size() > 0 && '*' == domain[0]) {

source/common/router/config_impl.h

@@ -52,6 +52,7 @@ class SslRedirector : public RedirectEntry {
 public:
   // Router::RedirectEntry
   std::string newPath(const Http::HeaderMap& headers) const override;
+  Http::Code redirectResponseCode() const override { return Http::Code::MovedPermanently; }
 };
 
 class SslRedirectRoute : public Route {
@@ -329,6 +330,7 @@ class RouteEntryImplBase : public RouteEntry,
 
   // Router::RedirectEntry
   std::string newPath(const Http::HeaderMap& headers) const override;
+  Http::Code redirectResponseCode() const override { return redirect_response_code_; }
 
   // Router::Route
   const RedirectEntry* redirectEntry() const override;
@@ -474,6 +476,7 @@ class RouteEntryImplBase : public RouteEntry,
   const std::multimap<std::string, std::string> opaque_config_;
 
   const DecoratorConstPtr decorator_;
+  const Http::Code redirect_response_code_;
 };
 
 /**

source/common/router/config_utility.cc

@@ -45,5 +45,23 @@ bool ConfigUtility::matchHeaders(const Http::HeaderMap& request_headers,
   return matches;
 }
 
+Http::Code ConfigUtility::parseRedirectResponseCode(
+    const envoy::api::v2::RedirectAction::RedirectResponseCode& code) {
+  switch (code) {
+  case envoy::api::v2::RedirectAction::MOVED_PERMANENTLY:
+    return Http::Code::MovedPermanently;
+  case envoy::api::v2::RedirectAction::FOUND:
+    return Http::Code::Found;
+  case envoy::api::v2::RedirectAction::SEE_OTHER:
+    return Http::Code::SeeOther;
+  case envoy::api::v2::RedirectAction::TEMPORARY_REDIRECT:
+    return Http::Code::TemporaryRedirect;
+  case envoy::api::v2::RedirectAction::PERMANENT_REDIRECT:
+    return Http::Code::PermanentRedirect;
+  default:
+    NOT_IMPLEMENTED;
+  }
+}
+
 } // namespace Router
 } // namespace Envoy

source/common/router/config_utility.h

@@ -4,6 +4,7 @@
 #include <string>
 #include <vector>
 
+#include "envoy/http/codes.h"
 #include "envoy/json/json_object.h"
 #include "envoy/upstream/resource_manager.h"
 
@@ -57,6 +58,14 @@ class ConfigUtility {
    */
   static bool matchHeaders(const Http::HeaderMap& headers,
                            const std::vector<HeaderData>& request_headers);
+
+  /**
+   * Returns the redirect HTTP Status Code enum parsed from proto.
+   * @param code supplies the RedirectResponseCode enum.
+   * @return Returns the Http::Code version of the RedirectResponseCode.
+   */
+  static Http::Code
+  parseRedirectResponseCode(const envoy::api::v2::RedirectAction::RedirectResponseCode& code);
 };
 
 } // namespace Router

source/common/router/router.cc

@@ -212,7 +212,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
   // Determine if there is a redirect for the request.
   if (route_->redirectEntry()) {
     config_.stats_.rq_redirect_.inc();
-    Http::Utility::sendRedirect(*callbacks_, route_->redirectEntry()->newPath(headers));
+    Http::Utility::sendRedirect(*callbacks_, route_->redirectEntry()->newPath(headers),
+                                route_->redirectEntry()->redirectResponseCode());
     return Http::FilterHeadersStatus::StopIteration;
   }
 

test/common/router/config_impl_test.cc

@@ -46,6 +46,12 @@ envoy::api::v2::RouteConfiguration parseRouteConfigurationFromJson(const std::st
   return route_config;
 }
 
+envoy::api::v2::RouteConfiguration parseRouteConfigurationFromV2Yaml(const std::string& yaml) {
+  envoy::api::v2::RouteConfiguration route_config;
+  MessageUtil::loadFromYaml(yaml, route_config);
+  return route_config;
+}
+
 TEST(RouteMatcherTest, TestRoutes) {
   std::string json = R"EOF(
 {
@@ -2927,6 +2933,48 @@ TEST(RoutEntryMetadataMatchTest, ParsesMetadata) {
   }
 }
 
+TEST(ConfigUtility, ParseResponseCode) {
+  const std::vector<std::pair<envoy::api::v2::RedirectAction::RedirectResponseCode, Http::Code>>
+      test_set = {std::make_pair(envoy::api::v2::RedirectAction::MOVED_PERMANENTLY,
+                                 Http::Code::MovedPermanently),
+                  std::make_pair(envoy::api::v2::RedirectAction::FOUND, Http::Code::Found),
+                  std::make_pair(envoy::api::v2::RedirectAction::SEE_OTHER, Http::Code::SeeOther),
+                  std::make_pair(envoy::api::v2::RedirectAction::TEMPORARY_REDIRECT,
+                                 Http::Code::TemporaryRedirect),
+                  std::make_pair(envoy::api::v2::RedirectAction::PERMANENT_REDIRECT,
+                                 Http::Code::PermanentRedirect)};
+  for (const auto& test_case : test_set) {
+    EXPECT_EQ(test_case.second, ConfigUtility::parseRedirectResponseCode(test_case.first));
+  }
+}
+
+TEST(RouteConfigurationV2, RedirectCode) {
+  std::string yaml = R"EOF(
+name: foo
+virtual_hosts:
+  - name: redirect
+    domains: [redirect.lyft.com]
+    routes:
+      - match: { prefix: "/"}
+        redirect: { host_redirect: new.lyft.com, response_code: TEMPORARY_REDIRECT }
+
+  )EOF";
+
+  NiceMock<Runtime::MockLoader> runtime;
+  NiceMock<Upstream::MockClusterManager> cm;
+  ConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), runtime, cm, true);
+
+  EXPECT_EQ(nullptr, config.route(genRedirectHeaders("www.foo.com", "/foo", true, true), 0));
+
+  {
+    Http::TestHeaderMapImpl headers = genRedirectHeaders("redirect.lyft.com", "/foo", false, false);
+    EXPECT_EQ("http://new.lyft.com/foo",
+              config.route(headers, 0)->redirectEntry()->newPath(headers));
+    EXPECT_EQ(Http::Code::TemporaryRedirect,
+              config.route(headers, 0)->redirectEntry()->redirectResponseCode());
+  }
+}
+
 } // namespace
 } // namespace Router
 } // namespace Envoy

test/common/router/router_test.cc

@@ -1405,6 +1405,7 @@ TEST_F(RouterTest, AltStatName) {
 TEST_F(RouterTest, Redirect) {
   MockRedirectEntry redirect;
   EXPECT_CALL(redirect, newPath(_)).WillOnce(Return("hello"));
+  EXPECT_CALL(redirect, redirectResponseCode()).WillOnce(Return(Http::Code::MovedPermanently));
   EXPECT_CALL(*callbacks_.route_, redirectEntry()).WillRepeatedly(Return(&redirect));
 
   Http::TestHeaderMapImpl response_headers{{":status", "301"}, {"location", "hello"}};
@@ -1415,6 +1416,20 @@ TEST_F(RouterTest, Redirect) {
   EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
 }
 
+TEST_F(RouterTest, RedirectFound) {
+  MockRedirectEntry redirect;
+  EXPECT_CALL(redirect, newPath(_)).WillOnce(Return("hello"));
+  EXPECT_CALL(redirect, redirectResponseCode()).WillOnce(Return(Http::Code::Found));
+  EXPECT_CALL(*callbacks_.route_, redirectEntry()).WillRepeatedly(Return(&redirect));
+
+  Http::TestHeaderMapImpl response_headers{{":status", "302"}, {"location", "hello"}};
+  EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
+  Http::TestHeaderMapImpl headers;
+  HttpTestUtility::addDefaultHeaders(headers);
+  router_.decodeHeaders(headers, true);
+  EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
+}
+
 TEST(RouterFilterUtilityTest, finalTimeout) {
   {
     NiceMock<MockRouteEntry> route;

test/mocks/router/mocks.h

@@ -34,6 +34,7 @@ class MockRedirectEntry : public RedirectEntry {
 
   // Router::Config
   MOCK_CONST_METHOD1(newPath, std::string(const Http::HeaderMap& headers));
+  MOCK_CONST_METHOD0(redirectResponseCode, Http::Code());
 };
 
 class TestCorsPolicy : public CorsPolicy {