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.

Sign up for GitHub

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

Allow customizing ResourceOptions in transformations #575

Merged
merged 4 commits into from
May 30, 2019
Merged

Allow customizing ResourceOptions in transformations #575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d2db36
Allow customizing ResourceOptions in transformations
May 29, 2019
8d86560
Add CHANGELOG
May 29, 2019
4268276
Add Python support
May 30, 2019
1f48d12
Update CHANGELOG
May 30, 2019
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
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

### Major changes

- None
- BREAKING: Change the recently added `transformations` callback in Python to match JavaScript API (https://github.com/pulumi/pulumi-kubernetes/pull/575)

### Improvements

- None
- Enable configuring `ResourceOptions` via `transformations` (https://github.com/pulumi/pulumi-kubernetes/pull/575).

### Bug fixes

Expand Down
15 changes: 9 additions & 6 deletions pkg/gen/nodejs-templates/yaml.ts.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

export interface ConfigFileOpts {
Expand All @@ -34,7 +34,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

export interface ConfigOpts {
Expand All @@ -45,7 +45,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

/** @ignore */ export function parse(
Expand Down Expand Up @@ -233,15 +233,18 @@ import * as outputApi from "../types/output";
}

/** @ignore */ function parseYamlObject(
obj: any, transformations?: ((o: any) => void)[], opts?: pulumi.CustomResourceOptions,
obj: any, transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[], opts?: pulumi.CustomResourceOptions,
): pulumi.Output<{name: string, resource: pulumi.CustomResource}>[] {
if (obj == null || Object.keys(obj).length == 0) {
return [];
}

// Allow users to change API objects before any validation.
// 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.
for (const t of transformations || []) {
t(obj);
t(obj, opts);
}

if (!("kind" in obj && "apiVersion" in obj)) {
Expand Down
14 changes: 12 additions & 2 deletions pkg/gen/python-templates/yaml.py.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
# *** Do not edit by hand unless you're certain you know what you are doing! ***

import json
from copy import deepcopy
from copy import copy, deepcopy
from inspect import getargspec
from typing import Callable, Dict, List, Optional

import pulumi.runtime
Expand Down Expand Up @@ -109,10 +110,19 @@ def _parse_yaml_object(obj, opts: Optional[pulumi.ResourceOptions] = None,
if not obj:
return []

# Create a copy of opts to pass into potentially mutating transforms that will be applied to this resource.
if opts is not None:
opts = copy(opts)
else:
opts = {}

# Allow users to change API objects before any validation.
if transformations is not None:
for t in transformations:
obj = t(obj)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW - note that this code previous did obj = t(obj) which is not the same semantics as JavaScript, and is possibly quite surprising. It's also not compatible with the change we want to make here.

I went ahead and changed it, though this is a breaking change. Curious if there's feedback on whether to take the breaking change to align these, or to adjust the PR somehow else to avoid it?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'd go with the breaking change. It was unintentional behavior introduced when I ported from the Node SDK, and isn't widely used yet. We had at least one user mention that it was surprising, so I'd been intending to fix it anyway.

if len(getargspec(t)[0]) == 2:
t(obj, opts)
else:
t(obj)

if "kind" not in obj or "apiVersion" not in obj:
raise Exception("Kubernetes resources require a kind and apiVersion: {}".format(json.dumps(obj)))
Expand Down
4 changes: 2 additions & 2 deletions sdk/nodejs/helm/v2/helm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ interface BaseChartOpts {
* Optional array of transformations to apply to resources that will be created by this chart prior to
* creation. Allows customization of the chart behaviour without directly modifying the chart itself.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

export interface ChartOpts extends BaseChartOpts {
Expand Down Expand Up @@ -179,7 +179,7 @@ export class Chart extends yaml.CollectionComponentResource {

parseTemplate(
yamlStream: string,
transformations: ((o: any) => void)[] | undefined,
transformations: ((o: any, opts: pulumi.CustomResourceOptions) => void)[] | undefined,
dependsOn: pulumi.Resource[]
): pulumi.Output<{ [key: string]: pulumi.CustomResource }> {
// NOTE: We must manually split the YAML stream because of js-yaml#456. Perusing the
Expand Down
15 changes: 9 additions & 6 deletions sdk/nodejs/yaml/yaml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

export interface ConfigFileOpts {
Expand All @@ -34,7 +34,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

export interface ConfigOpts {
Expand All @@ -45,7 +45,7 @@ import * as outputApi from "../types/output";
* A set of transformations to apply to Kubernetes resource definitions before registering
* with engine.
*/
transformations?: ((o: any) => void)[];
transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[];
}

/** @ignore */ export function parse(
Expand Down Expand Up @@ -2203,15 +2203,18 @@ import * as outputApi from "../types/output";
}

/** @ignore */ function parseYamlObject(
obj: any, transformations?: ((o: any) => void)[], opts?: pulumi.CustomResourceOptions,
obj: any, transformations?: ((o: any, opts: pulumi.CustomResourceOptions) => void)[], opts?: pulumi.CustomResourceOptions,
): pulumi.Output<{name: string, resource: pulumi.CustomResource}>[] {
if (obj == null || Object.keys(obj).length == 0) {
return [];
}

// Allow users to change API objects before any validation.
// 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.
for (const t of transformations || []) {
t(obj);
t(obj, opts);
}

if (!("kind" in obj && "apiVersion" in obj)) {
Expand Down
14 changes: 12 additions & 2 deletions sdk/python/pulumi_kubernetes/yaml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
# *** Do not edit by hand unless you're certain you know what you are doing! ***

import json
from copy import deepcopy
from copy import copy, deepcopy
from inspect import getargspec
from typing import Callable, Dict, List, Optional

import pulumi.runtime
Expand Down Expand Up @@ -144,10 +145,19 @@ def _parse_yaml_object(obj, opts: Optional[pulumi.ResourceOptions] = None,
if not obj:
return []

# Create a copy of opts to pass into potentially mutating transforms that will be applied to this resource.
if opts is not None:
opts = copy(opts)
else:
opts = {}

# Allow users to change API objects before any validation.
if transformations is not None:
for t in transformations:
obj = t(obj)
if len(getargspec(t)[0]) == 2:
t(obj, opts)
else:
t(obj)

if "kind" not in obj or "apiVersion" not in obj:
raise Exception("Kubernetes resources require a kind and apiVersion: {}".format(json.dumps(obj)))
Expand Down
15 changes: 15 additions & 0 deletions tests/examples/examples_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,21 @@ func TestExamples(t *testing.T) {
base.With(integration.ProgramTestOptions{
SkipRefresh: true, // Deployment controller changes object out-of-band.
Dir: path.Join(cwd, "helm"),
Verbose: true,
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
// Ensure that all `Services` have `status` marked as a `Secret`
for _, res := range stackInfo.Deployment.Resources {
if res.Type == tokens.Type("kubernetes:core/v1:Service") {
spec, has := res.Outputs["status"]
assert.True(t, has)
specMap, is := spec.(map[string]interface{})
assert.True(t, is)
sigKey, has := specMap[resource.SigKey]
assert.True(t, has)
assert.Equal(t, resource.SecretSig, sigKey)
}
}
},
}),

base.With(integration.ProgramTestOptions{
Expand Down
5 changes: 5 additions & 0 deletions tests/examples/helm/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ const nginx = new k8s.helm.v2.Chart("simple-nginx", {
} else {
obj.metadata = {namespace: namespaceName}
}
},
(obj: any, opts: pulumi.CustomResourceOptions) => {
if (obj.kind == "Service" && obj.apiVersion == "v1") {
opts.additionalSecretOutputs = ["status"];
}
}
]
});
Expand Down
14 changes: 14 additions & 0 deletions tests/examples/python/python_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,20 @@ func TestYaml(t *testing.T) {
// Verify root resource.
stackRes := stackInfo.Deployment.Resources[15]
assert.Equal(t, resource.RootStackType, stackRes.URN.Type())

// TODO[pulumi/pulumi#2782] Testing of secrets blocked on a bug in Python support for secrets.
// // Ensure that all `Pod` have `status` marked as a `Secret`
// for _, res := range stackInfo.Deployment.Resources {
// if res.Type == tokens.Type("kubernetes:core/v1:Pod") {
// spec, has := res.Outputs["apiVersion"]
// assert.True(t, has)
// specMap, is := spec.(map[string]interface{})
// assert.True(t, is)
// sigKey, has := specMap[resource.SigKey]
// assert.True(t, has)
// assert.Equal(t, resource.SecretSig, sigKey)
// }
// }
},
})
integration.ProgramTest(t, &options)
Expand Down
11 changes: 8 additions & 3 deletions tests/examples/python/yaml-test/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ def add_namespace(obj):
else:
obj["metadata"] = {"namespace": ns.metadata["name"]}

return obj

def secret_status(obj, opts):
if obj["kind"] == "Pod" and obj["apiVersion"] == "v1":
opts.additional_secret_outputs = ["apiVersion"]

cf_local = ConfigFile(
"yaml-test",
"manifest.yaml",
transformations=[add_namespace],
transformations=[
add_namespace,
# TODO[pulumi/pulumi#2782] Testing of secrets blocked on a bug in Python support for secrets.
# secret_status,
],
)

# Create resources from standard Kubernetes guestbook YAML example in the test namespace.
Expand Down