fix(update): preserve Kptfile formatting during upgrades by Jaisheesh-2006 · Pull Request #4427 · kptdev/kpt

Jaisheesh-2006 · 2026-03-06T07:09:31Z

Description

This PR fixes Kptfile formatting drift during package upgrades by switching upgrade-time Kptfile handling to an SDK-backed read/update flow and removing legacy rewrite logic from kptops helpers.

Motivation

During kpt pkg update, Kptfile rewrites could alter user formatting/comments in ways that were not intended.
Issue #4306 tracks this regression and expects upgrade behavior to preserve user-authored Kptfile structure as much as possible.

What Changed

Refactored upgrade-path Kptfile read/update operations to use SDK-backed Kptfile handling in kptfileutil.
Removed legacy custom Kptfile rewrite behavior in pkg/lib/kptops helper flows, reusing the centralized updater.
Hardened tests for cross-platform consistency:
- line ending normalization where needed
- environment-aware symlink assertions
- Kptfile-focused preservation coverage

Scope

This change targets upgrade and related Kptfile mutation paths only, with no intended functional change to unrelated package/resource update behavior.

Testing

Validated with targeted and package-level tests, including:

go test ./pkg/kptfile/... -count=1
go test ./pkg/lib/kptops -count=1
go test ./commands/pkg/update -count=1
go test ./internal/util/update/... -count=1

Issue

Fixes #4306

netlify · 2026-03-06T07:09:36Z

✅ Deploy Preview for kptdocs ready!

Name	Link
🔨 Latest commit	`a4335c5`
🔍 Latest deploy log	https://app.netlify.com/projects/kptdocs/deploys/69aaa59a1662410008b46324
😎 Deploy Preview	https://deploy-preview-4427--kptdocs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR addresses Kptfile formatting/comment drift during kpt pkg update by switching upgrade-time Kptfile mutations to an SDK-backed read/modify/write flow intended to preserve YAML structure and comments.

Changes:

Refactors kptops Kptfile update helpers to use kptfileutil.UpdateKptfileContent rather than re-marshalling via kyaml.
Adds SDK-based Kptfile write/update logic in kptfileutil to better preserve formatting/comments.
Hardens tests for cross-platform behavior (CRLF normalization, environment-aware symlink assertions) and adds Kptfile preservation coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/lib/kptops/render_test.go	Normalizes CRLF/LF in golden comparisons to reduce cross-platform diffs.
pkg/lib/kptops/clone_test.go	Adds tests asserting UpdateUpstream/UpdateName preserve comments/formatting.
pkg/lib/kptops/clone.go	Routes Kptfile mutations through `kptfileutil.UpdateKptfileContent` to preserve formatting.
pkg/kptfile/kptfileutil/util_test.go	Adds tests for comment/format preservation during `UpdateKptfile`.
pkg/kptfile/kptfileutil/util.go	Introduces SDK-based Kptfile write/update utilities and updates decode flow.
internal/util/get/get_test.go	Makes symlink assertions conditional on whether symlinks materialize in the test environment.
internal/builtins/pkg_context_test.go	Normalizes CRLF/LF for deterministic output comparison.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T07:14:06Z

internal/builtins/pkg_context_test.go

+			expected := strings.ReplaceAll(string(exp), "\r\n", "\n")
+			actual := strings.ReplaceAll(out.String(), "\r\n", "\n")
+			if diff := cmp.Diff(expected, actual); diff != "" {
 				t.Errorf("pkg context mistmach (-want +got):\n%s", diff)


Typo in test failure message: "pkg context mistmach" should be "pkg context mismatch".

Suggested change

t.Errorf("pkg context mistmach (-want +got):\n%s", diff)

t.Errorf("pkg context mismatch (-want +got):\n%s", diff)

Copilot · 2026-03-06T07:14:07Z

internal/util/get/get_test.go

+		assert.NotContains(t, cliOutput.String(), `[Warn] Ignoring symlink "config-symlink"`)
+	}

 	// verify the cloned contents do not contains symlinks


Grammar in comment: "verify the cloned contents do not contains symlinks" -> "do not contain symlinks".

Suggested change

// verify the cloned contents do not contains symlinks

// verify the cloned contents do not contain symlinks

Copilot · 2026-03-06T07:14:07Z

pkg/kptfile/kptfileutil/util_test.go

+    directory: package
+    ref: v1.0.0 # keep ref inline comment
+
+upstreamLock:
+  type: git


This test asserts "PreservesExactFormatting", but the input includes a blank line before upstreamLock: (line 754) while the expected output removes that blank line. Either keep the blank line in the expected output to truly test exact formatting, or rename/relax the test to only validate comment/field preservation.

Copilot · 2026-03-06T07:14:07Z

pkg/kptfile/kptfileutil/util.go

+	typedKptfile := &kptfilev1.KptFile{}
+	if err := sdkKptfile.Obj.As(typedKptfile); err != nil {
+		return "", err
+	}
+
+	mutator(typedKptfile)
+
+	if err := applyTypedKptfileToSDK(sdkKptfile, typedKptfile); err != nil {
+		return "", err


UpdateKptfileContent calls sdkKptfile.Obj.As(typedKptfile) but (unlike DecodeKptfile) does not strip SDK-internal annotations (config.kubernetes.io/index, internal.config.kubernetes.io/*). Those keys can then be written back into the user's Kptfile via applyTypedKptfileToSDK, leaking internal metadata and/or creating an empty annotations map. Consider filtering these keys (and nil-ing annotations when empty) before applying/writing.

Copilot · 2026-03-06T07:14:07Z

pkg/kptfile/kptfileutil/util.go

+	if val == nil || reflect.ValueOf(val).IsZero() {
+		_, err := obj.RemoveNestedField(fields...)
+		return err
+	}
+	return obj.SetNestedField(val, fields...)
+}
+


setOrRemoveNestedField removes the field only when val is nil or reflect.ValueOf(val).IsZero(). For non-nil but empty maps/slices, IsZero is false, so this can write empty objects (e.g. annotations: {}) and introduce structural/formatting drift. Consider treating empty maps/slices as absent and removing the nested field instead.

Suggested change

if val == nil || reflect.ValueOf(val).IsZero() {

_, err := obj.RemoveNestedField(fields...)

return err

}

return obj.SetNestedField(val, fields...)

}

if isEmptyValue(val) {

_, err := obj.RemoveNestedField(fields...)

return err

}

return obj.SetNestedField(val, fields...)

}

func isEmptyValue(val any) bool {

if val == nil {

return true

}

v := reflect.ValueOf(val)

if !v.IsValid() {

return true

}

// Treat nil pointers as empty.

if v.Kind() == reflect.Ptr {

if v.IsNil() {

return true

}

v = v.Elem()

}

// Treat empty maps, slices, and arrays as empty.

switch v.Kind() {

case reflect.Map, reflect.Slice, reflect.Array:

if v.Len() == 0 {

return true

}

}

return v.IsZero()

}

Use SDK-backed Kptfile read/update flow in upgrade paths, remove legacy rewrite behavior from kptops helpers, and harden tests for cross-platform stability. Fixes kptdev#4306 Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T10:05:19Z

pkg/kptfile/kptfileutil/util.go

+	typedKptfile := &kptfilev1.KptFile{}
+	if err := sdkKptfile.Obj.As(typedKptfile); err != nil {
+		return "", err
+	}
+
+	mutator(typedKptfile)
+
+	if err := applyTypedKptfileToSDK(sdkKptfile, typedKptfile); err != nil {
+		return "", err
+	}


UpdateKptfileContent reads a Kptfile via the SDK (Obj.As) but does not strip the SDK-internal metadata annotations (config.kubernetes.io/index, internal.config.kubernetes.io/*) the way DecodeKptfile does. If those keys are present after As, applyTypedKptfileToSDK will write them back out, causing internal annotations to leak into user Kptfiles. Consider deleting sdkInternalKptfileAnnotations (and nil-ing the map when empty) on typedKptfile before applying the mutator / writing.

Copilot · 2026-03-06T10:05:19Z

pkg/kptfile/kptfileutil/util.go

+func UpdateKptfileContent(content string, mutator func(*kptfilev1.KptFile)) (string, error) {
+	resources := map[string]string{kptfilev1.KptFileName: content}
+	sdkKptfile, err := fn.NewKptfileFromPackage(resources)
+	if err != nil {
+		return "", err
+	}


UpdateKptfileContent bypasses the version/Kind validation done by DecodeKptfile (checkKptfileVersion + KnownFields decode). Since clone/update paths previously went through DecodeKptfile, this can change behavior by allowing updates to deprecated/unknown Kptfile versions (or structurally invalid content) instead of failing early. Consider reusing checkKptfileVersion (and, if needed, the KnownFields validation) before calling fn.NewKptfileFromPackage to keep error behavior consistent.

Jaisheesh-2006 requested review from efiacor, kispaljr, liamfallon and mozesl-nokia as code owners March 6, 2026 07:09

Copilot AI review requested due to automatic review settings March 6, 2026 07:09

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working go Pull requests that update Go code labels Mar 6, 2026

Copilot started reviewing on behalf of Jaisheesh-2006 March 6, 2026 07:09 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Jaisheesh-2006 force-pushed the fix/4306-prevent-kptfile-upgrade branch from e6f980d to ffd9c47 Compare March 6, 2026 07:52

fix(update): preserve Kptfile formatting during upgrades

a4335c5

Use SDK-backed Kptfile read/update flow in upgrade paths, remove legacy rewrite behavior from kptops helpers, and harden tests for cross-platform stability. Fixes kptdev#4306 Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Copilot AI review requested due to automatic review settings March 6, 2026 09:59

Jaisheesh-2006 force-pushed the fix/4306-prevent-kptfile-upgrade branch from ffd9c47 to a4335c5 Compare March 6, 2026 09:59

Copilot started reviewing on behalf of Jaisheesh-2006 March 6, 2026 10:00 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(update): preserve Kptfile formatting during upgrades#4427

fix(update): preserve Kptfile formatting during upgrades#4427
Jaisheesh-2006 wants to merge 1 commit intokptdev:mainfrom
Jaisheesh-2006:fix/4306-prevent-kptfile-upgrade

Jaisheesh-2006 commented Mar 6, 2026

Uh oh!

netlify bot commented Mar 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	t.Errorf("pkg context mistmach (-want +got):\n%s", diff)
	t.Errorf("pkg context mismatch (-want +got):\n%s", diff)

	// verify the cloned contents do not contains symlinks
	// verify the cloned contents do not contain symlinks

-	if val == nil || reflect.ValueOf(val).IsZero() {
-		_, err := obj.RemoveNestedField(fields...)
-		return err
-	}
-	return obj.SetNestedField(val, fields...)
-}
+	if isEmptyValue(val) {
+		_, err := obj.RemoveNestedField(fields...)
+		return err
+	}
+	return obj.SetNestedField(val, fields...)
+}
+func isEmptyValue(val any) bool {
+	if val == nil {
+		return true
+	}
+	v := reflect.ValueOf(val)
+	if !v.IsValid() {
+		return true
+	}
+	// Treat nil pointers as empty.
+	if v.Kind() == reflect.Ptr {
+		if v.IsNil() {
+			return true
+		}
+		v = v.Elem()
+	}
+	// Treat empty maps, slices, and arrays as empty.
+	switch v.Kind() {
+	case reflect.Map, reflect.Slice, reflect.Array:
+		if v.Len() == 0 {
+			return true
+		}
+	}
+	return v.IsZero()
+}

Conversation

Jaisheesh-2006 commented Mar 6, 2026

Description

Motivation

What Changed

Scope

Testing

Issue

Uh oh!

netlify bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kptdocs ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify bot commented Mar 6, 2026 •

edited

Loading