siderolabs / talos

Talos Linux is a modern Linux distribution built for Kubernetes.
https://www.talos.dev
Mozilla Public License 2.0
6.71k stars 537 forks source link

talosctl machineconfig patch produces invalid result #7084

Closed KrystianMarek closed 1 year ago

KrystianMarek commented 1 year ago

Bug Report

talosctl machineconfig patch produces invalid result

Description

Steps to reproduce:

talosctl gen config [CLUSTER_NAME] https://[CLUSTER_VIP]:6443 

^ produces worker.yaml

talosctl machineconfig patch worker.yaml --patch @worker-patches.yaml -o worker-capi.yaml

worker-patches.yaml:

- op: add
  path: /cluster
  value:
    inlineManifests: []
- op: add
  path: /cluster/inlineManifests/0
  value:
    contents: |
      apiVersion: v1
      kind: Namespace
      metadata:
        labels:
          pod-security.kubernetes.io/enforce: privileged
          pod-security.kubernetes.io/audit: privileged
          pod-security.kubernetes.io/warn: privileged
        name: network-services
    name: network-services-namespace
- op: add
  path: /cluster/inlineManifests/1
  value:
    contents: |
      CILIUM_MANIFEST
    name: network-services-dependencies
- op: replace
  path: /cluster/controlPlane
  value:
    endpoint: https://CLUSTER_VIP:6443
- op: add
  path: /cluster/network
  value:
    cni: {}
- op: add
  path: /cluster/network/cni
  value:
    name: none
- op: add
  path: /cluster/proxy
  value:
    disabled: true
- op: add
  path: /cluster/externalCloudProvider
  value:
    enabled: true
    manifests:
    - https://github.com/equinix/cloud-provider-equinix-metal/releases/download/v3.6.0/deployment.yaml
- op: add
  path: /machine/kubelet/extraArgs
  value:
    cloud-provider: external

Resulting worker-capi.yaml misses multiple fields from original worker.yaml, like cluster.ca

I suppose the problem is with

- op: add
  path: /cluster
  value:
    inlineManifests: []

But without it talosctl validate -m cloud -c worker-capi.yaml fails with failure applying rfc6902 patches to talos machine config: add operation does not apply: doc is missing path: "/cluster/inlineManifests/0": missing value

Logs

N/A

Environment

smira commented 1 year ago

your patch should be (probably?):

- op: add
  path: /cluster/inlineManifests
  value: []

And I would recommend using strategic merge patches.

KrystianMarek commented 1 year ago

it works, thank you!