Support external certificate management for webhooks #344

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

What type of PR is this?

/kind feature

What this PR does / why we need it:

Note this PR was originally #233. I have taken it over it order to implement PR feedback and get things moving faster.

This PR adds support for external certificate management for Grove's webhook server. Users can now choose between:

Auto-provisioned certificates (default): Grove's built-in cert-controller automatically generates and manages self-signed certificates - best for development/testing
External certificates: Users can provide their own certificates (e.g., via cert-manager or manually) - best for production environments

Key changes:

Added autoProvision configuration option (config.server.webhooks.autoProvision) to toggle between auto-provisioned and external certificates
Added secretName configuration to specify the Kubernetes Secret containing webhook certificates
Added webhooks.caBundle Helm value to inject a custom CA bundle into webhook configurations
Added comprehensive e2e tests covering both certificate management modes and transitions between them

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

When autoProvision: true (default), behavior is unchanged - Grove generates and manages certificates automatically
When autoProvision: false, the operator expects certificates to exist in the specified Secret before starting
The e2e tests verify transitioning from cert-manager managed certificates to autoprovision mode and back
Webhook configurations now support an optional caBundle value for external CA certificates

Does this PR introduce a API change?

Added support for external certificate management. Users can now disable auto-provisioned certificates by setting `config.server.webhooks.autoProvision: false` and provide their own certificates (e.g., via cert-manager). A new `webhooks.caBundle` Helm value allows injecting custom CA bundles into webhook configurations.

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

Added docs/user-guide/certificate-management.md

Copilot

Pull request overview

This PR adds support for external certificate management for Grove's webhook server, allowing users to choose between automatic certificate provisioning (default) and external certificate management (e.g., via cert-manager). This provides flexibility for both development/testing environments and production deployments with enterprise PKI requirements.

Changes:

Added autoProvision and secretName configuration options to toggle between auto-provisioned and external certificates
Added caBundle Helm value to support custom CA certificates in webhook configurations
Implemented comprehensive e2e tests for certificate management mode transitions

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
operator/internal/controller/cert/cert.go	Modified `ManageWebhookCerts` to support both auto-provisioned and external certificate modes
operator/e2e/utils/k8s_client.go	Exported `applyYAMLData` function as `ApplyYAMLData` for use in new tests
operator/e2e/tests/cert_management_test.go	Added comprehensive e2e test suite for certificate management round-trip scenarios
operator/e2e/setup/helm.go	Added `UpgradeHelmChart` and `UninstallHelmChart` functions with timeout support
operator/e2e/setup/grove.go	Added `UpgradeGrove` function for upgrading Grove configuration
operator/e2e/dependencies.yaml	Added cert-manager as a test dependency
operator/e2e/dependencies.go	Added cert-manager to HelmCharts struct
operator/cmd/main.go	Updated main to pass new certificate configuration parameters
operator/charts/values.yaml	Added certificate management configuration options and documentation
operator/charts/templates/webhook-server-cert-secret.yaml	Made secret creation conditional based on autoProvision setting
operator/charts/templates/pcs-validating-webhook-config.yaml	Added support for custom annotations and caBundle
operator/charts/templates/pcs-defaulting-webhook-config.yaml	Added support for custom annotations and caBundle
operator/charts/templates/deployment.yaml	Made certificate directory and secret name configurable
operator/charts/templates/authorizer-webhook-config.yaml	Added support for custom annotations and caBundle
operator/charts/templates/_helpers.tpl	Added certificate configuration to config template
operator/api/config/v1alpha1/zz_generated.deepcopy.go	Added deep copy support for new AutoProvision field
operator/api/config/v1alpha1/types.go	Added SecretName and AutoProvision fields to WebhookServer struct
operator/api/config/v1alpha1/defaults.go	Added defaults for new certificate configuration fields
docs/user-guide/certificate-management.md	Added comprehensive certificate management documentation
docs/installation.md	Added reference to certificate management guide
docs/api-reference/operator-api.md	Updated API documentation with new certificate fields
.github/workflows/e2e-test.yaml	Added cert_management test to CI workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

operator/e2e/tests/cert_management_test.go

operator/api/config/v1alpha1/types.go

Ronkahn21

I will complete the review later today

operator/charts/values.yaml

operator/e2e/setup/grove.go

operator/e2e/setup/helm.go

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>

… test

Ronkahn21 · 2026-01-26T20:09:15Z

operator/e2e/setup/grove.go

