Release 2022.10.2

cloudflared's Makefile uses `shell go env GOOS` to determine the
LOCAL_OS regardless of it being provided. We therefore need pinned_go as
a dependency to run docker-generate-versions.

RELEASE_NOTES

@@ -1,3 +1,8 @@
+2022.10.2
+- 2022-10-18 TUN-6869: Fix Makefile complaining about missing GO packages
+- 2022-10-18 TUN-6864: Don't reuse port in quic unit tests
+- 2022-10-18 TUN-6868: Return left padded tracing ID when tracing identity is converted to string
+
 2022.10.1
 - 2022-10-16 TUN-6861: Trace ICMP on Windows
 - 2022-10-15 TUN-6860: Send access configuration keys to the edge

cfsetup.yaml

@@ -81,6 +81,7 @@ stretch: &stretch
   generate-versions-file:
     build_dir: *build_dir
     builddeps: 
+      - *pinned_go
       - build-essential
     post-cache:
       - make generate-docker-version

connection/quic_test.go

@@ -11,7 +11,6 @@ import (
 	"net/url"
 	"os"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -142,24 +141,33 @@ func TestQUICServer(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
+		test := test // capture range variable
 		t.Run(test.desc, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
-			var wg sync.WaitGroup
-			wg.Add(1)
+
+			quicListener, err := quic.Listen(udpListener, testTLSServerConfig, testQUICConfig)
+			require.NoError(t, err)
+
+			serverDone := make(chan struct{})
 			go func() {
-				defer wg.Done()
 				quicServer(
-					t, udpListener, testTLSServerConfig, testQUICConfig,
-					test.dest, test.connectionType, test.metadata, test.message, test.expectedResponse,
+					ctx, t, quicListener, test.dest, test.connectionType, test.metadata, test.message, test.expectedResponse,
 				)
+				close(serverDone)
 			}()
 
-			qc := testQUICConnection(udpListener.LocalAddr(), t)
-			go qc.Serve(ctx)
+			qc := testQUICConnection(udpListener.LocalAddr(), t, uint8(i))
 
-			wg.Wait()
+			connDone := make(chan struct{})
+			go func() {
+				qc.Serve(ctx)
+				close(connDone)
+			}()
+
+			<-serverDone
 			cancel()
+			<-connDone
 		})
 	}
 }
@@ -177,23 +185,16 @@ func (fakeControlStream) IsStopped() bool {
 }
 
 func quicServer(
+	ctx context.Context,
 	t *testing.T,
-	conn net.PacketConn,
-	tlsConf *tls.Config,
-	config *quic.Config,
+	listener quic.Listener,
 	dest string,
 	connectionType quicpogs.ConnectionType,
 	metadata []quicpogs.Metadata,
 	message []byte,
 	expectedResponse []byte,
 ) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	earlyListener, err := quic.Listen(conn, tlsConf, config)
-	require.NoError(t, err)
-
-	session, err := earlyListener.Accept(ctx)
+	session, err := listener.Accept(ctx)
 	require.NoError(t, err)
 
 	quicStream, err := session.OpenStreamSync(context.Background())
@@ -482,6 +483,7 @@ func TestBuildHTTPRequest(t *testing.T) {
 
 	log := zerolog.Nop()
 	for _, test := range tests {
+		test := test // capture range variable
 		t.Run(test.name, func(t *testing.T) {
 			req, err := buildHTTPRequest(context.Background(), test.connectRequest, test.body, &log)
 			assert.NoError(t, err)
@@ -519,7 +521,8 @@ func TestServeUDPSession(t *testing.T) {
 		edgeQUICSessionChan <- edgeQUICSession
 	}()
 
-	qc := testQUICConnection(val, t)
+	// Random index to avoid reusing port
+	qc := testQUICConnection(val, t, 28)
 	go qc.Serve(ctx)
 
 	edgeQUICSession := <-edgeQUICSessionChan
@@ -703,7 +706,7 @@ func (s mockSessionRPCServer) UnregisterUdpSession(ctx context.Context, sessionI
 	return nil
 }
 
-func testQUICConnection(udpListenerAddr net.Addr, t *testing.T) *QUICConnection {
+func testQUICConnection(udpListenerAddr net.Addr, t *testing.T, index uint8) *QUICConnection {
 	tlsClientConfig := &tls.Config{
 		InsecureSkipVerify: true,
 		NextProtos:         []string{"argotunnel"},
@@ -713,7 +716,7 @@ func testQUICConnection(udpListenerAddr net.Addr, t *testing.T) *QUICConnection
 	qc, err := NewQUICConnection(
 		testQUICConfig,
 		udpListenerAddr,
-		0,
+		index,
 		tlsClientConfig,
 		&mockOrchestrator{originProxy: &mockOriginProxyWithRequest{}},
 		&tunnelpogs.ConnectionOptions{},

tracing/identity.go

@@ -24,7 +24,7 @@ type Identity struct {
 
 // TODO: TUN-6604 Remove this. To reconstruct into Jaeger propagation format, convert tracingContext to tracing.Identity
 func (tc *Identity) String() string {
-	return fmt.Sprintf("%x%x:%x:0:%x", tc.traceIDUpper, tc.traceIDLower, tc.spanID, tc.flags)
+	return fmt.Sprintf("%016x%016x:%x:0:%x", tc.traceIDUpper, tc.traceIDLower, tc.spanID, tc.flags)
 }
 
 func (tc *Identity) MarshalBinary() ([]byte, error) {
@@ -68,14 +68,9 @@ func NewIdentity(trace string) (*Identity, error) {
 		return nil, fmt.Errorf("trace '%s' doesn't have exactly 4 parts separated by %s", trace, separator)
 	}
 	const base = 16
-	tracingID := parts[0]
-	if len(tracingID) == 0 {
-		return nil, fmt.Errorf("missing tracing ID")
-	}
-	if len(tracingID) != 32 {
-		// Correctly left pad the trace to a length of 32
-		left := traceID128bitsWidth - len(tracingID)
-		tracingID = strings.Repeat("0", left) + tracingID
+	tracingID, err := padTracingID(parts[0])
+	if err != nil {
+		return nil, err
 	}
 	traceIDUpper, err := strconv.ParseUint(tracingID[:16], base, 64)
 	if err != nil {
@@ -100,3 +95,16 @@ func NewIdentity(trace string) (*Identity, error) {
 		flags:        uint8(flags),
 	}, nil
 }
+
+func padTracingID(tracingID string) (string, error) {
+	if len(tracingID) == 0 {
+		return "", fmt.Errorf("missing tracing ID")
+	}
+	if len(tracingID) == traceID128bitsWidth {
+		return tracingID, nil
+	}
+	// Correctly left pad the trace to a length of 32
+	left := traceID128bitsWidth - len(tracingID)
+	paddedTracingID := strings.Repeat("0", left) + tracingID
+	return paddedTracingID, nil
+}

tracing/identity_test.go

@@ -10,40 +10,55 @@ func TestNewIdentity(t *testing.T) {
 	testCases := []struct {
 		testCase string
 		trace    string
-		valid    bool
+		expected string
 	}{
 		{
 			testCase: "full length trace",
 			trace:    "ec31ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
-			valid:    true,
+			expected: "ec31ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
 		},
 		{
 			testCase: "short trace ID",
 			trace:    "ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
-			valid:    true,
+			expected: "0000ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
+		},
+		{
+			testCase: "short trace ID with 0s in the middle",
+			trace:    "ad8a01fde11f000002efdce36873:52726f6cabc144f5:0:1",
+			expected: "0000ad8a01fde11f000002efdce36873:52726f6cabc144f5:0:1",
+		},
+		{
+			testCase: "short trace ID with 0s in the beginning and middle",
+			trace:    "001ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
+			expected: "0001ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0:1",
 		},
 		{
 			testCase: "no trace",
 			trace:    "",
-			valid:    false,
 		},
 		{
 			testCase: "missing flags",
 			trace:    "ec31ad8a01fde11fdcabe2efdce36873:52726f6cabc144f5:0",
-			valid:    false,
 		},
 		{
 			testCase: "missing separator",
 			trace:    "ec31ad8a01fde11fdcabe2efdce3687352726f6cabc144f501",
-			valid:    false,
 		},
 	}
 
 	for _, testCase := range testCases {
 		identity, err := NewIdentity(testCase.trace)
-		if testCase.valid {
-			require.NoError(t, err, testCase.testCase)
-			require.Equal(t, testCase.trace, identity.String())
+		if testCase.expected != "" {
+			require.NoError(t, err)
+			require.Equal(t, testCase.expected, identity.String())
+
+			serializedIdentity, err := identity.MarshalBinary()
+			require.NoError(t, err)
+			deserializedIdentity := new(Identity)
+			err = deserializedIdentity.UnmarshalBinary(serializedIdentity)
+			require.NoError(t, err)
+			require.Equal(t, identity, deserializedIdentity)
+
 		} else {
 			require.Error(t, err)
 			require.Nil(t, identity)