Skip to content

Control plane template extra args should be an array, not map #11562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joshfrench opened this issue Dec 11, 2024 · 4 comments
Closed

Control plane template extra args should be an array, not map #11562

joshfrench opened this issue Dec 11, 2024 · 4 comments
Labels
area/clusterclass Issues or PRs related to clusterclass area/control-plane Issues or PRs related to control-plane lifecycle management kind/bug Categorizes issue or PR as related to a bug. needs-priority Indicates an issue lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@joshfrench
Copy link
Contributor

joshfrench commented Dec 11, 2024

What steps did you take and what happened?

kube-apiserver allows some flags to be specified multiple times, such as --service-account-issuer and --oidc-required-claims but perhaps others. From the docs:

--service-account-issuer
...When this flag is specified multiple times, the first is used to generate tokens and all are used to determine which issuers are accepted.

Because KubeadmControlPlaneTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs is map[string]string, duplicate keys cannot be provided.

Applying a spec with the following keys:

kind: KubeadmControlPlaneTemplate
spec:
  template:
    spec:
      kubeadmConfigSpec:
        clusterConfiguration:
          apiServer:
            extraArgs:
              service-account-issuer: custom-issuer-1
              service-account-issuer: custom-issuer-2

The resulting kube-apiserver.yaml only contains the last issuer, having clobbered the duplicate key(s):

kind: Pod
spec:
  containers:
  - command:
    - kube-apiserver
    - ...
    - --requestheader-group-headers=X-Remote-Group
    - --service-account-issuer=custom-issuer-2
    - --requestheader-username-headers=X-Remote-User

The kubelet docs don't specifically mention whether any of its keys may be specified multiple times, but if so kubeletExtraArgs is also likely prone to this issue.

Because of this we must resort to jsonpatches for some api server args, but we'd rather 1) use the more ergonomic solution and 2) consolidate all of our args in a single place.

What did you expect to happen?

api server, controller manager, and kubelet extra args should support the full capabilities of their underlying components.

Cluster API version

v1.8.5

Kubernetes version

Client Version: v1.31.1
Server Version: v1.29.2

Anything else you would like to add?

Because order matters with --service-account-issuer, well-defined behavior around whether the extra args get prepended or appended would be helpful!

Label(s) to be applied

/kind bug
/area clusterclass

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. area/control-plane Issues or PRs related to control-plane lifecycle management needs-priority Indicates an issue lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 11, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If CAPI contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@joshfrench
Copy link
Contributor Author

Sorry, this should probably be:
/area clusterclass

@k8s-ci-robot k8s-ci-robot added the area/clusterclass Issues or PRs related to clusterclass label Dec 11, 2024
@chrischdi
Copy link
Member

We have this in our roadmap, however we cannot change the field in v1beta1 because it would be a breaking change to the API.

This is tracked in #10708

@joshfrench
Copy link
Contributor Author

Thanks, I'll follow along over there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterclass Issues or PRs related to clusterclass area/control-plane Issues or PRs related to control-plane lifecycle management kind/bug Categorizes issue or PR as related to a bug. needs-priority Indicates an issue lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

3 participants