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

router: Add path rewrite via regex #1

Closed
wants to merge 83 commits into from

Conversation

gabrieltaylor
Copy link
Owner

@gabrieltaylor gabrieltaylor commented Aug 31, 2018

Description:
Enables flexible path rewrites using regular expressions similar to the nginx rewrite directive.

The supported syntax is ^/foo/(bar)/(.*)$ /$1/$2 which would rewrite a path from /foo/bar/baz/123 to /bar/baz/123.

Relates to:
envoyproxy#2092
envoyproxy/data-plane-api#504
envoyproxy#4274

Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

ramaraochavali and others added 30 commits August 31, 2018 09:54
Description: Use SDS api that fetches secrets from remote SDS server. Secrets are stored in Secret Provider. Listeners and Clusters are updated when secrets are received.
Risk Level: Low
Testing: Unit tests and integration tests
Fixes envoyproxy#1194

Signed-off-by: Jimmy Chen [email protected]
 Google proto string incompatibility fix.
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Matt Rice <[email protected]>
In envoyproxy#4262, an ASSERT was added to guarantee that we wouldn't violate the
codec response contract regarding :status. This needed a corresponding
change in the H2 codec fuzzer.

Risk level: Low
Testing: Corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
Existing strftime uses did not correctly handle buffer overflow
conditions, where strftime returns 0 and the buffer contents are
undefined.

This was discovered by an internal equivalent of oss-fuzz.

Risk level: Low
Testing: Unit test and corpus entry added.

Signed-off-by: Harvey Tuch <[email protected]>
… closed connection (envoyproxy#4296)

Fixing and regression testing envoyproxy#3639

Risk Level: Low: avoids a no-op call during connection teardown
Testing: verified flow in integration test, wrote a regression unit test
Docs Changes: n/a
Release Notes: n/a
Fixes envoyproxy#3639

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Rama [email protected]

Description: Fixes minor doc issue
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Signed-off-by: Daniel Hochman <[email protected]>

Risk: trivial (Documentation only change)
Risk Level: doc-only
Testing: n/a doc-only change
Docs Changes: CONTRIBUTING.md change.
Release Notes: n/a

Signed-off-by: Dan Noé <[email protected]>
…4319)

When the server thread exits independently of the main test thread (e.g. exception catch), we
could race on the access to the server admin port via server_. We latch this in this PR to avoid
unsafe behavior. We might still have a stale address, but the request should then fail or timeout.

Fixes oss-fuzz issues https://oss-fuzz.com/v2/testcase-detail/5719024990683136 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10027.

Risk level: Low
Testing: TSAN integration tests.

Signed-off-by: Harvey Tuch <[email protected]>
When the RequestInfo start_time exceeds max limit, set to zero to avoid
undefined behavior. Before this patch, utility's fromRequestInfo experienced
similar error below:

/usr/include/c++/5/chrono:176:67: runtime error: signed integer overflow:
9799832698963886 * 1000 cannot be represented in type 'long int'

Fixes oss-fuzz issues:

* https://oss-fuzz.com/v2/testcase-detail/5647641023610880 (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10124)
* https://oss-fuzz.com/v2/testcase-detail/5701824317751296 (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10128)

Risk Level: Low
Testing: ASAN/UBSAN tests, corpus entries added.

Signed-off-by: Dhi Aurrahman <[email protected]>
…s marked dead

This PR makes sure we reset dynamic metadata wrapper when its parent (request info wrapper) is marked dead.

Risk Level: Low
Testing: Add integration testing
Docs Changes: N/A
Release Notes: N/A
Fixes envoyproxy#4305

Signed-off-by: Dhi Aurrahman <[email protected]>
fix static initialization fiasco problem in envoyproxy#4288
Risk Level: low

Signed-off-by: tianqian.zyf <[email protected]>
Build for Linux on Power (ppc64le). There is no luajit support for ppc64le, so
this PR puts in logic to not add that recipe to the sandbox speedup that
envoy uses for its build.

There were already some differences taken into account for Windows, so
hopefully this isn't too far out there.

Description: Build for Linux on Power (ppc64le).
Risk Level: low
Testing: manual + have built this into istio/proxy for a few releases now
Docs Changes: Would need to add a bit about Power support
Release Notes: same

Signed-off-by: Christy Norman <[email protected]>
Currently host level stats in clusters proto uses map so they are outputted in random order. This PR changes it to list so that the order is predictable.

Risk Level: Low
Testing: Added automated tests
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rama <[email protected]>
…nvoyproxy#4309)

