Proxy-WASM configuration protos. by jplevyak · Pull Request #9256 · envoyproxy/envoy

jplevyak · 2019-12-06T22:22:52Z

Signed-off-by: John Plevyak jplevyak@gmail.com

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

Description: Proxy_WASM configuration protos.
Risk Level: Low
Testing: N/A
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only · 2019-12-06T22:22:57Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9256 was opened by jplevyak.

see: more, trace.

htuch

Thanks for the split. I have a lot of feedback around readability; it's important to keep in mind that these are the docs that end users of Envoy see in https://www.envoyproxy.io/docs/envoy/latest/api/api, so we have really high standards around making them self contained, understandable by average Envoy users and as context free as possible.
/wait

htuch · 2019-12-06T23:21:13Z

+  // multiple filters/services are handled by the same vm_id and root_id and for logging/debugging.
+  string name = 1;
+
+  // A unique ID for a set of filters/services in a VM which will share a RootContext and Contexts


All docs in the API either should be consumable standalone or should have :ref: to something that explains concepts. At this point, the reader has no idea about root context or event contexts.

@PiotrSikora will be adding the ABI doc and we will reference it here.

I would prefer that the ABI doc lands first or we keep track with some annotations of every place we're missing these links.

@PiotrSikora Please attach ABI doc here.

htuch · 2019-12-06T23:22:46Z

+  // The Wasm runtime type (see source/extensions/common/wasm/well_known_names.h).
+  string runtime = 2;
+
+  // The Wasm code that Envoy will execute.


I think we need a WASM section in the Envoy architecture intro or configuration docs that we link to that explains things like file format etc.

@PiotrSikora can we push some of the ABI/docs to the envoyproxy/envoy repo?

mattklein123

Thanks will take another pass once the initial set of comments are addressed.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

mattklein123

Thanks, at a high level this looks good, but I think to review this there needs to be either a high level arch overview doc, or some other doc that succinctly describes the proposed architecture re: root contexts, etc. If there is a short gdoc we can read for now prior to merging into arch overview that's fine, but please provide that and then we can move forward with this.

/wait

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only · 2019-12-18T19:07:14Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9256 was synchronize by jplevyak.

see: more, trace.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak · 2019-12-24T00:25:30Z

Any suggestions for how to debug the CI issues. For some reason, despite producing titanic amounts of output, the one thing which is singularly missing is the actual error which caused the CI build to fail.

For example, envoy-linux (format) claims that there is a formatting error, but doesn't provide any error message. When I run the tests locally everything passes.

Any suggestions?

…m-upstream-protos

jplevyak · 2019-12-30T01:06:36Z

The CI formatting error "suggestion" is to delete the files that this PR adds.

Needless to say not very helpful. Any suggestions?

jplevyak · 2019-12-30T18:10:17Z

#9358 discusses the filter name issue

Signed-off-by: John Plevyak <jplevyak@gmail.com>

mattklein123

LGTM, thanks. Will defer to @htuch for final approval/merge.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

htuch

LGTM other than remaining threads. If you really want to reflect that this is still alpha, we should make this v4alpha for the latest API version.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

…m-upstream-protos Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak · 2020-01-21T18:36:32Z

RE: Warning for "allow_precompiled", I changed it, but I would say that nobody should be loading a wasm plugin from an untrusted source in any case if for no other reason than the existing ABI is sufficiently powerful to cause trouble without loading arbitrary code.

Signed-off-by: John Plevyak <jplevyak@gmail.com>

htuch

Thanks!

mattklein123 · 2020-01-22T18:39:27Z

Please merge master. I think this will need some doc updates for v3.

/wait

…m-upstream-protos

jplevyak · 2020-01-27T18:25:52Z

I merged master. What doc updates are you thinking?

mattklein123 · 2020-01-27T21:43:58Z

I merged master. What doc updates are you thinking?

I would have thought that the docs you added in the api-v2 tree would need replicating in the api-v3 tree. I'm not sure why it's not failing due to RST not being present in the tree. In a any case, if it's passing the build we can merge now and sort it out later. cc @htuch

Proxy-WASM configuration protos.

6bd8d54

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only Bot added the api label Dec 6, 2019

jplevyak mentioned this pull request Dec 6, 2019

Upstream ABI Part 1 (logging + stats) from envoy-wasm . #9217

Closed

mattklein123 assigned lizan, mattklein123 and htuch Dec 6, 2019

htuch suggested changes Dec 6, 2019

View reviewed changes

repokitteh-read-only Bot added the waiting label Dec 6, 2019

lizan reviewed Dec 6, 2019

