Skip to content
This repository has been archived by the owner on Jul 28, 2024. It is now read-only.

feat: support proxy set buffer bytes #76

Merged
merged 6 commits into from
Oct 12, 2020

Conversation

wimspaargaren
Copy link
Contributor

Resolve #45

We've tested the set buffer bytes function deployed in Istio and it seems to be working as expected.

Filter built tiny go version:

tinygo version  
tinygo version 0.16.0-dev darwin/amd64 (using go version go1.15 and LLVM version 10.0.1)

Built using:

tinygo build -o ./filters/myfilter/filter.wasm -target=wasi -wasm-abi=generic ./myfilter.go

Istio version

istioctl version                                                                                                                
client version: 1.7.0
control plane version: 1.7.0
data plane version: 1.7.0 (10 proxies)

Example usage:

        body := []byte(`{ "data": "my new body" }`)
	bufferData := &body[0]
	res := rawhostcall.ProxySetBufferBytes(types.BufferTypeHttpRequestBody, 0, len(body), bufferData, len(body))
	if res != types.StatusOK {
		// handle error
	}
	err = proxywasm.SetHttpRequestHeader("content-length", fmt.Sprintf("%d", len(body)))
	if err != nil {
		// handle error
	}

Envoy filter description:

Name:         wasm-myfilter
Namespace:    mycluster
Labels:       app.kubernetes.io/managed-by=Helm
Annotations:  meta.helm.sh/release-name: mycluster-cluster-config
              meta.helm.sh/release-namespace: mycluster
API Version:  networking.istio.io/v1alpha3
Kind:         EnvoyFilter
Metadata:
  Creation Timestamp:  2020-10-09T07:59:52Z
  Generation:          1
  Managed Fields:
    API Version:  networking.istio.io/v1alpha3
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:meta.helm.sh/release-name:
          f:meta.helm.sh/release-namespace:
        f:labels:
          .:
          f:app.kubernetes.io/managed-by:
      f:spec:
        .:
        f:configPatches:
        f:workloadSelector:
          .:
          f:labels:
            .:
            f:wasm_filter_myfilter:
    Manager:         Go-http-client
    Operation:       Update
    Time:            2020-10-09T07:59:52Z
  Resource Version:  2806
  Self Link:         /apis/networking.istio.io/v1alpha3/namespaces/mycluster/envoyfilters/wasm-myfilter
  UID:               fb324c3b-80aa-476a-9ab6-ecb2a5bbdf55
Spec:
  Config Patches:
    Apply To:  HTTP_FILTER
    Match:
      Context:  SIDECAR_INBOUND
      Listener:
        Filter Chain:
          Filter:
            Name:  envoy.http_connection_manager
            Sub Filter:
              Name:   autopseudo_filter
        Port Number:  80
      Proxy:
        Proxy Version:  ^1\.7.*
    Patch:
      Operation:  INSERT_BEFORE
      Value:
        Name:  myfilter_filter
        typed_config:
          @type:     type.googleapis.com/udpa.type.v1.TypedStruct
          type_url:  type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm
          Value:
            Config:
              Configuration:
                @type:  type.googleapis.com/google.protobuf.StringValue
                Value:  /gql
              root_id:  myfilter_root_id
              vm_config:
                Code:
                  Local:
                    Filename:  /var/local/lib/wasm-filters/myfilter/filter.wasm
                Runtime:       envoy.wasm.runtime.v8
                vm_id:         myfilter_filter
  Workload Selector:
    Labels:
      wasm_filter_myfilter:  enabled
Events:                       <none>

@mathetake
Copy link
Member

mathetake commented Oct 12, 2020

@wimspaargaren thanks for this and looks fine!

Could you please add the high level apis in proxywasm package using this raw hostcall like these? https://github.com/tetratelabs/proxy-wasm-go-sdk/blob/main/proxywasm/hostcall.go#L145-L160

proxywasm/hostcall.go Outdated Show resolved Hide resolved
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

Almost fine, but one thing left I would like to have is e2e test like this: https://github.com/tetratelabs/proxy-wasm-go-sdk/blob/main/e2e/e2e_test.go#L96-L119

examples/http_body/README.md Outdated Show resolved Hide resolved
assert.True(t, strings.Contains(out, `initial request body: { "example": "body" }`))
assert.True(t, strings.Contains(out, "on http request body finished"))
assert.False(t, strings.Contains(out, "failed to set request body"))
assert.False(t, strings.Contains(out, "failed to get request body"))
Copy link
Member

@mathetake mathetake Oct 12, 2020

Choose a reason for hiding this comment

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

should we assert string(r.Body) == "{ "another": "body" }"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well I wasn't sure on how to verify that. What we're doing is replacing the request body and the result is the response body from the staticreply filter. So the r.Body contains example body

Copy link
Member

Choose a reason for hiding this comment

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

ah, you are right. It seems fine as it is now 👍

@@ -19,6 +19,9 @@ test:
test.e2e:
docker run -it -w /tmp/proxy-wasm-go -v $(shell pwd):/tmp/proxy-wasm-go getenvoy/proxy-wasm-go-sdk-ci:istio-${ISTIO_VERSION} go test -v ./e2e

test.e2e.single:
Copy link
Member

Choose a reason for hiding this comment

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

Thanks:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, saves quite some time :)

Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

Thanks @wimspaargaren for your contribution 🎖️ 💯

@mathetake mathetake merged commit 5a6a7b0 into tetratelabs:main Oct 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support buffer mutation
2 participants