Making sure if waitForHalfClose stops waiting due to the short waitFor timeout of 5ms, rather than a spurious event occurring, that the test continues waiting for half close.

Risk Level: low (test only)
Testing: tcp_proxy_integration_testx1000 test/integration:all x1
Docs Changes: n./a
Release Notes: n/a
Fixes envoyproxy#4031

Signed-off-by: Alyssa Wilk <[email protected]>
…e with simulated timers (envoyproxy#4257)

Unifies the TimeSource so that system and monotonic sources are always handled similarly (mocks, real-time, or in the future, simulated), and derives a new interface, Event::TimeSystem, which managers timers. This is a pure refactor; none of the operation or semantics of any production or test code is changed with this PR. This is another step in resolving envoyproxy#4160.

Risk Level: medium, just because PR is large and merge conflicts happen, but it should be pretty safe
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <[email protected]>
)

This PR added a new principal_name of type StringMatcher to rbac Authenticated and mark the existing user field as deprecated. This gives us more flexibility to express more matching rules against peer certificate.

Risk Level: Low
Testing: Added unit tests

Signed-off-by: Yangmin Zhu <[email protected]>
…roxy#4232)

Description: FakeConnectionBase::waitForDisconnect and FakeHttpConnection::waitForNewStream were returning assertion successes when they timed out, because an AssertionResult constructed with a (non-empty) string counts as a success. Fix that.
Risk Level: Low (test only)
Testing: bazel test //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Michael Behr <[email protected]>
When all domains of all virtual hosts had wildcard characters and there
was a default virtual host, RouteMatcher::findVirtualHost() used to
erroneously ignore the wildcard suffixes.

This patch fixes the issue and introduces a unit test that covers this
case.

Risk level: Medium. The fix is straightforward, but users who rely on
the erroneous behavior might be affected.

Testing: Introduced TestRoutesWithWildcardAndDefaultOnly

Signed-off-by: Tal Nordan [email protected]
* Fix crash in tcp_proxy.

Closing the upstream connection is not safe from the Filter destructor,
because it triggers events back into the downstream connection, which
is partially destructed.

Ensure that the upstream connection is closed before the destructor is called.

Fixes envoyproxy#4310

Signed-off-by: Greg Greenway <[email protected]>
Description:
The ssl_ctx_ fields of the ServerSslSocketFactory and ClientSslSocketFactory are accessed and mutated from different threads without external serialization. This is a bug since instances of std::shared_ptr are not thread-safe (though different instances pointing to the same object are). This patch fixes the bug by using a mutex to serialize accesses.

Risk Level: Low
Testing: ran test suite
Docs Changes: n/a
Release Notes: n/a
…4286)

Modifies the Thrift router to allow protocols to upgrade on initial
data from the downstream connection and upgrade an upstream connection
before transmitting a request. As part of this work, Transport and
Protocol objects are reused across downstream connections and for
an upstream's request and response.

*Risk Level*: low, upgrade path unused
*Testing*: unit tests
*Docs Changes*: n/a
*Release Notes*: n/a

Signed-off-by: Stephan Zuercher <[email protected]>
Add a terminal attribute to request hash policy.

Think about a case where customers want to hash on a cookie if it's present but if it's not present, do best-effort sticky based on something like IP so the customer has a stable hash.

This "terminal" allows request hashing to have the ability of "if A not working, fallback to B.", which also saves time to generate the hash.

Changes:
* Add a terminal attribute to HashMethod, which shortcircuit the hash generating process if a policy is marked terminal and there is a hash computed already.

Signed-off-by: Xin Zhuang [email protected]

Description: Add terminal attribute to request hash.
Risk Level: Low
Testing: unit tests.
…on (envoyproxy#4288)

Risk Level: low
Testing: N/A
Docs Changes:
Release Notes:
Fixes envoyproxy#4278

Signed-off-by: tianqian.zyf <[email protected]>
OS X's localhost interface often takes longer to fail to connect
than Linux. Modifies RawConnectionDriver to detect when its
underlying connection is connected or failed and pauses the
echo_integration_test until that state is reached before
checking the connection state.

Risk Level: low, test-only changes
Testing: existing tests
Doc Changes: n/a
Release Notes: n/a

Signed-off-by: Stephan Zuercher [email protected]
Tony Allen and others added 27 commits September 12, 2018 16:16
CI for data-plane-api has been failing since gogoproto was updated at envoyproxy/data-plane-api@43bc9ad. This patch makes it so that we also copy over the entirety of the WORKSPACE.

I am creating this PR so I can test if the CI passes successfully.

Risk Level:
Low

Testing:
Running CI in PR.

Fixes envoyproxy#4218

Signed-off-by: Tony Allen <[email protected]>
Rather than use the downstream connection's sequence ID, track
sequence IDs per upstream connection.

*Risk Level*: low
*Testing*: unit tests
*Docs Changes*: n/a
*Release Notes*: n/a

Signed-off-by: Stephan Zuercher <[email protected]>
* Add docs for /stats filtering
* add filtering capability to /stats

Signed-off-by: James Buckland <[email protected]>
…4412)

