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

feat(kubernetes-etcd-backup): skip tls verify #1292

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

in0rdr
Copy link
Contributor

@in0rdr in0rdr commented Aug 7, 2024

etcdctl can be run with the --insecure-skip-tls-verify to skip tls verification of the etcd endpoint.

This is useful in some deployments, for instance, when the etcd cluster is external to Kubernetes and the Kubernetes endpoint name (e.g., etcd.kube-system.svc.cluster.local) does not match the names in the certificates of the external etcd cluster.

Checklist

  • This PR contains a description of the changes I'm making
  • I updated the version in Chart.yaml
    • bumped the minor version
  • I updated the changelog with an artifacthub.io/changes annotation in Chart.yaml, check the example in the documentation.
  • I updated applicable README.md files using pre-commit run
  • I documented any high-level concepts I'm introducing in docs/
  • CI is currently green and this is ready for review
  • I am ready to test changes after they are applied and released

@in0rdr in0rdr requested a review from a team as a code owner August 7, 2024 07:24
@in0rdr in0rdr requested review from inisitijitty and tongpu August 7, 2024 07:24
@github-actions github-actions bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 7, 2024
@4censord
Copy link
Contributor

4censord commented Aug 7, 2024

Wouldn't it be better to just specify the correct hostname?
Or is that not an option in your case?

@in0rdr
Copy link
Contributor Author

in0rdr commented Aug 7, 2024

Wouldn't it be better to just specify the correct hostname? Or is that not an option in your case?

Of course would be better, but the nodes are external to the cluster and these DNS names are not resolvable from within the Kubernetes cluster. The cluster internal etcd service in the kube-system namespace resolves these external endpoints though.

@4censord
Copy link
Contributor

4censord commented Aug 7, 2024

Hmm, how does the kubelet resolve/verify etdc then?
Or does the kubelet just not verify as well?

@tongpu
Copy link
Member

tongpu commented Aug 7, 2024

This is useful in some deployments, for instance, when the etcd cluster is external to Kubernetes and the Kubernetes endpoint name (e.g., etcd.kube-system.svc.cluster.local) does not match the names in the certificates of the external etcd cluster.

AFAIK only the Kubernetes API server talks directly to etcd, so only they would need to have the correct hostname configured.

@eyenx
Copy link
Member

eyenx commented Aug 7, 2024

Hmm, how does the kubelet resolve/verify etdc then? Or does the kubelet just not verify as well?

The IPs are hardcoded in the Endpoints CR. The TLS skip-verify is only needed cause Nutanix's Implementation of ETCD Cluster uses a certificate that is only valid for

  • hostname (not resolvable within cluster)
  • hostname.cluster.local (no idea why that would help)
  • *.cluster.local (does not help as well)

The only way I see not having to set TLS SKIP Verify is to make sure the hostnames are resolvable within the cluster. Or using directly IPs (which are in the SAN of the cert, as we are using them for grabbing etcd metrics for prometheus with insecureSkipVerify: false, but iwth serverName: etcd.cluster.local)

@hairmare
Copy link
Contributor

hairmare commented Aug 7, 2024

The TLS skip-verify is only needed cause Nutanix's Implementation of ETCD

Should we also reflect this issue in upstream Nutanix so it gets unborked at some point?

@4censord
Copy link
Contributor

4censord commented Aug 7, 2024

ETCD Cluster uses a certificate that is only valid for

  • *.cluster.local (does not help as well)

Now I'm even more confused, because shouldn't that be valid for the internal service etcd.kube-system.svc.cluster.local?

@tongpu
Copy link
Member

tongpu commented Aug 7, 2024

ETCD Cluster uses a certificate that is only valid for

  • *.cluster.local (does not help as well)

Now I'm even more confused, because shouldn't that be valid for the internal service etcd.kube-system.svc.cluster.local?

No, because the wildcard is only valid for a single subdomain, but not for a subdomain of a subdomain (of a subdomain), as is the case with service.namespace.svc.cluster.local.

@4censord
Copy link
Contributor

4censord commented Aug 7, 2024

oh, so for TLS certs it differs from how wildcards work for DNS. Because with dns, *.cluster.local would also resolve to service.namespace.svc.cluster.local
I did not know that.

@eyenx
Copy link
Member

eyenx commented Aug 7, 2024

The TLS skip-verify is only needed cause Nutanix's Implementation of ETCD

Should we also reflect this issue in upstream Nutanix so it gets unborked at some point?

I'll discuss this with the customer tomorrow.

@eyenx
Copy link
Member

eyenx commented Aug 7, 2024

This needs now a rebase

Andreas Gruhler and others added 5 commits August 7, 2024 16:48
etcdctl can be run with the `--insecure-skip-tls-verify` to skip tls
verification of the etcd endpoint.

This is useful in some deployments, for instance, when the etcd
cluster is external to Kubernetes and the Kubernetes endpoint name (e.g.,
`etcd.kube-system.svc.cluster.local`) does not match the names in the
certificates of the external etcd cluster.
@in0rdr in0rdr force-pushed the feat/etcd-backup/skip-verify branch from 5623eef to 0879794 Compare August 7, 2024 14:54
@in0rdr
Copy link
Contributor Author

in0rdr commented Aug 7, 2024

rebased, I think I almost need to bump to 1.3.0 now

@in0rdr
Copy link
Contributor Author

in0rdr commented Aug 7, 2024

@eyenx you only keep the latest artifacthub annotation right?

@eyenx
Copy link
Member

eyenx commented Aug 7, 2024

yes, bump to 1.3.0 and just add the your change to artifacthub

@in0rdr in0rdr merged commit 12aa40c into adfinis:main Aug 7, 2024
3 checks passed
@in0rdr in0rdr deleted the feat/etcd-backup/skip-verify branch August 7, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants