router: add filter_state internal redirect predicate by juanmolle · Pull Request #45904 · envoyproxy/envoy

juanmolle · 2026-06-30T20:09:24Z

Commit Message:
router: add filter_state internal redirect predicate

Adds the envoy.internal_redirect_predicates.filter_state predicate, which decides
whether to follow an internal redirect based on a boolean filter state object. The
predicate reads a BoolAccessor at the configured filter_state_key and accepts
the redirect if the value is true (or false if invert: true).

Also adds a generic envoy.bool filter state factory that accepts: true, t,
yes, y, 1 for true; false, f, no, n, 0 for false (case-insensitive).

Example configuration:

internal_redirect_policy:
  predicates:
    - name: envoy.internal_redirect_predicates.filter_state
      typed_config:
        "@type": type.googleapis.com/envoy.extensions.internal_redirect.filter_state.v3.FilterStateConfig
        filter_state_key: envoy.internal_redirect.gate
        invert: false

Setting from Lua:

handle:streamInfo():filterState():set("envoy.internal_redirect.gate", "envoy.bool", "true")

Risk Level: Low — self-contained extension, inert unless explicitly configured.

Testing: Unit tests for predicate and factory. Integration tests with Docker validate
end-to-end behavior.

Docs Changes: Added envoy.bool factory to well_known_filter_state.rst.

Release Notes: Added changelogs/current/new_features/router__added-filter-state-internal-redirect-predicate.rst.

[Generative AI usage disclosure: Generative AI (Claude) was used to assist with this change;
I have reviewed and understand all of the code.]

Adds the envoy.internal_redirect_predicates.filter_state predicate, which decides whether to follow an internal redirect by comparing a filter state string object (set earlier in the request by another filter) against a configured value: allow_value follows only on a match, deny_value follows unless it matches, and allow_if_absent controls behavior when the object is missing (default: do not follow). No change for existing configs; the predicate has effect only when added to an InternalRedirectPolicy. Uses a raw string with a required oneof allow_value / deny_value rather than a StringMatcher, because the predicate factory does not receive a CommonFactoryContext (needed by StringMatcher for regex/custom). This stays additively upgradeable to a StringMatcher option later without an API break. Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

repokitteh-read-only · 2026-06-30T20:09:30Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #45904 was opened by juanmolle.

see: more, trace.

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

repokitteh-read-only · 2026-06-30T22:01:08Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #45904 was ready_for_review by juanmolle.

see: more, trace.

adisuissa

Thanks.
Left a high-level question about the design.

adisuissa · 2026-07-08T14:49:08Z

+// whether an otherwise-enabled internal redirect is actually taken, without
+// changing route matching.
+//
+// The predicate reads the filter-state object named ``filter_state_key`` (a


From a high-level perspective:
it seems that in the future this will have a matcher that is dependent on the expected type. The most basic one will be a string matcher, but the API should be designed in a way that allows other types of matchers.

I thought that using oneof will allow in the future to add others like string matcher with backward compatibility

oneof policy {
string allow_value = 2; // existing - exact string match
string deny_value = 3; // existing - exact string match
StringMatcher allow_string_matcher = 5; // NEW - regex, prefix, suffix, etc.
StringMatcher deny_string_matcher = 6; // NEW - regex, prefix, suffix, etc.
}

if you have something better in mind let me know I can change it

IIUC, @adisuissa means that the type of data store in the filter state might be more than a string.

From an API point of view, the policy and the filter_state_key is coupled by the type of the filter state's type.

What happens if the filter state pointed by filter_state_key stores something other than string?

An alternative might be:

