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.

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>

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

juanmolle commented Jun 30, 2026 •

edited

Loading