+	for k, v := range c.Webhooks.Annotations {
+		annotations[k] = v
+	}
+
+	webhookAnnotations := map[string]interface{}{"annotations": annotations}
+
+	return map[string]interface{}{
+		"installCRDs": c.InstallCRDs,
+		"config": map[string]interface{}{
+			"server": map[string]interface{}{
+				"webhooks": map[string]interface{}{


Manual map nesting like this is pretty brittle—if we rename a field in GroveConfig but forget to update this string key, the Helm chart will break silently.

Can we refactor this to use json.Marshal/Unmarshal or a library like mapstructure?

Ronkahn21 · 2026-01-26T20:14:15Z

operator/e2e/setup/grove.go

+//
+// This approach avoids wasteful rebuilds while staying compatible with the Skaffold installation.
+func UpdateGroveConfiguration(ctx context.Context, restConfig *rest.Config, config *GroveConfig, logger *utils.Logger) error {
+	chartDir, err := getGroveChartDir()


From a SOLID perspective the path should be passed to the function

Ronkahn21 · 2026-01-26T20:22:40Z

operator/e2e/utils/k8s_client.go

-func applyYAMLData(ctx context.Context, yamlData []byte, namespace string, restConfig *rest.Config, logger *Logger) ([]AppliedResource, error) {
+// ApplyYAMLData is the common function that applies YAML data to Kubernetes
+func ApplyYAMLData(ctx context.Context, yamlData []byte, namespace string, restConfig *rest.Config, logger *Logger) ([]AppliedResource, error) {
 	dynamicClient, restMapper, err := createKubernetesClients(restConfig)


I would prefer to have a way we not to create client obj in every function that interact with the cluster, in long session it could take up to a minute

Ronkahn21 · 2026-01-26T20:25:46Z

operator/internal/controller/cert/cert.go

+// ManageWebhookCerts manages webhook certificates based on the CertProvisionMode configuration.
+// When mode=auto: uses cert-controller for automatic certificate generation and management.
+// When mode=manual: waits for externally provided certificates (e.g., from cert-manager, cluster admin).
+func ManageWebhookCerts(mgr ctrl.Manager, certDir string, secretName string, authorizerEnabled bool, certProvisionMode configv1alpha1.CertProvisionMode, certsReadyCh chan struct{}) error {


can you please add unit testing for the new functionality. It will make sure it does not break when we might have more the current two modes

gflarity requested review from Ronkahn21, sanjaychatterjee, shayasoolin and unmarshall as code owners January 17, 2026 16:51

gflarity mentioned this pull request Jan 17, 2026

Manage external cert #233

Closed

gflarity force-pushed the cert_management_cleanup branch from 6c64a4d to f5681f1 Compare January 18, 2026 18:20

renormalize self-assigned this Jan 20, 2026

Ronkahn21 requested a review from Copilot January 22, 2026 09:11

Copilot AI reviewed Jan 22, 2026

View reviewed changes

operator/e2e/tests/cert_management_test.go Outdated Show resolved Hide resolved

Ronkahn21 reviewed Jan 22, 2026

View reviewed changes

operator/api/config/v1alpha1/types.go Outdated Show resolved Hide resolved

Ronkahn21 reviewed Jan 22, 2026

View reviewed changes

operator/charts/values.yaml Show resolved Hide resolved

operator/e2e/setup/grove.go Outdated Show resolved Hide resolved

operator/e2e/setup/grove.go Outdated Show resolved Hide resolved

operator/e2e/setup/helm.go Show resolved Hide resolved

kangclzjc and others added 19 commits January 22, 2026 14:01

manage external cert

db09e87

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

customize ca

e0d5652

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

refactor

55a91fa

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

add pcs webhook

74f3316

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

add e2e

2749596

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

fix conflict

1e4e410

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

check

5d1c702

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

fix default value

376e81c

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

use poll

f6fbda1

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

Cert-manager to Autoprovision e2e test flow

d7c78a7

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

Refactor cert-manager & autoprovision tests into separate functions

54cf6c9

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

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

35ed8b7

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

Poll ClusterIssuer resource

8c5aa3a

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

Update cert-manager-ctl image version

6a87670

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

refactor

825d05d

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

refactor

edd4800

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

unify log format

1b01663

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

reuse image

2d7355d

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

fix version

dffd572

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

gflarity added 11 commits January 22, 2026 14:02

actually use secretName

26afe4b

remove unused clusterTopologyValidationWebhook from cert-manager test

cdb1efb

webhooks are cluster scoped, remove unused namespaces

3fa4c9f

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

a75184e

add a comment explaining if statement

1cc2fa5

rename test to make it less cert manager specific

2c07e42

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

b89431e

… test

add cert management tests to e2e matrix

c45b072

add documentation for external certificate management

211adc1

make E2E tests more robust for CI

7fd443e

PR feedback

df66b3b

gflarity force-pushed the cert_management_cleanup branch from f5681f1 to df66b3b Compare January 22, 2026 20:36

gflarity requested a review from Ronkahn21 January 22, 2026 20:37

gflarity added 2 commits January 22, 2026 15:44

remove trouble shooting guide

611165f

add some more documentation to the grove.go file in the setup package

5a584f1

Ronkahn21 reviewed Jan 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support external certificate management for webhooks #344

Support external certificate management for webhooks #344

Uh oh!

gflarity commented Jan 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Ronkahn21 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ronkahn21 Jan 26, 2026

Uh oh!

Ronkahn21 Jan 26, 2026

Uh oh!

Ronkahn21 Jan 26, 2026

Uh oh!

Ronkahn21 Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Support external certificate management for webhooks #344

Are you sure you want to change the base?

Support external certificate management for webhooks #344

Uh oh!

Conversation

gflarity commented Jan 17, 2026 • 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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Ronkahn21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ronkahn21 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Ronkahn21 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Ronkahn21 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Ronkahn21 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gflarity commented Jan 17, 2026 •

edited

Loading