gcr.io/kpt-fn/set-namespace deletes comments in file

[howardjohn]

Start with file:

# This object and its stored inventory is subsequently
# used to calculate the set of objects to automatically
# delete (prune), when an object is omitted from further
# applies. When applied, this "inventory object" is also
# used to identify the entire set of objects to delete.
#
# NOTE: The name of this inventory template file
# does NOT have any impact on group-related functionality
# such as deletion or pruning.
#
apiVersion: v1 # NOTE: auto-generated. Some fields should NOT be modified.
kind: ConfigMap
metadata:
  # DANGER: Do not change the inventory object namespace.
  # Changing the namespace will cause a loss of continuity
  # with previously applied grouped objects. Set deletion
  # and pruning functionality will be impaired.
  namespace: metallb-system
  # NOTE: The name of the inventory object does NOT have
  # any impact on group-related functionality such as
  # deletion or pruning.
  name: inventory-82029660
  labels:
    # DANGER: Do not change the value of this label.
    # Changing this value will cause a loss of continuity
    # with previously applied grouped objects. Set deletion
    # and pruning functionality will be impaired.
    cli-utils.sigs.k8s.io/inventory-id: e947bb0e-56f3-450c-96ed-8c5ecff10017

Run:
kpt fn run --image gcr.io/kpt-functions/set-namespace .

Results:

diff --git a/inventory-template.yaml b/inventory-template.yaml
index f40ca14..398c152 100644
--- a/inventory-template.yaml
+++ b/inventory-template.yaml
@@ -1,5 +1,3 @@
-# This object and its stored inventory is subsequently
-# used to calculate the set of objects to automatically
 # delete (prune), when an object is omitted from further
 # applies. When applied, this "inventory object" is also
 # used to identify the entire set of objects to delete.

Run again:

diff --git a/inventory-template.yaml b/inventory-template.yaml
index f40ca14..28e9ded 100644
--- a/inventory-template.yaml
+++ b/inventory-template.yaml
@@ -1,7 +1,3 @@
-# This object and its stored inventory is subsequently
-# used to calculate the set of objects to automatically
-# delete (prune), when an object is omitted from further
-# applies. When applied, this "inventory object" is also
 # used to identify the entire set of objects to delete.
 #
 # NOTE: The name of this inventory template file

kpt version is 0.39.2`.

I am not sure if this is a bug with set-namespaces or kpt in general.

[mikebz]

@mengqiy can you please take a look?

[Shell32-Natsu]

Looks like function gcr.io/kpt-functions/set-namespace is old set-namespace function.

[howardjohn]

I am not 100% sure but with some brief testing it seems like it may happen with any function, or at least another function (https://github.com/mgoltzsche/khelm) has the same issue.

[mengqiy]

I think this has been fixed in GoogleContainerTools/kpt-functions-catalog#222.
@howardjohn Can you please try to use image gcr.io/kpt-fn/set-namespace:v0.1 which is our latest image

[howardjohn]

gcr.io/kpt-fn/set-namespace:v0.1 seems to have an even more critical issue - it deletes all my files with config.kubernetes.io/local-config: "true" configs (including the file actually declaratively configuring set-namespace to run!).

It also seems a lot more destructive, completely re-ordering the YAML which has the side effect of massive diffs and some removed comments (when the comment is on the same line as yaml, like a: b # comment)

[mengqiy]

it deletes all my files with config.kubernetes.io/local-config: "true" configs (including the file actually declaratively configuring set-namespace to run!).

A work around is separate the sources and fn configs by directories. And then use kpt fn run --fn-path fn-config/ resources/.

The docs related to config.kubernetes.io/local-config:

• https://github.com/kubernetes-sigs/kustomize/blob/master/cmd/config/docs/api-conventions/config-io.md#configkubernetesiolocal-config
• https://github.com/GoogleContainerTools/kpt/releases/tag/v0.34.0

It's not very clear to me what is the desired behavior for kpt fn and functions when dealing with config.kubernetes.io/local-config: "true". @droot @frankfarzan @Shell32-Natsu Can you someone clarify it?

If the function config object is in items of the input resourceList, gcr.io/kpt-fn/set-namespace:v0.1 currently will NOT output it in items in the output resourceList.
Current behavior is not a problem for the new kpt fn UX, but it can be a problem for existing kpt (v0.39.x) when the resources and fn configs are not separated by directories.

@frankfarzan We should standardize if functions should drop its fn config from resourceList.items. WDYT?

completely re-ordering the YAML which has the side effect of massive diffs and some removed comments (when the comment is on the same line as yaml, like a: b # comment)

This formatting issue may happen. We are working on a "format" function in GoogleContainerTools/kpt-functions-catalog#242. We will recommend our users to always use it as the last one in the mutators.

some removed comments (when the comment is on the same line as yaml, like a: b # comment)

@howardjohn Can you please provide steps to reproduce this? e.g. example resources.

[mikebz]

pipeline-validate example is a good repro of disappearing files:

~/kpt/package-examples/pipeline-validate » kpt fn render .

[mengqiy]

We have found the root cause of why pipeline-validate package is not working as expected:
#1933

[mengqiy]

I didn't find any existing functions deleting comments in the catalog. I created some test cases to ensure that in GoogleContainerTools/kpt-functions-catalog#324

[mikebz]

thanks for taking a look and adding tests to make sure we don't have these problems in the future!

[mengqiy]

Currently all of our mutator functions are written in golang. They works fine.
But it appears there is an issue when a mutator function is written with the TS SDK. It is tracked in #2069

[mengqiy]

The work about functions should not drop resources with local-config is tracked in #1933

[mengqiy]

kpt copies comments from input to output after running the functions. However, we found some bugs and they are being fixed in PR kubernetes-sigs/kustomize#3965.
We will need to upgrade kpt's kyaml dependency to pick up the fix.

[mengqiy]

With #2169 merged, we can close this.