Skip to content

Commit

Permalink
Use JSON schema when parsing YAML
Browse files Browse the repository at this point in the history
The fields of the Kubernetes core resource types are tagged with Go's
`"json:"` serialization metadata. They expect things like JavaScript's
`Date` to be serialized as string.

Fixes #501.
  • Loading branch information
hausdorff committed Jul 13, 2019
1 parent 375cf3e commit c6d600c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
(https://github.com/pulumi/pulumi-kubernetes/pull/638).
- Return useful errors when we fail to fetch URL YAML
(https://github.com/pulumi/pulumi-kubernetes/pull/638).
- Use JSON_SCHEMA when parsing Kubernetes YAML, to conform with the expectations of the Kubernetes
core resource types. (https://github.com/pulumi/pulumi-kubernetes/pull/638).

## 0.25.2 (July 11, 2019)

Expand Down
16 changes: 12 additions & 4 deletions pkg/gen/nodejs-templates/yaml.ts.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import * as pulumi from "@pulumi/pulumi";
import * as fs from "fs";
import * as glob from "glob";
import * as jsyaml from "js-yaml";
import fetch from "node-fetch";
import * as k8s from "../index";
import * as outputApi from "../types/output";
Expand Down Expand Up @@ -66,6 +65,16 @@ import * as outputApi from "../types/output";
resourcePrefix?: string;
}

/** @ignore */ export function yamlLoadAll(text: string): any[] {
// NOTE[pulumi-kubernetes#501]: Use `loadAll` with `JSON_SCHEMA` here instead of
// `safeLoadAll` because the latter is incompatible with `JSON_SCHEMA`. It is
// important to use `JSON_SCHEMA` here because the fields of the Kubernetes core
// API types are all tagged with `json:`, and they don't deal very well with things
// like dates.
const jsyaml = require("js-yaml");

This comment has been minimized.

Copy link
@lukehoban

lukehoban Jul 14, 2019

Member

Why was this moved to be a local require instead of an import?

This comment has been minimized.

Copy link
@hausdorff

hausdorff Jul 15, 2019

Author Contributor

Our use of js-yaml is sufficiently subtle that I don't want anyone to use this package outside of this specific function.

return jsyaml.loadAll(text, undefined, {schema: jsyaml.JSON_SCHEMA});
}
/** @ignore */ export function parse(
config: ConfigGroupOpts, opts?: pulumi.CustomResourceOptions
): pulumi.Output<{[key: string]: pulumi.CustomResource}> {
Expand Down Expand Up @@ -112,9 +121,8 @@ import * as outputApi from "../types/output";
}
for (const text of yamlTexts) {
const objs = jsyaml.safeLoadAll(text);
const docResources = parseYamlDocument({
objs: objs,
objs: yamlLoadAll(text),
transformations: config.transformations,
resourcePrefix: config.resourcePrefix
},
Expand Down Expand Up @@ -258,7 +266,7 @@ import * as outputApi from "../types/output";
this.resources = pulumi.output(text.then(t => {
try {
return parseYamlDocument({
objs: jsyaml.safeLoadAll(t),
objs: yamlLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})
Expand Down
16 changes: 12 additions & 4 deletions sdk/nodejs/yaml/yaml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import * as pulumi from "@pulumi/pulumi";
import * as fs from "fs";
import * as glob from "glob";
import * as jsyaml from "js-yaml";
import fetch from "node-fetch";
import * as k8s from "../index";
import * as outputApi from "../types/output";
Expand Down Expand Up @@ -66,6 +65,16 @@ import * as outputApi from "../types/output";
resourcePrefix?: string;
}

/** @ignore */ export function yamlLoadAll(text: string): any[] {

This comment has been minimized.

Copy link
@lukehoban

lukehoban Jul 14, 2019

Member

Why is this exported and ignored? Why not just define it locally in this file? It's only used locally.

// NOTE[pulumi-kubernetes#501]: Use `loadAll` with `JSON_SCHEMA` here instead of
// `safeLoadAll` because the latter is incompatible with `JSON_SCHEMA`. It is
// important to use `JSON_SCHEMA` here because the fields of the Kubernetes core
// API types are all tagged with `json:`, and they don't deal very well with things
// like dates.
const jsyaml = require("js-yaml");
return jsyaml.loadAll(text, undefined, {schema: jsyaml.JSON_SCHEMA});
}

/** @ignore */ export function parse(
config: ConfigGroupOpts, opts?: pulumi.CustomResourceOptions
): pulumi.Output<{[key: string]: pulumi.CustomResource}> {
Expand Down Expand Up @@ -112,9 +121,8 @@ import * as outputApi from "../types/output";
}

for (const text of yamlTexts) {
const objs = jsyaml.safeLoadAll(text);
const docResources = parseYamlDocument({
objs: objs,
objs: yamlLoadAll(text),
transformations: config.transformations,
resourcePrefix: config.resourcePrefix
},
Expand Down Expand Up @@ -2234,7 +2242,7 @@ import * as outputApi from "../types/output";
this.resources = pulumi.output(text.then(t => {
try {
return parseYamlDocument({
objs: jsyaml.safeLoadAll(t),
objs: yamlLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})
Expand Down

0 comments on commit c6d600c

Please sign in to comment.