From d7a3e8abad2e9d1e7cd108475e62b09cdead5d31 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Tue, 30 May 2023 13:47:40 +0100 Subject: [PATCH 1/3] fix: SpecReader to escape embedded JSON files --- specs/spec_reader.go | 13 +++++++++++++ specs/spec_reader_test.go | 34 ++++++++++++++++++++++++++++++++++ specs/testdata/creds2.json | 4 ++++ 3 files changed, 51 insertions(+) create mode 100644 specs/testdata/creds2.json diff --git a/specs/spec_reader.go b/specs/spec_reader.go index 347fa455..7f8a2454 100644 --- a/specs/spec_reader.go +++ b/specs/spec_reader.go @@ -2,10 +2,12 @@ package specs import ( "bytes" + "encoding/json" "fmt" "math/rand" "os" "path/filepath" + "reflect" "regexp" "strings" @@ -33,6 +35,17 @@ func expandFileConfig(cfg []byte) ([]byte, error) { expandErr = err return nil } + var isJSON any + if err := json.Unmarshal(content, &isJSON); err == nil { + k := reflect.TypeOf(isJSON).Kind() + if k == reflect.Map || k == reflect.Slice { + buffer := &bytes.Buffer{} + encoder := json.NewEncoder(buffer) + encoder.SetEscapeHTML(false) + expandErr = encoder.Encode(string(content)) + return bytes.TrimSuffix(buffer.Bytes(), []byte{'\n'}) + } + } return content }) return cfg, expandErr diff --git a/specs/spec_reader_test.go b/specs/spec_reader_test.go index 654a9989..2d8d7703 100644 --- a/specs/spec_reader_test.go +++ b/specs/spec_reader_test.go @@ -7,6 +7,7 @@ import ( "runtime" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -287,6 +288,39 @@ spec: } } +func TestExpandFileJSON(t *testing.T) { + cfg := []byte(` +kind: source +spec: + name: gcp + path: cloudquery/gcp + version: v1.0.0 + table_concurrency: 10 + registry: local + destinations: [postgresql] + service_account_key_json: ${file:./testdata/creds2.json} + `) + expectedCfg := []byte(` +kind: source +spec: + name: gcp + path: cloudquery/gcp + version: v1.0.0 + table_concurrency: 10 + registry: local + destinations: [postgresql] + service_account_key_json: "{\n \"key\": \"foo\",\n \"secret\": \"bar\"\n}\n" + `) + expandedCfg, err := expandFileConfig(cfg) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + expectedCfg = bytes.ReplaceAll(expectedCfg, []byte(`\n`), []byte(`\r\n`)) + } + assert.Equal(t, expectedCfg, expandedCfg) +} + func TestExpandEnv(t *testing.T) { os.Setenv("TEST_ENV_CREDS", "mytestcreds") os.Setenv("TEST_ENV_CREDS2", "anothercredtest") diff --git a/specs/testdata/creds2.json b/specs/testdata/creds2.json new file mode 100644 index 00000000..ddb02535 --- /dev/null +++ b/specs/testdata/creds2.json @@ -0,0 +1,4 @@ +{ + "key": "foo", + "secret": "bar" +} From 024cdd127c50dc5ad07e8e2a29a24f91e9d345ee Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Mon, 5 Jun 2023 15:10:10 +0100 Subject: [PATCH 2/3] clean up, add e2e test case for Env expansion --- specs/spec_reader.go | 35 +++++++++++++++++++--------- specs/spec_reader_test.go | 48 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/specs/spec_reader.go b/specs/spec_reader.go index 7f8a2454..17dd32c3 100644 --- a/specs/spec_reader.go +++ b/specs/spec_reader.go @@ -26,6 +26,29 @@ type SpecReader struct { var fileRegex = regexp.MustCompile(`\$\{file:([^}]+)\}`) var envRegex = regexp.MustCompile(`\$\{([^}]+)\}`) +// readEncodeIfJSON reads the content of the file and if it is a JSON object or array, it encodes it as a string to satisfy YAML requirements +// It will suppress any JSON unmarshalling errors and return the original content. +func readEncodeIfJSON(content []byte) ([]byte, error) { + var isJSON any + if err := json.Unmarshal(content, &isJSON); err != nil { + return content, nil + } + + k := reflect.TypeOf(isJSON).Kind() + if k == reflect.Map || k == reflect.Slice { + buffer := &bytes.Buffer{} + encoder := json.NewEncoder(buffer) + encoder.SetEscapeHTML(false) + if err := encoder.Encode(string(content)); err != nil { + return content, err + } + + return bytes.TrimSuffix(buffer.Bytes(), []byte{'\n'}), nil + } + + return content, nil +} + func expandFileConfig(cfg []byte) ([]byte, error) { var expandErr error cfg = fileRegex.ReplaceAllFunc(cfg, func(match []byte) []byte { @@ -35,17 +58,7 @@ func expandFileConfig(cfg []byte) ([]byte, error) { expandErr = err return nil } - var isJSON any - if err := json.Unmarshal(content, &isJSON); err == nil { - k := reflect.TypeOf(isJSON).Kind() - if k == reflect.Map || k == reflect.Slice { - buffer := &bytes.Buffer{} - encoder := json.NewEncoder(buffer) - encoder.SetEscapeHTML(false) - expandErr = encoder.Encode(string(content)) - return bytes.TrimSuffix(buffer.Bytes(), []byte{'\n'}) - } - } + content, expandErr = readEncodeIfJSON(content) return content }) return cfg, expandErr diff --git a/specs/spec_reader_test.go b/specs/spec_reader_test.go index 2d8d7703..25e914d6 100644 --- a/specs/spec_reader_test.go +++ b/specs/spec_reader_test.go @@ -318,7 +318,7 @@ spec: if runtime.GOOS == "windows" { expectedCfg = bytes.ReplaceAll(expectedCfg, []byte(`\n`), []byte(`\r\n`)) } - assert.Equal(t, expectedCfg, expandedCfg) + assert.Equal(t, string(expectedCfg), string(expandedCfg)) } func TestExpandEnv(t *testing.T) { @@ -366,3 +366,49 @@ spec: t.Fatal("expected error, got nil") } } + +func TestExpandEnvJSONNewlines(t *testing.T) { + os.Setenv("TEST_ENV_CREDS3", `{ + "type": "service_account", + "private_key": "-----BEGIN PRIVATE KEY-----\nMIItest\n\n-----END PRIVATE KEY-----\n", + "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test%40test.iam.gserviceaccount.com" +} +`) + cfg := []byte(` +kind: source +spec: + name: test + registry: local + path: /path/to/source + version: v1.0.0 + tables: [ "some_table" ] + destinations: [ "test2" ] + spec: + credentials: ${TEST_ENV_CREDS3} + otherstuff: 2 +--- +kind: destination +spec: + name: test2 + registry: local + path: /path/to/dest +`) + + f, err := os.CreateTemp("", "testcase*.yaml") + assert.NoError(t, err) + defer os.Remove(f.Name()) + assert.NoError(t, os.WriteFile(f.Name(), cfg, 0644)) + + expectedCreds := map[string]any{ + "type": "service_account", + "private_key": "-----BEGIN PRIVATE KEY-----\nMIItest\n\n-----END PRIVATE KEY-----\n", + "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test%40test.iam.gserviceaccount.com", + } + + s, err := NewSpecReader([]string{f.Name()}) + assert.NoError(t, err) + + assert.Equal(t, 1, len(s.Sources)) + sp := s.Sources[0].Spec.(map[string]any) + assert.Equal(t, expectedCreds, sp["credentials"]) +} From f682e0074cb19f732a53e9b51fcfb2645ae3739b Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Mon, 5 Jun 2023 17:47:01 +0100 Subject: [PATCH 3/3] Fixes and cleanups, solve #10987 too --- specs/spec_reader.go | 11 +++++++++-- specs/spec_reader_test.go | 11 +++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/specs/spec_reader.go b/specs/spec_reader.go index 17dd32c3..e97b099f 100644 --- a/specs/spec_reader.go +++ b/specs/spec_reader.go @@ -58,7 +58,10 @@ func expandFileConfig(cfg []byte) ([]byte, error) { expandErr = err return nil } - content, expandErr = readEncodeIfJSON(content) + content, err = readEncodeIfJSON(content) + if expandErr == nil { + expandErr = err + } return content }) return cfg, expandErr @@ -74,7 +77,11 @@ func expandEnv(cfg []byte) ([]byte, error) { expandErr = fmt.Errorf("env variable %s not found", envVar) return nil } - return []byte(content) + newcontent, err := readEncodeIfJSON([]byte(content)) + if expandErr == nil { + expandErr = err + } + return newcontent }) return cfg, expandErr diff --git a/specs/spec_reader_test.go b/specs/spec_reader_test.go index 25e914d6..33706e77 100644 --- a/specs/spec_reader_test.go +++ b/specs/spec_reader_test.go @@ -368,12 +368,13 @@ spec: } func TestExpandEnvJSONNewlines(t *testing.T) { - os.Setenv("TEST_ENV_CREDS3", `{ + expectedCreds := `{ "type": "service_account", "private_key": "-----BEGIN PRIVATE KEY-----\nMIItest\n\n-----END PRIVATE KEY-----\n", "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test%40test.iam.gserviceaccount.com" } -`) +` + os.Setenv("TEST_ENV_CREDS3", expectedCreds) cfg := []byte(` kind: source spec: @@ -399,12 +400,6 @@ spec: defer os.Remove(f.Name()) assert.NoError(t, os.WriteFile(f.Name(), cfg, 0644)) - expectedCreds := map[string]any{ - "type": "service_account", - "private_key": "-----BEGIN PRIVATE KEY-----\nMIItest\n\n-----END PRIVATE KEY-----\n", - "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test%40test.iam.gserviceaccount.com", - } - s, err := NewSpecReader([]string{f.Name()}) assert.NoError(t, err)