Add `AutoProvider` certificate manager #227

ArkaSaha30 · 2025-10-23T21:04:19Z

This PR will add AutoProvider certificate manager as a part of generating default self-signed certificates:

interface functions
reconciler logic
e2e tests

etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k get pods
NAME                   READY   STATUS    RESTARTS   AGE
etcdcluster-sample-0   1/1     Running   0          34m
etcdcluster-sample-1   1/1     Running   0          34m
etcdcluster-sample-2   1/1     Running   0          33m


etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k describe pod etcdcluster-sample-0  
Name:             etcdcluster-sample-0
Namespace:        default
Priority:         0
Service Account:  default
Node:             kind-control-plane/172.18.0.2
Start Time:       Mon, 27 Oct 2025 23:47:43 +0530
Labels:           app=etcdcluster-sample
                  apps.kubernetes.io/pod-index=0
                  controller=etcdcluster-sample
                  controller-revision-hash=etcdcluster-sample-754c4cfbcd
                  statefulset.kubernetes.io/pod-name=etcdcluster-sample-0
Annotations:      <none>
Status:           Running
IP:               10.244.0.9
IPs:
  IP:           10.244.0.9
Controlled By:  StatefulSet/etcdcluster-sample
Containers:
  etcd:
    Container ID:  containerd://faa47a05746374496ff1b3d0147a93efc4d72bc07572e68183df1df9decdcdbf
    Image:         gcr.io/etcd-development/etcd:v3.5.21
    Image ID:      gcr.io/etcd-development/etcd@sha256:fd158fbe55240e252947bbd2e8dddc217997ff43978071fac2bd202b6ad15c03
    Ports:         2379/TCP, 2380/TCP
    Host Ports:    0/TCP, 0/TCP
    Command:
      /usr/local/bin/etcd
    Args:
      --name=$(POD_NAME)
      --listen-peer-urls=http://0.0.0.0:2380
      --listen-client-urls=http://0.0.0.0:2379
      --initial-advertise-peer-urls=http://$(POD_NAME).etcdcluster-sample.$(POD_NAMESPACE).svc.cluster.local:2380
      --advertise-client-urls=http://$(POD_NAME).etcdcluster-sample.$(POD_NAMESPACE).svc.cluster.local:2379
    State:          Running
      Started:      Mon, 27 Oct 2025 23:47:52 +0530
    Ready:          True
    Restart Count:  0
    Environment Variables from:
      etcdcluster-sample-state  ConfigMap  Optional: false
    Environment:
      POD_NAME:       etcdcluster-sample-0 (v1:metadata.name)
      POD_NAMESPACE:  default (v1:metadata.namespace)
    Mounts:
      /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-kwksc (ro)
Conditions:
  Type                        Status
  PodReadyToStartContainers   True 
  Initialized                 True 
  Ready                       True 
  ContainersReady             True 
  PodScheduled                True 
Volumes:
  server-secret:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  etcdcluster-sample-server-tls
    Optional:    false
  peer-secret:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  etcdcluster-sample-peer-tls
    Optional:    false
  kube-api-access-kwksc:
    Type:                    Projected (a volume that contains injected data from multiple sources)
    TokenExpirationSeconds:  3607
    ConfigMapName:           kube-root-ca.crt
    ConfigMapOptional:       <nil>
    DownwardAPI:             true
QoS Class:                   BestEffort
Node-Selectors:              <none>
Tolerations:                 node.kubernetes.io/not-ready:NoExecute op=Exists for 300s
                             node.kubernetes.io/unreachable:NoExecute op=Exists for 300s
Events:
  Type    Reason     Age   From               Message
  ----    ------     ----  ----               -------
  Normal  Scheduled  34m   default-scheduler  Successfully assigned default/etcdcluster-sample-0 to kind-control-plane
  Normal  Pulling    34m   kubelet            Pulling image "gcr.io/etcd-development/etcd:v3.5.21"
  Normal  Pulled     34m   kubelet            Successfully pulled image "gcr.io/etcd-development/etcd:v3.5.21" in 8.646s (8.646s including waiting). Image size: 20653564 bytes.
  Normal  Created    34m   kubelet            Created container: etcd
  Normal  Started    34m   kubelet            Started container etcd

etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k get secrets
NAME                            TYPE                DATA   AGE
etcdcluster-sample-client-tls   kubernetes.io/tls   3      35m
etcdcluster-sample-peer-tls     kubernetes.io/tls   3      35m
etcdcluster-sample-server-tls   kubernetes.io/tls   3      35m

etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k describe secret etcdcluster-sample-client-tls
Name:         etcdcluster-sample-client-tls
Namespace:    default
Labels:       <none>
Annotations:  <none>

Type:  kubernetes.io/tls

