fix(ui): switch from js-yaml to yaml. Fixes #12205

[js-yaml](https://www.npmjs.com/package/js-yaml) has had no releases in over 3 years. Also, the fact it only supports YAML 1.2 is leading to bugs, since Kubernetes uses YAML 1.1 (see kubernetes/kubernetes#34146). This switches over to [yaml](https://www.npmjs.com/package/yaml), which had a release last month, is [licensed under ISC](https://github.com/eemeli/yaml/blob/main/LICENSE), and supports YAML 1.1. I also added test cases for the parsing/stringifying code. Signed-off-by: Mason Malone <[email protected]>
argoproj · Oct 13, 2024 · 1693f40 · 1693f40
1 parent ad114b0
commit 1693f40
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 11 deletions.
diff --git a/ui/package.json b/ui/package.json
@@ -24,7 +24,6 @@
         "cronstrue": "^2.50.0",
         "dagre": "^0.8.5",
         "history": "^4.10.1",
-        "js-yaml": "^4.1.0",
         "moment": "^2.30.1",
         "monaco-editor": "^0.45.0",
         "prop-types": "^15.8.1",
@@ -37,7 +36,8 @@
         "react-router-dom": "^4.2.2",
         "remark-gfm": "^3.0.0",
         "superagent": "^8.1.2",
-        "swagger-ui-react": "^5.17.12"
+        "swagger-ui-react": "^5.17.12",
+        "yaml": "^2.5.1"
     },
     "devDependencies": {
         "@babel/core": "^7.22.11",
@@ -48,7 +48,6 @@
         "@types/dagre": "^0.7.52",
         "@types/history": "^4.6.2",
         "@types/jest": "^26.0.15",
-        "@types/js-yaml": "^4.0.9",
         "@types/prop-types": "^15.7.11",
         "@types/react": "^16.8.5",
         "@types/react-autocomplete": "^1.8.10",

diff --git a/ui/src/app/shared/components/object-parser.test.ts b/ui/src/app/shared/components/object-parser.test.ts
@@ -0,0 +1,127 @@
+import {parse, stringify} from './object-parser';
+import {exampleWorkflowTemplate} from '../examples';
+
+describe('parse', () => {
+    it('handles a valid JSON string', () => {
+        expect(parse('{}')).toEqual({});
+        expect(parse('{"a": 1}')).toEqual({a: 1});
+    });
+
+    it('handles a malformed JSON string', () => {
+        expect(() => parse('{1}')).toThrow();
+    });
+
+    it('handles a valid YAML string', () => {
+        expect(parse('')).toEqual(null);
+        expect(parse('a: 1')).toEqual({a: 1});
+    });
+
+    it('handles a malformed YAML string', () => {
+        expect(() => parse('!foo')).toThrow();
+    });
+
+    it('parses a YAML string as YAML 1.1, not YAML 1.2', () => {
+        expect(parse('foo: 0755')).toEqual({foo: 493});
+    });
+});
+
+describe('stringify', () => {
+    const testWorkflowTemplate = exampleWorkflowTemplate('test');
+    testWorkflowTemplate.metadata.name = 'test-workflowtemplate';
+
+    it('encodes to YAML', () => {
+        // Can't use toMatchInlineSnapshot() until we upgrade to Jest 30: https://github.com/jestjs/jest/issues/14305
+        expect(stringify(testWorkflowTemplate, 'yaml')).toEqual(`\
+metadata:
+  name: test-workflowtemplate
+  namespace: test
+  labels:
+    example: "true"
+spec:
+  workflowMetadata:
+    labels:
+      example: "true"
+  entrypoint: argosay
+  arguments:
+    parameters:
+      - name: message
+        value: hello argo
+  templates:
+    - name: argosay
+      inputs:
+        parameters:
+          - name: message
+            value: "{{workflow.parameters.message}}"
+      container:
+        name: main
+        image: argoproj/argosay:v2
+        command:
+          - /argosay
+        args:
+          - echo
+          - "{{inputs.parameters.message}}"
+  ttlStrategy:
+    secondsAfterCompletion: 300
+  podGC:
+    strategy: OnPodCompletion
+`);
+    });
+
+    it('encodes to JSON', () => {
+        expect(stringify(testWorkflowTemplate, 'json')).toEqual(`{
+  "metadata": {
+    "name": "test-workflowtemplate",
+    "namespace": "test",
+    "labels": {
+      "example": "true"
+    }
+  },
+  "spec": {
+    "workflowMetadata": {
+      "labels": {
+        "example": "true"
+      }
+    },
+    "entrypoint": "argosay",
+    "arguments": {
+      "parameters": [
+        {
+          "name": "message",
+          "value": "hello argo"
+        }
+      ]
+    },
+    "templates": [
+      {
+        "name": "argosay",
+        "inputs": {
+          "parameters": [
+            {
+              "name": "message",
+              "value": "{{workflow.parameters.message}}"
+            }
+          ]
+        },
+        "container": {
+          "name": "main",
+          "image": "argoproj/argosay:v2",
+          "command": [
+            "/argosay"
+          ],
+          "args": [
+            "echo",
+            "{{inputs.parameters.message}}"
+          ]
+        }
+      }
+    ],
+    "ttlStrategy": {
+      "secondsAfterCompletion": 300
+    },
+    "podGC": {
+      "strategy": "OnPodCompletion"
+    }
+  }
+}`);
+    });
+});
diff --git a/ui/src/app/shared/components/object-parser.ts b/ui/src/app/shared/components/object-parser.ts
@@ -1,12 +1,17 @@
-import jsyaml from 'js-yaml';
+import YAML from 'yaml';
 
 export function parse<T>(value: string): T {
     if (value.startsWith('{')) {
         return JSON.parse(value);
     }
-    return jsyaml.load(value) as T;
+    return YAML.parse(value, {
+        // Default is YAML 1.2, but Kubernetes uses YAML 1.1, which leads to subtle bugs.
+        // See https://github.com/argoproj/argo-workflows/issues/12205#issuecomment-2111572189
+        version: '1.1',
+        strict: false
+    }) as T;
 }
 
 export function stringify<T>(value: T, type: string) {
-    return type === 'yaml' ? jsyaml.dump(value, {noRefs: true}) : JSON.stringify(value, null, '  ');
+    return type === 'yaml' ? YAML.stringify(value, {aliasDuplicateObjects: false}) : JSON.stringify(value, null, '  ');
 }
diff --git a/ui/yarn.lock b/ui/yarn.lock
@@ -2080,11 +2080,6 @@
     jest-diff "^26.0.0"
     pretty-format "^26.0.0"
 
-"@types/js-yaml@^4.0.9":
-  version "4.0.9"
-  resolved "https://registry.yarnpkg.com/@types/js-yaml/-/js-yaml-4.0.9.tgz#cd82382c4f902fed9691a2ed79ec68c5898af4c2"
-  integrity sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==
-
 "@types/json-schema@^7.0.12", "@types/json-schema@^7.0.5", "@types/json-schema@^7.0.8", "@types/json-schema@^7.0.9":
   version "7.0.15"
   resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.15.tgz#596a1747233694d50f6ad8a7869fcb6f56cf5841"
@@ -10355,6 +10350,11 @@ yallist@^4.0.0:
   resolved "https://registry.yarnpkg.com/yallist/-/yallist-4.0.0.tgz#9bb92790d9c0effec63be73519e11a35019a3a72"
   integrity sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==
 
+yaml@^2.5.1:
+  version "2.5.1"
+  resolved "https://registry.yarnpkg.com/yaml/-/yaml-2.5.1.tgz#c9772aacf62cb7494a95b0c4f1fb065b563db130"
+  integrity sha512-bLQOjaX/ADgQ20isPJRvF0iRUHIxVhYvr53Of7wGcWlO2jvtUlH5m87DsmulFVxRpNLOnI4tB6p/oh8D7kpn9Q==
+
 [email protected]:
   version "20.2.9"
   resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-20.2.9.tgz#2eb7dc3b0289718fc295f362753845c41a0c94ee"