Skip to content

CNTRLPLANE-3213: Enable configurable PKI for managed certificate rotation #2958

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.

Sign up for GitHub

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

Open
sanchezl wants to merge 2 commits into
base: master
Choose a base branch
from
Open

CNTRLPLANE-3213: Enable configurable PKI for managed certificate rotation #2958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b4dc98
vendor: bump library-go
sanchezl Apr 10, 2026
75e8e1d
CNTRLPLANE-2014: Enable configurable PKI for managed certificate rota…
sanchezl Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ require (
require (
github.com/openshift/api v0.0.0-20260320151444-324a1bcb9f55
github.com/openshift/client-go v0.0.0-20260320040014-4b5fc2cdad98
github.com/openshift/library-go v0.0.0-20260303171201-5d9eb6295ff6
github.com/openshift/library-go v0.0.0-20260409165127-c57da2bf5720
github.com/openshift/machine-config-operator v0.0.1-0.20250724162154-ab14c8e2843b
k8s.io/apiextensions-apiserver v0.35.2
k8s.io/apiserver v0.35.2
k8s.io/client-go v0.35.2
sigs.k8s.io/controller-tools v0.20.1
)
Expand Down Expand Up @@ -154,7 +155,6 @@ require (
google.golang.org/genproto/googleapis/api v0.0.0-20260226221140-a57be14db171 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260226221140-a57be14db171 // indirect
gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
k8s.io/apiserver v0.35.2 // indirect
k8s.io/gengo/v2 v2.0.0-20251215205346-5ee0d033ba5b // indirect
k8s.io/kms v0.35.2 // indirect
k8s.io/kube-aggregator v0.35.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ github.com/openshift/build-machinery-go v0.0.0-20251023084048-5d77c1a5e5af h1:Ui
github.com/openshift/build-machinery-go v0.0.0-20251023084048-5d77c1a5e5af/go.mod h1:8jcm8UPtg2mCAsxfqKil1xrmRMI3a+XU2TZ9fF8A7TE=
github.com/openshift/client-go v0.0.0-20260320040014-4b5fc2cdad98 h1:Ssuo/zELWqb7pFCwzB3QGEA4QeLW948hL2AhWq2SWjs=
github.com/openshift/client-go v0.0.0-20260320040014-4b5fc2cdad98/go.mod h1:8O4jIKdcr5YR9FFQEeokYoCplCUN+j9hZj4u/2yg0As=
github.com/openshift/library-go v0.0.0-20260303171201-5d9eb6295ff6 h1:xjqy0OolrFdJ+ofI/aD0+2k9+MSk5anP5dXifFt539Q=
github.com/openshift/library-go v0.0.0-20260303171201-5d9eb6295ff6/go.mod h1:D797O/ssKTNglbrGchjIguFq+DbyRYdeds5w4/VTrKM=
github.com/openshift/library-go v0.0.0-20260409165127-c57da2bf5720 h1:ljxETzJc/vDgMJlaSB8GRryBHZOAIENA2Wo5arD8avM=
github.com/openshift/library-go v0.0.0-20260409165127-c57da2bf5720/go.mod h1:3bi4pLpYRdVd1aEhsHfRTJkwxwPLfRZ+ZePn3RmJd2k=
github.com/openshift/machine-config-operator v0.0.1-0.20250724162154-ab14c8e2843b h1:LvoFr/2IEj0BWy7mKBdR7ueAHpMJGju1EkEIZrXa+DM=
github.com/openshift/machine-config-operator v0.0.1-0.20250724162154-ab14c8e2843b/go.mod h1:UL1OVkRAUkB4aaFZrLlSvuY0jayfdF+o+ZxKiKaaArc=
github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/add_networkconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ import (
"github.com/openshift/cluster-network-operator/pkg/controller/infrastructureconfig"
"github.com/openshift/cluster-network-operator/pkg/controller/ingressconfig"
"github.com/openshift/cluster-network-operator/pkg/controller/operconfig"
"github.com/openshift/cluster-network-operator/pkg/controller/pki"
"github.com/openshift/cluster-network-operator/pkg/controller/proxyconfig"
signer "github.com/openshift/cluster-network-operator/pkg/controller/signer"
)

func init() {
// AddToManagerFuncs is a list of functions to create controllers and add them to a manager.
// Note: pki.Add is called separately in operator.go with additional parameters.
AddToManagerFuncs = append(AddToManagerFuncs,
pki.Add,
egress_router.Add,
proxyconfig.Add,
operconfig.Add,
Expand Down
90 changes: 36 additions & 54 deletions pkg/controller/pki/pki_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,28 @@ package pki

import (
"context"
"crypto/x509"
"fmt"
"log"
"reflect"
"strings"
"time"

netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/network/v1"
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
"github.com/openshift/cluster-network-operator/pkg/controller/eventrecorder"
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
"github.com/openshift/cluster-network-operator/pkg/names"

features "github.com/openshift/api/features"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/crypto"
"github.com/openshift/library-go/pkg/operator/certrotation"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/pki"
"github.com/pkg/errors"

features "github.com/openshift/api/features"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
crclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -46,8 +45,8 @@ const (
)

// Add attaches our control loop to the manager and watches for PKI objects
func Add(mgr manager.Manager, status *statusmanager.StatusManager, _ cnoclient.Client, featureGates featuregates.FeatureGate) error {
r, err := newPKIReconciler(mgr, status, featureGates)
func Add(mgr manager.Manager, status *statusmanager.StatusManager, featureGates featuregates.FeatureGate, pkiProfileProvider pki.PKIProfileProvider) error {
r, err := newPKIReconciler(mgr, status, featureGates, pkiProfileProvider)
if err != nil {
return err
}
Expand Down Expand Up @@ -76,11 +75,15 @@ type PKIReconciler struct {
status *statusmanager.StatusManager

// one PKI per CA
pkis map[types.NamespacedName]*pki
pkis map[types.NamespacedName]*operatorPKI
// For computing status
pkiErrs map[types.NamespacedName]error

certDuration time.Duration

// pkiProfileProvider is non-nil when the ConfigurablePKI feature gate is
// enabled. It provides the PKI profile for certificate key configuration.
pkiProfileProvider pki.PKIProfileProvider
}

// The periodic resync interval.
Expand All @@ -90,7 +93,7 @@ var ResyncPeriod = 5 * time.Minute

// newPKIReconciler creates the toplevel reconciler that receives PKI updates
// and configures the CertRotationController accordingly
func newPKIReconciler(mgr manager.Manager, status *statusmanager.StatusManager, featureGates featuregates.FeatureGate) (reconcile.Reconciler, error) {
func newPKIReconciler(mgr manager.Manager, status *statusmanager.StatusManager, featureGates featuregates.FeatureGate, pkiProfileProvider pki.PKIProfileProvider) (reconcile.Reconciler, error) {
clientset, err := kubernetes.NewForConfig(mgr.GetConfig())
if err != nil {
return nil, err
Expand All @@ -106,9 +109,10 @@ func newPKIReconciler(mgr manager.Manager, status *statusmanager.StatusManager,
status: status,
clientset: clientset,

pkis: map[types.NamespacedName]*pki{},
pkiErrs: map[types.NamespacedName]error{},
certDuration: certDuration,
pkis: map[types.NamespacedName]*operatorPKI{},
pkiErrs: map[types.NamespacedName]error{},
certDuration: certDuration,
pkiProfileProvider: pkiProfileProvider,
}, nil
}

Expand Down Expand Up @@ -139,7 +143,7 @@ func (r *PKIReconciler) Reconcile(ctx context.Context, request reconcile.Request
}
}
if existing == nil {
existing, err = newPKI(obj, r.clientset, r.mgr, r.certDuration)
existing, err = newPKI(obj, r.clientset, r.mgr, r.certDuration, r.pkiProfileProvider)
if err != nil {
log.Println(err)
r.pkiErrs[request.NamespacedName] =
Expand Down Expand Up @@ -179,15 +183,15 @@ func (r *PKIReconciler) setStatus() {
}
}

// pki is the internal type that represents a single PKI CRD. It manages the
// operatorPKI is the internal type that represents a single PKI CRD. It manages the
// business of reconciling the certificate objects
type pki struct {
type operatorPKI struct {
spec netopv1.OperatorPKISpec
controller factory.Controller
}

// newPKI creates a CertRotationController for the supplied configuration
func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr manager.Manager, certDuration time.Duration) (*pki, error) {
func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr manager.Manager, certDuration time.Duration, pkiProfileProvider pki.PKIProfileProvider) (*operatorPKI, error) {
spec := config.Spec

// Ugly: the existing cache + informers used as part of the controller-manager
Expand All @@ -209,12 +213,14 @@ func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr ma
AdditionalAnnotations: certrotation.AdditionalAnnotations{
JiraComponent: names.ClusterNetworkOperatorJiraComponent,
},
Validity: 10 * OneYear,
Refresh: 9 * OneYear,
Informer: inf.Core().V1().Secrets(),
Lister: inf.Core().V1().Secrets().Lister(),
Client: clientset.CoreV1(),
EventRecorder: &eventrecorder.LoggingRecorder{},
CertificateName: "network.signer",
PKIProfileProvider: pkiProfileProvider,
Comment on lines +216 to +217
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Compute CertificateName from the OperatorPKI resource name.

Hardcoding "network.signer" and "network.peer" makes every OperatorPKI resolve the same PKI profile. That breaks per-resource algorithm selection for resources like ovn vs network-node-identity.

Proposed fix 
-			CertificateName:    "network.signer",
+			CertificateName:    fmt.Sprintf("network.%s-signer", config.Name),
 			PKIProfileProvider: pkiProfileProvider,
@@
-			CertificateName:    "network.peer",
+			CertificateName:    fmt.Sprintf("network.%s-peer", config.Name),
 			PKIProfileProvider: pkiProfileProvider,

Also applies to: 242-243

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/controller/pki/pki_controller.go` around lines 216 - 217, CertificateName
is being hardcoded to "network.signer"/"network.peer" which forces all
OperatorPKI resources to use the same profile; instead compute CertificateName
from the OperatorPKI resource name (e.g., use the OperatorPKI object's Name to
produce "<resourceName>.signer" or "<resourceName>.peer") when constructing the
PKI profile (refer to the CertificateName field assignment alongside
PKIProfileProvider in the code that builds the profile for OperatorPKI). Update
both places where "network.signer"/"network.peer" are set so they derive from
the OperatorPKI resource name rather than using the literal strings.

Validity: 10 * OneYear,
Refresh: 9 * OneYear,
Informer: inf.Core().V1().Secrets(),
Lister: inf.Core().V1().Secrets().Lister(),
Client: clientset.CoreV1(),
EventRecorder: &eventrecorder.LoggingRecorder{},
},
certrotation.CABundleConfigMap{
Namespace: config.Namespace,
Expand All @@ -233,15 +239,13 @@ func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr ma
AdditionalAnnotations: certrotation.AdditionalAnnotations{
JiraComponent: names.ClusterNetworkOperatorJiraComponent,
},
Validity: certDuration,
Refresh: certDuration / 2,
CertCreator: &certrotation.ServingRotation{
CertificateName: "network.peer",
PKIProfileProvider: pkiProfileProvider,
Validity: certDuration,
Refresh: certDuration / 2,
CertCreator: &certrotation.PeerRotation{
Hostnames: func() []string { return []string{spec.TargetCert.CommonName} },

// Force the certificate to also be client
CertificateExtensionFn: []crypto.CertificateExtensionFunc{
toClientCert,
},
UserInfo: &user.DefaultInfo{Name: spec.TargetCert.CommonName},
},
Lister: inf.Core().V1().Secrets().Lister(),
Informer: inf.Core().V1().Secrets(),
Expand All @@ -252,7 +256,7 @@ func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr ma
nil,
)

out := &pki{
out := &operatorPKI{
controller: cont,
}
config.Spec.DeepCopyInto(&out.spec)
Expand All @@ -265,29 +269,7 @@ func newPKI(config *netopv1.OperatorPKI, clientset *kubernetes.Clientset, mgr ma
}

// sync causes the underlying cert controller to try and reconcile
func (p *pki) sync() error {
func (p *operatorPKI) sync() error {
runOnceCtx := context.WithValue(context.Background(), certrotation.RunOnceContextKey, true) //nolint:staticcheck
return p.controller.Sync(runOnceCtx, nil)
}

// toClientCert is a certificate "decorator" that adds ClientAuth to the
// list of ExtendedKeyUsages. This allows the generated certificate to be
// used for both client and server auth.
func toClientCert(cert *x509.Certificate) error {
if len(cert.ExtKeyUsage) == 0 {
return nil
}

found := false
for _, u := range cert.ExtKeyUsage {
if u == x509.ExtKeyUsageClientAuth {
found = true
break
}
}

if !found {
cert.ExtKeyUsage = append(cert.ExtKeyUsage, x509.ExtKeyUsageClientAuth)
}
return nil
}
30 changes: 21 additions & 9 deletions pkg/controller/signer/signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package signer
import (
c "crypto"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"errors"
Expand All @@ -21,8 +20,6 @@ func newCertificateTemplate(certReq *x509.CertificateRequest, certDuration time.
template := &x509.Certificate{
Subject: certReq.Subject,

SignatureAlgorithm: x509.SHA512WithRSA,

NotBefore: time.Now().Add(-1 * time.Second),
NotAfter: time.Now().Add(certDuration),
SerialNumber: big.NewInt(serialNumber),
Expand Down Expand Up @@ -69,13 +66,28 @@ func decodeCertificate(pemBytes []byte) (*x509.Certificate, error) {
return x509.ParseCertificate(block.Bytes)
}

func decodePrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) {
func decodePrivateKey(pemBytes []byte) (c.Signer, error) {
block, _ := pem.Decode(pemBytes)
if block == nil || block.Type != "RSA PRIVATE KEY" {
fmt.Println(block.Type)
err := errors.New("PEM block type must be RSA PRIVATE KEY")
return nil, err
if block == nil {
return nil, errors.New("no PEM block found in private key data")
}

return x509.ParsePKCS1PrivateKey(block.Bytes)
switch block.Type {
case "RSA PRIVATE KEY":
return x509.ParsePKCS1PrivateKey(block.Bytes)
case "PRIVATE KEY":
key, err := x509.ParsePKCS8PrivateKey(block.Bytes)
if err != nil {
return nil, fmt.Errorf("failed to parse PKCS8 private key: %w", err)
}
signer, ok := key.(c.Signer)
if !ok {
return nil, fmt.Errorf("parsed private key does not implement crypto.Signer")
}
return signer, nil
case "EC PRIVATE KEY":
return x509.ParseECPrivateKey(block.Bytes)
default:
return nil, fmt.Errorf("unsupported PEM block type: %s", block.Type)
}
}
26 changes: 26 additions & 0 deletions pkg/operator/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"time"

configv1 "github.com/openshift/api/config/v1"
features "github.com/openshift/api/features"
configclient "github.com/openshift/client-go/config/clientset/versioned"
configinformers "github.com/openshift/client-go/config/informers/externalversions"
"github.com/openshift/library-go/pkg/controller/controllercmd"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/loglevel"
"github.com/openshift/library-go/pkg/operator/management"
"github.com/openshift/library-go/pkg/operator/managementstatecontroller"
pkipkg "github.com/openshift/library-go/pkg/pki"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -27,6 +29,7 @@ import (
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
"github.com/openshift/cluster-network-operator/pkg/controller"
"github.com/openshift/cluster-network-operator/pkg/controller/connectivitycheck"
pkictrl "github.com/openshift/cluster-network-operator/pkg/controller/pki"
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
"github.com/openshift/cluster-network-operator/pkg/hypershift"
"github.com/openshift/cluster-network-operator/pkg/names"
Expand Down Expand Up @@ -140,12 +143,35 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
return err
}

// Set up PKI profile provider if ConfigurablePKI is enabled.
// Register the PKI informer on the existing configInformers factory,
// re-start to pick it up (idempotent for already-running informers),
// and wait for cache sync before controllers start reconciling.
var pkiProfileProvider pkipkg.PKIProfileProvider
if featureGates.Enabled(features.FeatureGateConfigurablePKI) {
configInformers.Config().V1alpha1().PKIs().Informer()
configInformers.Start(wait.NeverStop)
for t, synced := range configInformers.WaitForCacheSync(wait.NeverStop) {
if !synced {
return fmt.Errorf("failed to sync config informer for %v", t)
}
}
Comment on lines +146 to +158
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read the relevant section of the operator.go file
cat -n pkg/operator/operator.go | sed -n '140,170p'

Repository: openshift/cluster-network-operator

Length of output: 1500

🏁 Script executed:

# Check imports to understand available context/wait utilities
head -n 50 pkg/operator/operator.go | cat -n

Repository: openshift/cluster-network-operator

Length of output: 2361

🏁 Script executed:

# Search for other WaitForCacheSync usages in the codebase to understand patterns
rg 'WaitForCacheSync' -A 3 -B 1

Repository: openshift/cluster-network-operator

Length of output: 37944

🏁 Script executed:

# Look at the broader function context around lines 146-158 to check available variables
cat -n pkg/operator/operator.go | sed -n '100,180p'

Repository: openshift/cluster-network-operator

Length of output: 3832

🏁 Script executed:

# Find the function signature that contains this code block
sed -n '50,100p' pkg/operator/operator.go | cat -n

Repository: openshift/cluster-network-operator

Length of output: 2544

🏁 Script executed:

# Search for the function name containing line 146-162
rg -B 50 'Set up PKI profile provider if ConfigurablePKI' pkg/operator/operator.go | head -70

Repository: openshift/cluster-network-operator

Length of output: 1808

Bound the PKI informer sync wait.

Line 154 uses WaitForCacheSync(wait.NeverStop), which can hang operator startup indefinitely if the PKIs informer never syncs. This turns configuration or RBAC issues into a permanently hung operator rather than fast failure. Apply a timeout consistent with the feature gate initialization pattern used earlier (line 137).

Suggested fix 
 	if featureGates.Enabled(features.FeatureGateConfigurablePKI) {
 		configInformers.Config().V1alpha1().PKIs().Informer()
 		configInformers.Start(wait.NeverStop)
-		for t, synced := range configInformers.WaitForCacheSync(wait.NeverStop) {
+		syncCtx, cancel := context.WithTimeout(ctx, time.Minute)
+		defer cancel()
+		for t, synced := range configInformers.WaitForCacheSync(syncCtx.Done()) {
 			if !synced {
-				return fmt.Errorf("failed to sync config informer for %v", t)
+				return fmt.Errorf("failed to sync config informer for %v before timeout", t)
 			}
 		}
 		pkiProfileProvider = pkipkg.NewClusterPKIProfileProvider(
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/operator.go` around lines 146 - 158, The PKI informer sync uses
wait.NeverStop which can hang startup; change it to use a bounded stop channel
with a timeout (matching the feature-gate init pattern used earlier) so the
configInformers.Start/WaitForCacheSync call fails fast on timeout. Concretely,
create a timeout channel (e.g., stopCh := make(chan struct{}); start a goroutine
that closes stopCh after the same timeout duration used earlier, pass stopCh to
configInformers.Start and configInformers.WaitForCacheSync, and return an error
if any informer (from the map returned by configInformers.WaitForCacheSync)
failed to sync before the timeout; keep the logic around
featureGates.Enabled(features.FeatureGateConfigurablePKI) and the existing error
message in fmt.Errorf.

pkiProfileProvider = pkipkg.NewClusterPKIProfileProvider(
configInformers.Config().V1alpha1().PKIs().Lister(),
)
}

// Add controller-runtime controllers
klog.Info("Adding controller-runtime controllers")
if err := controller.AddToManager(o.manager, o.StatusManager, o.client, featureGates); err != nil {
return fmt.Errorf("failed to add controllers to manager: %w", err)
}

// Add PKI controller separately — it needs the PKI profile provider
if err := pkictrl.Add(o.manager, o.StatusManager, featureGates, pkiProfileProvider); err != nil {
return fmt.Errorf("failed to add pki controller: %w", err)
}

// Initialize individual (non-controller-runtime) controllers

// logLevelController reacts to changes in the operator spec loglevel
Expand Down
Loading