`kpt fn render` reformats causes undesired field reordering

[morgante]

When using kpt fn render, comments in my yaml files are arbitrarily relocated and disconnected from their intended location.

For example, if I run kpt fn render on this ConfigMap:

apiVersion: v1
kind: ConfigMap
metadata:
  name: setters-config
data:
  # This should be the name of your Config Controller instance
  cluster-name: cluster-name
  # This should be the project where you deployed Config Controller
  project-id: project-id
  project-number: "1234567890123"
  # You can leave these defaults
  namespace: config-control
  deployment-repo: deployment-repo
  source-repo: source-repo

It will reorganize the entire file and disconnect comments from their intended location:

apiVersion: v1
kind: ConfigMap
metadata: # kpt-merge: /setters-config
  name: setters-config
data:
  # You can leave these defaults
  namespace: config-control
  # This should be the name of your Config Controller instance
  cluster-name: cluster-name
  deployment-repo: deployment-repo
  # This should be the project where you deployed Config Controller
  project-id: project-id
  project-number: "1234567890123"
  source-repo: source-repo

Desired result: The order of fields in the data map should not be changed and comments should stay connected to the correct lines.

kpt version: v1.0-beta

[mengqiy]

If you look at the yaml lines with comment, the comments are still tied to the same yaml line after the fields reordering.

  # This should be the name of your Config Controller instance
  cluster-name: cluster-name

  # This should be the project where you deployed Config Controller
  project-id: project-id

  # You can leave these defaults
  namespace: config-control

The comments preservation works as expected. It seems what causes the problem here is field reordering by formatting yaml.

[morgante]

Right, they're still tied to the immediately following line. The problem is the whole map being rearranged.

[phanimarupaka]

Here is how formatting works. https://catalog.kpt.dev/format/v0.1/?id=functionconfig

This falls into the category of unrecognized fields and are being sorted. I will try to figure out a way to handle this, probably not sort the unrecognized fields and retain their original order. Need to see if there is any downside due to that approach.

How big of a problem is it ? If you run render again, the order is not changed correct ? I see the problem with this comment though # You can leave these defaults. You want the order to be preserved for this. Is there a work around?

[morgante]

How big of a problem is it ? If you run render again, the order is not changed correct ? I see the problem with this comment though # You can leave these defaults. You want the order to be preserved for this. Is there a work around?

It's not a blocker, but meaningful friction if we're trying to encourage in-place hydration. I'm not aware of any workaround where my goal is preserved (putting the defaultable values at the bottom).

[bzub]

https://github.com/GoogleContainerTools/kpt/issues/2301

[stefanhenseler]

@phanimarupaka I think this is really something we need to look at. Our users complain about this as well. This behavior is really odd. Kustomizations suddenly look like this:

namespace: helix-pomerium
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
metadata:
  name: kustomization
  annotations:
    config.kubernetes.io/local-config: 'true'
resources:
  - ../upstream

Especially if you have a preexisting base and suddenly, after running the setter function, you got a hundred changes and the comments are all over the place. I agree, it's not as bad if you recreate a package from scratch and the ordering is already there. I think the behavior should be similar to the old kpt set command if that's possible.

[phanimarupaka]

@stefanhenseler This is being actively worked on. Here is the PR for reference. https://github.com/kubernetes-sigs/kustomize/pull/4021

We will include it in the next kpt release.

[stefanhenseler]

Ah ok thanks, I think this will solve it for us.

[phanimarupaka]

The PR to fix this issue has been merged and the fix will be available in beta.2 release of kpt.