Skip to content
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

validateKubeConfig ignores CertificateAuthority and only looks at CertificateAuthorityData #2739

Closed
rohitagarwal003 opened this issue Aug 3, 2022 · 2 comments · Fixed by kubernetes/kubernetes#111783
Assignees
Labels
area/pki PKI and certificate related issues kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@rohitagarwal003
Copy link
Member

rohitagarwal003 commented Aug 3, 2022

What keywords did you search in kubeadm issues before filing this one?

"got the wrong CA cert"

Is this a BUG REPORT or FEATURE REQUEST?

BUG REPORT

Versions

kubeadm version (use kubeadm version): v1.23.6

Environment:

  • Kubernetes version (use kubectl version): v1.23.6
  • Cloud provider or hardware configuration: AWS EC2 c6i
  • OS (e.g. from /etc/os-release): Amazon Linux 2
  • Others: Using an external CA (i.e. no ca.key in /etc/kubernetes/pki)

What happened?

Got the following error while running kubeadm init phase kubelet-start --config=/path/to/kubeadm.conf

the controller-manager.conf file does not exists or it is not valid: a kubeconfig file "/etc/kubernetes/controller-manager.conf" exists already but has got the wrong CA cert

What you expected to happen?

Didn't expect this error to happen, because our controller-manager kubeconfig has:

apiVersion: v1
clusters:
- cluster:
    certificate-authority: /etc/kubernetes/pki/ca.crt
    server: https://xyz.example.com:6443
  name: xyz
contexts:
- context:
    cluster: xyz
    user: controller-manager
  name: default
current-context: default
kind: Config
preferences: {}
users:
- name: controller-manager
  user:
    client-certificate: /etc/kubernetes/pki/controller-manager.crt
    client-key: /etc/kubernetes/pki/controller-manager.key

The root CA is the exact same file.

How to reproduce it (as minimally and precisely as possible)?

  1. Use an external CA (i.e. no ca.key in /etc/kubernetes/pki.
  2. Use a controller-manager conf that uses certificate-authority instead of certificate-authority-data.
  3. Run kubeadm init phase kubelet-start --config=/path/to/kubeadm.conf

Anything else we need to know?

https://github.com/kubernetes/kubernetes/blob/v1.23.6/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go#L240 is looking at just CertificateAuthorityData. It should also read the contents of the file pointed by CertificateAuthority if CertificateAuthorityData is empty.

@neolit123
Copy link
Member

neolit123 commented Aug 3, 2022

looking at just CertificateAuthorityData. It should also read the contents of the file pointed by CertificateAuthority if CertificateAuthorityData is empty.

hi, this was never a supported use case and i believe you are the first user requesting it.

what prevents you from embedding the client cert/key like kubeadm does by default?

i would not mind a change to validate external files, but we are in code freeze for 1.25 and i think this is a feature and not a bug fix, thus not backportable to < .25 too.

@neolit123 neolit123 added priority/backlog Higher priority than priority/awaiting-more-evidence. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 3, 2022
@neolit123 neolit123 added this to the v1.26 milestone Aug 3, 2022
@neolit123 neolit123 added the area/pki PKI and certificate related issues label Aug 3, 2022
@SataQiu
Copy link
Member

SataQiu commented Aug 10, 2022

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/pki PKI and certificate related issues kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
3 participants