View reviewed changes

Comment thread api/envoy/config/wasm/v2/wasm.proto Outdated

Comment thread api/envoy/config/wasm/v2/wasm.proto Outdated

Comment thread api/envoy/config/wasm/v2/wasm.proto Outdated

mattklein123 requested changes Dec 9, 2019

View reviewed changes

Comment thread api/envoy/config/wasm/v2/wasm.proto Outdated

Comment thread api/envoy/config/wasm/v2/wasm.proto Outdated

Address comments.

65083ee

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only Bot removed the waiting label Dec 12, 2019

jplevyak added 3 commits December 12, 2019 22:42

Address comments.

0d63b3c

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Address comments.

e849767

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Fix annotation.

0391532

Signed-off-by: John Plevyak <jplevyak@gmail.com>

mattklein123 requested changes Dec 13, 2019

View reviewed changes

Comment thread api/envoy/config/wasm/v2alpha/wasm.proto Outdated

Comment thread api/envoy/config/wasm/v2alpha/wasm.proto Outdated

repokitteh-read-only Bot added the waiting label Dec 13, 2019

Address comments.

bdfcdde

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only Bot removed the waiting label Dec 18, 2019

mattklein123 added the waiting label Dec 20, 2019

Fix format and add any.proto.

51fcb4d

Signed-off-by: John Plevyak <jplevyak@gmail.com>

repokitteh-read-only Bot removed the waiting label Dec 23, 2019

Fix formatting.

9154661

Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak mentioned this pull request Dec 24, 2019

Differentiate a Filter's "name" and a "type". #9466

Closed

Merge branch 'master' of https://github.com/envoyproxy/envoy into was…

cbe7e25

…m-upstream-protos

jplevyak added 2 commits January 16, 2020 23:42

Address comments.

f8c1fb1

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Address comments.

3abf419

Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak requested review from htuch and mattklein123 January 16, 2020 23:50

mattklein123 previously approved these changes Jan 17, 2020

View reviewed changes

repokitteh-read-only Bot removed the api label Jan 17, 2020

PiotrSikora reviewed Jan 17, 2020

View reviewed changes

Comment thread api/envoy/config/wasm/v2alpha/wasm.proto

Address comments.

494fa9c

Signed-off-by: John Plevyak <jplevyak@gmail.com>

jplevyak dismissed mattklein123’s stale review via 494fa9c January 17, 2020 22:15

repokitteh-read-only Bot added the api label Jan 17, 2020

jplevyak requested review from PiotrSikora, lizan and mattklein123 January 17, 2020 22:16

PiotrSikora suggested changes Jan 17, 2020

View reviewed changes

Comment thread api/envoy/config/wasm/v2alpha/wasm.proto

htuch reviewed Jan 19, 2020

View reviewed changes

jplevyak added 3 commits January 21, 2020 18:29

Address comments.

0573d68

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Update comments.

04302b2

Signed-off-by: John Plevyak <jplevyak@gmail.com>

Merge branch 'master' of https://github.com/envoyproxy/envoy into was…

28d6e53

…m-upstream-protos Signed-off-by: John Plevyak <jplevyak@gmail.com>

Address comments.

6ad0fa1

Signed-off-by: John Plevyak <jplevyak@gmail.com>

htuch approved these changes Jan 21, 2020

View reviewed changes

repokitteh-read-only Bot removed the api label Jan 21, 2020

repokitteh-read-only Bot added the waiting label Jan 22, 2020

Merge branch 'master' of https://github.com/envoyproxy/envoy into was…

2936b69

…m-upstream-protos

repokitteh-read-only Bot removed the waiting label Jan 27, 2020

mattklein123 merged commit bd637c4 into envoyproxy:master Jan 27, 2020

Conversation

jplevyak commented Dec 6, 2019

Uh oh!

repokitteh-read-only Bot commented Dec 6, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

htuch Dec 6, 2019

Choose a reason for hiding this comment

Uh oh!

jplevyak Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

htuch Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

jplevyak Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

htuch Dec 6, 2019

Choose a reason for hiding this comment

Uh oh!

jplevyak Dec 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

repokitteh-read-only Bot commented Dec 18, 2019

Uh oh!

jplevyak commented Dec 24, 2019

Uh oh!

jplevyak commented Dec 30, 2019

Uh oh!

jplevyak commented Dec 30, 2019

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

jplevyak commented Jan 21, 2020

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

mattklein123 commented Jan 22, 2020

Uh oh!

jplevyak commented Jan 27, 2020

Uh oh!

mattklein123 commented Jan 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants