From 4e54bd72d58b49c4bf065b04814274c5d80f065a Mon Sep 17 00:00:00 2001
From: Mateusz Hawrus <48822818+nieomylnieja@users.noreply.github.com>
Date: Thu, 27 Jun 2024 13:13:50 +0200
Subject: [PATCH] fix: Double quoted JSON handling (#471)

## Motivation

The issue is extensively covered in this issue:
https://github.com/goccy/go-yaml/issues/455

## Summary

Added test case and replaced `goccy/go-yaml` with our fork.
For now I'm pinning the commit SHA from the PR branch.
I'm not sure if the upstream is actively maintained anymore, we might
just want to go further with the fork instead of the replace clause.

## Related changes

- https://github.com/goccy/go-yaml/pull/457
- https://github.com/nobl9/go-yaml/pull/3

## Testing

Test case was supplied.

## Release Notes

Fixed incorrect handling of double quoted JSON literals which caused
errors when unmarshaling CloudWatch v1alpha SLOs
with certain `json` queries.
---
 go.mod                                        |  3 ++
 go.sum                                        |  4 +--
 manifest/v1alpha/parser/parser_test.go        |  9 ++++++
 .../parser/test_data/cloudwatch_slo.yaml      | 29 +++++++++++++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 manifest/v1alpha/parser/test_data/cloudwatch_slo.yaml

diff --git a/go.mod b/go.mod
index 22f07112..d4990362 100644
--- a/go.mod
+++ b/go.mod
@@ -31,3 +31,6 @@ require (
 	golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
+
+// We might in the end decide to just go with the fork direcly.
+replace github.com/goccy/go-yaml => github.com/nobl9/go-yaml v0.0.0-20240626115914-6b82fd0d61b9
diff --git a/go.sum b/go.sum
index 8410ed77..9343bf24 100644
--- a/go.sum
+++ b/go.sum
@@ -19,8 +19,6 @@ github.com/go-playground/universal-translator v0.17.0 h1:icxd5fm+REJzpZx7ZfpaD87
 github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA=
 github.com/go-playground/validator/v10 v10.4.1 h1:pH2c5ADXtd66mxoE0Zm9SUhxE20r7aM3F26W0hOn+GE=
 github.com/go-playground/validator/v10 v10.4.1/go.mod h1:nlOn6nFhuKACm19sB/8EGNn9GlaMV7XkbRSipzJ0Ii4=
-github.com/goccy/go-yaml v1.11.3 h1:B3W9IdWbvrUu2OYQGwvU1nZtvMQJPBKgBUuweJjLj6I=
-github.com/goccy/go-yaml v1.11.3/go.mod h1:wKnAMd44+9JAAnGQpWVEgBzGt3YuTaQ4uXoHvE4m7WU=
 github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
 github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
@@ -42,6 +40,8 @@ github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovk
 github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
 github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
 github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
+github.com/nobl9/go-yaml v0.0.0-20240626115914-6b82fd0d61b9 h1:HWAY7k8zA8T3dgQRg6llwyWshguD6bIeaVXukNrnen4=
+github.com/nobl9/go-yaml v0.0.0-20240626115914-6b82fd0d61b9/go.mod h1:wKnAMd44+9JAAnGQpWVEgBzGt3YuTaQ4uXoHvE4m7WU=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
diff --git a/manifest/v1alpha/parser/parser_test.go b/manifest/v1alpha/parser/parser_test.go
index 5d091c58..82785287 100644
--- a/manifest/v1alpha/parser/parser_test.go
+++ b/manifest/v1alpha/parser/parser_test.go
@@ -136,6 +136,15 @@ func Test_ParseObject_EnsureAllKindsAreParsed(t *testing.T) {
 	}
 }
 
+// References:
+// - https://github.com/nobl9/go-yaml/pull/3 (fork)
+// - https://github.com/nobl9/go-yaml/pull/457 (upstream)
+func Test_ParseObject_DoubleQuotedJSONHandling(t *testing.T) {
+	data, format := readParserTestFile(t, "cloudwatch_slo.yaml")
+	_, err := ParseObject(data, manifest.KindSLO, format)
+	require.NoError(t, err)
+}
+
 func readParserTestFile(t *testing.T, filename string) ([]byte, manifest.ObjectFormat) {
 	t.Helper()
 	data, err := parserTestData.ReadFile(filepath.Join("test_data", filename))
diff --git a/manifest/v1alpha/parser/test_data/cloudwatch_slo.yaml b/manifest/v1alpha/parser/test_data/cloudwatch_slo.yaml
new file mode 100644
index 00000000..90780895
--- /dev/null
+++ b/manifest/v1alpha/parser/test_data/cloudwatch_slo.yaml
@@ -0,0 +1,29 @@
+apiVersion: n9/v1alpha
+kind: SLO
+metadata:
+  name: cloudwatch-json
+  project: cloudwatch
+spec:
+  budgetingMethod: Occurrences
+  description: ""
+  indicator:
+    metricSource:
+      kind: Agent
+      name: cloudwatch
+      project: cloudwatch
+  objectives:
+    - displayName: ""
+      op: lte
+      rawMetric:
+        query:
+          cloudWatch:
+            json: "[{\"Id\": \"e1\",\"Expression\": \"MAX(FILL(METRICS(), 0))\",\"Period\": 60},{\"Id\": \"m1\",\"MetricStat\": {\"Metric\": {\"Namespace\": \"AWS/ApplicationELB\",\"MetricName\": \"TargetResponseTime\",\"Dimensions\": [{\"Name\": \"LoadBalancer\",\"Value\": \"app/123/123\"},{\"Name\": \"TargetGroup\",\"Value\": \"targetgroup/123/123\"}]},\"Period\": 60,\"Stat\": \"Average\"},\"ReturnData\": false}]"
+            region: eu-central-1
+      target: 0.8
+      value: 0.9
+      name: objective-1
+  service: cloudwatch-service
+  timeWindows:
+    - count: 1
+      isRolling: true
+      unit: Hour
\ No newline at end of file