Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/codeql-daily.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@181d5eefc20863364f96762470ba6f862bdef56b # codeql-bundle-v3.29.2
uses: github/codeql-action/init@d6bbdef45e766d081b84a2def353b0055f728d3e # codeql-bundle-v3.29.3
# Override language selection by uncommenting this and choosing your languages
with:
languages: cpp
Expand Down Expand Up @@ -75,6 +75,6 @@ jobs:
git clean -xdf

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@181d5eefc20863364f96762470ba6f862bdef56b # codeql-bundle-v3.29.2
uses: github/codeql-action/analyze@d6bbdef45e766d081b84a2def353b0055f728d3e # codeql-bundle-v3.29.3
with:
trap-caching: false
4 changes: 2 additions & 2 deletions .github/workflows/codeql-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:

- name: Initialize CodeQL
if: ${{ env.BUILD_TARGETS != '' }}
uses: github/codeql-action/init@181d5eefc20863364f96762470ba6f862bdef56b # codeql-bundle-v3.29.2
uses: github/codeql-action/init@d6bbdef45e766d081b84a2def353b0055f728d3e # codeql-bundle-v3.29.3
with:
languages: cpp
trap-caching: false
Expand Down Expand Up @@ -112,6 +112,6 @@ jobs:

- name: Perform CodeQL Analysis
if: ${{ env.BUILD_TARGETS != '' }}
uses: github/codeql-action/analyze@181d5eefc20863364f96762470ba6f862bdef56b # codeql-bundle-v3.29.2
uses: github/codeql-action/analyze@d6bbdef45e766d081b84a2def353b0055f728d3e # codeql-bundle-v3.29.3
with:
trap-caching: false
2 changes: 1 addition & 1 deletion .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ jobs:
retention-days: 5

- name: "Upload to code-scanning"
uses: github/codeql-action/upload-sarif@181d5eefc20863364f96762470ba6f862bdef56b # v3.29.2
uses: github/codeql-action/upload-sarif@d6bbdef45e766d081b84a2def353b0055f728d3e # v3.29.3
with:
sarif_file: results.sarif
43 changes: 18 additions & 25 deletions envoy/matcher/matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,22 @@ class Action {
}
};

using ActionConstSharedPtr = std::shared_ptr<const Action>;
using ActionPtr = std::unique_ptr<Action>;
using ActionFactoryCb = std::function<ActionPtr()>;

template <class ActionFactoryContext> class ActionFactory : public Config::TypedFactory {
public:
virtual ActionConstSharedPtr
createAction(const Protobuf::Message& config, ActionFactoryContext& action_factory_context,
ProtobufMessage::ValidationVisitor& validation_visitor) PURE;
virtual ActionFactoryCb
createActionFactoryCb(const Protobuf::Message& config,
ActionFactoryContext& action_factory_context,
ProtobufMessage::ValidationVisitor& validation_visitor) PURE;

std::string category() const override { return "envoy.matching.action"; }
};

