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: support ECDSA private keys #667

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

avorima
Copy link
Contributor

@avorima avorima commented Jan 14, 2025

Support ECDSA keys for certificates.

Copy link

netlify bot commented Jan 14, 2025

Deploy Preview for kamaji-documentation canceled.

Name Link
🔨 Latest commit 3bbdc7e
🔍 Latest deploy log https://app.netlify.com/sites/kamaji-documentation/deploys/6786d65dd4881d000874a71f

@prometherion
Copy link
Member

We can provide this feature, however, I'm wondering how you're deploying ECDSA based certificates.

@avorima
Copy link
Contributor Author

avorima commented Jan 14, 2025

We're using cert-manager with a pre-provided cluster issuer.

@avorima
Copy link
Contributor Author

avorima commented Jan 14, 2025

I actually have a related change that fixes cleanup for TCPs that did not successfully complete datastore setup.

@prometherion
Copy link
Member

prometherion commented Jan 15, 2025

The cert manager support is something I always looked for in Kamaji but never had a user story or a proper enhancement proposal (#463).

Please, may I ask you to share which certificates are you creating (as well as manifests) and how the integration works?

I'll draft everything as a documentation later to reduce your burden, unless you're willing to put it down in the docs.

@avorima
Copy link
Contributor Author

avorima commented Jan 15, 2025

We only use it for the etcds and kamaji had problems to set up the certificates for it without this patch.

It's just something like

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: etcd-server
spec:
  commonName: etcd-server
  dnsNames:
  - '*.etcd-headless.default.svc.cluster.local'
  - '*.etcd.default.svc.cluster.local'
  - etcd.default.svc.cluster.local
  - etcd.default.svc
  - etcd.default
  - etcd
  - localhost
  duration: 87600h0m0s
  ipAddresses:
  - 127.0.0.1
  issuerRef:
    kind: ClusterIssuer
    name: etcd-issuer
  secretName: etcd-server
  usages:
  - client auth
  - server auth
  - signing
  - key encipherment

where the etcd-issuer is provided by the infra team that manages PKI in all environments.

@prometherion prometherion added the enhancement New feature or request label Jan 17, 2025
Comment on lines +232 to +233
$(HELM) repo add jetstack https://charts.jetstack.io
$(HELM) upgrade --install cert-manager jetstack/cert-manager --namespace certmanager-system --create-namespace --set "installCRDs=true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: what's wrong with Bitnami's chart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like they have a broken dependency or something. Either way, bitnami charts tend to have some extra stuff which isn't really needed in most cases (IMO).

https://github.com/clastix/kamaji/actions/runs/12776386026/job/35614933651

Release "cert-manager" does not exist. Installing it now.
index.go:346: skipping loading invalid entry for chart "airflow" "" from /home/runner/.cache/helm/repository/bitnami-index.yaml: validation: chart.metadata.version is required
Error: failed to download "bitnami/cert-manager"

@prometherion prometherion merged commit f29e219 into clastix:master Jan 17, 2025
10 checks passed
@avorima avorima deleted the feat-ecdsa-keys branch January 17, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants