TUN-1828: Update declarative tunnel config struct

tunnelrpc/pogs/config_test.go

@@ -1,6 +1,8 @@
 package pogs
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
@@ -10,15 +12,18 @@ import (
 	capnp "zombiezen.com/go/capnproto2"
 )
 
-func TestCloudflaredConfig(t *testing.T) {
-	addDoHProxyConfigs := func(c *CloudflaredConfig) {
+func TestClientConfig(t *testing.T) {
+	addDoHProxyConfigs := func(c *ClientConfig) {
 		c.DoHProxyConfigs = []*DoHProxyConfig{
 			sampleDoHProxyConfig(),
 		}
 	}
-	addReverseProxyConfigs := func(c *CloudflaredConfig) {
+	addReverseProxyConfigs := func(c *ClientConfig) {
 		c.ReverseProxyConfigs = []*ReverseProxyConfig{
 			sampleReverseProxyConfig(),
+			sampleReverseProxyConfig(func(c *ReverseProxyConfig) {
+				c.ChunkedEncoding = false
+			}),
 			sampleReverseProxyConfig(func(c *ReverseProxyConfig) {
 				c.Origin = sampleHTTPOriginConfig()
 			}),
@@ -31,23 +36,23 @@ func TestCloudflaredConfig(t *testing.T) {
 		}
 	}
 
-	testCases := []*CloudflaredConfig{
-		sampleCloudflaredConfig(),
-		sampleCloudflaredConfig(addDoHProxyConfigs),
-		sampleCloudflaredConfig(addReverseProxyConfigs),
-		sampleCloudflaredConfig(addDoHProxyConfigs, addReverseProxyConfigs),
+	testCases := []*ClientConfig{
+		sampleClientConfig(),
+		sampleClientConfig(addDoHProxyConfigs),
+		sampleClientConfig(addReverseProxyConfigs),
+		sampleClientConfig(addDoHProxyConfigs, addReverseProxyConfigs),
 	}
 	for i, testCase := range testCases {
 		_, seg, err := capnp.NewMessage(capnp.SingleSegment(nil))
-		capnpEntity, err := tunnelrpc.NewCloudflaredConfig(seg)
+		capnpEntity, err := tunnelrpc.NewClientConfig(seg)
 		if !assert.NoError(t, err) {
 			t.Fatal("Couldn't initialize a new message")
 		}
-		err = MarshalCloudflaredConfig(capnpEntity, testCase)
+		err = MarshalClientConfig(capnpEntity, testCase)
 		if !assert.NoError(t, err, "testCase index %v failed to marshal", i) {
 			continue
 		}
-		result, err := UnmarshalCloudflaredConfig(capnpEntity)
+		result, err := UnmarshalClientConfig(capnpEntity)
 		if !assert.NoError(t, err, "testCase index %v failed to unmarshal", i) {
 			continue
 		}
@@ -208,18 +213,17 @@ func TestWebSocketOriginConfig(t *testing.T) {
 //////////////////////////////////////////////////////////////////////////////
 // Functions to generate sample data for ease of testing
 
-func sampleCloudflaredConfig(overrides ...func(*CloudflaredConfig)) *CloudflaredConfig {
-	// strip the location and monotonic clock reading so that assert.Equals()
-	// will work correctly
-	now := time.Now().UTC().Round(0)
-	sample := &CloudflaredConfig{
-		Timestamp:              now,
+func sampleClientConfig(overrides ...func(*ClientConfig)) *ClientConfig {
+	sample := &ClientConfig{
+		Version:                uint64(1337),
 		AutoUpdateFrequency:    21 * time.Hour,
 		MetricsUpdateFrequency: 11 * time.Minute,
 		HeartbeatInterval:      5 * time.Second,
 		MaxFailedHeartbeats:    9001,
 		GracePeriod:            31 * time.Second,
+		NumHAConnections:       49,
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
@@ -232,6 +236,7 @@ func sampleDoHProxyConfig(overrides ...func(*DoHProxyConfig)) *DoHProxyConfig {
 		ListenPort: 53,
 		Upstreams:  []string{"https://1.example.com", "https://2.example.com"},
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
@@ -240,13 +245,14 @@ func sampleDoHProxyConfig(overrides ...func(*DoHProxyConfig)) *DoHProxyConfig {
 
 func sampleReverseProxyConfig(overrides ...func(*ReverseProxyConfig)) *ReverseProxyConfig {
 	sample := &ReverseProxyConfig{
-		TunnelID:           "hijk",
+		TunnelHostname:     "hijk.example.com",
 		Origin:             &HelloWorldOriginConfig{},
 		Retries:            18,
 		ConnectionTimeout:  5 * time.Second,
-		ChunkedEncoding:    false,
+		ChunkedEncoding:    true,
 		CompressionQuality: 4,
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
@@ -265,6 +271,7 @@ func sampleHTTPOriginConfig(overrides ...func(*HTTPOriginConfig)) *HTTPOriginCon
 		MaxIdleConnections:    19,
 		IdleConnectionTimeout: 17 * time.Second,
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
@@ -275,6 +282,7 @@ func sampleUnixSocketOriginConfig(overrides ...func(*UnixSocketOriginConfig)) *U
 	sample := &UnixSocketOriginConfig{
 		Path: "/var/lib/file.sock",
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
@@ -285,8 +293,71 @@ func sampleWebSocketOriginConfig(overrides ...func(*WebSocketOriginConfig)) *Web
 	sample := &WebSocketOriginConfig{
 		URL: "ssh://example.com",
 	}
+	sample.ensureNoZeroFields()
 	for _, f := range overrides {
 		f(sample)
 	}
 	return sample
 }
+
+func (c *ClientConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{"DoHProxyConfigs", "ReverseProxyConfigs"})
+}
+
+func (c *DoHProxyConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{})
+}
+
+func (c *ReverseProxyConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{})
+}
+
+func (c *HTTPOriginConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{})
+}
+
+func (c *UnixSocketOriginConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{})
+}
+
+func (c *WebSocketOriginConfig) ensureNoZeroFields() {
+	ensureNoZeroFieldsInSample(reflect.ValueOf(c), []string{})
+}
+
+// ensureNoZeroFieldsInSample checks that all fields in the sample struct,
+// except those listed in `allowedZeroFieldNames`, are initialized to nonzero
+// values. Note that the value has to be a pointer for reflection to work
+// correctly:
+//     e := &ExampleStruct{ ... }
+//     ensureNoZeroFieldsInSample(reflect.ValueOf(e), []string{})
+//
+// Context:
+// Our tests work by taking a sample struct and marshalling/unmarshalling it.
+// This makes them easy to write, but introduces some risk: if we don't
+// include a field in the sample value, it won't be covered under tests.
+// This check reduces that risk by requiring fields to be either initialized
+// or explicitly uninitialized.
+// https://bitbucket.cfdata.org/projects/TUN/repos/cloudflared/pull-requests/151/overview?commentId=348012
+func ensureNoZeroFieldsInSample(ptrToSampleValue reflect.Value, allowedZeroFieldNames []string) {
+	sampleValue := ptrToSampleValue.Elem()
+	structType := ptrToSampleValue.Type().Elem()
+
+	allowedZeroFieldSet := make(map[string]bool)
+	for _, name := range allowedZeroFieldNames {
+		if _, ok := structType.FieldByName(name); !ok {
+			panic(fmt.Sprintf("struct %v has no field %v", structType.Name(), name))
+		}
+		allowedZeroFieldSet[name] = true
+	}
+
+	for i := 0; i < structType.NumField(); i++ {
+		if allowedZeroFieldSet[structType.Field(i).Name] {
+			continue
+		}
+
+		zeroValue := reflect.Zero(structType.Field(i).Type)
+		if reflect.DeepEqual(zeroValue.Interface(), sampleValue.Field(i).Interface()) {
+			panic(fmt.Sprintf("In the sample value for struct %v, field %v was not initialized", structType.Name(), structType.Field(i).Name))
+		}
+	}
+}