diff --git a/go.mod b/go.mod index 246f5e993..fc0849e58 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/fatih/color v1.19.0 github.com/google/go-cmp v0.7.0 github.com/kong/go-apiops v0.4.2 - github.com/kong/go-database-reconciler v1.38.0 + github.com/kong/go-database-reconciler v1.39.0 github.com/kong/go-kong v0.75.1 github.com/mitchellh/go-homedir v1.1.0 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index d961029a3..a3c8c24bf 100644 --- a/go.sum +++ b/go.sum @@ -225,8 +225,8 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/kong/go-apiops v0.4.2 h1:t+k42WVdxm5xOKsKTkkTIKK6l2QAhTerWqRa5muZVhM= github.com/kong/go-apiops v0.4.2/go.mod h1:Xt99d90LallLVwYJAGaufiNbBdsK0KKboe7gR4Ryths= -github.com/kong/go-database-reconciler v1.38.0 h1:KecRJLDRMrH/r/BTORQ17x4coZBOv8vKXTP/K9rlVzg= -github.com/kong/go-database-reconciler v1.38.0/go.mod h1:4YhUoBTMO5smJ0BuWaSfxknbhjrKx9Cg4hvUMR2EW8M= +github.com/kong/go-database-reconciler v1.39.0 h1:va3qjNe4urz+g+nZwmQQndvtUZtIY1ji3oA29aY4yfA= +github.com/kong/go-database-reconciler v1.39.0/go.mod h1:4YhUoBTMO5smJ0BuWaSfxknbhjrKx9Cg4hvUMR2EW8M= github.com/kong/go-kong v0.75.1 h1:2W4bZQUvXYGsZ7PvbZKkWnjrZmEiKueSY1yf6xv2jwY= github.com/kong/go-kong v0.75.1/go.mod h1:Wx5aTcMjyUnIF94M5NYFWb/EnuEkqB5STrWvybFSYYQ= github.com/kong/go-slugify v1.0.0 h1:vCFAyf2sdoSlBtLcrmDWUFn0ohlpKiKvQfXZkO5vSKY= diff --git a/tests/integration/diff_test.go b/tests/integration/diff_test.go index c0a1f1fc7..5a2c232e5 100644 --- a/tests/integration/diff_test.go +++ b/tests/integration/diff_test.go @@ -5,6 +5,7 @@ package integration import ( "context" "os" + "strings" "testing" "github.com/kong/go-database-reconciler/pkg/utils" @@ -1194,3 +1195,125 @@ func Test_RequestTermination_Dump_Diff(t *testing.T) { assert.Contains(t, diffOut, "Deleted: 0", "expected no diff between Kong state and the dumped file, got:\n%s", diffOut) } + +// Environment variables with short values that could cause false positives +var maskingFixEnvVars = map[string]string{ + "DECK_REDIS_DB": "0", + "DECK_SYNC_RATE": "1", + "DECK_RETRIES": "5", + "DECK_SERVICE_HOST": "example.com", + "DECK_REGEX_PRIORITY": "0", + "DECK_RATE_LIMIT": "100", +} + +// Test_Diff_Masking_ShortValues_NoCorruption verifies that short env var values +// like "0", "1", "5" don't corrupt UUIDs or other unrelated values. +// test scope: +// - 3.x +func Test_Diff_Masking_ShortValues_NoCorruption(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range maskingFixEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + require.NoError(t, sync(ctx, "testdata/diff/009-mask-short-values/initial.yaml")) + + out, err := diff("testdata/diff/009-mask-short-values/kong.yaml") + require.NoError(t, err) + + assert.Contains(t, out, "b35b3ec2-fa1c-4f6c-825e-c38141562c76", + "UUID should not be corrupted by short env var values") + assert.Contains(t, out, "a1234567-89ab-cdef-0123-456789abcdef", + "UUID should not be corrupted by short env var values") + + assert.NotContains(t, out, "[masked]3ec", + "The '0' in UUID should not cause substring masking") + assert.NotContains(t, out, "b35b3ec[masked]", + "The '2' in UUID should not cause substring masking") +} + +// Test_Diff_Masking_JSONOutput_ValidJSON verifies that when integers are masked, +// the JSON output remains valid (masked values are properly quoted). +// test scope: +// - 3.x +func Test_Diff_Masking_JSONOutput_ValidJSON(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range maskingFixEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + require.NoError(t, sync(ctx, "testdata/diff/009-mask-short-values/initial.yaml")) + + out, err := diff("testdata/diff/009-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + assert.NotEmpty(t, out, "JSON output should not be empty") + + if strings.Contains(out, "[masked]") { + assert.Contains(t, out, `"[masked]"`, + "Masked values in JSON should be quoted strings") + } + + assert.Contains(t, out, "b35b3ec2-fa1c-4f6c-825e-c38141562c76", + "UUID should not be corrupted in JSON output") +} + +// Test_Diff_Masking_ExactValueMatch verifies that masking uses exact value matching, +// not substring matching. A value like "100" should NOT be masked just because +// DECK_REDIS_DB=0 (the "0" in "100" should not trigger masking). +// test scope: +// - 3.x +func Test_Diff_Masking_ExactValueMatch(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range maskingFixEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + require.NoError(t, sync(ctx, "testdata/diff/009-mask-short-values/initial.yaml")) + + out, err := diff("testdata/diff/009-mask-short-values/kong.yaml") + require.NoError(t, err) + + assert.Contains(t, out, "60000", + "Timeout value should not be masked - '0' in '60000' is not an exact match") + + assert.Contains(t, out, "8080", + "Port value should not be masked - '0' in '8080' is not an exact match") +} + +// Test_Sync_Masking_ValidJSON verifies that sync command with --json-output +// produces valid JSON when env var values include integers. +// test scope: +// - 3.x +func Test_Sync_Masking_ValidJSON(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range maskingFixEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + require.NoError(t, sync(ctx, "testdata/diff/009-mask-short-values/initial.yaml")) + + out, err := diff("testdata/diff/009-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + if strings.Contains(out, "masked") { + assert.NotRegexp(t, `:\s*\[masked\][^"]`, out, + "Masked integer values should be quoted in JSON output") + } +} diff --git a/tests/integration/sync_test.go b/tests/integration/sync_test.go index 9264e36e9..4f3b64136 100644 --- a/tests/integration/sync_test.go +++ b/tests/integration/sync_test.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "os" + "strings" "testing" "time" @@ -12185,3 +12186,154 @@ func testSyncPluginConditionalKonnectImpl(t *testing.T) { testKongState(t, client, true, false, expectedStatePostSync, nil) } + +// Environment variables with short values that could cause false positives +var syncMaskingEnvVars = map[string]string{ + "DECK_REDIS_DB": "0", + "DECK_SYNC_RATE": "1", + "DECK_RETRIES": "5", + "DECK_SERVICE_HOST": "example.com", + "DECK_REGEX_PRIORITY": "0", + "DECK_RATE_LIMIT": "100", +} + +// Test_Sync_Masking_JSONOutput_ValidJSON verifies that sync with --json-output +// produces valid JSON when env var values include short integers like "0". +// This tests the masking fix - short values shouldn't corrupt the JSON output. +// test scope: +// - 3.x +func Test_Sync_Masking_JSONOutput_ValidJSON(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range syncMaskingEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + out, err := syncWithOutput(ctx, "testdata/sync/053-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + assert.NotEmpty(t, out, "JSON output should not be empty") + assert.Contains(t, out, `"summary"`, + "JSON output should contain summary field") + + assert.Contains(t, out, `"creating"`, + "JSON output should contain creating field") + + if strings.Contains(out, "[masked]") { + assert.Contains(t, out, `"[masked]"`, + "Masked values in JSON should be quoted strings") + // Check for invalid unquoted [masked] + assert.NotRegexp(t, `:\s*\[masked\][^"]`, out, + "Masked integer values should be quoted in JSON output") + } + + assert.NotContains(t, out, "b35b3ec[masked]", + "UUID should not be corrupted by short env var values in sync output") + assert.NotContains(t, out, "[masked]3ec2", + "UUID should not be corrupted by substring matching") +} + +// Test_Sync_Masking_ShortValues_NoCorruption verifies that short env var values +// don't corrupt UUIDs or other values in the sync JSON output. +// test scope: +// - 3.x +func Test_Sync_Masking_ShortValues_NoCorruption(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range syncMaskingEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + out, err := syncWithOutput(ctx, "testdata/sync/053-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + assert.NotRegexp(t, `:\s*\[masked\][,\s\n\}]`, out, + "Masked values should be quoted strings, not unquoted [masked]") + + if strings.Contains(out, "[masked]") { + assert.Contains(t, out, `"[masked]"`, + "Masked values must be quoted for valid JSON") + } + + client, err := getTestClient() + require.NoError(t, err) + + services, err := client.Services.ListAll(ctx) + require.NoError(t, err) + + var foundService *kong.Service + for _, svc := range services { + if svc.Name != nil && *svc.Name == "mask-test-service" { + foundService = svc + break + } + } + require.NotNil(t, foundService, "Service should be created via sync") + + assert.Equal(t, "b35b3ec2-fa1c-4f6c-825e-c38141562c76", *foundService.ID, + "Service UUID should not be corrupted") + + assert.Equal(t, "example.com", *foundService.Host) + assert.Equal(t, 5, *foundService.Retries) +} + +// Test_Sync_Masking_ExactValueMatch verifies that masking uses exact value matching +// in the sync command's JSON output. +// test scope: +// - 3.x +func Test_Sync_Masking_ExactValueMatch(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range syncMaskingEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + out, err := syncWithOutput(ctx, "testdata/sync/053-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + assert.NotContains(t, out, "6[masked]", + "The '0' in '60000' should not cause partial masking") + assert.NotContains(t, out, "[masked][masked][masked][masked]", + "Multiple '0's should not create corrupted output") + + if strings.Contains(out, "[masked]") { + assert.Contains(t, out, `"[masked]"`, + "Masked values should be quoted strings in JSON") + } +} + +// Test_Sync_Masking_ReSyncIdempotency verifies that re-syncing the same file +// with env vars doesn't cause spurious changes (idempotency). +// test scope: +// - 3.x +func Test_Sync_Masking_ReSyncIdempotency(t *testing.T) { + runWhen(t, "kong", ">=3.0.0") + setup(t) + + for k, v := range syncMaskingEnvVars { + t.Setenv(k, v) + } + + ctx := context.Background() + + require.NoError(t, sync(ctx, "testdata/sync/053-mask-short-values/kong.yaml")) + + out, err := syncWithOutput(ctx, "testdata/sync/053-mask-short-values/kong.yaml", "--json-output") + require.NoError(t, err) + + assert.Contains(t, out, `"creating": []`, + "No entities should be created on re-sync") + assert.Contains(t, out, `"updating": []`, + "No entities should be updated on re-sync") + assert.Contains(t, out, `"deleting": []`, + "No entities should be deleted on re-sync") +} diff --git a/tests/integration/test_utils.go b/tests/integration/test_utils.go index 37059737f..4de340573 100644 --- a/tests/integration/test_utils.go +++ b/tests/integration/test_utils.go @@ -349,6 +349,28 @@ func sync(ctx context.Context, kongFile string, opts ...string) error { return deckCmd.ExecuteContext(ctx) } +func syncWithOutput(ctx context.Context, kongFile string, opts ...string) (string, error) { + deckCmd := cmd.NewRootCmd() + args := []string{"gateway", "sync", kongFile} + if len(opts) > 0 { + args = append(args, opts...) + } + deckCmd.SetArgs(args) + + // overwrite default standard output + r, w, _ := os.Pipe() + color.Output = w + + // execute decK command + cmdErr := deckCmd.ExecuteContext(ctx) + + // read command output + w.Close() + out, _ := io.ReadAll(r) + + return stripansi.Strip(string(out)), cmdErr +} + func multiFileSync(ctx context.Context, kongFiles []string, opts ...string) error { deckCmd := cmd.NewRootCmd() args := []string{"gateway", "sync"} diff --git a/tests/integration/testdata/diff/009-mask-short-values/initial.yaml b/tests/integration/testdata/diff/009-mask-short-values/initial.yaml new file mode 100644 index 000000000..5d658c4e3 --- /dev/null +++ b/tests/integration/testdata/diff/009-mask-short-values/initial.yaml @@ -0,0 +1,16 @@ +_format_version: "3.0" +services: + - name: test-service + id: b35b3ec2-fa1c-4f6c-825e-c38141562c76 + host: old-example.com + port: 8080 + protocol: http + read_timeout: 60000 + write_timeout: 60000 + retries: 3 + routes: + - name: test-route + id: a1234567-89ab-cdef-0123-456789abcdef + paths: + - /api + regex_priority: 1 diff --git a/tests/integration/testdata/diff/009-mask-short-values/kong.yaml b/tests/integration/testdata/diff/009-mask-short-values/kong.yaml new file mode 100644 index 000000000..5ce293e59 --- /dev/null +++ b/tests/integration/testdata/diff/009-mask-short-values/kong.yaml @@ -0,0 +1,23 @@ +_format_version: "3.0" +services: + - name: test-service + id: b35b3ec2-fa1c-4f6c-825e-c38141562c76 + host: ${{ env "DECK_SERVICE_HOST" }} + port: 8080 + protocol: http + read_timeout: 60000 + write_timeout: 60000 + retries: ${{ env "DECK_RETRIES" }} + routes: + - name: test-route + id: a1234567-89ab-cdef-0123-456789abcdef + paths: + - /api + regex_priority: ${{ env "DECK_REGEX_PRIORITY" }} +plugins: + - name: rate-limiting + id: c2222222-2222-2222-2222-222222222222 + config: + minute: ${{ env "DECK_RATE_LIMIT" }} + redis_database: ${{ env "DECK_REDIS_DB" }} + diff --git a/tests/integration/testdata/sync/053-mask-short-values/kong.yaml b/tests/integration/testdata/sync/053-mask-short-values/kong.yaml new file mode 100644 index 000000000..068f21f7f --- /dev/null +++ b/tests/integration/testdata/sync/053-mask-short-values/kong.yaml @@ -0,0 +1,23 @@ +_format_version: "3.0" +services: + - name: mask-test-service + id: b35b3ec2-fa1c-4f6c-825e-c38141562c76 + host: ${{ env "DECK_SERVICE_HOST" }} + port: 8080 + protocol: http + read_timeout: 60000 + write_timeout: 60000 + retries: ${{ env "DECK_RETRIES" }} + routes: + - name: mask-test-route + id: a1234567-89ab-cdef-0123-456789abcdef + paths: + - /api + regex_priority: ${{ env "DECK_REGEX_PRIORITY" }} +plugins: + - name: rate-limiting + id: c2222222-2222-2222-2222-222222222222 + config: + minute: ${{ env "DECK_RATE_LIMIT" }} + redis_database: ${{ env "DECK_REDIS_DB" }} +