Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full Set Of Changes For Pattern Match and Rewrite #22207

Merged

Conversation

silverstar194
Copy link
Contributor

@silverstar194 silverstar194 commented Jul 14, 2022

Commit Message:
Full Set Of Changes For Pattern Match and Rewrite

Additional Description:
This PR will implement issue detailed here and described below: #7763

Match Patterns and Templates

Wildcard support based on match patterns and templates.

  • A match pattern matches an incoming URL path.
    Match patterns support glob operators to match URL text and variable definitions to bind matched text to names.

  • Template patterns are used to re-write URLs.
    Template patterns build new URLs and may reference variables bound by a match pattern.

Match Examples

  • /**.m3u8 would match /foo.m3u8 and /foo/bar.m3u8.
  • /{dir_name}/*.ts would match /example/file.ts and bind dir_name="example" for a later template match to use.
  • /{dir_name}/**.ts would match /example/path/file.ts and bind dir_name="example" for a later template match to use. This would also match /example/.ts, which may or may not be a desired behavior.
  • /{path=v1/*}/{file=*.ts} would match /v1/example/movie.ts (binding path="v1/example" and file="movie"), but would not match /v0/example/movie.ts.

See post for full details and example:
#7763 (comment)

Risk Level:
Testing:
Unit tests. Both both internal matching/rewrite library and config/data plane changes.

Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

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

see: more, trace.

@repokitteh-read-only
Copy link

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

🐱

Caused by: #22207 was opened by silverstar194.

see: more, trace.

Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
…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 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
…ter-filter-matching-lib-control-plane

Signed-off-by: silverstar195 <[email protected]>
@silverstar194 silverstar194 requested review from RyanTheOptimist and zuercher and removed request for RyanTheOptimist and zuercher September 9, 2022 13:51
source/common/http/async_client_impl.cc Outdated Show resolved Hide resolved
source/common/http/async_client_impl.h Outdated Show resolved Hide resolved
source/common/http/async_client_impl.h Outdated Show resolved Hide resolved
tools/spelling/spelling_dictionary.txt Outdated Show resolved Hide resolved
test/per_file_coverage.sh Outdated Show resolved Hide resolved
test/mocks/router/mocks.h Outdated Show resolved Hide resolved
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
@silverstar194
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #22207 (comment) was created by @silverstar194.

see: more, trace.

@silverstar194
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #22207 (comment) was created by @silverstar194.

see: more, trace.

Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

I think we definitely need to sort out the test coverage issues, but we're probably ready for @zuercher to take a cross-company review.

tools/spelling/spelling_dictionary.txt Outdated Show resolved Hide resolved
test/per_file_coverage.sh Outdated Show resolved Hide resolved
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
Signed-off-by: silverstar195 <[email protected]>
@silverstar194
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #22207 (comment) was created by @silverstar194.

see: more, trace.

Signed-off-by: silverstar195 <[email protected]>
@silverstar194 silverstar194 requested review from RyanTheOptimist and zuercher and removed request for RyanTheOptimist September 12, 2022 15:54
@silverstar194
Copy link
Contributor Author

@zuercher Could you take a look?

Copy link
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Thanks! I think this looks good.

@RyanTheOptimist
Copy link
Contributor

@htuch can you give this an API review?

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

/lgtm api

@htuch htuch merged commit 8cfc61f into envoyproxy:main Sep 14, 2022
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.

5 participants