Skip to content

Add path_matcher + prefix_rewrite validation#23160

Merged
adisuissa merged 270 commits intoenvoyproxy:mainfrom
silverstar194:pattern_template-prefix_rewrite-fix
Sep 19, 2022
Merged

Add path_matcher + prefix_rewrite validation#23160
adisuissa merged 270 commits intoenvoyproxy:mainfrom
silverstar194:pattern_template-prefix_rewrite-fix

Conversation

@silverstar194
Copy link
Copy Markdown
Contributor

Commit Message:
Add path_matcher + prefix_rewrite validation

Additional Description:
You should not be able to perform a prefix_rewrite when matching with a path_matcher.

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]

[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
…er-filter-matching-lib-control-plane

# Conflicts:
#	envoy/router/BUILD
#	source/extensions/extensions_build_config.bzl
#	source/extensions/extensions_metadata.yaml
#	tools/extensions/extensions_schema.yaml

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
…ite-extention-lib

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
…ter-filter-matching-lib-control-plane

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: silverstar195 <seanmaloney@google.com>

# Conflicts:
#	api/envoy/config/route/v3/route_components.proto
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
…g-lib-control-plane

Signed-off-by: silverstar195 <seanmaloney@google.com>
…g-lib-control-plane

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
…-fuzzing

Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
Signed-off-by: silverstar195 <seanmaloney@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

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: #23160 was opened by silverstar194.

see: more, trace.

@silverstar194
Copy link
Copy Markdown
Contributor Author

silverstar194 commented Sep 19, 2022

/assign adisuissa

1 similar comment
@silverstar194
Copy link
Copy Markdown
Contributor Author

/assign adisuissa

@silverstar194 silverstar194 marked this pull request as ready for review September 19, 2022 14:20
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@adisuissa adisuissa merged commit 36fdc75 into envoyproxy:main Sep 19, 2022
"Specify only one of prefix_rewrite, regex_rewrite or path_rewrite_policy");
}

if (!route.route().prefix_rewrite().empty() && path_matcher_ != nullptr) {
Copy link
Copy Markdown
Member

@tyxia tyxia Sep 21, 2022

Choose a reason for hiding this comment

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

nit: We should be able to directly use prefix_rewrite_ that has already been initialized, right? This avoids redundant function call to route() and prefix_rewrite().

We can even merge this into the if block in line 659. But I think merging will be a bit micro-optimization and keep the separate two checks here helps readability.

Changing to prefix_rewrite_ is good enough IMO.

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Sep 21, 2022

/assign @tyxia

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants