kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
11.09k stars 2.26k forks source link

Allow custom merge keys? #5110

Open ciis0 opened 1 year ago

ciis0 commented 1 year ago

Eschewed features

What would you like to have added?

An option to add x-kubernetes-merge-key options to exiting OpenAPI resources (e.g. io.k8s.api.core.v1.ConfigMapVolumeSource)

maybe something like:

# kustomization.yaml
configurations:
- merge-keys.yml

# merge-keys.yml
mergeKeys:
- resource: io.k8s.api.core.v1.ConfigMapVolumeSource
  property: items
  key: key
  strategy: merge

Why is this needed?

Some locations in kubernetes OpenAPI are missing merge keys, some intentionally, some likely bugs:

A StatefulSets' volumeClaimTemplates are missing merge keys, so you cannot use SMP for setting the setting or overwriting the storageClass of a volumeClaimTemplate. From kubernetes PoV this is intentional as patching a STS volumeClaimTemplate is illegal, you need to recreate the STS for that, so k8s does not really have a motivation to fix it. :) (https://github.com/kubernetes/kubernetes/issues/74819, https://github.com/kubernetes-sigs/kustomize/issues/819)

ConfigMapVolumeSource's items are missing merge keys, so you cannot use a SMP for appending a key/path pair to an existing configMap volume. This likely is a bug.

Can you accomplish the motivating task without this feature, and if so, how?

Not really.

What other solutions have you considered?

JSON Patches: As both volumeClaimTemplates and configMap volume items are a list-of-maps, one would need to rely on list order.

Anything else we should know?

An example:

# sts.yml
apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: ss
spec:
  selector:
    matchLabels:
      ss: ss
  template:
    metadata:
      labels:
        ss: ss
    spec:
      volumes:
        - name: config-volume
          configMap:
            name: config
            items:
              - key: foo
                path: foo
              - key: bar
              - path: bar
      containers:
        - name: foo
          image: foo
  serviceName: sts
  volumeClaimTemplates:
    - apiVersion: v1
      kind: PersistentVolumeClaim
      metadata:
        name: pvc
      spec:
        accessModes:
          - ReadWriteOnce
        resources:
          requests:
            storage: 5Gi
# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- sts.yml

patches:
  - patch: |-
      kind: StatefulSet
      apiVersion: apps/v1
      metadata:
         name: ss
      spec:
        template:
          spec:
            volumes:
              - name: config
                configMap:
                  items:
                    - key: ll
                      path: ll
        volumeClaimTemplates:
          - apiVersion: v1
            kind: PersistentVolumeClaim
            metadata:
              name: pvc
            spec:
              storageClassName: thin

with

$ kustomize version
v5.0.1

$ kustomize build .

this results in:

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: ss
spec:
  selector:
    matchLabels:
      ss: ss
  serviceName: sts
  template:
    metadata:
      labels:
        ss: ss
    spec:
      containers:
      - image: foo
        name: foo
      volumes:
      - configMap:
          items:
          - key: ll
            path: ll
          name: config
        name: config-volume
  volumeClaimTemplates:
  - apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      name: pvc
    spec:
      storageClassName: thin

while my expected output would be

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: ss
spec:
  selector:
    matchLabels:
      ss: ss
  serviceName: sts
  template:
    metadata:
      labels:
        ss: ss
    spec:
      containers:
      - image: foo
        name: foo
      volumes:
      - configMap:
          items:
          - key: foo
            path: foo
          - key: bar
            path: bar
          - key: ll
            path: ll
          name: config
        name: config-volume
  volumeClaimTemplates:
  - apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      name: pvc
    spec:
      accessModes:
      - ReadWriteOnce
      resources:
        requests:
          storage: 5Gi
      storageClassName: thin

Feature ownership

natasha41575 commented 1 year ago

I think allowing custom merge keys easily is definitely something we want to support, but we will have to look at it holisitcally with https://github.com/kubernetes-sigs/kustomize/issues/3944 and https://github.com/kubernetes-sigs/kustomize/issues/3945.

We are currently working on a roadmap for the rest of the year and putting those two issues as one of the higher priorities. I will add a note that the design for those issues should address this one as well.

/triage accepted /priority important long-term /assign

k8s-ci-robot commented 1 year ago

@natasha41575: The label(s) priority/important, priority/long-term cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/5110#issuecomment-1502255989): >I think allowing custom merge keys easily is definitely something we want to support, but we will have to look at it holisitcally with https://github.com/kubernetes-sigs/kustomize/issues/3944 and https://github.com/kubernetes-sigs/kustomize/issues/3945. > >We are currently working on a roadmap for the rest of the year and putting those two issues as one of the higher priorities. I will add a note that the design for those issues should address this one as well. > >/triage accepted >/priority important long-term >/assign Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
ElSamhaa commented 1 year ago

FWIW, replacements don't seem to have any effect on volumeClaimTemplates too

k8s-triage-robot commented 5 days ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted