From 6b5d2562af41687a76022a6b4d362734807d8701 Mon Sep 17 00:00:00 2001 From: Aenimus Date: Fri, 24 Jan 2025 23:18:11 +0000 Subject: [PATCH 1/5] feat: also add handshake for static execution configs --- router/core/graph_server.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 35236bfcdd..a9a1c922f2 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -5,6 +5,7 @@ import ( "crypto/ecdsa" "errors" "fmt" + "github.com/wundergraph/cosmo/router/pkg/execution_config" otelmetric "go.opentelemetry.io/otel/metric" "net/http" "net/url" @@ -102,6 +103,16 @@ type ( // newGraphServer creates a new server instance. func newGraphServer(ctx context.Context, r *Router, routerConfig *nodev1.RouterConfig, proxy ProxyFunc) (*graphServer, error) { + /* Older versions of composition will not populate a compatibility version. + * Currently, all "old" router execution configurations are compatible as there have been no breaking + * changes. + * Upon the first breaking change to the execution config, an unpopulated compatibility version will + * also be unsupported (and the logic for IsRouterCompatibleWithExecutionConfig will need to be updated). + */ + if !execution_config.IsRouterCompatibleWithExecutionConfig(r.logger, routerConfig.CompatibilityVersion) { + return nil, fmt.Errorf(`the comapitibility version "%s" is not compatible with this router version`, routerConfig.CompatibilityVersion) + } + ctx, cancel := context.WithCancel(ctx) s := &graphServer{ context: ctx, From 860e575f6ecf2f7decbffc31d7616d874abf3c46 Mon Sep 17 00:00:00 2001 From: Aenimus Date: Fri, 24 Jan 2025 23:19:41 +0000 Subject: [PATCH 2/5] chore: remove previous handshake --- router/core/router.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/router/core/router.go b/router/core/router.go index eb4f6b5a38..f3e1a8ad9f 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1105,16 +1105,6 @@ func (r *Router) Start(ctx context.Context) error { return nil } - /* Older versions of composition will not populate a compatibility version. - * Currently, all "old" router execution configurations are compatible as there have been no breaking - * changes. - * Upon the first breaking change to the execution config, an unpopulated compatibility version will - * also be unsupported (and the logic for IsRouterCompatibleWithExecutionConfig will need to be updated). - */ - if !execution_config.IsRouterCompatibleWithExecutionConfig(r.logger, cfg.CompatibilityVersion) { - return nil - } - if err := r.newServer(ctx, cfg); err != nil { r.logger.Error("Failed to update server with new config", zap.Error(err)) return nil From b8b6c3a9a647b683924af0c728f9c014ace3d230 Mon Sep 17 00:00:00 2001 From: Aenimus Date: Fri, 24 Jan 2025 23:22:10 +0000 Subject: [PATCH 3/5] chore: fix typo --- router/core/graph_server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index a9a1c922f2..e06dec4c42 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -110,7 +110,7 @@ func newGraphServer(ctx context.Context, r *Router, routerConfig *nodev1.RouterC * also be unsupported (and the logic for IsRouterCompatibleWithExecutionConfig will need to be updated). */ if !execution_config.IsRouterCompatibleWithExecutionConfig(r.logger, routerConfig.CompatibilityVersion) { - return nil, fmt.Errorf(`the comapitibility version "%s" is not compatible with this router version`, routerConfig.CompatibilityVersion) + return nil, fmt.Errorf(`the compatibility version "%s" is not compatible with this router version`, routerConfig.CompatibilityVersion) } ctx, cancel := context.WithCancel(ctx) From 7a61461df05cbd46c0e64098604f14f1c8af5534 Mon Sep 17 00:00:00 2001 From: Aenimus Date: Sun, 26 Jan 2025 18:31:20 +0000 Subject: [PATCH 4/5] chore: respond to PR feedback --- router/pkg/execution_config/compatibility.go | 36 +++++++++---------- .../execution_config/compatibility_test.go | 8 ++--- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/router/pkg/execution_config/compatibility.go b/router/pkg/execution_config/compatibility.go index 57d3f476c5..b3fc98bb80 100644 --- a/router/pkg/execution_config/compatibility.go +++ b/router/pkg/execution_config/compatibility.go @@ -8,20 +8,20 @@ import ( ) const ( - // ExecutionConfigVersionThreshold should ONLY be updated if there is a breaking change in the router execution config. - ExecutionConfigVersionThreshold = 1 - compatibilityVersionParseErrorMessage = "Failed to parse compatibility version." - executionConfigVersionParseErrorMessage = "Failed to parse router execution config version of compatibility version." + // RouterCompatibilityVersionThreshold should ONLY be updated if there is a breaking change in the router execution config. + RouterCompatibilityVersionThreshold = 1 + compatibilityVersionParseErrorMessage = "Failed to parse compatibility version." + routerCompatibilityVersionParseErrorMessage = "Failed to parse router execution config version of compatibility version." ) func IsRouterCompatibleWithExecutionConfig(logger *zap.Logger, compatibilityVersion string) bool { if compatibilityVersion == "" { return true } - /* A compatibility version is composed thus: : - * A router version supports a maximum router execution configuration version (ExecutionConfigVersionThreshold). - * In the event the execution config version exceeds ExecutionConfigVersionThreshold, an error will request for - * the router version be upgraded. + /* A compatibility version is composed thus: : + * A router version supports a maximum router compatibility version (RouterCompatibilityVersionThreshold). + * In the event the execution config version exceeds RouterCompatibilityVersionThreshold, an error will request the + * router version be upgraded. * If the router version requires a newer router execution configuration version, a warning will explain that some * new features may be unavailable or functionality/behaviour may have changed. */ @@ -30,25 +30,25 @@ func IsRouterCompatibleWithExecutionConfig(logger *zap.Logger, compatibilityVers logger.Error(compatibilityVersionParseErrorMessage, zap.String("compatibility_version", compatibilityVersion)) return false } - routerExecutionVersion, err := strconv.ParseInt(segments[0], 10, 32) + routerCompatibilityVersion, err := strconv.ParseInt(segments[0], 10, 32) if err != nil { - logger.Error(executionConfigVersionParseErrorMessage, zap.String("compatibility_version", compatibilityVersion)) + logger.Error(routerCompatibilityVersionParseErrorMessage, zap.String("compatibility_version", compatibilityVersion)) return false } switch { - case routerExecutionVersion == ExecutionConfigVersionThreshold: + case routerCompatibilityVersion == RouterCompatibilityVersionThreshold: return true - case routerExecutionVersion > ExecutionConfigVersionThreshold: + case routerCompatibilityVersion > RouterCompatibilityVersionThreshold: logger.Error( - executionConfigVersionThresholdExceededError(routerExecutionVersion), - zap.Int64("execution_config_version", routerExecutionVersion), + executionConfigVersionThresholdExceededError(routerCompatibilityVersion), + zap.Int64("router_compatibility_version", routerCompatibilityVersion), zap.String("composition_package_version", segments[1]), ) return false default: logger.Warn( - executionConfigVersionInsufficientWarning(routerExecutionVersion), - zap.Int64("execution_config_version", routerExecutionVersion), + executionConfigVersionInsufficientWarning(routerCompatibilityVersion), + zap.Int64("router_compatibility_version", routerCompatibilityVersion), zap.String("composition_package_version", segments[1]), ) return true @@ -58,7 +58,7 @@ func IsRouterCompatibleWithExecutionConfig(logger *zap.Logger, compatibilityVers func executionConfigVersionThresholdExceededError(executionConfigVersion int64) string { return fmt.Sprintf( "This router version supports a router execution config version up to %d. The router execution config version supplied is %d. Please upgrade your router version.", - ExecutionConfigVersionThreshold, + RouterCompatibilityVersionThreshold, executionConfigVersion, ) } @@ -66,7 +66,7 @@ func executionConfigVersionThresholdExceededError(executionConfigVersion int64) func executionConfigVersionInsufficientWarning(executionConfigVersion int64) string { return fmt.Sprintf( "This router version requires a minimum router execution config version of %d to support all functionality. The router execution config version supplied is %d. Please create a new execution configuration.", - ExecutionConfigVersionThreshold, + RouterCompatibilityVersionThreshold, executionConfigVersion, ) } diff --git a/router/pkg/execution_config/compatibility_test.go b/router/pkg/execution_config/compatibility_test.go index be8afdba56..a3f3496718 100644 --- a/router/pkg/execution_config/compatibility_test.go +++ b/router/pkg/execution_config/compatibility_test.go @@ -21,7 +21,7 @@ func TestExecutionConfiguration(t *testing.T) { t.Run("same compatibility version is supported", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) - assert.True(t, IsRouterCompatibleWithExecutionConfig(logger, fmt.Sprintf("%d:0.1.0", ExecutionConfigVersionThreshold))) + assert.True(t, IsRouterCompatibleWithExecutionConfig(logger, fmt.Sprintf("%d:0.1.0", RouterCompatibilityVersionThreshold))) assert.Equal(t, 0, len(logs.All())) }) @@ -58,7 +58,7 @@ func TestExecutionConfiguration(t *testing.T) { assert.False(t, IsRouterCompatibleWithExecutionConfig(logger, compatibilityVersion)) logsSlice := logs.All() assert.Equal(t, 1, len(logsSlice)) - assert.Equal(t, executionConfigVersionParseErrorMessage, logsSlice[0].Message) + assert.Equal(t, routerCompatibilityVersionParseErrorMessage, logsSlice[0].Message) assert.Equal(t, zapcore.ErrorLevel, logsSlice[0].Level) assert.Equal(t, 1, len(logsSlice[0].Context)) assert.Equal(t, zap.String("compatibility_version", compatibilityVersion), logsSlice[0].Context[0]) @@ -67,7 +67,7 @@ func TestExecutionConfiguration(t *testing.T) { t.Run("return an error if the maximum execution config version threshold of the router is exceeded", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) - nextVersion := int64(ExecutionConfigVersionThreshold + 1) + nextVersion := int64(RouterCompatibilityVersionThreshold + 1) compVersion := "0.1.0" compatibilityVersion := fmt.Sprintf("%d:%s", nextVersion, compVersion) assert.False(t, IsRouterCompatibleWithExecutionConfig(logger, compatibilityVersion)) @@ -83,7 +83,7 @@ func TestExecutionConfiguration(t *testing.T) { t.Run("return a warning if the execution config version is insufficient", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) - previousVersion := int64(ExecutionConfigVersionThreshold - 1) + previousVersion := int64(RouterCompatibilityVersionThreshold - 1) compVersion := "0.1.0" compatibilityVersion := fmt.Sprintf("%d:%s", previousVersion, compVersion) assert.True(t, IsRouterCompatibleWithExecutionConfig(logger, compatibilityVersion)) From a5b9c3a88c6125a70aed3935c49d3086168376e5 Mon Sep 17 00:00:00 2001 From: Aenimus Date: Sun, 26 Jan 2025 19:39:58 +0000 Subject: [PATCH 5/5] chore: fix tests --- router/pkg/execution_config/compatibility_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/router/pkg/execution_config/compatibility_test.go b/router/pkg/execution_config/compatibility_test.go index a3f3496718..90c2585582 100644 --- a/router/pkg/execution_config/compatibility_test.go +++ b/router/pkg/execution_config/compatibility_test.go @@ -51,7 +51,7 @@ func TestExecutionConfiguration(t *testing.T) { assert.Equal(t, zap.String("compatibility_version", compatibilityVersion), logsSlice[0].Context[0]) }) - t.Run("return an error if execution config version is unparsable", func(t *testing.T) { + t.Run("return an error if the router compatibility version is unparsable", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) compatibilityVersion := "a:0.1.0" @@ -64,7 +64,7 @@ func TestExecutionConfiguration(t *testing.T) { assert.Equal(t, zap.String("compatibility_version", compatibilityVersion), logsSlice[0].Context[0]) }) - t.Run("return an error if the maximum execution config version threshold of the router is exceeded", func(t *testing.T) { + t.Run("return an error if the maximum router compatibility version threshold of the router is exceeded", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) nextVersion := int64(RouterCompatibilityVersionThreshold + 1) @@ -76,11 +76,11 @@ func TestExecutionConfiguration(t *testing.T) { assert.Equal(t, executionConfigVersionThresholdExceededError(nextVersion), logsSlice[0].Message) assert.Equal(t, zapcore.ErrorLevel, logsSlice[0].Level) assert.Equal(t, 2, len(logsSlice[0].Context)) - assert.Equal(t, zap.Int64("execution_config_version", nextVersion), logsSlice[0].Context[0]) + assert.Equal(t, zap.Int64("router_compatibility_version", nextVersion), logsSlice[0].Context[0]) assert.Equal(t, zap.String("composition_package_version", compVersion), logsSlice[0].Context[1]) }) - t.Run("return a warning if the execution config version is insufficient", func(t *testing.T) { + t.Run("return a warning if the router compatibility version is insufficient", func(t *testing.T) { observed, logs := observer.New(zapcore.DebugLevel) logger := newLogger(observed) previousVersion := int64(RouterCompatibilityVersionThreshold - 1) @@ -92,7 +92,7 @@ func TestExecutionConfiguration(t *testing.T) { assert.Equal(t, executionConfigVersionInsufficientWarning(previousVersion), logsSlice[0].Message) assert.Equal(t, zapcore.WarnLevel, logsSlice[0].Level) assert.Equal(t, 2, len(logsSlice[0].Context)) - assert.Equal(t, zap.Int64("execution_config_version", previousVersion), logsSlice[0].Context[0]) + assert.Equal(t, zap.Int64("router_compatibility_version", previousVersion), logsSlice[0].Context[0]) assert.Equal(t, zap.String("composition_package_version", compVersion), logsSlice[0].Context[1]) }) }