Skip to content

Cherrypick 647aea1 from envoyproxy/envoy #109

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 9, 2019
Merged

Cherrypick 647aea1 from envoyproxy/envoy #109

merged 1 commit into from
Oct 9, 2019

Conversation

nrjpoddar
Copy link
Member

@nrjpoddar nrjpoddar commented Oct 3, 2019 •
edited
Loading

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@nrjpoddar nrjpoddar requested a review from howardjohn October 3, 2019 14:27
proto: re-implement MessageUtil::hash function to consistently hash A…
a54b9c6 
…ny recursively (envoyproxy#8231)

Use TextFormat::Print with SetExpandAny(true) instead of SerializeAsString to calculate hash with Any expanded.

Risk Level: Med
Testing: CI, regression test

Fixes envoyproxy#5744.

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
(cherry picked from commit 647aea1)
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Oct 3, 2019
@nrjpoddar
Copy link
Member Author

@howardjohn @rlenglet are there no CI integrations for this repo?

@mandarjog mandarjog requested review from kyessenov and removed request for howardjohn October 3, 2019 14:49
@mandarjog
Copy link

Is this a cherry pick?

@nrjpoddar
Copy link
Member Author

Yes cherry-pick of 647aea1 from envoyproxy/envoy to get stable serialization for Protobuf.Any

@nrjpoddar nrjpoddar changed the title proto: re-implement MessageUtil::hash function to consistently hash A… Cherrypick 647aea1 from envoyproxy/envoy Oct 3, 2019
@kyessenov
Copy link

Ping @JimmyCYJ -- this fixes SDS, too, I think.

@kyessenov
Copy link

This doesn't look like a clean cherry-pick. Did you amend the commit?

@nrjpoddar
Copy link
Member Author

This doesn't look like a clean cherry-pick. Did you amend the commit?

Yes, there was a conflict in header includes:

#include "envoy/config/grpc_credential/v2alpha/file_based_metadata.pb.h"

#include "common/common/base64.h"

rlenglet
rlenglet suggested changes Oct 3, 2019
View reviewed changes
Copy link

@rlenglet rlenglet left a comment

Choose a reason for hiding this comment

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

Please someone else review and approve.
I'm not rejecting the PR, I'm only blocking merging it until after 1.3.2 is released on October 8, to keep the release-1.3 history as-is.
I'll approve this PR afterwards to include into 1.3.3.

@nrjpoddar
Copy link
Member Author

Ping @JimmyCYJ -- this fixes SDS, too, I think.

@JimmyCYJ what SDS issue are we talking about here?

@kyessenov can I get a lgtm?

@howardjohn howardjohn mentioned this pull request Oct 7, 2019
Closed
lambdai
lambdai approved these changes Oct 7, 2019
View reviewed changes
Copy link

@lambdai lambdai left a comment

Choose a reason for hiding this comment

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

lgtm

This one is a MUST-HAVE in 1.3.3

@nrjpoddar
Copy link
Member Author

@rlenglet can this merged now?

@rlenglet
Copy link

rlenglet commented Oct 8, 2019

No, 1.3.2 hasn't been released. I'll merge this after we do some cleanup post-1.3.2.

@rlenglet
Copy link

rlenglet commented Oct 9, 2019

@nrjpoddar Please add an item for this PR into the 1.3.3 release note draft.

@rlenglet rlenglet merged commit dfec978 into istio:release-1.3 Oct 9, 2019
@rlenglet rlenglet mentioned this pull request Oct 10, 2019
Closed
howardjohn pushed a commit that referenced this pull request Mar 3, 2020
@lizan @PiotrSikora
buffer: draining any zero byte fragments (envoyproxy#9837) (#109) (#123)
1ca8596 
Given that we allow creating zero byte fragments, it'd be good to proactively drain them. For example if someone is doing timing instrumentation and wants to know when Network::Connection data is written to the kernel, it could be useful to have a zero byte sentinel.

Risk Level: Low (I don't think anyone is adding zero byte fragments yet)
Testing: new unit test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Co-authored-by: Lizan Zhou <lizan@tetrate.io>
istio-testing pushed a commit that referenced this pull request Mar 3, 2020
@PiotrSikora @lizan
buffer: draining any zero byte fragments (envoyproxy#9837) (#109) (#172)
e4001fa 
Given that we allow creating zero byte fragments, it'd be good to proactively drain them. For example if someone is doing timing instrumentation and wants to know when Network::Connection data is written to the kernel, it could be useful to have a zero byte sentinel.

Risk Level: Low (I don't think anyone is adding zero byte fragments yet)
Testing: new unit test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

Co-authored-by: Lizan Zhou <lizan@tetrate.io>
fpesce pushed a commit that referenced this pull request Jun 30, 2020
@lizan
buffer: draining any zero byte fragments (envoyproxy#9837) (#109)
f6a1127 
Given that we allow creating zero byte fragments, it'd be good to proactively drain them. For example if someone is doing timing instrumentation and wants to know when Network::Connection data is written to the kernel, it could be useful to have a zero byte sentinel.

Risk Level: Low (I don't think anyone is adding zero byte fragments yet)
Testing: new unit test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
brian-avery pushed a commit that referenced this pull request Jun 30, 2020
@lizan
buffer: draining any zero byte fragments (envoyproxy#9837) (#109) (#123)
92bd1e9 
Given that we allow creating zero byte fragments, it'd be good to proactively drain them. For example if someone is doing timing instrumentation and wants to know when Network::Connection data is written to the kernel, it could be useful to have a zero byte sentinel.

Risk Level: Low (I don't think anyone is adding zero byte fragments yet)
Testing: new unit test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Co-authored-by: Lizan Zhou <lizan@tetrate.io>
Miss-you pushed a commit to Miss-you/envoy that referenced this pull request Nov 17, 2020
@majinghe
Merge pull request istio#109 from helight/zh_trans_index
297c5b9 
zh translation: index
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyessenov kyessenov kyessenov approved these changes

+2 more reviewers

@rlenglet rlenglet rlenglet approved these changes

@lambdai lambdai lambdai approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@rlenglet rlenglet

Labels
cla: yes size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nrjpoddar @googlebot @mandarjog @kyessenov @rlenglet @lambdai @istio-testing