Skip to content

ABI version and enumeration constants #11

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

Closed
esnible opened this issue Sep 15, 2020 · 5 comments
Closed

ABI version and enumeration constants #11

esnible opened this issue Sep 15, 2020 · 5 comments

Comments

@esnible
Copy link

esnible commented Sep 15, 2020

I noticed a recent change added an enumeration to the FilterHeadersStatus
return codes for ContinueAndDontEndStream. https://github.com/envoyproxy/envoy/blob/master/include/envoy/http/filter.h#L75

Envoy WASM doesn't yet have the change [ https://github.com/envoyproxy/envoy-wasm/blob/master/include/envoy/http/filter.h ]

The AssemblyScript runtime has its own version of the enumeration, at https://github.com/solo-io/proxy-runtime/blob/master/assembly/runtime.ts#L96 , which doesn't include the new enumeration. The current AssemblyScript runtime advertises itself as proxy_abi_version_0_2_0.

How will this work in practice when Envoy WASM picks up the new enumeration from Envoy? Do we expect

  • Envoy-Wasm will pick up the Envoy change but re-order the enumeration so binary enum constant values don't change?
  • Envoy-Wasm will reject plugins with ABI older version point release, forcing filter developers to recompile?
  • Envoy-Wasm will including enumeration mapping, and proxy_abi_version_0_2_0 filters can continue to return 4 for FilterHeadersStatus.StopAllIterationAndBuffer alongside proxy_abi_version_0_2_x filters returning 5 for proxy_abi_version_0_2_0?
@mandarjog
Copy link

@PiotrSikora this seems really bad that enums are remapped and can crash proxy.

@mandarjog
Copy link

@soya3129 Can you comment on why the enums were remapped in https://github.com/envoyproxy/envoy/pull/7756/files instead of adding a new enum at the end?

@PiotrSikora
Copy link
Member

How will this work in practice when Envoy WASM picks up the new enumeration from Envoy? Do we expect

  • Envoy-Wasm will pick up the Envoy change but re-order the enumeration so binary enum constant values don't change?

Proxy-Wasm ABI is independent of Envoy, the values are mapped and not passed-through, so the renumeration in Envoy doesn't matter at all (unless there are some bugs or leftovers in Envoy-Wasm).

  • Envoy-Wasm will reject plugins with ABI older version point release, forcing filter developers to recompile?

Envoy supports multiple versions of the ABI at the same time, it currently supports 0.1.0, 0.2.0 and 0.2.1.

  • Envoy-Wasm will including enumeration mapping, and proxy_abi_version_0_2_0 filters can continue to return 4 for FilterHeadersStatus.StopAllIterationAndBuffer alongside proxy_abi_version_0_2_x filters returning 5 for proxy_abi_version_0_2_0?

Filters should return values defined in the ABI, not enum values from Envoy.

@esnible
Copy link
Author

esnible commented Sep 28, 2020

Filters should return values defined in the ABI, not enum values from Envoy.

Can you point me to the authoritative ABI return values list?

@mathetake
Copy link
Contributor

I believe this will be resolved by our project overhaul. Please refer to #39 (comment) for the context. Thanks!

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

No branches or pull requests

4 participants