tls_certificates are necessary else validation errors with
"A single TLS certificate is required for server contexts".

context_config_impl and relevant tests have also been updated
to clarify that tls_certificates is required in the server context.
The existing error is still output if more than one certificate is detected.

Risk Level: Low, clarifying error message and docs fix
Testing: bazel test
Docs Changes: Included
Fixes envoyproxy#4408

Signed-off-by: Christopher M. Luciano <[email protected]>
Explains what the different values output means and writes to stderr
instead of stdout. This should make the error output appear in the
testlogs.

Risk Level: Low, change to test error reporting
Testing: bazel test //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Snow Pettersen <[email protected]>
…wn source file (envoyproxy#4406)

Making this change so others can depend on metadatamatchcriteria_impl without needing
config_impl. Also update comments on metadata_match to specify that these fields only apply for the subset load balancer.

Context: https://github.com/envoyproxy/envoy/pull/4402/files#r216856765

Risk Level: low
Testing: tests, new and old, pass
Doc Changes: clarified docs
Release Notes: n/a

Signed-off-by: Brian Ramos <[email protected]>
These functions are equivalent to calling methods on the std::atomic
objects themselves, and the method invocations are easier to read.

Risk Level: Low
Testing: added format test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alex Konradi <[email protected]>
JwksFetcher wraps up HTTP acquisition of JWKS strings converting them into a concrete type on the way.
JwksFetcher is reusable so can be used in a wider context.

Tests updated and fixed where necessary.

We are in the process of implementing a new Envoy filter based on the design presented here and wish to re-use existing logic in the jwt_authn filter. We've split out the logic we're interested in into a new class called JwksFetcher. Later PRs will re-use the split out logic an OpenID Connect filter.

This is the second crack at this PR (see envoyproxy#4225 which went horribly wrong after following the DCO rebase guidelines).

Risk Level: Medium
Testing: Add replacement and additional unit tests for the logic that's been moved.

Signed-off-by: Nick A. Smith <[email protected]>
…envoyproxy#4402)

This change adds the ability to attach metadata to the clusters and weighted clusters to allow users to filter a down to instances that match metadata criteria provided. This is used only when the subset load balancer is enabled and leverages what exists already for http metadata matching.

Changes include:
- construct MetadataMatchCriteria objects from protobufs
- expose through Thrift::Router::RouteEntry and use in router impl

Risk Level: LOW
Testing: tests, new and old, pass
Docs Changes: added description for new proto fields. docs build successfully.
Release Notes: n/a

Signed-off-by: Brian Ramos <[email protected]>
Wires up route configuration to allow specifying what hosts should be
reattempted during retry host selection.
Risk Level: Medium, some changes made to the router. Otherwise new optional feature
Testing: unit and integration test
Docs Changes: n/a
Release Notes: n/a

Part of envoyproxy#3958

Signed-off-by: Snow Pettersen <[email protected]>
…4418)

In this case, the early close fix only worked if the upstream connection
hadn't yet been formed. Instead, handle early close as long as there
isn't a pending oneway request. Also discovered a case where the
Unframed transport causes an RPC to start on a zero-length buffer with
end_stream set.

Risk Level: low
Testing: added tests, ran 2000x each
Doc Changes: n/a
Release Notes: n/a
Fixes: envoyproxy#4416

Signed-off-by: Stephan Zuercher <[email protected]>
Add an overload action in the http connection manager to immediately close new streams in case of envoy overload (issue envoyproxy#373).

Signed-off-by: Elisha Ziskind <[email protected]>
There is an inconsistency on how xDS and ADS handle failures. ADS increments update_failure stat for network failures and update_rejected stat for schema/validation issues where as xDS just increments update_failure in both the cases. Also update_rejected stat is un documented. This PR addresses both.

Risk Level: Low
Testing: Automated tests
Docs Changes: documented stats
Release Notes: updated

Signed-off-by: Rama <[email protected]>
Add a new field local_credentail into GoogleGrpc which supports Envoy to use gRPC local channel credentials.
Updated gRPC library to 1.15.0 release, which provides new methods that we need in order to use local channel credentials. See grpc/grpc#15909.
Certain Google gRPC features, such as passing Google default call credential, only works with a valid channel credential. Local credential is a valid channel credential.

Risk Level: Low
Testing: Unit test

Signed-off-by: JimmyCYJ <[email protected]>
As per https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#making-the-compilation-faster, moving ctor/dtor definition out of the header files can lead to faster builds.
Risk Level: Low, minor refactor
Testing: Run CI
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Snow Pettersen <[email protected]>
… client error (envoyproxy#4199)

Ext_Authz HTTP client has been modified so that 5xx errors received from the authorization server will set the filter response status to error instead of denied and HTTP status code field to Forbidden. The gRPC client has been also modified in order to return HTTP status code Forbidden whenever an error between the client and the authorization server occurs.

Risk Level: low
Testing: unit tests, manual tests.
Docs Changes: not needed.
Fixes issue: envoyproxy#4124.

Signed-off-by: Gabriel <[email protected]>
envoyproxy#4344)

Extract common buffer helper functions.

This is split out and rebased with upstream as per discussion in envoyproxy#3502 (comment).

Risk Level: Medium
Testing: Unit tests added for buffer and byte_order changes
Docs Changes: N/A
Release Notes: Per discussion with @zuercher this change itself shouldn't need an entry in version_history.rst

Signed-off-by: Jason Jian <[email protected]>
…System or TimeSource (envoyproxy#4434)

In order to fully control time during tests, direct calls to system-provided time functions must not be used. These had previously not been fixed. In at least one case I think there was real-time deltas being compared in a test, which really needed to be mocked, so that change had to occur in this PR. Another step toward resolution of envoyproxy#4160

Risk Level: medium, due to size of PR.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <[email protected]>
Add an alternate TimeSystem implementation which keeps manually-adjusted time and timer-callbacks making sense together. This is intended for use in most if not all unit tests, and many integration tests. This is another step to resolve envoyproxy#4160.

Risk Level: low -- this is a new test-only class that's not used anywhere yet.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <[email protected]>
…oxy#4436)

When Envoy is stuck during secondary cluster initialization because management server does not respond back with Endpoints of any cluster, Envoy just prints the line
"cm init: initializing secondary clusters" and stays there. It is very difficult to debug which cluster initialization is causing Envoy to wait for ever.

Added couple of log lines that would help to figure out that details.

Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rama <[email protected]>
)

 Refactor SdsApi to support dynamic certificate validation context, and support Envoy to fetch certificate validation context from remote server via SDS API.
Risk Level: Low
Testing: Unit tests and integration tests.
Fixes envoyproxy#1194

Signed-off-by: JimmyCYJ <[email protected]>
…ime. (envoyproxy#4441)

The new SimulatedTimeSystem is a little terser to use than mock time, and more powerful because it's integrated with timers. This PR takes it around the block for a test spin by using it in lieu of a mock, exposing an opportunity for an overload to make it even terser to use.

Risk Level: low
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <[email protected]>
To encourage users to use v2 configuration. Related to envoyproxy#2100.

Risk Level: N/A, documentation change.
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Taiki Ono <[email protected]>
Implements the Twitter variant of the Thrift binary protocol,
as implemented by the finagle library.

*Risk Level*: low
*Testing*: unit tests
*Docs Changes*: updated API docs
*Release Notes*: n/a

Signed-off-by: Stephan Zuercher <[email protected]>
…roxy#4448)

According to comment in PR envoyproxy#4355, we want to avoid std::dynamic_pointer_cast in SecretManagerImpl. This PR creates a template class and moves findOrCreate method into the template class.

Risk Level: Low
Testing: Existing unit tests and integration tests.

Signed-off-by: JimmyCYJ <[email protected]>
Plumbs through the max_host_selection_count parameter from retry policy config -> router
Risk Level: Low
Testing: UT
Docs Changes: n/a
Release Notes: n/a
Part of envoyproxy#3958

Signed-off-by: Snow Pettersen <[email protected]>
Signed-off-by: Gabriel Taylor Russ <[email protected]>
@gabrieltaylor
Copy link
Owner Author

The path functionality this was adding would be better added as trie.

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.