string filter_state_key = 1; enum MatchAction { UNSPECIFIED = 0; ALLOW = 1; DENY = 2; } MatchAction match_action = 2; oneof matcher { string string_value = 1; // other types of matcher for extension }

With all the above said, isn't it simpler for this to just read a boolean from a filter state that dictates whether to follow the redirect? What's the use case that requires a string value to be compared against?

I didn't see a boolean could work and perhaps it is cleaner
Do you have something like this in mind?

message FilterStateConfig { // Name of the filter-state boolean object. If true, follows the redirect. string redirect_enabled_key = 1 [(validate.rules).string = {min_len: 1}]; // Whether to follow the redirect when the filter-state object is absent. // Defaults to false (do not redirect if the gate wasn't explicitly set). bool redirect_if_absent = 2; }

I think this is a lot cleaner while being really flexible - whichever filter producing the filter state can decide what to do.

the code is cleaner too (and just a couple of lines) I will do some exploratory test and update the PR, thanks

adisuissa · 2026-07-08T14:52:55Z

Assigning internal_redirect code-owners:
/assign @botengyao @penguingao

penguingao

Thanks for the contribution. This looks great. Just need to figure out the API discussion.

penguingao · 2026-07-10T16:36:57Z

+// whether an otherwise-enabled internal redirect is actually taken, without
+// changing route matching.
+//
+// The predicate reads the filter-state object named ``filter_state_key`` (a


IIUC, @adisuissa means that the type of data store in the filter state might be more than a string.

From an API point of view, the policy and the filter_state_key is coupled by the type of the filter state's type.

What happens if the filter state pointed by filter_state_key stores something other than string?

An alternative might be:

string filter_state_key = 1; enum MatchAction { UNSPECIFIED = 0; ALLOW = 1; DENY = 2; } MatchAction match_action = 2; oneof matcher { string string_value = 1; // other types of matcher for extension }

With all the above said, isn't it simpler for this to just read a boolean from a filter state that dictates whether to follow the redirect? What's the use case that requires a string value to be compared against?

penguingao · 2026-07-10T16:47:55Z

+A fourth predicate :ref:`filter_state
+<envoy_v3_api_msg_extensions.internal_redirect.filter_state.v3.FilterStateConfig>`
+can be used to gate the redirect on a filter state value set earlier in the request by another
+filter, allowing a per-request decision about whether an internal redirect is followed.


looks like we just need it to be per-request, in which case, it's really just a boolean to be delivered.

penguingao · 2026-07-10T16:49:16Z

+        const envoy::extensions::internal_redirect::filter_state::v3::FilterStateConfig&>(
+        config, ProtobufMessage::getStrictValidationVisitor());
+
+    using ProtoConfig = envoy::extensions::internal_redirect::filter_state::v3::FilterStateConfig;


If you move this above line 19, you can use it with the call to downcastAndValidate as well.

penguingao · 2026-07-10T17:00:24Z

+  EXPECT_EQ(predicate->name(), "envoy.internal_redirect_predicates.filter_state");
+}
+
+// allow_value: follow only when the gate equals the configured value.


the tests here are not "config_test" - we might want to just rename this file to reflect the nature of this test file.

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

juanmolle · 2026-07-13T17:17:13Z

+    format_string:
+      text_format_source:
+        inline_string: "true"
+


I don't know if there was a reason to not have this generic bool, I added it to be able to write bool directly in filter state, if this is not correct let me know and I will remove and store "true" as string in the filter state

juanmolle · 2026-07-17T11:45:03Z

@penguingao @adisuissa If you have some time to take a look, I will appreciate

penguingao

Thanks! This looks great to me modulo a few nits.

penguingao · 2026-07-17T12:20:54Z

@@ -0,0 +1,8 @@
+Added :ref:`filter_state
+<envoy_v3_api_msg_extensions.internal_redirect.filter_state.v3.FilterStateConfig>`,
+a new internal redirect predicate that gates redirect decisions on a boolean filter-state


nit: s/a new internal redirect/an internal redirect

this will not be new in some future point.

penguingao · 2026-07-17T12:21:37Z

+object set earlier in the request lifecycle (for example by a Lua filter, ext_proc,
+``set_filter_state``, or a dynamic module). The predicate follows the redirect when the
+boolean value is true, enabling per-request redirect control without changing route
+matching. Also added the generic ``envoy.bool`` filter state factory for creating


nit: "Also added ..." here belongs to release note instead of the static documentation.

penguingao · 2026-07-17T12:24:25Z

+  std::unique_ptr<StreamInfo::FilterState::Object>
+  createFromBytes(absl::string_view data) const override {
+    bool value = false;
+    (void)absl::SimpleAtob(data, &value);


I'd prefer we throw if this is false.

it's better to reject bad config than taking an assumption.

penguingao · 2026-07-17T12:26:27Z

+  }
+
+private:
+  using ProtoConfig = envoy::extensions::internal_redirect::filter_state::v3::FilterStateConfig;


is this still useful?

- changelog: use 'an' instead of 'a new', remove envoy.bool sentence - GenericBoolObjectFactory: throw on invalid input instead of defaulting to false - config.h: remove duplicate ProtoConfig typedef Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

- changelog: use 'an' instead of 'a new', remove envoy.bool sentence - GenericBoolObjectFactory: throw on invalid input instead of defaulting to false - config.h: move ProtoConfig typedef to private class scope to avoid duplication Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

penguingao · 2026-07-17T15:20:42Z

This LGTM.

@botengyao for API approval and merge, unless @adisuissa has more feedbacks.

paul-r-gall · 2026-07-21T17:10:58Z

/retest

paul-r-gall · 2026-07-21T17:11:09Z

@botengyao ping for review

botengyao

here is a pass, thanks for adding this support.

botengyao · 2026-07-24T12:20:39Z

+  createFromBytes(absl::string_view data) const override {
+    bool value = false;
+    if (!absl::SimpleAtob(data, &value)) {
+      throw EnvoyException(fmt::format("Invalid boolean value: '{}'", data));


This can happen in request time, and we want to avoid the exceptions, e.g., config HTTP set_filter_state with factory_key: envoy.bool and a value such as %REQ(x-redirect-enabled)%. and then send x-redirect-enabled: garbage, it will crash. We need to return nullptr here, and is this tested? I don't see it.

@botengyao thanks for the review!

As pointed out in review, throwing an exception for invalid boolean values can crash at request time when values come from headers (e.g., using %REQ(x-redirect-enabled)% with garbage input). Return nullptr instead to gracefully handle invalid input, consistent with other filter state factories that return nullptr on bad input. Added test for invalid value from request header. Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

Add tests for setting boolean filter_state values directly from request headers using format_string (e.g., %REQ(x-enable-failover)%). Tests cover: - Valid header values (1, false) create filter_state correctly - Invalid header values (garbage) return nullptr without crashing This validates the fix from PR review where createFromBytes() now returns nullptr instead of throwing for invalid boolean values. Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

repokitteh-read-only Bot added the api label Jun 30, 2026

juanmolle added 2 commits June 30, 2026 18:08

add tests

1ead99f

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

fix format

1632538

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

juanmolle marked this pull request as ready for review June 30, 2026 22:00

juanmolle requested a review from botengyao as a code owner June 30, 2026 22:00

repokitteh-read-only Bot assigned adisuissa Jun 30, 2026

adisuissa reviewed Jul 8, 2026

View reviewed changes

repokitteh-read-only Bot assigned botengyao and penguingao Jul 8, 2026

penguingao reviewed Jul 10, 2026

View reviewed changes

router: simplify filter_state predicate to use boolean

862c0d3

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

juanmolle force-pushed the internal-redirect-filter-state-predicate branch from 16a6b09 to 862c0d3 Compare July 13, 2026 17:00

fix changelog: remove incorrect invert field reference

0ee4779

Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

juanmolle commented Jul 13, 2026

View reviewed changes

juanmolle requested review from adisuissa and penguingao July 13, 2026 17:17

Merge branch 'main' into internal-redirect-filter-state-predicate

e4b7ef7

penguingao approved these changes Jul 17, 2026

View reviewed changes

juanmolle added 2 commits July 17, 2026 10:13

address PR feedback

b0b2164

- changelog: use 'an' instead of 'a new', remove envoy.bool sentence - GenericBoolObjectFactory: throw on invalid input instead of defaulting to false - config.h: remove duplicate ProtoConfig typedef Signed-off-by: Juan Manuel Olle <jolle@salesforce.com>

botengyao requested changes Jul 24, 2026

View reviewed changes

juanmolle force-pushed the internal-redirect-filter-state-predicate branch from 8240502 to 4da4a30 Compare July 24, 2026 18:07

Uh oh!

Conversation

juanmolle commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only Bot commented Jun 30, 2026

Uh oh!

repokitteh-read-only Bot commented Jun 30, 2026

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adisuissa commented Jul 8, 2026

Uh oh!

penguingao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juanmolle commented Jul 17, 2026

Uh oh!

penguingao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penguingao commented Jul 17, 2026

Uh oh!

paul-r-gall commented Jul 21, 2026

Uh oh!

paul-r-gall commented Jul 21, 2026

Uh oh!

botengyao left a comment

Choose a reason for hiding this comment

Uh oh!

botengyao Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

juanmolle commented Jun 30, 2026 •

edited

Loading

botengyao Jul 24, 2026 •

edited

Loading