Manage external cert by kangclzjc · Pull Request #233 · ai-dynamo/grove

kangclzjc · 2025-10-24T09:00:38Z

What type of PR is this?

Currently we do the certificate generation ourselves. If they already have certificate in their system, we should manage external existing certificate by mounting them

What this PR does / why we need it:

We add support for allowing user to use external cert

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a API change?

Additional documentation e.g., enhancement proposals, usage docs, etc.:

gflarity · 2025-10-27T21:09:35Z

+	// If set to false, you must provide your own certificates via Secret.
+	// Default: true
+	// +optional
+	AutoProvision *bool `json:"autoProvision,omitempty"`


Hmm... maybe just default to auto provision in the code if SecretName isn't set, then the user doesn't have to remember to disable and we don't need to track this variable.

You're right. It seems that we can detect secretName or certFilesPath user want to mount to decide whether to auto provision

gflarity

As discussed, let's simplify the logic by enabling automatic certs when the secretName isn't provided.

kangclzjc · 2025-11-04T02:48:44Z

As discussed, let's simplify the logic by enabling automatic certs when the secretName isn't provided.
Currently， this ps will implement these feature:

If enable certManagerEnabled, then certmanager should handle the certificate/secret creation process
if certFilesPath is not empty, then we only mount the certfiles into grove operator pod
if certManagerEnabled and certFilesPath are not provided, then we use the default auto provision logic which currently is cert controller

gflarity

Thanks, left an example that I implemented a while ago (running in production for some time now). The only difference is we need to enable auto provision. Otherwise you can just follow this.

gflarity · 2025-12-01T14:38:52Z

Sorry for the delay, planning to review this today.

gflarity

Thanks @kangclzjc, gave it a quick review today. Looks good, I'll do a slower review tomorrow morning first thing. In the mean time:

One thing we'll need is a simple E2E test to verifying everything works using cert manager. Please take a look at the current E2E tests, all it really needs to do is install cert manager, then update the helm values to use it. If you start a simple grove deployment successfully, it means the webhooks are still working with this change.

There is code in the cluster setup utilities for installing a helm chart (cert manager). You should be able to reuse some of that logic to install cert manager for this specific test. Please let me know if you have any questions.

kangclzjc · 2025-12-02T01:04:33Z

Thanks @kangclzjc, gave it a quick review today. Looks good, I'll do a slower review tomorrow morning first thing. In the mean time:

One thing we'll need is a simple E2E test to verifying everything works using cert manager. Please take a look at the current E2E tests, all it really needs to do is install cert manager, then update the helm values to use it. If you stat a simple grove deployment successfully, it means the webhooks are still working with this change.

There code in the cluster setup utilities for installing a helm chart (cert manager). You should be able to reuse some of that logic to install cert manager for this specific test. Please let me know if you have any questions.

Thanks @gflarity , let me check the E2E test first. And then do a simple E2E test to verifying everything works using cert manager.

gflarity

Just a couple small things, otherwise looks good. Just need the E2E test(s).

gflarity

Left some feedback on the E2E test.

gflarity · 2026-01-02T18:54:17Z

Just circling back on this @kangclz so we can get it merged. What's the current status?

Please resolve any comments you believe have now been addressed.

Also, looks like the E2E tests are failing. Please try rebasing from main to see if that helps with the tests.

… test

Signed-off-by: kangclzjc <kangz@nvidia.com>

Signed-off-by: Bashorun97 <bashorun.emma@gmail.com>

Signed-off-by: Emmanuel Bashorun <bashorun.emma@gmail.com>

Signed-off-by: kangclzjc <kangz@nvidia.com>

Cert management

gflarity · 2026-01-17T16:51:56Z

Closing this PR in favour of #344

kangclzjc requested review from sanjaychatterjee and unmarshall as code owners October 24, 2025 09:00

kangclzjc marked this pull request as draft October 24, 2025 09:00

kangclzjc mentioned this pull request Oct 24, 2025

feat: add cert-manager support as an option #145

Closed

gflarity reviewed Oct 27, 2025

View reviewed changes

gflarity requested changes Oct 29, 2025

View reviewed changes

kangclzjc marked this pull request as ready for review November 4, 2025 02:36

kangclzjc force-pushed the cert_manage branch from faba093 to fc4f28a Compare November 4, 2025 02:38

gflarity reviewed Nov 5, 2025

View reviewed changes

Comment thread operator/charts/templates/authorizer-webhook-config.yaml Outdated

gflarity reviewed Nov 5, 2025

View reviewed changes

Comment thread operator/charts/templates/pcs-defaulting-webhook-config.yaml

gflarity reviewed Nov 5, 2025

View reviewed changes

Comment thread operator/charts/templates/pcs-validating-webhook-config.yaml Outdated

gflarity reviewed Nov 5, 2025

View reviewed changes

Comment thread operator/charts/templates/webhook-server-cert-secret.yaml Outdated

