Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions bindata/assets/controller-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: v1
kind: ConfigMap
metadata:
namespace: openshift-service-ca
name: service-ca-controller-config
data:
controller-config.yaml: |
apiVersion: operator.openshift.io/v1alpha1
kind: GenericOperatorConfig
8 changes: 8 additions & 0 deletions bindata/assets/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ spec:
image: ${IMAGE}
imagePullPolicy: IfNotPresent
command: ["service-ca-operator", "controller"]
args:
- --config=/var/run/configmaps/config/controller-config.yaml
- --terminate-on-files=/var/run/configmaps/config/controller-config.yaml
ports:
- containerPort: 8443
securityContext:
Expand All @@ -45,13 +48,18 @@ spec:
name: signing-key
- mountPath: /var/run/configmaps/signing-cabundle
name: signing-cabundle
- mountPath: /var/run/configmaps/config
name: config
volumes:
- name: signing-key
secret:
secretName: signing-key
- name: signing-cabundle
configMap:
name: signing-cabundle
- name: config
configMap:
name: service-ca-controller-config
Comment on lines +51 to +62

@coderabbitai coderabbitai Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Apply the required workload hardening controls.

This workload still lacks explicit allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, capabilities.drop: ["ALL"], CPU/memory limits, and liveness/readiness probes. Also set automountServiceAccountToken: false unless this controller requires in-cluster API credentials, and ensure its namespace has a restrictive NetworkPolicy.