// On match, we either return the action to perform or another match tree to match against.
template <class DataType> struct OnMatch {
const ActionConstSharedPtr action_;
const ActionFactoryCb action_cb_;
const MatchTreeSharedPtr<DataType> matcher_;
bool keep_matching_{};
};
Expand All @@ -128,39 +130,30 @@ template <class DataType> class OnMatchFactory {
// - The match could not be completed due to lack of data (isInsufficientData() will return true.)
// - The match was completed, no match found (isNoMatch() will return true.)
// - The match was completed, match found (isMatch() will return true, action() will return the
// ActionConstSharedPtr.)
// ActionFactoryCb.)
struct MatchResult {
public:
MatchResult(ActionConstSharedPtr cb) : result_(std::move(cb)) {}
MatchResult(ActionFactoryCb cb) : result_(std::move(cb)) {}
static MatchResult noMatch() { return MatchResult(NoMatch{}); }
static MatchResult insufficientData() { return MatchResult(InsufficientData{}); }
bool isInsufficientData() const { return absl::holds_alternative<InsufficientData>(result_); }
bool isComplete() const { return !isInsufficientData(); }
bool isNoMatch() const { return absl::holds_alternative<NoMatch>(result_); }
bool isMatch() const { return absl::holds_alternative<ActionConstSharedPtr>(result_); }
const ActionConstSharedPtr& action() const {
ASSERT(isMatch());
return absl::get<ActionConstSharedPtr>(result_);
}
// Returns the action by move. The caller must ensure that the MatchResult is not used after
// this call.
ActionConstSharedPtr actionByMove() {
ASSERT(isMatch());
return absl::get<ActionConstSharedPtr>(std::move(result_));
}
bool isMatch() const { return absl::holds_alternative<ActionFactoryCb>(result_); }
ActionFactoryCb actionFactory() const { return absl::get<ActionFactoryCb>(result_); }
ActionPtr action() const { return actionFactory()(); }

private:
struct InsufficientData {};
struct NoMatch {};
using Result = absl::variant<ActionConstSharedPtr, NoMatch, InsufficientData>;
using Result = absl::variant<ActionFactoryCb, NoMatch, InsufficientData>;
Result result_;
MatchResult(NoMatch) : result_(NoMatch{}) {}
MatchResult(InsufficientData) : result_(InsufficientData{}) {}
};

// Callback to execute against skipped matches' actions.
using SkippedMatchCb = std::function<void(const ActionConstSharedPtr&)>;

using SkippedMatchCb = std::function<void(ActionFactoryCb)>;
/**
* MatchTree provides the interface for performing matches against the data provided by DataType.
*/
Expand Down Expand Up @@ -194,19 +187,19 @@ template <class DataType> class MatchTree {
// Parent result's keep_matching skips the nested result.
if (on_match->keep_matching_ && nested_result.isMatch()) {
if (skipped_match_cb) {
skipped_match_cb(nested_result.action());
skipped_match_cb(nested_result.actionFactory());
}
return MatchResult::noMatch();
}
return nested_result;
}
if (on_match->action_ && on_match->keep_matching_) {
if (on_match->action_cb_ && on_match->keep_matching_) {
if (skipped_match_cb) {
skipped_match_cb(on_match->action_);
skipped_match_cb(on_match->action_cb_);
}
return MatchResult::noMatch();
}
return MatchResult{on_match->action_};
return MatchResult{on_match->action_cb_};
}
};

