Skip to content

Commit

Permalink
fix(ui): switch from js-yaml to yaml. Fixes #12205
Browse files Browse the repository at this point in the history
[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]>
  • Loading branch information
MasonM committed Oct 13, 2024
1 parent ad114b0 commit f165571
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 5 deletions.
4 changes: 2 additions & 2 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
127 changes: 127 additions & 0 deletions ui/src/app/shared/components/object-parser.test.ts
Original file line number Diff line number Diff line change
@@ -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"
}
}
}`);
});
});
11 changes: 8 additions & 3 deletions ui/src/app/shared/components/object-parser.ts
Original file line number Diff line number Diff line change
@@ -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
schema: 'yaml-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, ' ');
}
5 changes: 5 additions & 0 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10355,6 +10355,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"
Expand Down

0 comments on commit f165571

Please sign in to comment.