Data
====
ca.crt:   769 bytes
tls.crt:  769 bytes
tls.key:  365 bytes

etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k describe secret etcdcluster-sample-peer-tls  
Name:         etcdcluster-sample-peer-tls
Namespace:    default
Labels:       <none>
Annotations:  <none>

Type:  kubernetes.io/tls

Data
====
tls.crt:  774 bytes
tls.key:  365 bytes
ca.crt:   774 bytes

etcd-operator on  auto-cert-provider [$?] via 🐳 colima via 🐹 v1.25.3 on ☁️  (us-east-1) on ☁️  [email protected] 
❯ k describe secret etcdcluster-sample-server-tls
Name:         etcdcluster-sample-server-tls
Namespace:    default
Labels:       <none>
Annotations:  <none>

Type:  kubernetes.io/tls

Data
====
ca.crt:   769 bytes
tls.crt:  769 bytes
tls.key:  365 bytes

k8s-ci-robot · 2025-10-23T21:04:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ArkaSaha30
Once this PR has been reviewed and has the lgtm label, please assign ahrtr for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ahrtr · 2025-10-28T12:59:33Z

@abdurrehman107 @ballista01 @ivanvc @hakman PTAL.

Reviewing this PR is also an opportunity to get familiar with the certificate design & implementation.

Reference: https://docs.google.com/document/d/1k8os77Hqxe36nUaZ31YqBGBuXcgdozVPppJYOfwUDFA/edit?tab=t.0

ahrtr · 2025-10-28T14:56:53Z

cc @frederiko

internal/controller/utils.go

ahrtr · 2025-10-28T19:30:01Z

pkg/certificate/auto/provider.go

+		return nil, fmt.Errorf("failed to get certificate: %w", err)
+	}
+
+	return &interfaces.Config{}, nil


We should get the info from the certificate included in the secret.

Hi @ahrtr , I explored getting the original config from the secret itself but it will be a complicated one since we are manipulating the config and passing it to transport.SelfCert() which in turn creats the certificate.
Another workaround is to retrieve the certificateConfig from the etcdCluster object via the certificateSecret.ownerRef.Name, but it will cause a circular dependency

What can be the suggested solution here?
cc @ivanvc

but it will be a complicated one

Where is the complication? I don't see a PoC so far, nor a reasonable explanation.

The easiest solution is to just save the config passed to EnsureCertificateSecret, and return it in GetCertificateConfig. The problem is that it isn't the final real configuration that we used to create the certificate.

Sure, I have updated the GetCertificateConfig to return the saved user-defined config

ivanvc

Thanks for the pull request, Arka. I left some observations, comments, and suggestions.

pkg/certificate/auto/provider.go

api/v1alpha1/etcdcluster_types.go

internal/controller/utils.go

pkg/certificate/auto/provider.go

test/e2e/auto_provider_test.go

pkg/certificate/auto/provider.go

ArkaSaha30 · 2025-10-30T10:29:23Z

/test pull-etcd-operator-test-e2e

ivanvc · 2025-11-09T05:26:35Z

@ArkaSaha30, where do we stand in this pull request? Is this ready for another review?

pkg/certificate/auto/provider.go

This commit will add the initial scaffolding for AutoProvider. Signed-off-by: ArkaSaha30 <[email protected]>

Signed-off-by: ArkaSaha30 <[email protected]>

neolit123

thanks for the updates
generally LGTM, just a few minor comments from me

neolit123 · 2025-12-01T18:46:40Z

internal/controller/utils.go

+	autoConfig := ec.Spec.TLS.ProviderCfg.AutoCfg
+	duration, err := time.ParseDuration(autoConfig.ValidityDuration)
+	if err != nil {
+		log.Printf("Failed to parse ValidityDuration: %s", err)


should it return an error here?

neolit123 · 2025-12-01T18:47:46Z

internal/controller/utils.go

+		log.Printf("Failed to parse ValidityDuration: %s", err)
+	}
+
+	var getAltNames certInterface.AltNames


maybe just call it altNames

neolit123 · 2025-12-01T18:48:07Z

