Skip to content

replace std::string with absl::string_view#11728

Closed
yashwant121 wants to merge 68 commits into
envoyproxy:masterfrom
yashwant121:clutter_remove
Closed

replace std::string with absl::string_view#11728
yashwant121 wants to merge 68 commits into
envoyproxy:masterfrom
yashwant121:clutter_remove

Conversation

@yashwant121
Copy link
Copy Markdown

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

@yashwant121 yashwant121 reopened this Jun 24, 2020
@yashwant121 yashwant121 marked this pull request as ready for review June 24, 2020 10:51
@asraa
Copy link
Copy Markdown
Contributor

asraa commented Jun 24, 2020

Thanks! Could you please fix format and update the description since this is ready to review?

@yashwant121
Copy link
Copy Markdown
Author

@asraa, Just one more doubt.
I am still not able to find any instance of casting of std::string_view to std::string as you asked earlier https://github.com/envoyproxy/envoy/pull/11568#issuecomment-644761527]. I think you have addressed most of the casting part here #11287.
Please give me some example where its happening rn in codebase,So that I can address this issue in this PR or make a new PR for the same.

@asraa
Copy link
Copy Markdown
Contributor

asraa commented Jun 24, 2020

Sure, you could look for std::string casting in those PRs: https://github.com/envoyproxy/envoy/pull/11287/files, https://github.com/envoyproxy/envoy/pull/11285/files

Please take a look, there are many more.
source/common/tracing/http_tracer_impl.cc:285
test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc:712
test/integration/http2_integration_test.cc:1341

htuch and others added 24 commits June 24, 2020 18:48
This shows that the EDS onConfigUpdate() is 2-3x slower when working
with v2 config and doing version conversion with original version
recovery. Followup PRs will optimize.

Relates to envoyproxy#11362 and envoyproxy#10875.

Risk level: Low
Testing: Ran benchmark with -c opt binary.

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…y#11480)

Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
)

Signed-off-by: Sam Flattery <samflattery@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: Shikugawa <rei@tetrate.io>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
This is to help with envoyproxy#9784

Risk Level: Low (added a single test)
Testing: a new unit test and integration test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Risk Level: Low (connect only)
Testing: fixed up tests
Docs Changes: no
Release Notes: no
Runtime guard: no - connect in Alpha
Part of envoyproxy#1451

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…voyproxy#11458)

Follow-on to envoyproxy#10811. The previous change would strip the
transfer-encoding: chunked header if returned by an upstream
during upgrade, but the http/1.1 codec adds the header back.
RFC 7230, Section 3.3.1 requires that no such header appear
in a 1xx or 204 response.

Use runtime feature "envoy.reloadable_features.strict_1xx_and_204_response_headers"
to disable.

Risk Level: medium
Testing: updated tests
Docs Changes: n/a
Release Notes: updated

Signed-off-by: Stephan Zuercher <zuercher@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: Martin Matusiak <numerodix@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…#11385)

Fixes crashes (panic: not reached) due to not implemented protos being fuzzed.

- load balancing policy LOAD_BALANCING_POLICY_CONFIG not implemented, causing server_fuzz_test to crash
- connect matcher not supported in Jwt authentication filter, but matcher available @qiwzhang

Risk Level: Low
Testing: Regression testcases added

Fixes OSS-fuzz issues
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=21876
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17762

Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Add DrainStrategy enum to Options with Graceful and Immediate
Disable probabilistic drain in DrainManager if DrainStrategy == Immediate

