Fix nil pointer during http call dispatch on RootContexts (#111)

mathetake · web-flow · commit 1701721dd995 · 2021-01-04T10:33:19.000+09:00
diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
@@ -105,6 +105,11 @@ func Test_E2E(t *testing.T) {
 		staticReply: 8010,
 		admin:       28310,
 	}, accessLogger))
+	t.Run("dispatch_call_on_tick", testRunnerGetter(envoyPorts{
+		endpoint:    11011,
+		staticReply: 8011,
+		admin:       28311,
+	}, dispatchCallOnTick))
 }
 
 type runner = func(t *testing.T, nps envoyPorts, stdErr *bytes.Buffer)
@@ -346,3 +351,12 @@ func accessLogger(t *testing.T, ps envoyPorts, stdErr *bytes.Buffer) {
 	fmt.Println(out)
 	assert.Contains(t, out, exp)
 }
+
+func dispatchCallOnTick(t *testing.T, ps envoyPorts, stdErr *bytes.Buffer) {
+	time.Sleep(3 * time.Second)
+	out := stdErr.String()
+	fmt.Println(out)
+	for i := 1; i < 6; i++ {
+		assert.Contains(t, out, fmt.Sprintf("called! %d", i))
+	}
+}
diff --git a/examples/dispatch_call_on_tick/README.md b/examples/dispatch_call_on_tick/README.md
@@ -0,0 +1,37 @@
+## dispatch_call_on_tick
+
+this example periodically dispatches http calls
+
+```
+wasm log dispatch: called! 1
+wasm log dispatch: called! 1
+wasm log dispatch: called! 1
+wasm log dispatch: called! 2
+wasm log dispatch: called! 2
+wasm log dispatch: called! 2
+wasm log dispatch: called! 3
+wasm log dispatch: called! 3
+wasm log dispatch: called! 3
+wasm log dispatch: called! 4
+wasm log dispatch: called! 4
+wasm log dispatch: called! 4
+wasm log dispatch: called! 5
+wasm log dispatch: called! 5
+wasm log dispatch: called! 5
+wasm log dispatch: called! 6
+wasm log dispatch: called! 6
+wasm log dispatch: called! 6
+wasm log dispatch: called! 7
+wasm log dispatch: called! 7
+wasm log dispatch: called! 7
+wasm log dispatch: called! 8
+wasm log dispatch: called! 8
+wasm log dispatch: called! 8
+wasm log dispatch: called! 9
+wasm log dispatch: called! 9
+wasm log dispatch: called! 9
+wasm log dispatch: called! 10
+wasm log dispatch: called! 10
+wasm log dispatch: called! 10
+
+```
diff --git a/examples/dispatch_call_on_tick/envoy.yaml b/examples/dispatch_call_on_tick/envoy.yaml
@@ -0,0 +1,93 @@
+static_resources:
+  listeners:
+    - name: main
+      address:
+        socket_address:
+          address: 0.0.0.0
+          port_value: 18000
+      filter_chains:
+        - filters:
+            - name: envoy.http_connection_manager
+              typed_config:
+                "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+                stat_prefix: ingress_http
+                codec_type: auto
+                route_config:
+                  name: local_route
+                  virtual_hosts:
+                    - name: local_service
+                      domains:
+                        - "*"
+                      routes:
+                        - match:
+                            prefix: "/"
+                          route:
+                            cluster: web_service
+                http_filters:
+                  - name: envoy.filters.http.wasm
+                    typed_config:
+                      "@type": type.googleapis.com/udpa.type.v1.TypedStruct
+                      type_url: type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm
+                      value:
+                        config:
+                          name: "dispatch"
+                          root_id: "dispatch"
+                          vm_config:
+                            vm_id: "dispatch"
+                            runtime: "envoy.wasm.runtime.v8"
+                            code:
+                              local:
+                                filename: "./examples/dispatch_call_on_tick/main.go.wasm"
+                  - name: envoy.filters.http.router
+                    typed_config: {}
+
+    - name: staticreply
+      address:
+        socket_address:
+          address: 127.0.0.1
+          port_value: 8099
+      filter_chains:
+        - filters:
+            - name: envoy.http_connection_manager
+              typed_config:
+                "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+                stat_prefix: ingress_http
+                codec_type: auto
+                route_config:
+                  name: local_route
+                  virtual_hosts:
+                    - name: local_service
+                      domains:
+                        - "*"
+                      routes:
+                        - match:
+                            prefix: "/"
+                          direct_response:
+                            status: 200
+                            body:
+                              inline_string: "example body\n"
+                http_filters:
+                  - name: envoy.filters.http.router
+                    typed_config: {}
+
+  clusters:
+    - name: web_service
+      connect_timeout: 0.25s
+      type: STATIC
+      lb_policy: ROUND_ROBIN
+      load_assignment:
+        cluster_name: mock_service
+        endpoints:
+          - lb_endpoints:
+              - endpoint:
+                  address:
+                    socket_address:
+                      address: 127.0.0.1
+                      port_value: 8099
+
+admin:
+  access_log_path: "/dev/null"
+  address:
+    socket_address:
+      address: 0.0.0.0
+      port_value: 8001
diff --git a/examples/dispatch_call_on_tick/main.go b/examples/dispatch_call_on_tick/main.go
@@ -0,0 +1,59 @@
+// Copyright 2020 Tetrate
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm"
+)
+
+const tickMilliseconds uint32 = 100
+
+func main() {
+	proxywasm.SetNewRootContext(newRootContext)
+}
+
+type rootContext struct {
+	// you must embed the default context so that you need not to reimplement all the methods by yourself
+	proxywasm.DefaultRootContext
+	contextID uint32
+}
+
+func newRootContext(contextID uint32) proxywasm.RootContext {
+	return &rootContext{contextID: contextID}
+}
+
+// override
+func (ctx *rootContext) OnVMStart(vmConfigurationSize int) bool {
+	if err := proxywasm.SetTickPeriodMilliSeconds(tickMilliseconds); err != nil {
+		proxywasm.LogCriticalf("failed to set tick period: %v", err)
+	}
+	proxywasm.LogInfof("set tick period milliseconds: %d", tickMilliseconds)
+	return true
+}
+
+func (ctx *rootContext) OnTick() {
+	hs := [][2]string{{":method", "GET"}, {":authority", "some_authority"}, {":path", "/path/to/service"}, {"accept", "*/*"}}
+	if _, err := proxywasm.DispatchHttpCall("web_service", hs, "", [][2]string{},
+		5000, callback); err != nil {
+		proxywasm.LogCriticalf("dispatch httpcall failed: %v", err)
+	}
+}
+
+var cnt int
+
+func callback(numHeaders, bodySize, numTrailers int) {
+	cnt++
+	proxywasm.LogInfof("called! %d", cnt)
+}
diff --git a/examples/dispatch_call_on_tick/main_test.go b/examples/dispatch_call_on_tick/main_test.go
@@ -0,0 +1,48 @@
+package main
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/tetratelabs/proxy-wasm-go-sdk/proxytest"
+	"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm/types"
+)
+
+func TestRootContext_OnTick(t *testing.T) {
+	opt := proxytest.NewEmulatorOption().
+		WithNewRootContext(newRootContext)
+	host := proxytest.NewHostEmulator(opt)
+	defer host.Done() // release the host emulation lock so that other test cases can insert their own host emulation
+
+	host.StartVM() // call OnVMStart
+
+	assert.Equal(t, tickMilliseconds, host.GetTickPeriod())
+
+	for i := 1; i < 10; i++ {
+		host.Tick() // call OnTick
+		attrs := host.GetCalloutAttributesFromContext(proxytest.RootContextID)
+		require.Equal(t, len(attrs), i)                            // verify DispatchHttpCall is called
+		host.PutCalloutResponse(attrs[0].CalloutID, nil, nil, nil) // receive callout response
+
+		logs := host.GetLogs(types.LogLevelInfo)
+		require.Greater(t, len(logs), 0)
+		msg := logs[len(logs)-1]
+
+		assert.True(t, strings.Contains(msg, fmt.Sprintf("called! %d", i)))
+	}
+
+}
+
+func TestRootContext_OnVMStart(t *testing.T) {
+	opt := proxytest.NewEmulatorOption().
+		WithNewRootContext(newRootContext)
+	host := proxytest.NewHostEmulator(opt)
+	defer host.Done() // release the host emulation lock so that other test cases can insert their own host emulation
+
+	host.StartVM() // call OnVMStart
+	assert.Equal(t, tickMilliseconds, host.GetTickPeriod())
+}
diff --git a/proxytest/http.go b/proxytest/http.go
@@ -303,7 +303,7 @@ func (h *httpHostEmulator) ProxySendLocalResponse(statusCode uint32,
 // impl HostEmulator
 func (h *httpHostEmulator) HttpFilterInitContext() (contextID uint32) {
 	contextID = getNextContextID()
-	proxywasm.ProxyOnContextCreate(contextID, rootContextID)
+	proxywasm.ProxyOnContextCreate(contextID, RootContextID)
 	h.httpStreams[contextID] = &httpStreamState{action: types.ActionContinue}
 	return
 }
@@ -465,12 +465,12 @@ func (h *httpHostEmulator) HttpFilterGetSentLocalResponse(contextID uint32) *Loc
 
 // impl HostEmulator
 func (h *httpHostEmulator) CallOnLogForAccessLogger(requestHeaders, responseHeaders [][2]string) {
-	h.httpStreams[rootContextID] = &httpStreamState{
+	h.httpStreams[RootContextID] = &httpStreamState{
 		requestHeaders:   requestHeaders,
 		responseHeaders:  responseHeaders,
 		requestTrailers:  nil,
 		responseTrailers: nil,
 	}
 
-	proxywasm.ProxyOnLog(rootContextID)
+	proxywasm.ProxyOnLog(RootContextID)
 }
diff --git a/proxytest/network.go b/proxytest/network.go
@@ -113,7 +113,7 @@ func (n *networkHostEmulator) NetworkFilterPutDownstreamData(contextID uint32, d
 // impl HostEmulator
 func (n *networkHostEmulator) NetworkFilterInitConnection() (contextID uint32) {
 	contextID = getNextContextID()
-	proxywasm.ProxyOnContextCreate(contextID, rootContextID)
+	proxywasm.ProxyOnContextCreate(contextID, RootContextID)
 	proxywasm.ProxyOnNewConnection(contextID)
 	n.streamStates[contextID] = &streamState{}
 	return
diff --git a/proxytest/proxytest.go b/proxytest/proxytest.go
@@ -56,12 +56,12 @@ type HostEmulator interface {
 }
 
 const (
-	rootContextID uint32 = 1 // TODO: support multiple rootContext
+	RootContextID uint32 = 1 // TODO: support multiple rootContext
 )
 
 var (
 	hostMux       = sync.Mutex{}
-	nextContextID = rootContextID + 1
+	nextContextID = RootContextID + 1
 )
 
 type hostEmulator struct {
@@ -92,7 +92,7 @@ func NewHostEmulator(opt *EmulatorOption) HostEmulator {
 	proxywasm.SetNewHttpContext(opt.newHttpContext)
 
 	// create root context: TODO: support multiple root contexts
-	proxywasm.ProxyOnContextCreate(rootContextID, 0)
+	proxywasm.ProxyOnContextCreate(RootContextID, 0)
 
 	return emulator
 }
diff --git a/proxytest/root.go b/proxytest/root.go
@@ -141,7 +141,7 @@ func (r *rootHostEmulator) ProxyEnqueueSharedQueue(queueID uint32, valueData *by
 
 	// note that this behavior is not accurate for some old host implementations:
 	//	see: https://github.com/proxy-wasm/proxy-wasm-cpp-host/pull/36
-	proxywasm.ProxyOnQueueReady(rootContextID, queueID) // Note that this behavior is not accurate on Istio before 1.8.x
+	proxywasm.ProxyOnQueueReady(RootContextID, queueID) // Note that this behavior is not accurate on Istio before 1.8.x
 	return types.StatusOK
 }
 
@@ -362,7 +362,7 @@ func (r *rootHostEmulator) GetTickPeriod() uint32 {
 
 // impl HostEmulator
 func (r *rootHostEmulator) Tick() {
-	proxywasm.ProxyOnTick(rootContextID)
+	proxywasm.ProxyOnTick(RootContextID)
 }
 
 // impl HostEmulator
@@ -378,12 +378,12 @@ func (r *rootHostEmulator) GetCalloutAttributesFromContext(contextID uint32) []H
 
 // impl HostEmulator
 func (r *rootHostEmulator) StartVM() {
-	proxywasm.ProxyOnVMStart(rootContextID, len(r.vmConfiguration))
+	proxywasm.ProxyOnVMStart(RootContextID, len(r.vmConfiguration))
 }
 
 // impl HostEmulator
 func (r *rootHostEmulator) StartPlugin() {
-	proxywasm.ProxyOnConfigure(rootContextID, len(r.pluginConfiguration))
+	proxywasm.ProxyOnConfigure(RootContextID, len(r.pluginConfiguration))
 }
 
 // impl HostEmulator
@@ -393,17 +393,17 @@ func (r *rootHostEmulator) PutCalloutResponse(calloutID uint32, headers, trailer
 		body              []byte
 	}{headers: headers, trailers: trailers, body: body}
 
-	// rootContextID, calloutID uint32, numHeaders, bodySize, numTrailers in
+	// RootContextID, calloutID uint32, numHeaders, bodySize, numTrailers in
 	r.activeCalloutID = calloutID
 	defer func() {
 		r.activeCalloutID = 0
 		delete(r.httpCalloutResponse, calloutID)
 		delete(r.httpCalloutIDToContextID, calloutID)
 	}()
-	proxywasm.ProxyOnHttpCallResponse(rootContextID, calloutID, len(headers), len(body), len(trailers))
+	proxywasm.ProxyOnHttpCallResponse(RootContextID, calloutID, len(headers), len(body), len(trailers))
 }
 
 // impl HostEmulator
 func (r *rootHostEmulator) FinishVM() {
-	proxywasm.ProxyOnDone(rootContextID)
+	proxywasm.ProxyOnDone(RootContextID)
 }
diff --git a/proxywasm/abi_l7_test.go b/proxywasm/abi_l7_test.go
diff --git a/proxywasm/vmstate.go b/proxywasm/vmstate.go
diff --git a/proxywasm/vmstate_test.go b/proxywasm/vmstate_test.go

Original file line number	Diff line number	Diff line change
`@@ -56,12 +56,12 @@ type HostEmulator interface {`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`const (`
`59`		`- rootContextID uint32 = 1 // TODO: support multiple rootContext`
	`59`	`+ RootContextID uint32 = 1 // TODO: support multiple rootContext`
`60`	`60`	`)`
`61`	`61`
`62`	`62`	`var (`
`63`	`63`	`hostMux = sync.Mutex{}`
`64`		`- nextContextID = rootContextID + 1`
	`64`	`+ nextContextID = RootContextID + 1`
`65`	`65`	`)`
`66`	`66`
`67`	`67`	`type hostEmulator struct {`
`@@ -92,7 +92,7 @@ func NewHostEmulator(opt *EmulatorOption) HostEmulator {`
`92`	`92`	`proxywasm.SetNewHttpContext(opt.newHttpContext)`
`93`	`93`
`94`	`94`	`// create root context: TODO: support multiple root contexts`
`95`		`- proxywasm.ProxyOnContextCreate(rootContextID, 0)`
	`95`	`+ proxywasm.ProxyOnContextCreate(RootContextID, 0)`
`96`	`96`
`97`	`97`	`return emulator`
`98`	`98`	`}`