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

Allow for Usage of Cert-Manager in helm chart for webhook certificate #677

Closed
knechtionscoding opened this issue Sep 5, 2023 · 10 comments
Closed
Labels
kind/feature Categorizes issue or PR as related to a new feature. v1 Issues requiring resolution by the v1 milestone

Comments

@knechtionscoding
Copy link

Description

What problem are you trying to solve?

I'd like to be able to use cert-manager to manage: https://github.com/aws/karpenter/blob/main/charts/karpenter/templates/secret-webhook-cert.yaml rather than relying on a pre-sync hook or some other mechanism.

How important is this feature to you?

Reasonably important. Allows us to control the issuer, renewal timing, and other features of the certificate.

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@knechtionscoding knechtionscoding added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 5, 2023
@bwagner5
Copy link
Contributor

bwagner5 commented Sep 5, 2023

We're thinking of just getting rid of our webhook altogether in-favor of using CEL. Curious on your thoughts: #103

@knechtionscoding
Copy link
Author

@bwagner5 I think that'd be great. Would remove extra maintenance requirements from karpenter's side and just allow for native tooling to be used.

I'm curious if there's a k8s versioning restriction on CEL? Or is it all K8s versions? If there is, it might make sense to add an option to still disable the webhook cert for those of us not on 1.28, or have slower upgrade cycles?

@bwagner5
Copy link
Contributor

bwagner5 commented Sep 6, 2023

CEL has been available as a Beta feature (enabled by default) since K8s 1.25. Karpenter supports down to K8s 1.23 right now. So we'd need to setup a flag for this or wait until early next year to roll out CEL support as the default.

You are able to disable the webhooks by setting the DISABLE_WEBHOOK=true environment variable when installing Karpenter.

@knechtionscoding
Copy link
Author

@bwagner5 Because I haven't looked into the webhooks specifically for Karpenter, do I lose out on anything by disabling them? Are they validating webhooks? If so I might spend some time helping convert from this to CEL.

@bwagner5
Copy link
Contributor

bwagner5 commented Sep 7, 2023

We'd love the help!!!

We have some validating and some mutating (defaulting) webhooks. Most of the validations are fairly simple, so we're hoping that CEL is able to model those well enough, but it would be great if you'd be willing to help with that.

If you disable the webhooks, you lose the validations found below and you'd also lose any defaulting of the Provisioner. For example, if you give a provisioner with no requirement, we default AMD64 and on-demand.

Here's the validations for the AWSNodeTemplate: https://github.com/aws/karpenter/blob/main/pkg/apis/v1alpha1/awsnodetemplate_validation.go

And here is the validations for the Provisioner:
https://github.com/aws/karpenter-core/blob/main/pkg/apis/v1alpha5/provisioner_validation.go

@knechtionscoding
Copy link
Author

@bwagner5 I opened: aws/karpenter-provider-aws#4586 I'm not sure if you want the PR here or in karpenter-core. Let me know and I can redirect it

@ellistarn
Copy link
Contributor

ellistarn commented Sep 11, 2023

We'll need PRs in both Karpenter and karpenter-core, depending whether or not the validation is AWS specific or not:
https://github.com/aws/karpenter-core/blob/main/pkg/apis/v1beta1/nodepool_validation.go
https://github.com/aws/karpenter/blob/main/pkg/apis/v1beta1/nodeclass_validation.go

@njtran
Copy link
Contributor

njtran commented Nov 2, 2023

This should be closed as part of v0.33.0

@njtran njtran added the v1 Issues requiring resolution by the v1 milestone label Nov 2, 2023
@njtran njtran transferred this issue from aws/karpenter-provider-aws Nov 2, 2023
@jonathan-innis
Copy link
Member

We can close this out now that v0.33 is released.

@jonathan-innis
Copy link
Member

Closed by #821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. v1 Issues requiring resolution by the v1 milestone
Projects
None yet
5 participants