Add response flags into cel expression context (#8827)

bianpengyuan · lizan · commit c60da468c270 · 2019-11-04T17:05:13.000-08:00
Description: Add response flags into expression context
Risk Level: low
Testing: unit test
Docs Changes:
Release Notes:

Signed-off-by: Pengyuan Bian &lt;bianpengyuan@google.com&gt;
diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst
@@ -81,6 +81,7 @@ The following attributes are exposed to the language runtime:
    response.headers, string map, All response headers
    response.trailers, string map, All response trailers
    response.size, int, Size of the response body
+   response.flags, int, Additional details about the response beyond the standard response code
    source.address, string, Downstream connection remote address
    source.port, int, Downstream connection remote port
    destination.address, string, Downstream connection local address
diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h
@@ -357,6 +357,11 @@ class StreamInfo {
    */
   virtual bool hasAnyResponseFlag() const PURE;
 
+  /**
+   * @return response flags encoded as an integer.
+   */
+  virtual uint64_t responseFlags() const PURE;
+
   /**
    * @return upstream host description.
    */
diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h
@@ -123,6 +123,8 @@ struct StreamInfoImpl : public StreamInfo {
 
   bool hasAnyResponseFlag() const override { return response_flags_ != 0; }
 
+  uint64_t responseFlags() const override { return response_flags_; }
+
   void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override {
     upstream_host_ = host;
   }
diff --git a/source/extensions/filters/common/expr/BUILD b/source/extensions/filters/common/expr/BUILD
@@ -29,6 +29,7 @@ envoy_cc_library(
     hdrs = ["context.h"],
     deps = [
         "//source/common/http:utility_lib",
+        "//source/common/stream_info:utility_lib",
         "@com_google_cel_cpp//eval/public:cel_value",
     ],
 )
diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc
@@ -129,6 +129,8 @@ absl::optional<CelValue> ResponseWrapper::operator[](CelValue key) const {
     return CelValue::CreateMap(&headers_);
   } else if (value == Trailers) {
     return CelValue::CreateMap(&trailers_);
+  } else if (value == Flags) {
+    return CelValue::CreateInt64(info_.responseFlags());
   }
   return {};
 }
diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h
@@ -34,6 +34,7 @@ constexpr absl::string_view Duration = "duration";
 constexpr absl::string_view Response = "response";
 constexpr absl::string_view Code = "code";
 constexpr absl::string_view Trailers = "trailers";
+constexpr absl::string_view Flags = "flags";
 
 // Per-request or per-connection metadata
 constexpr absl::string_view Metadata = "metadata";
diff --git a/test/common/stream_info/stream_info_impl_test.cc b/test/common/stream_info/stream_info_impl_test.cc
@@ -121,6 +121,7 @@ TEST_F(StreamInfoImplTest, ResponseFlagTest) {
         << fmt::format("Flag: {} was expected to be set", flag);
   }
   EXPECT_TRUE(stream_info.hasAnyResponseFlag());
+  EXPECT_EQ(0xFFF, stream_info.responseFlags());
 
   StreamInfoImpl stream_info2(Http::Protocol::Http2, test_time_.timeSystem());
   stream_info2.setResponseFlag(FailedLocalHealthCheck);
diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h
@@ -49,6 +49,7 @@ class TestStreamInfo : public StreamInfo::StreamInfo {
   void setResponseFlag(Envoy::StreamInfo::ResponseFlag response_flag) override {
     response_flags_ |= response_flag;
   }
+  uint64_t responseFlags() const override { return response_flags_; }
   void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override {
     upstream_host_ = host;
   }
diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc
@@ -195,6 +195,7 @@ TEST(Context, ResponseAttributes) {
 
   EXPECT_CALL(info, responseCode()).WillRepeatedly(Return(404));
   EXPECT_CALL(info, bytesSent()).WillRepeatedly(Return(123));
+  EXPECT_CALL(info, responseFlags()).WillRepeatedly(Return(0x1));
 
   {
     auto value = response[CelValue::CreateString(Undefined)];
@@ -250,6 +251,12 @@ TEST(Context, ResponseAttributes) {
     ASSERT_TRUE(header.value().IsString());
     EXPECT_EQ("b", header.value().StringOrDie().value());
   }
+  {
+    auto value = response[CelValue::CreateString(Flags)];
+    EXPECT_TRUE(value.has_value());
+    ASSERT_TRUE(value.value().IsInt64());
+    EXPECT_EQ(0x1, value.value().Int64OrDie());
+  }
 }
 
 TEST(Context, ConnectionAttributes) {
diff --git a/test/mocks/stream_info/mocks.h b/test/mocks/stream_info/mocks.h
@@ -52,6 +52,7 @@ class MockStreamInfo : public StreamInfo {
   MOCK_CONST_METHOD0(bytesSent, uint64_t());
   MOCK_CONST_METHOD1(hasResponseFlag, bool(ResponseFlag));
   MOCK_CONST_METHOD0(hasAnyResponseFlag, bool());
+  MOCK_CONST_METHOD0(responseFlags, uint64_t());
   MOCK_CONST_METHOD0(upstreamHost, Upstream::HostDescriptionConstSharedPtr());
   MOCK_METHOD1(setUpstreamLocalAddress, void(const Network::Address::InstanceConstSharedPtr&));
   MOCK_CONST_METHOD0(upstreamLocalAddress, const Network::Address::InstanceConstSharedPtr&());

Original file line number	Diff line number	Diff line change
`@@ -123,6 +123,8 @@ struct StreamInfoImpl : public StreamInfo {`
`123`	`123`
`124`	`124`	`bool hasAnyResponseFlag() const override { return response_flags_ != 0; }`
`125`	`125`
	`126`	`+ uint64_t responseFlags() const override { return response_flags_; }`
	`127`	`+`
`126`	`128`	`void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override {`
`127`	`129`	`upstream_host_ = host;`
`128`	`130`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ envoy_cc_library(`
`29`	`29`	`hdrs = ["context.h"],`
`30`	`30`	`deps = [`
`31`	`31`	`"//source/common/http:utility_lib",`
	`32`	`+ "//source/common/stream_info:utility_lib",`
`32`	`33`	`"@com_google_cel_cpp//eval/public:cel_value",`
`33`	`34`	`],`
`34`	`35`	`)`
Original file line number	Diff line number	Diff line change
`@@ -129,6 +129,8 @@ absl::optional<CelValue> ResponseWrapper::operator[](CelValue key) const {`
`129`	`129`	`return CelValue::CreateMap(&headers_);`
`130`	`130`	`} else if (value == Trailers) {`
`131`	`131`	`return CelValue::CreateMap(&trailers_);`
	`132`	`+ } else if (value == Flags) {`
	`133`	`+ return CelValue::CreateInt64(info_.responseFlags());`
`132`	`134`	`}`
`133`	`135`	`return {};`
`134`	`136`	`}`
Original file line number	Diff line number	Diff line change
`@@ -121,6 +121,7 @@ TEST_F(StreamInfoImplTest, ResponseFlagTest) {`
`121`	`121`	`<< fmt::format("Flag: {} was expected to be set", flag);`
`122`	`122`	`}`
`123`	`123`	`EXPECT_TRUE(stream_info.hasAnyResponseFlag());`
	`124`	`+ EXPECT_EQ(0xFFF, stream_info.responseFlags());`
`124`	`125`
`125`	`126`	`StreamInfoImpl stream_info2(Http::Protocol::Http2, test_time_.timeSystem());`
`126`	`127`	`stream_info2.setResponseFlag(FailedLocalHealthCheck);`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ class TestStreamInfo : public StreamInfo::StreamInfo {`
`49`	`49`	`void setResponseFlag(Envoy::StreamInfo::ResponseFlag response_flag) override {`
`50`	`50`	`response_flags_ \|= response_flag;`
`51`	`51`	`}`
	`52`	`+ uint64_t responseFlags() const override { return response_flags_; }`
`52`	`53`	`void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override {`
`53`	`54`	`upstream_host_ = host;`
`54`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -195,6 +195,7 @@ TEST(Context, ResponseAttributes) {`
`195`	`195`
`196`	`196`	`EXPECT_CALL(info, responseCode()).WillRepeatedly(Return(404));`
`197`	`197`	`EXPECT_CALL(info, bytesSent()).WillRepeatedly(Return(123));`
	`198`	`+ EXPECT_CALL(info, responseFlags()).WillRepeatedly(Return(0x1));`
`198`	`199`
`199`	`200`	`{`
`200`	`201`	`auto value = response[CelValue::CreateString(Undefined)];`
`@@ -250,6 +251,12 @@ TEST(Context, ResponseAttributes) {`
`250`	`251`	`ASSERT_TRUE(header.value().IsString());`
`251`	`252`	`EXPECT_EQ("b", header.value().StringOrDie().value());`
`252`	`253`	`}`
	`254`	`+ {`
	`255`	`+ auto value = response[CelValue::CreateString(Flags)];`
	`256`	`+ EXPECT_TRUE(value.has_value());`
	`257`	`+ ASSERT_TRUE(value.value().IsInt64());`
	`258`	`+ EXPECT_EQ(0x1, value.value().Int64OrDie());`
	`259`	`+ }`
`253`	`260`	`}`
`254`	`261`
`255`	`262`	`TEST(Context, ConnectionAttributes) {`