kangclzjc requested a review from gflarity November 12, 2025 06:19

kangclzjc force-pushed the cert_manage branch from a0bb706 to 53ca78c Compare November 17, 2025 09:01

gflarity requested changes Nov 19, 2025

View reviewed changes

Comment thread operator/charts/templates/authorizer-webhook-config.yaml Outdated

kangclzjc force-pushed the cert_manage branch from 0cc29d4 to 4c83e1b Compare November 25, 2025 03:52

kangclzjc requested a review from gflarity November 26, 2025 08:47

gflarity requested changes Dec 1, 2025

View reviewed changes

gflarity requested changes Dec 2, 2025

View reviewed changes

Comment thread docs/api-reference/operator-api.md Outdated

Comment thread operator/charts/values.yaml Outdated

Comment thread operator/charts/values.yaml Outdated

kangclzjc force-pushed the cert_manage branch 2 times, most recently from dabcda6 to 46d8d99 Compare December 15, 2025 05:02

kangclzjc requested a review from gflarity December 15, 2025 08:38

gflarity requested changes Dec 16, 2025

View reviewed changes

briefgaming mentioned this pull request Dec 29, 2025

Cert-manager to Autoprovision e2e test flow kangclzjc/grove#1

Merged

kangclzjc force-pushed the cert_manage branch from 68612bc to 766953a Compare December 31, 2025 01:56

kangclzjc force-pushed the cert_manage branch from 766953a to f30a30d Compare January 4, 2026 01:16

gflarity and others added 24 commits January 16, 2026 13:23

add generic UpgradeGrove helper and refactor cert-manager test to use it

4679119

add a comment explaining if statement

48f45fa

rename test to make it less cert manager specific

61b0681

update CM tests to check certificates, and condensed it into a single…

c9ea3a6

… test

add cert management tests to e2e matrix

be210d9

manage external cert

63dad10

Signed-off-by: kangclzjc <kangz@nvidia.com>

customize ca

3d9d971

Signed-off-by: kangclzjc <kangz@nvidia.com>

refactor

621b257

Signed-off-by: kangclzjc <kangz@nvidia.com>

add pcs webhook

8ff2100

Signed-off-by: kangclzjc <kangz@nvidia.com>

add e2e

22870d1

Signed-off-by: kangclzjc <kangz@nvidia.com>

fix conflict

932dc50

Signed-off-by: kangclzjc <kangz@nvidia.com>

check

40827df

Signed-off-by: kangclzjc <kangz@nvidia.com>

fix default value

118662b

Signed-off-by: kangclzjc <kangz@nvidia.com>

use poll

23e3456

Signed-off-by: kangclzjc <kangz@nvidia.com>

Cert-manager to Autoprovision e2e test flow

3131db3

Signed-off-by: Bashorun97 <bashorun.emma@gmail.com>

Refactor cert-manager & autoprovision tests into separate functions

20cd841

Signed-off-by: Bashorun97 <bashorun.emma@gmail.com>

Add cert-manager images to dependencies.yaml for pre-pulling

2a6a467

Signed-off-by: Bashorun97 <bashorun.emma@gmail.com>

Poll ClusterIssuer resource

15e0334

Signed-off-by: Emmanuel Bashorun <bashorun.emma@gmail.com>

Update cert-manager-ctl image version

421d6fa

Signed-off-by: Emmanuel Bashorun <bashorun.emma@gmail.com>

refactor

c266391

Signed-off-by: kangclzjc <kangz@nvidia.com>

refactor

37bc9a8

Signed-off-by: kangclzjc <kangz@nvidia.com>

unify log format

3dc1f26

Signed-off-by: kangclzjc <kangz@nvidia.com>

reuse image

08b46cc

Signed-off-by: kangclzjc <kangz@nvidia.com>

fix version

32f9b96

Signed-off-by: kangclzjc <kangz@nvidia.com>

gflarity force-pushed the cert_manage branch from a79c827 to 32f9b96 Compare January 16, 2026 21:22

kangclzjc added 2 commits January 17, 2026 19:32

Merge branch 'cert_manage' into cert_management

4803f56

Merge pull request #3 from gflarity/cert_management

846a565

Cert management

gflarity closed this Jan 17, 2026

gflarity mentioned this pull request Jan 17, 2026

Support external certificate management for webhooks #344

Merged

Conversation

kangclzjc commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a API change?

Additional documentation e.g., enhancement proposals, usage docs, etc.:

Uh oh!

gflarity Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

kangclzjc Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

gflarity left a comment

Choose a reason for hiding this comment

Uh oh!

kangclzjc commented Nov 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gflarity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gflarity commented Dec 1, 2025

Uh oh!

gflarity left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kangclzjc commented Dec 2, 2025

Uh oh!

gflarity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gflarity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gflarity commented Jan 2, 2026

Uh oh!

gflarity commented Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kangclzjc commented Oct 24, 2025 •

edited

Loading

gflarity left a comment •

edited

Loading