Skip to content

rbac: apply the shadow_rules_stat_prefix to shadow stats in addition to dynamic metadata #15391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 2 commits into from
Mar 12, 2021
Merged

rbac: apply the shadow_rules_stat_prefix to shadow stats in addition to dynamic metadata #15391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6638506
rbac: apply the shadow_rules_stat_prefix to shadow stats in addition …
yangminzhu Mar 10, 2021
fcdfbe2
update link
yangminzhu Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/root/configuration/http/http_filters/rbac_filter.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ The RBAC filter outputs statistics in the *http.<stat_prefix>.rbac.* namespace.
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stat_prefix>` comes from the
owning HTTP connection manager.

For the shadow rule statistics `shadow_allowed` and `shadow_denied`, the :ref:`shadow_rules_stat_prefix <envoy_v3_api_field_extensions.filters.http.rbac.v3.RBAC.shadow_rules_stat_prefix>`
can be used to add an extra prefix to output the statistics in the *http.<stat_prefix>.rbac.<shadow_rules_stat_prefix>.* namespace.

.. csv-table::
:header: Name, Type, Description
:widths: 1, 1, 2
Expand All @@ -51,6 +54,9 @@ Dynamic Metadata

The RBAC filter emits the following dynamic metadata.

For the shadow rules dynamic metadata `shadow_effective_policy_id` and `shadow_engine_result`, the :ref:`shadow_rules_stat_prefix <envoy_v3_api_field_extensions.filters.http.rbac.v3.RBAC.shadow_rules_stat_prefix>`
can be used to add an extra prefix to the corresponding dynamic metadata key.

.. csv-table::
:header: Name, Type, Description
:widths: 1, 1, 2
Expand Down
6 changes: 6 additions & 0 deletions docs/root/configuration/listeners/network_filters/rbac_filter.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ Statistics

The RBAC network filter outputs statistics in the *<stat_prefix>.rbac.* namespace.

For the shadow rule statistics `shadow_allowed` and `shadow_denied`, the :ref:`shadow_rules_stat_prefix <envoy_v3_api_field_extensions.filters.network.rbac.v3.RBAC.shadow_rules_stat_prefix>`
can be used to add an extra prefix to output the statistics in the *<stat_prefix>.rbac.<shadow_rules_stat_prefix>.* namespace.

.. csv-table::
:header: Name, Type, Description
:widths: 1, 1, 2
Expand All @@ -41,6 +44,9 @@ Dynamic Metadata

The RBAC filter emits the following dynamic metadata.

For the shadow rules dynamic metadata `shadow_effective_policy_id` and `shadow_engine_result`, the :ref:`shadow_rules_stat_prefix <envoy_v3_api_field_extensions.filters.network.rbac.v3.RBAC.shadow_rules_stat_prefix>`
can be used to add an extra prefix to the corresponding dynamic metadata key.

.. csv-table::
:header: Name, Type, Description
:widths: 1, 1, 2
Expand Down
6 changes: 4 additions & 2 deletions source/extensions/filters/common/rbac/utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ namespace Filters {
namespace Common {
namespace RBAC {

RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, Stats::Scope& scope) {
RoleBasedAccessControlFilterStats
generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope) {
const std::string final_prefix = prefix + "rbac.";
return {ALL_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
return {ENFORCE_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))
SHADOW_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix + shadow_prefix))};
}

std::string responseDetail(const std::string& policy_id) {
Expand Down
19 changes: 13 additions & 6 deletions source/extensions/filters/common/rbac/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,29 @@ namespace Common {
namespace RBAC {

/**
* All stats for the RBAC filter. @see stats_macros.h
* All stats for the enforced rules in RBAC filter. @see stats_macros.h
*/
#define ALL_RBAC_FILTER_STATS(COUNTER) \
#define ENFORCE_RBAC_FILTER_STATS(COUNTER) \
COUNTER(allowed) \
COUNTER(denied) \
COUNTER(denied)

/**
* All stats for the shadow rules in RBAC filter. @see stats_macros.h
*/
#define SHADOW_RBAC_FILTER_STATS(COUNTER) \
COUNTER(shadow_allowed) \
COUNTER(shadow_denied)

/**
* Wrapper struct for RBAC filter stats. @see stats_macros.h
* Wrapper struct for shadow rules in RBAC filter stats. @see stats_macros.h
*/
struct RoleBasedAccessControlFilterStats {
ALL_RBAC_FILTER_STATS(GENERATE_COUNTER_STRUCT)
ENFORCE_RBAC_FILTER_STATS(GENERATE_COUNTER_STRUCT)
SHADOW_RBAC_FILTER_STATS(GENERATE_COUNTER_STRUCT)
};

RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, Stats::Scope& scope);
RoleBasedAccessControlFilterStats
generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope);

template <class ConfigType>
std::unique_ptr<RoleBasedAccessControlEngineImpl> createEngine(const ConfigType& config) {
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/filters/http/rbac/rbac_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace RBACFilter {
RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config,
const std::string& stats_prefix, Stats::Scope& scope)
: stats_(Filters::Common::RBAC::generateStats(stats_prefix, scope)),
: stats_(Filters::Common::RBAC::generateStats(stats_prefix,
proto_config.shadow_rules_stat_prefix(), scope)),
shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()),
engine_(Filters::Common::RBAC::createEngine(proto_config)),
shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)) {}
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/filters/network/rbac/rbac_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ namespace RBACFilter {

RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope)
: stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), scope)),
: stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(),
proto_config.shadow_rules_stat_prefix(), scope)),
shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()),
engine_(Filters::Common::RBAC::createEngine(proto_config)),
shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)),
Expand Down
20 changes: 20 additions & 0 deletions test/extensions/filters/http/rbac/rbac_filter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ TEST_F(RoleBasedAccessControlFilterTest, Allowed) {
EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_.decodeMetadata(metadata_map));
EXPECT_EQ(1U, config_->stats().allowed_.value());
EXPECT_EQ(1U, config_->stats().shadow_denied_.value());
EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("testrbac.denied", config_->stats().denied_.name());
EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

Buffer::OwnedImpl data("");
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false));
Expand All @@ -146,6 +150,10 @@ TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) {
EXPECT_EQ(0U, config_->stats().denied_.value());
EXPECT_EQ(0U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(1U, config_->stats().shadow_denied_.value());
EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("testrbac.denied", config_->stats().denied_.name());
EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

Buffer::OwnedImpl data("");
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false));
Expand Down Expand Up @@ -183,6 +191,10 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) {
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, true));
EXPECT_EQ(1U, config_->stats().denied_.value());
EXPECT_EQ(1U, config_->stats().shadow_allowed_.value());
EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("testrbac.denied", config_->stats().denied_.name());
EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at(HttpFilterNames::get().Rbac);
EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value());
Expand Down Expand Up @@ -222,6 +234,10 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldLog) {
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false));
EXPECT_EQ(1U, config_->stats().allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("testrbac.denied", config_->stats().denied_.name());
EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

Buffer::OwnedImpl data("");
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false));
Expand All @@ -241,6 +257,10 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) {
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false));
EXPECT_EQ(1U, config_->stats().allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("testrbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("testrbac.denied", config_->stats().denied_.name());
EXPECT_EQ("testrbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("testrbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

Buffer::OwnedImpl data("");
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data, false));
Expand Down
30 changes: 29 additions & 1 deletion test/extensions/filters/network/rbac/filter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test {

envoy::extensions::filters::network::rbac::v3::RBAC config;
config.set_stat_prefix("tcp.");
config.set_shadow_rules_stat_prefix("prefix_");

if (with_policy) {
envoy::config::rbac::v3::Policy policy;
Expand All @@ -45,7 +46,6 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test {
shadow_policy_rules->add_rules()->set_destination_port(456);
shadow_policy.add_principals()->set_any(true);
config.mutable_shadow_rules()->set_action(action);
config.set_shadow_rules_stat_prefix("prefix_");
(*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy;
}

Expand Down Expand Up @@ -125,6 +125,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithOneTimeEnforcement) {
EXPECT_EQ(0U, config_->stats().denied_.value());
EXPECT_EQ(0U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(1U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());
}

TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement) {
Expand All @@ -142,6 +146,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement
EXPECT_EQ(0U, config_->stats().denied_.value());
EXPECT_EQ(0U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(2U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());
}

TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) {
Expand All @@ -157,6 +165,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) {
EXPECT_EQ(0U, config_->stats().denied_.value());
EXPECT_EQ(0U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(1U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());
}

TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) {
Expand All @@ -172,6 +184,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) {
EXPECT_EQ(0U, config_->stats().denied_.value());
EXPECT_EQ(0U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());
}

TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) {
Expand All @@ -187,6 +203,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) {
EXPECT_EQ(1U, config_->stats().denied_.value());
EXPECT_EQ(1U, config_->stats().shadow_allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

auto filter_meta =
stream_info_.dynamicMetadata().filter_metadata().at(NetworkFilterNames::get().Rbac);
Expand All @@ -206,6 +226,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldLog) {
EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false));
EXPECT_EQ(1U, config_->stats().allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

checkAccessLogMetadata(true);
}
Expand All @@ -221,6 +245,10 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldNotLog) {
EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data_, false));
EXPECT_EQ(1U, config_->stats().allowed_.value());
EXPECT_EQ(0U, config_->stats().shadow_denied_.value());
EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name());
EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name());
EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name());

checkAccessLogMetadata(false);
}
Expand Down