As per path instructions, Kubernetes workloads require these security controls, limits, probes, and least-privilege defaults.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/assets/deployment.yaml` around lines 48 - 59, Harden the Kubernetes
workload in deployment.yaml by adding allowPrivilegeEscalation: false,
readOnlyRootFilesystem: true, capabilities.drop: ["ALL"], CPU and memory limits,
and liveness/readiness probes to the container specification. Set
automountServiceAccountToken: false unless the controller explicitly needs
in-cluster API credentials, and add or ensure a restrictive NetworkPolicy exists
for the workload’s namespace.

Source: Path instructions

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not in the scope of the PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ingvagabund Understood—this hardening work is outside the scope of the centralized TLS configuration change. I’ll treat it as a follow-up rather than a blocker for this PR. Would you like me to open a GitHub issue to track it?

nodeSelector:
node-role.kubernetes.io/master: ""
priorityClassName: "system-cluster-critical"
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ require (
k8s.io/kube-aggregator v0.35.1
k8s.io/kubernetes v1.35.1
k8s.io/utils v0.0.0-20251002143259-bc988d571ff4
sigs.k8s.io/yaml v1.6.0
)

require (
Expand Down Expand Up @@ -119,7 +120,6 @@ require (
sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96 // indirect
sigs.k8s.io/randfill v1.0.0 // indirect
sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect
sigs.k8s.io/yaml v1.6.0 // indirect
)

replace (
Expand Down
65 changes: 65 additions & 0 deletions pkg/operator/configobservation/configobserver_controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package configobservation

import (
"k8s.io/client-go/tools/cache"

configinformers "github.com/openshift/client-go/config/informers/externalversions"
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/operator/configobserver"
libgoapiserver "github.com/openshift/library-go/pkg/operator/configobserver/apiserver"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"
"github.com/openshift/library-go/pkg/operator/v1helpers"
)

// Listers combines the required listers for config observation
type Listers struct {
apiServerLister configlistersv1.APIServerLister
resourceSyncer resourcesynccontroller.ResourceSyncer
cacheSyncs []cache.InformerSynced
}

// APIServerLister returns the APIServer lister
func (l Listers) APIServerLister() configlistersv1.APIServerLister {
return l.apiServerLister
}

// ResourceSyncer returns the resource syncer
func (l Listers) ResourceSyncer() resourcesynccontroller.ResourceSyncer {
return l.resourceSyncer
}

// PreRunHasSynced returns the cache sync functions
func (l Listers) PreRunHasSynced() []cache.InformerSynced {
return l.cacheSyncs
}

// NewConfigObserverController creates a config observer controller for service-ca-operator
func NewConfigObserverController(
operatorClient v1helpers.OperatorClient,
configInformer configinformers.SharedInformerFactory,
resourceSyncer resourcesynccontroller.ResourceSyncer,
eventRecorder events.Recorder,
) factory.Controller {
informers := []factory.Informer{
operatorClient.Informer(),
configInformer.Config().V1().APIServers().Informer(),
}

return configobserver.NewConfigObserver(
"service-ca",
operatorClient,
eventRecorder,
Listers{
apiServerLister: configInformer.Config().V1().APIServers().Lister(),
resourceSyncer: resourceSyncer,
cacheSyncs: []cache.InformerSynced{
operatorClient.Informer().HasSynced,
configInformer.Config().V1().APIServers().Informer().HasSynced,
},
},
informers,
libgoapiserver.ObserveTLSSecurityProfile,
)
}
9 changes: 9 additions & 0 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/openshift/library-go/pkg/operator/status"
"github.com/openshift/library-go/pkg/operator/v1helpers"
"github.com/openshift/service-ca-operator/pkg/controller/api"
"github.com/openshift/service-ca-operator/pkg/operator/configobservation"
"github.com/openshift/service-ca-operator/pkg/operator/operatorclient"
)

Expand Down Expand Up @@ -107,6 +108,13 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
controllerContext.EventRecorder,
)

configObserver := configobservation.NewConfigObserverController(
operatorClient,
configInformers,
resourceSyncController,
controllerContext.EventRecorder,
)

klog.Infof("Fetching FeatureGates")
stopChan := ctx.Done()
featureGateAccessor := featuregates.NewFeatureGateAccess(
Expand Down Expand Up @@ -189,6 +197,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
operatorLogLevelController.Run,
clusterOperatorStatus.Run,
resourceSyncController.Run,
configObserver.Run,
} {
go controllerRunner(ctx, 1)
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,14 @@ func (c *serviceCAOperator) syncControllers(ctx context.Context, operatorConfig
return err
}

// Sync the controller config with TLS settings
configModified, err := c.manageControllerConfig(ctx, operatorConfig)
if err != nil {
return err
}

// Sync the controller.
_, err = c.manageDeployment(ctx, operatorConfig, needsDeploy || caModified, shouldScheduleOnWorkers(infrastructure))
_, err = c.manageDeployment(ctx, operatorConfig, needsDeploy || caModified || configModified, shouldScheduleOnWorkers(infrastructure))
if err != nil {
return err
}
Expand Down
42 changes: 42 additions & 0 deletions pkg/operator/sync_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@ import (
"bytes"
"context"
"crypto/x509"
"encoding/json"
"fmt"
"os"
"sort"
"strings"
"time"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/yaml"

"github.com/openshift/library-go/pkg/pki"
"github.com/openshift/service-ca-operator/pkg/operator/metrics"

Expand Down Expand Up @@ -230,6 +234,44 @@ func (c *serviceCAOperator) manageSignerCABundle(ctx context.Context, forceUpdat
return mod, err
}

func (c *serviceCAOperator) manageControllerConfig(ctx context.Context, operatorConfig *operatorv1.ServiceCA) (bool, error) {
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/controller-config.yaml"))

var observedConfig map[string]interface{}
if len(operatorConfig.Spec.ObservedConfig.Raw) > 0 {
if err := json.Unmarshal(operatorConfig.Spec.ObservedConfig.Raw, &observedConfig); err != nil {
return false, fmt.Errorf("failed to unmarshal observedConfig: %w", err)
}
}

config := map[string]interface{}{
"apiVersion": "operator.openshift.io/v1alpha1",
"kind": "GenericOperatorConfig",
}

servingInfo := map[string]interface{}{}
if minTLSVersion, found, err := unstructured.NestedString(observedConfig, "servingInfo", "minTLSVersion"); err == nil && found && minTLSVersion != "" {
servingInfo["minTLSVersion"] = minTLSVersion
}
if cipherSuites, found, err := unstructured.NestedStringSlice(observedConfig, "servingInfo", "cipherSuites"); err == nil && found && len(cipherSuites) > 0 {
servingInfo["cipherSuites"] = cipherSuites
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

if len(servingInfo) > 0 {
config["servingInfo"] = servingInfo
}

configYAML, err := yaml.Marshal(config)
if err != nil {
return false, fmt.Errorf("failed to marshal controller config: %w", err)
}

required.Data["controller-config.yaml"] = string(configYAML)

_, mod, err := resourceapply.ApplyConfigMap(ctx, c.corev1Client, c.eventRecorder, required)
return mod, err
}

func (c *serviceCAOperator) manageDeployment(ctx context.Context, options *operatorv1.ServiceCA, forceDeployment, runOnWorkers bool) (bool, error) {
required := resourceread.ReadDeploymentV1OrDie(bindata.MustAsset("assets/deployment.yaml"))
required.Spec.Template.Spec.Containers[0].Image = os.Getenv("CONTROLLER_IMAGE")
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading