Skip to content

Update Envoy-WASM SHA to latest with HTTP inspector fixes.#2549

Merged
istio-testing merged 1 commit intoistio:release-1.4from
PiotrSikora:envoy_5a7a1fd1aa
Nov 11, 2019
Merged

Update Envoy-WASM SHA to latest with HTTP inspector fixes.#2549
istio-testing merged 1 commit intoistio:release-1.4from
PiotrSikora:envoy_5a7a1fd1aa

Conversation

@PiotrSikora
Copy link
Contributor

Pulling in istio/envoy#120.

Signed-off-by: Piotr Sikora piotrsikora@google.com

Pulling in istio/envoy#120.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora requested review from a team, howardjohn and yxue November 11, 2019 11:28
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 11, 2019
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 11, 2019
@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

5 similar comments
@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@jwendell
Copy link
Member

/retest

@rshriram
Copy link
Member

Why is this getting an exception when the ALS one is not? Shouldn't we wait until we decide on what to do about the proxy version (basing off envoy stable or master) ?

@rshriram
Copy link
Member

/hold

@rshriram rshriram added the do-not-merge Block automatic merging of a PR. label Nov 11, 2019
Copy link
Member

@rshriram rshriram left a comment

Choose a reason for hiding this comment

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

I would like to see a clear explanation for this dual policy of blocking specific upstream changes until envoy release while still cherry picking ones into our fork and syncing to istio/proxy.

@PiotrSikora
Copy link
Contributor Author

@rshriram this is a security fix.

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@howardjohn howardjohn dismissed rshriram’s stale review November 11, 2019 21:11

this has been answered and blocking 1.4.0

@howardjohn
Copy link
Member

/retest

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit

@PiotrSikora
Copy link
Contributor Author

PiotrSikora commented Nov 11, 2019

@rshriram could you remove the do-not-merge label, since it's blocking merge and the release?

@howardjohn howardjohn removed the do-not-merge Block automatic merging of a PR. label Nov 11, 2019
@howardjohn
Copy link
Member

I think @rshriram 's discussion is reasonable, but we should not block the release on this

@rshriram
Copy link
Member

My point is that we have a very buggy ALS system in 1.4 release today. And fixes for those are not allowed because of deviation from the baseline release. So why is this ? We could turn off sniffing (we already do) in inbound. So, this might as well wait for an entire release ?

@PiotrSikora
Copy link
Contributor Author

@rshriram the whole point of stable releases of Envoy and using them in Istio Proxy is to support bugfixes such as the ALS one. I don't recall anybody not allowing it to be cherry-picked...

@PiotrSikora
Copy link
Contributor Author

/test proxy-presubmit-asan

@istio-testing istio-testing merged commit ed3b56d into istio:release-1.4 Nov 11, 2019
@lizan
Copy link
Contributor

lizan commented Nov 12, 2019

@PiotrSikora @rshriram yeah sure let's cherry-pick, PTAL istio/envoy#121

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

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants