Skip to content

fix: router_check_tool load runtime flags before running tests#45997

Open
rgctlln wants to merge 1 commit into
envoyproxy:mainfrom
rgctlln:fix-router_check-runtime-flags
Open

fix: router_check_tool load runtime flags before running tests#45997
rgctlln wants to merge 1 commit into
envoyproxy:mainfrom
rgctlln:fix-router_check-runtime-flags

Conversation

@rgctlln

@rgctlln rgctlln commented Jul 6, 2026

Copy link
Copy Markdown

Commit Message: router_check_tool: Load runtime flags before running the tests.
Additional Description: This adds a new field to the Validation configuration to load runtime flags. More context in #35662 (comment)
Risk Level: Low
Testing: Extended existing tests. Without re2.max_program_size.error_level flag set the test fails.

bazel-bin/test/tools/router_check/router_check_tool -c test/tools/router_check/test/config/DefaultRuntime.yaml -t test/tools/router_check/test/config/DefaultRuntime.golden.proto.json
Fixes: #36660

@repokitteh-read-only

Copy link
Copy Markdown

Hi @rgctlln, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #45997 was opened by rgctlln.

see: more, trace.

@rgctlln rgctlln had a problem deploying to external-contributors July 6, 2026 16:00 — with GitHub Actions Error
Signed-off-by: Kirill Alferov <k.g.alferov@tbank.ru>
Signed-off-by: Kirill Alferov <kirillalfa94@gmail.com>
@rgctlln rgctlln force-pushed the fix-router_check-runtime-flags branch from 0ec63c9 to 08ca2b1 Compare July 6, 2026 16:53
@rgctlln rgctlln requested a deployment to external-contributors July 6, 2026 16:53 — with GitHub Actions Waiting

@adisuissa adisuissa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Left a few questions.
/wait-any

RouterCheckTool RouterCheckTool::create(const std::string& router_config_file,
const std::string& validation_config_file,
const bool disable_deprecation_check) {
// TODO(hennna): Allow users to load a full config and extract the route configuration from it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed?
Is it because of this PR or a previous one?

Envoy::RouterCheckTool checktool =
Envoy::RouterCheckTool::create(options.configPath(), options.disableDeprecationCheck());
Envoy::RouterCheckTool checktool = Envoy::RouterCheckTool::create(
options.configPath(), options.testPath(), options.disableDeprecationCheck());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is testPath() converted to validation_config_file?

repeated ValidationItem tests = 1 [(validate.rules).repeated .min_items = 1];

// The runtime flags to be loaded before running the tests.
google.protobuf.Struct runtime = 2;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be something other than Struct (specifically defined as a normal runtime layer)?
One option is to use the message type defined in api/envoy/service/runtime/v3/rtds.proto. What are the tradeoffs, and why did you choose use Struct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants