Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various usability problems in yaml.* #638

Merged
merged 5 commits into from
Jul 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

### Bug fixes

- Allow `yaml.ConfigGroup` to take URLs as argument
(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).
- Don't render emoji on Windows. (https://github.com/pulumi/pulumi-kubernetes/pull/634)
- Emit a useful error message (rather than a useless one) if we fail to parse the YAML data in
`kubernetes:config:kubeconfig` (https://github.com/pulumi/pulumi-kubernetes/pull/636).
Expand Down
58 changes: 45 additions & 13 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;
}

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");
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 All @@ -74,10 +83,18 @@ import * as outputApi from "../types/output";
if (config.files !== undefined) {
let files: string[] = [];
if (typeof config.files === 'string') {
files = glob.sync(config.files);
if (isUrl(config.files)) {
files = [config.files];
} else {
files = glob.sync(config.files);
}
} else {
for (const file of config.files) {
files.push(...glob.sync(file));
if (isUrl(file)) {
files.push(file);
} else {
files.push(...glob.sync(file));
}
}
}

Expand All @@ -104,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 @@ -235,20 +251,36 @@ import * as outputApi from "../types/output";
super("kubernetes:yaml:ConfigFile", name, config, opts);
const fileId = config && config.file || name;
let text: Promise<string>;
if (fileId.startsWith("http://") || fileId.startsWith("https://")) {
text = fetch(fileId).then(r => r.text())
if (isUrl(fileId)) {
text = fetch(fileId).then(r => {
if (r.ok) {
return r.text()
} else {
throw Error(`Error fetching YAML file '${fileId}': ${r.status} ${r.statusText}`);
}
});
} else {
text = Promise.resolve(fs.readFileSync(fileId).toString());
}

this.resources = pulumi.output(text.then(t => parseYamlDocument({
objs: jsyaml.safeLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})));
this.resources = pulumi.output(text.then(t => {
try {
return parseYamlDocument({
objs: yamlLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})
} catch (e) {
throw Error(`Error fetching YAML file '${fileId}': ${e}`);
}
}));
}
}

/** @ignore */ function isUrl(s: string): boolean {
return s.startsWith("http://") || s.startsWith("https://")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be instead attempt to parse a URL with https://nodejs.org/api/url.html#url_class_url and return true if parsing succeeds? This would allow us to handle file URLs, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I do see that what you've written matches the prior behavior of ConfigFile, so no urgency here)

}

/** @ignore */ function parseYamlDocument(
config: ConfigOpts,
opts?: pulumi.CustomResourceOptions,
Expand Down Expand Up @@ -285,7 +317,7 @@ import * as outputApi from "../types/output";
// Create a copy of opts to pass into potentially mutating transforms that will be applied to this resource.
opts = Object.assign({}, opts);

// Allow users to change API objects before any validation.
// Allow users to change API objects before any validation.
for (const t of transformations || []) {
t(obj, opts);
}
Expand Down
58 changes: 45 additions & 13 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;
}

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");
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 All @@ -74,10 +83,18 @@ import * as outputApi from "../types/output";
if (config.files !== undefined) {
let files: string[] = [];
if (typeof config.files === 'string') {
files = glob.sync(config.files);
if (isUrl(config.files)) {
files = [config.files];
} else {
files = glob.sync(config.files);
}
} else {
for (const file of config.files) {
files.push(...glob.sync(file));
if (isUrl(file)) {
files.push(file);
} else {
files.push(...glob.sync(file));
}
}
}

Expand All @@ -104,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 @@ -2211,20 +2227,36 @@ import * as outputApi from "../types/output";
super("kubernetes:yaml:ConfigFile", name, config, opts);
const fileId = config && config.file || name;
let text: Promise<string>;
if (fileId.startsWith("http://") || fileId.startsWith("https://")) {
text = fetch(fileId).then(r => r.text())
if (isUrl(fileId)) {
text = fetch(fileId).then(r => {
if (r.ok) {
return r.text()
} else {
throw Error(`Error fetching YAML file '${fileId}': ${r.status} ${r.statusText}`);
}
});
} else {
text = Promise.resolve(fs.readFileSync(fileId).toString());
}

this.resources = pulumi.output(text.then(t => parseYamlDocument({
objs: jsyaml.safeLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})));
this.resources = pulumi.output(text.then(t => {
try {
return parseYamlDocument({
objs: yamlLoadAll(t),
transformations: config && config.transformations || [],
resourcePrefix: config && config.resourcePrefix || undefined
}, {parent: this})
} catch (e) {
throw Error(`Error fetching YAML file '${fileId}': ${e}`);
}
}));
}
}

/** @ignore */ function isUrl(s: string): boolean {
return s.startsWith("http://") || s.startsWith("https://")
}

/** @ignore */ function parseYamlDocument(
config: ConfigOpts,
opts?: pulumi.CustomResourceOptions,
Expand Down Expand Up @@ -2261,7 +2293,7 @@ import * as outputApi from "../types/output";
// Create a copy of opts to pass into potentially mutating transforms that will be applied to this resource.
opts = Object.assign({}, opts);

// Allow users to change API objects before any validation.
// Allow users to change API objects before any validation.
for (const t of transformations || []) {
t(obj, opts);
}
Expand Down
13 changes: 13 additions & 0 deletions tests/integration/istio/step1/charts/istio-init/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: v1
appVersion: 1.1.0
description: Helm chart to initialize Istio CRDs
engine: gotpl
icon: https://istio.io/favicons/android-192x192.png
keywords:
- istio
- crd
name: istio-init
sources:
- http://github.com/istio/istio
tillerVersion: '>=2.7.2-0'
version: 1.1.0
77 changes: 77 additions & 0 deletions tests/integration/istio/step1/charts/istio-init/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Istio

[Istio](https://istio.io/) is an open platform for providing a uniform way to integrate microservices, manage traffic flow across microservices, enforce policies and aggregate telemetry data.

## Introduction

This chart bootstraps Istio's [CRDs](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#customresourcedefinitions)
which are an internal implementation detail of Istio. CRDs define data structures for storing runtime configuration
specified by a human operator.

This chart must be run to completion prior to running other Istio charts, or other Istio charts will fail to initialize.

## Prerequisites

- Kubernetes 1.9 or newer cluster with RBAC (Role-Based Access Control) enabled is required
- Helm 2.7.2 or newer or alternately the ability to modify RBAC rules is also required

## Resources Required

The chart deploys pods that consume minimal resources.

## Installing the Chart

1. If a service account has not already been installed for Tiller, install one:
```
$ kubectl apply -f install/kubernetes/helm/helm-service-account.yaml
```

1. If Tiller has not already been installed in your cluster, Install Tiller on your cluster with the service account:
```
$ helm init --service-account tiller
```

1. Install the Istio initializer chart:
```
$ helm install install/kubernetes/helm/istio-init --name istio-init --namespace istio-system
```

> Although you can install the `istio-init` chart to any namespace, it is recommended to install `istio-init` in the same namespace(`istio-system`) as other Istio charts.

## Configuration

The Helm chart ships with reasonable defaults. There may be circumstances in which defaults require overrides.
To override Helm values, use `--set key=value` argument during the `helm install` command. Multiple `--set` operations may be used in the same Helm operation.

Helm charts expose configuration options which are currently in alpha. The currently exposed options are explained in the following table:

| Parameter | Description | Values | Default |
| --- | --- | --- | --- |
| `global.hub` | Specifies the HUB for most images used by Istio | registry/namespace | `docker.io/istio` |
| `global.tag` | Specifies the TAG for most images used by Istio | valid image tag | `0.8.latest` |
| `global.imagePullPolicy` | Specifies the image pull policy | valid image pull policy | `IfNotPresent` |


## Uninstalling the Chart

> Uninstalling this chart does not delete Istio's registered CRDs. Istio by design expects
> CRDs to leak into the Kubernetes environment. As CRDs contain all runtime configuration
> data in CutomResources the Istio designers feel it is better to explicitly delete this
> configuration rather then unexpectedly lose it.

To uninstall/delete the `istio-init` release but continue to track the release:
```
$ helm delete istio-init
```

To uninstall/delete the `istio-init` release completely and make its name free for later use:
```
$ helm delete istio-init --purge
```

> Warning: Deleting CRDs will delete any configuration that you have made to Istio.

To delete all CRDs, run the following command
```
$ for i in istio-init/files/*crd*yaml; do kubectl delete -f $i; done
```
Loading