Expand Down
15 changes: 8 additions & 7 deletions source/common/listener_manager/filter_chain_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ class FilterChainNameActionFactory : public Matcher::ActionFactory<FilterChainAc
Logger::Loggable<Logger::Id::config> {
public:
std::string name() const override { return "filter-chain-name"; }
Matcher::ActionConstSharedPtr createAction(const Protobuf::Message& config,
FilterChainActionFactoryContext&,
ProtobufMessage::ValidationVisitor&) override {
return std::make_shared<FilterChainNameAction>(
dynamic_cast<const ProtobufWkt::StringValue&>(config).value());
Matcher::ActionFactoryCb createActionFactoryCb(const Protobuf::Message& config,
FilterChainActionFactoryContext&,
ProtobufMessage::ValidationVisitor&) override {
const auto& name = dynamic_cast<const ProtobufWkt::StringValue&>(config);
return [value = name.value()]() { return std::make_unique<FilterChainNameAction>(value); };
}
ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<ProtobufWkt::StringValue>();
Expand Down Expand Up @@ -573,8 +573,9 @@ FilterChainManagerImpl::findFilterChainUsingMatcher(const Network::ConnectionSoc
Matcher::evaluateMatch<Network::MatchingData>(*matcher_, data);
ASSERT(match_result.isComplete(), "Matching must complete for network streams.");
if (match_result.isMatch()) {
return match_result.action()->getTyped<Configuration::FilterChainBaseAction>().get(
filter_chains_by_name_, info);
const Matcher::ActionPtr action = match_result.action();
return action->getTyped<Configuration::FilterChainBaseAction>().get(filter_chains_by_name_,
info);
}
return default_filter_chain_.get();
}
Expand Down
8 changes: 4 additions & 4 deletions source/common/matcher/matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,10 @@ class MatchTreeFactory : public OnMatchFactory<DataType> {
on_match.action().typed_config(), server_factory_context_.messageValidationVisitor(),
factory);

auto action = factory.createAction(*message, action_factory_context_,
server_factory_context_.messageValidationVisitor());
return [action, keep_matching = on_match.keep_matching()] {
return OnMatch<DataType>{action, {}, keep_matching};
auto action_factory = factory.createActionFactoryCb(
*message, action_factory_context_, server_factory_context_.messageValidationVisitor());
return [action_factory, keep_matching = on_match.keep_matching()] {
return OnMatch<DataType>{action_factory, {}, keep_matching};
};
}

Expand Down
27 changes: 14 additions & 13 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1825,13 +1825,14 @@ RouteConstSharedPtr VirtualHostImpl::getRouteFromEntries(const RouteCallback& cb
Matcher::evaluateMatch<Http::HttpMatchingData>(*matcher_, data);

if (match_result.isMatch()) {
const auto result = match_result.actionByMove();
const Matcher::ActionPtr result = match_result.action();
if (result->typeUrl() == RouteMatchAction::staticTypeUrl()) {
return getRouteFromRoutes(
cb, headers, stream_info, random_value,
{std::dynamic_pointer_cast<const RouteEntryImplBase>(std::move(result))});
const RouteMatchAction& route_action = result->getTyped<RouteMatchAction>();

return getRouteFromRoutes(cb, headers, stream_info, random_value, {route_action.route()});
} else if (result->typeUrl() == RouteListMatchAction::staticTypeUrl()) {
const RouteListMatchAction& action = result->getTyped<RouteListMatchAction>();

return getRouteFromRoutes(cb, headers, stream_info, random_value, action.routes());
}
PANIC("Action in router matcher should be Route or RouteList");
Expand Down Expand Up @@ -2139,24 +2140,24 @@ const Envoy::Config::TypedMetadata& NullConfigImpl::typedMetadata() const {
return DefaultRouteMetadataPack::get().typed_metadata_;
}

Matcher::ActionConstSharedPtr
RouteMatchActionFactory::createAction(const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
Matcher::ActionFactoryCb RouteMatchActionFactory::createActionFactoryCb(
const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
const auto& route_config =
MessageUtil::downcastAndValidate<const envoy::config::route::v3::Route&>(config,
validation_visitor);
auto route = THROW_OR_RETURN_VALUE(
RouteCreator::createAndValidateRoute(route_config, context.vhost, context.factory_context,
validation_visitor, false),
RouteEntryImplBaseConstSharedPtr);
return route;

return [route]() { return std::make_unique<RouteMatchAction>(route); };
}
REGISTER_FACTORY(RouteMatchActionFactory, Matcher::ActionFactory<RouteActionContext>);

Matcher::ActionConstSharedPtr
RouteListMatchActionFactory::createAction(const Protobuf::Message& config,
RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
Matcher::ActionFactoryCb RouteListMatchActionFactory::createActionFactoryCb(
const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
const auto& route_config =
MessageUtil::downcastAndValidate<const envoy::config::route::v3::RouteList&>(
config, validation_visitor);
Expand All @@ -2168,7 +2169,7 @@ RouteListMatchActionFactory::createAction(const Protobuf::Message& config,
validation_visitor, false),
RouteEntryImplBaseConstSharedPtr));
}
return std::make_shared<RouteListMatchAction>(std::move(routes));
return [routes]() { return std::make_unique<RouteListMatchAction>(routes); };
}
REGISTER_FACTORY(RouteListMatchActionFactory, Matcher::ActionFactory<RouteActionContext>);

Expand Down
13 changes: 6 additions & 7 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,6 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
public Matchable,
public DirectResponseEntry,
public PathMatchCriterion,
public Matcher::ActionBase<envoy::config::route::v3::Route>,
public std::enable_shared_from_this<RouteEntryImplBase>,
Logger::Loggable<Logger::Id::router> {
protected:
Expand Down Expand Up @@ -1192,9 +1191,9 @@ class RouteMatchAction : public Matcher::ActionBase<envoy::config::route::v3::Ro
// Registered factory for RouteMatchAction.
class RouteMatchActionFactory : public Matcher::ActionFactory<RouteActionContext> {
public:
Matcher::ActionConstSharedPtr
createAction(const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
Matcher::ActionFactoryCb
createActionFactoryCb(const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
std::string name() const override { return "route"; }
ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::route::v3::Route>();
Expand All @@ -1218,9 +1217,9 @@ class RouteListMatchAction : public Matcher::ActionBase<envoy::config::route::v3
// Registered factory for RouteListMatchAction.
class RouteListMatchActionFactory : public Matcher::ActionFactory<RouteActionContext> {
public:
Matcher::ActionConstSharedPtr
createAction(const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
Matcher::ActionFactoryCb
createActionFactoryCb(const Protobuf::Message& config, RouteActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
std::string name() const override { return "route_match_action"; }
ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::route::v3::RouteList>();
Expand Down
13 changes: 7 additions & 6 deletions source/extensions/filters/common/rbac/engine_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ namespace Filters {
namespace Common {
namespace RBAC {

Envoy::Matcher::ActionConstSharedPtr
ActionFactory::createAction(const Protobuf::Message& config, ActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
Envoy::Matcher::ActionFactoryCb
ActionFactory::createActionFactoryCb(const Protobuf::Message& config, ActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) {
const auto& action_config =
MessageUtil::downcastAndValidate<const envoy::config::rbac::v3::Action&>(config,
validation_visitor);
Expand All @@ -25,7 +25,7 @@ ActionFactory::createAction(const Protobuf::Message& config, ActionContext& cont
context.has_log_ = true;
}

return std::make_shared<Action>(name, action);
return [name, action]() { return std::make_unique<Action>(name, action); };
}

REGISTER_FACTORY(ActionFactory, Envoy::Matcher::ActionFactory<ActionContext>);
Expand Down Expand Up @@ -138,17 +138,18 @@ bool RoleBasedAccessControlMatcherEngineImpl::handleAction(
Envoy::Matcher::evaluateMatch<Http::HttpMatchingData>(*matcher_, data);
ASSERT(result.isComplete());
if (result.isMatch()) {
const auto& action = result.action()->getTyped<Action>();
auto action = result.action()->getTyped<Action>();
if (effective_policy_id != nullptr) {
*effective_policy_id = action.name();
}

// If there is at least an LOG action in matchers, we have to turn on and off for shared log
// metadata every time when there is a connection or request.
const auto rbac_action = action.action();
auto rbac_action = action.action();
if (has_log_) {
generateLog(info, mode_, rbac_action == envoy::config::rbac::v3::RBAC::LOG);
}

switch (rbac_action) {
PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
case envoy::config::rbac::v3::RBAC::ALLOW:
Expand Down
6 changes: 3 additions & 3 deletions source/extensions/filters/common/rbac/engine_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ class Action : public Envoy::Matcher::ActionBase<envoy::config::rbac::v3::Action

class ActionFactory : public Envoy::Matcher::ActionFactory<ActionContext> {
public:
Envoy::Matcher::ActionConstSharedPtr
createAction(const Protobuf::Message& config, ActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
Envoy::Matcher::ActionFactoryCb
createActionFactoryCb(const Protobuf::Message& config, ActionContext& context,
ProtobufMessage::ValidationVisitor& validation_visitor) override;
std::string name() const override { return "envoy.filters.rbac.action"; }
ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return std::make_unique<envoy::config::rbac::v3::Action>();
Expand Down
Loading