Skip to content

Commit

Permalink
Prune read-only fields from resource inputs (#2571)
Browse files Browse the repository at this point in the history
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes
<!--Give us a brief description of what you've done and what it solves.
-->

This PR improves the handling of the read-only metadata fields of
Kubernetes objects, such as `creationTimestamp` and `resourceVersion`.
The provider now ignores these fields when they appear as resource
inputs, to address a few quirks:
1. Unparseable values do not cause an API Server error during preview.
2. A subsequent refresh does not show a difference to the `__inputs`
property.
3. The behavior is now uniform for standard resource types and for
custom resource types.

An integration test is provided to show that such fields are ignored as
inputs, and are still available as outputs. Manual tests confirmed
identical behavior for custom resource objects.

### Related issues (optional)
Closes #2351

Related:
- #2511
- #2445

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
  • Loading branch information
EronWright committed Sep 21, 2023
1 parent d2a3b8c commit 550b206
Show file tree
Hide file tree
Showing 10 changed files with 164 additions and 27 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
installs based on the source distribution.
- Return mapping information for terraform conversions (https://github.com/pulumi/pulumi-kubernetes/pull/2457)
- feature: added skipUpdateUnreachable flag to proceed with the updates without failing (https://github.com/pulumi/pulumi-kubernetes/pull/2528)

- helm.v3.Release: Detect changes to local charts (https://github.com/pulumi/pulumi-kubernetes/pull/2568)
- Ignore read-only inputs in Kubernetes object metadata (https://github.com/pulumi/pulumi-kubernetes/pull/2571)

## 4.1.1 (August 23, 2023)

Expand Down
3 changes: 0 additions & 3 deletions provider/pkg/clients/unstructured.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ func Normalize(uns *unstructured.Unstructured) (*unstructured.Unstructured, erro
}
}

// Remove output-only fields
unstructured.RemoveNestedField(result.Object, "metadata", "creationTimestamp")

return result, nil
}

Expand Down
15 changes: 0 additions & 15 deletions provider/pkg/clients/unstructured_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,20 +272,6 @@ var (
},
}

secretWithCreationTimestampUnstructured = &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "v1",
"kind": "Secret",
"metadata": map[string]any{
"creationTimestamp": "2023-07-20T23:54:21Z",
"name": "foo",
},
"stringData": map[string]any{
"foo": "bar",
},
},
}