internal/controller/utils.go


 func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client, certName string) error {
-	cert, certErr := certificate.NewProvider(certificate.ProviderType(ec.Spec.TLS.Provider), c)
+	// tls field is present but spec is empty


Suggested change

// tls field is present but spec is empty

// The TLS field is present but spec is empty

neolit123 · 2025-12-01T18:48:43Z

internal/controller/utils.go

+func createClientCertificate(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client, scheme *runtime.Scheme) error {
 	certName := getClientCertName(ec.Name)
 	createClientCertErr := createCertificate(ec, ctx, c, certName)
+	patchCertErr := patchCertificateSecret(ctx, ec, c, scheme, certName)


just use a single err and check it on every stage.

neolit123 · 2025-12-01T18:49:23Z

internal/controller/utils.go

+	patchCertErr := patchCertificateSecret(ctx, ec, c, scheme, serverCertName)
+	if patchCertErr != nil {
+		return fmt.Errorf("patching certificate secret: %s with ownerReference failed: %w", serverCertName, patchCertErr)


same comments as above

neolit123 · 2025-12-01T18:54:35Z

pkg/certificate/auto/provider.go

+
+func (ac *Provider) EnsureCertificateSecret(ctx context.Context, secretName, namespace string,
+	cfg *interfaces.Config) error {
+	// save the user defined AutoConfig so that it can be returned from GetCertificateConfig


Suggested change

// save the user defined AutoConfig so that it can be returned from GetCertificateConfig

// Save the user defined AutoConfig so that it can be returned from GetCertificateConfig

neolit123 · 2025-12-01T18:56:46Z

internal/controller/utils.go

+	if err := controllerutil.SetControllerReference(ec, getCertSecret, scheme); err != nil {
+		return err
+	}
+	if updateErr := c.Update(ctx, getCertSecret); updateErr != nil {


neolit123 · 2025-12-01T19:01:38Z

pkg/certificate/auto/provider.go

+		}
+	}
+
+	log.Printf("hosts: %v", hosts)


maybe add a more detailed log msg, like "calling SelfCert with hosts: ...`

neolit123 · 2025-12-01T19:02:14Z

pkg/certificate/auto/provider.go

+
+	caPEM, err := os.ReadFile(caPath)
+	if err != nil || len(caPEM) == 0 {
+		// use certPEM when CA file is not found or empty


Suggested change

// use certPEM when CA file is not found or empty

// Use certPEM when CA file is not found or empty

neolit123 · 2025-12-01T19:02:27Z

pkg/certificate/auto/provider.go

+
+	// Validate cert and key pair
+	if _, err := tls.X509KeyPair(certPEM, keyPEM); err != nil {
+		return fmt.Errorf("generated keypair invalid: %w", err)


Suggested change

return fmt.Errorf("generated keypair invalid: %w", err)

return fmt.Errorf("generated keypair is invalid: %w", err)

palnabarun · 2025-12-02T15:12:31Z

internal/controller/etcdcluster_controller.go

 	// Ensure the operator has TLS credentials when the cluster requests TLS.
 	if ec.Spec.TLS != nil {
-		if err := createClientCertificate(ctx, ec, r.Client); err != nil {
+		if err := createClientCertificate(ctx, ec, r.Client, r.Scheme); err != nil {


r.Client also has the Scheme embedded. Can't that be used instead of passing Scheme through the call chain?

palnabarun · 2025-12-02T15:14:33Z

internal/controller/utils.go

+
+func patchCertificateSecret(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client, scheme *runtime.Scheme, certSecretName string) error {
+	getCertSecret := &corev1.Secret{}
+	getErr := c.Get(ctx, client.ObjectKey{Name: certSecretName, Namespace: ec.Namespace}, getCertSecret)


also, the Get call and the error check can be folded to the same block.

palnabarun · 2025-12-02T15:17:34Z

pkg/certificate/auto/provider.go

+
+var userAutoConfig *interfaces.Config
+
+func (ac *Provider) EnsureCertificateSecret(ctx context.Context, secretName, namespace string,


Whenever the "name, namespace" pattern is used, an object key can be used. This ensures consistency. See github.com/kubernetes-sigs/controller-runtime/blob/main/pkg/client/example_test.go for example.

Suggested change

func (ac *Provider) EnsureCertificateSecret(ctx context.Context, secretName, namespace string,

func (ac *Provider) EnsureCertificateSecret(ctx context.Context, secretKey ctrlruntimeclient.ObjectKey

palnabarun · 2025-12-02T15:20:31Z

pkg/certificate/auto/provider.go

+		return interfaces.ErrTLSCert
+	}
+
+	decodeCertificatePem, _ := pem.Decode(certificateData)


Do we not need an err check?

palnabarun · 2025-12-02T15:21:43Z

pkg/certificate/auto/provider.go

+	err := ac.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, &secret)
+	if k8serrors.IsNotFound(err) {
+		return nil
+	}
+	if err != nil {
+		return err
+	}


Suggested change

err := ac.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, &secret)

if k8serrors.IsNotFound(err) {

return nil

}

if err != nil {

return err

}

if err := ac.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, &secret); err != nil {

if k8serrors.IsNotFound(err) {

return nil

}

return err

}

palnabarun · 2025-12-02T15:23:02Z

pkg/certificate/auto/provider.go

+
+// parsePrivateKey parses the private key from the PEM-encoded data.
+func parsePrivateKey(privateKeyData []byte) (crypto.PrivateKey, error) {
+	block, _ := pem.Decode(privateKeyData)


same question as above

palnabarun · 2025-12-02T15:23:27Z

pkg/certificate/auto/provider.go

+	if err != nil {
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse private key: %w", err)
+		}
+	}


Suggested change

if err != nil {

if err != nil {

return nil, fmt.Errorf("failed to parse private key: %w", err)

}

}

if err != nil {

return nil, fmt.Errorf("failed to parse private key: %w", err)

}

ahrtr · 2025-12-02T17:11:08Z

@ArkaSaha30 pls feedback after you resolve all the review comments, thx

k8s-ci-robot added the do-not-merge/work-in-progress label Oct 23, 2025

k8s-ci-robot added size/M size/L and removed size/M labels Oct 23, 2025

ArkaSaha30 force-pushed the auto-cert-provider branch 3 times, most recently from f819caf to 82651bc Compare October 27, 2025 18:17

ArkaSaha30 changed the title ~~[WIP] Add AutoProvider interface functions~~ Add AutoProvider interface functions Oct 27, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress label Oct 27, 2025

ArkaSaha30 requested review from ahrtr and ivanvc October 27, 2025 18:49

k8s-ci-robot added size/XL and removed size/L labels Oct 28, 2025

ArkaSaha30 force-pushed the auto-cert-provider branch from 276d55e to 9886706 Compare October 28, 2025 16:10

ArkaSaha30 changed the title ~~Add AutoProvider interface functions~~ Add AutoProvider certificate manager Oct 28, 2025

ahrtr reviewed Oct 28, 2025

View reviewed changes

ivanvc reviewed Oct 28, 2025

View reviewed changes

ArkaSaha30 force-pushed the auto-cert-provider branch 4 times, most recently from 9d90c9e to e2c11fd Compare October 30, 2025 09:56

neolit123 reviewed Nov 26, 2025

View reviewed changes

pkg/certificate/auto/provider.go Show resolved Hide resolved

ArkaSaha30 force-pushed the auto-cert-provider branch from e2c11fd to ba83138 Compare November 30, 2025 15:29

Add initial scaffolding for AutoProvider

3aada97

This commit will add the initial scaffolding for AutoProvider. Signed-off-by: ArkaSaha30 <[email protected]>

ArkaSaha30 force-pushed the auto-cert-provider branch from ba83138 to 6027cab Compare November 30, 2025 15:30

ArkaSaha30 added 3 commits November 30, 2025 21:39

Implement AutoProvider interface

4d02745

Signed-off-by: ArkaSaha30 <[email protected]>

Add reconciler logic for AutoProvider

6a66942

Signed-off-by: ArkaSaha30 <[email protected]>

Add e2e tests for AutoProvider

ccf14e3

Signed-off-by: ArkaSaha30 <[email protected]>

ArkaSaha30 force-pushed the auto-cert-provider branch from 6027cab to ccf14e3 Compare November 30, 2025 16:10

ArkaSaha30 requested review from ahrtr, ivanvc and neolit123 November 30, 2025 16:11

neolit123 reviewed Dec 1, 2025

View reviewed changes

palnabarun reviewed Dec 2, 2025

View reviewed changes

	// tls field is present but spec is empty
	// The TLS field is present but spec is empty

	// save the user defined AutoConfig so that it can be returned from GetCertificateConfig
	// Save the user defined AutoConfig so that it can be returned from GetCertificateConfig

	// use certPEM when CA file is not found or empty
	// Use certPEM when CA file is not found or empty

	return fmt.Errorf("generated keypair invalid: %w", err)
	return fmt.Errorf("generated keypair is invalid: %w", err)


		var userAutoConfig *interfaces.Config

		func (ac *Provider) EnsureCertificateSecret(ctx context.Context, secretName, namespace string,

Add AutoProvider certificate manager #227

Are you sure you want to change the base?

Add AutoProvider certificate manager #227

Conversation

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

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 23, 2025

Uh oh!

ahrtr commented Oct 28, 2025

Uh oh!

ahrtr commented Oct 28, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArkaSaha30 Nov 9, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivanvc 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!

ArkaSaha30 commented Oct 30, 2025

Uh oh!

ivanvc commented Nov 9, 2025

Uh oh!

Uh oh!

neolit123 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahrtr commented Dec 2, 2025

Uh oh!

Reviewers

Assignees

Add `AutoProvider` certificate manager #227

Add `AutoProvider` certificate manager #227

ArkaSaha30 commented Oct 23, 2025 •

edited

Loading

ArkaSaha30 Nov 9, 2025 •

edited

Loading