Add integration tests
Risk Level: Low.
Testing: Integration tests, verify that the race condition from envoyproxy#11240 does not occur if the probabilistic drain is disabled.

Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…er.sh (envoyproxy#11534)

Signed-off-by: weixiao-huang <hwx.simle@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
The aim is to fix one of the issues encountered while cross-compiling
Envoy (see envoyproxy#11446 and bazel-contrib/rules_foreign_cc#407).

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: Ranjith Kumar <ranjith.dakshana2015@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
… streaming mode (envoyproxy#11456)

Currently httpStatus is not being rewritten when trailers only gRPC streamed response.
Risk Level: low
Testing: added integration tests
Docs Changes: not changed, bugfix
Release Notes: added line to bugfixes section

Signed-off-by: Konstantin Belyalov <arsenicus@mail.ru>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Added test case with "upgrade" request/response in fuzz test. Enabled "upgrade" option by adding an EXPECT_CALL in conn_manager_impl_fuzz_test.cc.
Fuzz test covers mutateRequestHeaders() and mutateResponseHeaders() with above
changes.

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: Florin Coras <fcoras@cisco.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
This is a prerequisite for porting envoy to c++17 as c++17
forbids the initialization of absl::string_view() with nullptr.
The most recent patch of cel-cpp master fixes an instance of this
issue.

Signed-off-by: Yifan Yang <needyyang@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…es (envoyproxy#11368)

Description:
Created _sqlutils_ library to be shared for common functionality between SQL filters. 
mysql and postgres filters will use that library to create filter metadata based on SQL query.
mysql filter was already producing metadata but postges will use the new library as described in envoyproxy#11065. 

Risk Level:
Low: No new functionality has been added and only mysql_proxy filter is affected

Testing:
Added unit tests.

Docs Changes:
No.

Release Notes:
No.

Fixes: envoyproxy#11320

Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Description: groups drain close tests together
Risk Level: n/a (test only)

Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…11554)

split out from envoyproxy#11327

There's a bit of transitive ugliness: declaring the extensions requires security posture, requires stub build files, requires codeowners before the code move, but it'll be pretty short lived.

Risk Level: Low (mostly only APIs)
Testing: n/a
Docs Changes: some of the new docs
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Moving the Generic class definitions into the include file, in preparation for pluggable upstreams
split out from envoyproxy#11327

Risk Level: Low (mostly only header changes)
Testing: n/a
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
As (now) commented, when codecs access runtime, it creates a destruction order invariant that the fake upstreams are torn down before the server (and its runtime). Fixing this in the base integration test and cleaning up test sub-classes.

Risk Level: n/a (test only)
Testing: http2 test passes cleanly with 200 runs (where before it flaked out)
Docs Changes: n/a
Release Notes: n/a
Fixes envoyproxy#11538

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
…yproxy#11161)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
tomocy and others added 13 commits June 24, 2020 20:38
Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Fixes envoyproxy#11535 

Signed-off-by: tomocy <tomocy.dev@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Implement custom O(1) header registration for header maps.

- Remove virtual inheritence from header maps.
- Implement variable inline storage for concrete header maps.
- Various TODO/logic cleanups from previous header refactors.
- Demonstrate what custom header registration looks like for the
  CORS filter. More cleanup of this type can be done.

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
For grpc requests, error message will write to grpc-message header.
LocalReplyConfig::body_format still applies. If it is JSON format, grpc-message header value will be in json too.

But there is a bug when json format is used, it generates an invalid json format as:

grpc-message: '{"code":401,"message":"Jwt is missing"%0A'
It should be:

grpc-message: '{"code":401,"message":"Jwt is missing"}'
Risk Level: Low
Testing: integration test

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Test added by envoyproxy#11414 uses RequestHeaderMapImpl constructor made private
in envoyproxy#11546

Signed-off-by: Florin Coras <fcoras@cisco.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Risk Level: Med
Testing: CI
Docs Changes: Changed clang references
Release Notes: Added

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
close_connections_on_host_health_failure was disabled by the refactoring to
make tcp_proxy use the TCP connection pool implementation.

To fix it we:
* Add a method to immediately close all active connections on a TCP conn pool.
* Invoke this method instead of drainConnections() if close_connections_on_host_health_failure is set.

Signed-off-by: Jonathan Oddy <jonathan.oddy@transferwise.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #11728 was synchronize by yashwant121.

see: more, trace.

@yashwant121 yashwant121 deleted the clutter_remove branch June 24, 2020 17:55
@yashwant121 yashwant121 restored the clutter_remove branch June 24, 2020 17:58
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.