secretNormalizedUnstructured = &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "v1",
Expand Down Expand Up @@ -343,7 +329,6 @@ func TestNormalize(t *testing.T) {
{"CRD with status", args{uns: crdStatusUnstructured}, crdUnstructured, false},
{"Secret with stringData input", args{uns: secretUnstructured}, secretNormalizedUnstructured, false},
{"Secret with data input", args{uns: secretNormalizedUnstructured}, secretNormalizedUnstructured, false},
{"Secret with creationTimestamp set on input", args{uns: secretWithCreationTimestampUnstructured}, secretNormalizedUnstructured, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
27 changes: 19 additions & 8 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
oldInputs := propMapToUnstructured(olds)
newInputs := propMapToUnstructured(news)

newInputs, err = normalize(newInputs)
newInputs, err = normalizeInputs(newInputs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1561,11 +1561,11 @@ func (k *kubeProvider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*p

oldInputs, oldLive := parseCheckpointObject(oldState)

oldInputs, err = normalize(oldInputs)
oldInputs, err = normalizeInputs(oldInputs)
if err != nil {
return nil, err
}
newInputs, err = normalize(newInputs)
newInputs, err = normalizeInputs(newInputs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2006,7 +2006,7 @@ func (k *kubeProvider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*p
oldInputs := propMapToUnstructured(oldInputsPM)
_, oldLive := parseCheckpointObject(oldState)

oldInputs, err = normalize(oldInputs)
oldInputs, err = normalizeInputs(oldInputs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2238,7 +2238,7 @@ func (k *kubeProvider) Update(
return nil, pkgerrors.Wrapf(err, "update failed because malformed resource inputs")
}
newInputs := propMapToUnstructured(newResInputs)
newInputs, err = normalize(newInputs)
newInputs, err = normalizeInputs(newInputs)
if err != nil {
return nil, err
}
Expand All @@ -2264,7 +2264,7 @@ func (k *kubeProvider) Update(
// annotation during preview.
removeLastAppliedConfigurationAnnotation(oldLive, oldInputs)

oldInputs, err = normalize(oldInputs)
oldInputs, err = normalizeInputs(oldInputs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2744,17 +2744,28 @@ func shouldNormalize(uns *unstructured.Unstructured) bool {
return kinds.KnownGroupVersions.Has(uns.GetAPIVersion())
}

// normalize converts an Unstructured resource into a normalized form so that semantically equivalent representations
// normalizeInputs converts an Unstructured resource into a normalized form so that semantically equivalent representations
// are set to the same output shape. This is important to avoid generating diffs for inputs that will produce the same
// result on the cluster.
func normalize(uns *unstructured.Unstructured) (*unstructured.Unstructured, error) {
func normalizeInputs(uns *unstructured.Unstructured) (*unstructured.Unstructured, error) {
if shouldNormalize(uns) {
normalized, err := clients.Normalize(uns)
if err != nil {
return nil, err
}
uns = pruneLiveState(normalized, uns)
}

// Remove read-only fields
// see: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#objectmeta-v1-meta
unstructured.RemoveNestedField(uns.Object, "metadata", "creationTimestamp")
unstructured.RemoveNestedField(uns.Object, "metadata", "deletionGracePeriodSeconds")
unstructured.RemoveNestedField(uns.Object, "metadata", "deletionTimestamp")
unstructured.RemoveNestedField(uns.Object, "metadata", "generation")
unstructured.RemoveNestedField(uns.Object, "metadata", "managedFields")
unstructured.RemoveNestedField(uns.Object, "metadata", "resourceVersion")
unstructured.RemoveNestedField(uns.Object, "metadata", "uid")

return uns, nil
}

Expand Down
3 changes: 3 additions & 0 deletions tests/sdk/nodejs/metadata/step1/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: metadata
description: Tests object metadata handling.
runtime: nodejs
41 changes: 41 additions & 0 deletions tests/sdk/nodejs/metadata/step1/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2016-2022, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as pulumi from "@pulumi/pulumi";
import * as k8s from "@pulumi/kubernetes";

const appLabels = { app: "nginx" };
const deployment = new k8s.apps.v1.Deployment("nginx", {
metadata: {
// this program attempts to set some read-only properties
creationTimestamp: "invalid-step1",
deletionTimestamp: "invalid-step1",
resourceVersion: "invalid-step1",
uid: "invalid-step1",
// and some good properties
annotations: {
"foo": "bar"
}
},
spec: {
selector: { matchLabels: appLabels },
replicas: 1,
template: {
metadata: { labels: appLabels },
spec: { containers: [{ name: "nginx", image: "nginx" }] }
}
}
});
export const name = deployment.metadata.name;
export const resourceVersion = deployment.metadata.resourceVersion;
12 changes: 12 additions & 0 deletions tests/sdk/nodejs/metadata/step1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "metadata",
"version": "0.1.0",
"dependencies": {
"@pulumi/pulumi": "latest"
},
"devDependencies": {
},
"peerDependencies": {
"@pulumi/kubernetes": "latest"
}
}
22 changes: 22 additions & 0 deletions tests/sdk/nodejs/metadata/step1/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"compilerOptions": {
"outDir": "bin",
"target": "es6",
"module": "commonjs",
"moduleResolution": "node",
"declaration": true,
"sourceMap": true,
"stripInternal": true,
"experimentalDecorators": true,
"pretty": true,
"noFallthroughCasesInSwitch": true,
"noImplicitAny": true,
"noImplicitReturns": true,
"forceConsistentCasingInFileNames": true,
"strictNullChecks": true
},
"files": [
"index.ts"
]
}

41 changes: 41 additions & 0 deletions tests/sdk/nodejs/metadata/step2/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2016-2022, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as pulumi from "@pulumi/pulumi";
import * as k8s from "@pulumi/kubernetes";

const appLabels = { app: "nginx" };
const deployment = new k8s.apps.v1.Deployment("nginx", {
metadata: {
// step 2 has changed values that are expected to be ignored
creationTimestamp: "invalid-step2",
deletionTimestamp: "invalid-step2",
resourceVersion: "invalid-step2",
uid: "invalid-step2",
// and some good properties
annotations: {
"foo": "bar"
}
},
spec: {
selector: { matchLabels: appLabels },
replicas: 1,
template: {
metadata: { labels: appLabels },
spec: { containers: [{ name: "nginx", image: "nginx" }] }
}
}
});
export const name = deployment.metadata.name;
export const resourceVersion = deployment.metadata.resourceVersion;
25 changes: 25 additions & 0 deletions tests/sdk/nodejs/nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,31 @@ func TestQuery(t *testing.T) {
integration.ProgramTest(t, &test)
}

// TestReadonlyMetadata tests the behavior of read-only metadata fields.
func TestReadonlyMetadata(t *testing.T) {

test := baseOptions.With(integration.ProgramTestOptions{
Dir: filepath.Join("metadata", "step1"),
Quick: true,
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
// Verify that the resourceVersion (a read-only property) is available as an output.
resourceVersion, ok := stackInfo.Outputs["resourceVersion"]
assert.Truef(t, ok, "missing expected output \"resourceVersion\"")
assert.NotEmptyf(t, resourceVersion, "resourceVersion is empty")
assert.NotEqual(t, "invalid-step1", resourceVersion)
},
EditDirs: []integration.EditDir{
{
Dir: filepath.Join("metadata", "step2"),
Additive: true,
// Verify that changes to some read-only values are ignored
ExpectNoChanges: true,
},
},
})
integration.ProgramTest(t, &test)
}

func TestRenderYAML(t *testing.T) {
// Create a temporary directory to hold rendered YAML manifests.
dir, err := ioutil.TempDir("", "")
Expand Down

0 comments on commit 550b206

Please sign in to comment.