Skip to content
Open
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require (
github.com/mikefarah/yq/v3 v3.0.0-20201202084205-8846255d1c37
github.com/onsi/ginkgo/v2 v2.31.0
github.com/openshift/api v0.0.0-20260204104751-e09e5a4ebcd0
github.com/openshift/library-go v0.0.0-20260204111611-b7d4fa0e292a
github.com/operator-framework/api v0.44.0
github.com/operator-framework/operator-lifecycle-manager v0.0.0-00010101000000-000000000000
github.com/operator-framework/operator-registry v1.72.0
Expand Down Expand Up @@ -155,7 +156,6 @@ require (
github.com/opencontainers/image-spec v1.1.1 // indirect
github.com/opencontainers/runtime-spec v1.3.0 // indirect
github.com/openshift/client-go v0.0.0-20260108185524-48f4ccfc4e13 // indirect
github.com/openshift/library-go v0.0.0-20260204111611-b7d4fa0e292a // indirect
github.com/otiai10/copy v1.14.1 // indirect
github.com/otiai10/mint v1.6.3 // indirect
github.com/pkg/errors v0.9.1 // indirect
Expand Down
6 changes: 6 additions & 0 deletions pkg/manifests/csv.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ spec:
verbs:
- get
- list
- apiGroups:
- "config.openshift.io"
resources:
- apiservers
verbs:
- get
Comment on lines +72 to +77

Copy link
Copy Markdown

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:

#!/bin/bash
# Description: Confirm the ServiceAccount that runs the APIServer watcher has
# list/watch on config.openshift.io/apiservers, not only get.

echo "Controller APIServer watch/read sites:"
rg -n -C5 --type=go 'configv1\.APIServer|Watches\(.*APIServer|For\(.*APIServer'

echo
echo "RBAC rules mentioning config.openshift.io/apiservers:"
rg -n -C6 --iglob '*.yaml' --iglob '*.yml' 'config\.openshift\.io|apiservers|olm-operator-serviceaccount'

Repository: openshift/operator-framework-olm

Length of output: 50389


🏁 Script executed:

echo "=== pkg/manifests/csv.yaml (lines 60-85) ==="
sed -n '60,85p' pkg/manifests/csv.yaml

echo -e "\n=== staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml (lines 58-85) ==="
sed -n '58,85p' staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml

echo -e "\n=== staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml (lines 255-280) ==="
sed -n '255,280p' staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml

echo -e "\n=== Check which ServiceAccount runs the manager in controller.go ==="
rg -n 'ServiceAccountName|ServiceAccount' pkg/package-server-manager/controller.go -A 2 -B 2 | head -30

Repository: openshift/operator-framework-olm

Length of output: 2768


Add list and watch verbs to the APIServer RBAC rules. The added rules grant only get, which works for direct reads, but the controller registers a watch on configv1.APIServer (controller.go:201) via a SharedInformer. Informers require list and watch permissions to function.

Update all three RBAC definitions to include these verbs:

  • pkg/manifests/csv.yaml#L72-L77
  • staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml#L70-L75
  • staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml#L267-L272
Example fix for pkg/manifests/csv.yaml
            - apiGroups:
                - "config.openshift.io"
              resources:
                - apiservers
              verbs:
                - get
                - list
                - watch
📍 Affects 3 files
  • pkg/manifests/csv.yaml#L72-L77 (this comment)
  • staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml#L70-L75
  • staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml#L267-L272
🤖 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 `@pkg/manifests/csv.yaml` around lines 72 - 77, The APIServer RBAC rules across
three files currently only grant the `get` verb, but the controller registers a
watch on configv1.APIServer via SharedInformer which requires `list` and `watch`
permissions. Update the verbs array in all three locations to include these
missing permissions. In pkg/manifests/csv.yaml at lines 72-77, add `list` and
`watch` verbs to the apiservers rule. Apply the same change to
staging/operator-lifecycle-manager/deploy/chart/templates/_packageserver.clusterserviceversion.yaml
at lines 70-75, and to
staging/operator-lifecycle-manager/deploy/upstream/quickstart/olm.yaml at lines
267-272. Each location has the same RBAC block with apiGroups containing
"config.openshift.io" and resources containing "apiservers" that needs to be
updated.

deployments:
- name: packageserver
spec:
Expand Down
9 changes: 5 additions & 4 deletions pkg/package-server-manager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,18 @@ func getTopologyModeFromInfra(infra *configv1.Infrastructure) bool {
// resource matches that of the codified defaults and high availability configurations, where
// codified defaults are defined by the csv returned by the manifests.NewPackageServerCSV
// function.
func ensureCSV(log logr.Logger, image string, interval string, csv *olmv1alpha1.ClusterServiceVersion, highlyAvailableMode bool) (bool, error) {
func ensureCSV(log logr.Logger, image string, interval string, flags []string, csv *olmv1alpha1.ClusterServiceVersion, highlyAvailableMode bool) (bool, error) {

flags := []string{}
runFlags := []string{}
if interval != "" {
flags = append(flags, "--interval", interval)
runFlags = append(runFlags, "--interval", interval)
}
runFlags = append(runFlags, flags...)
expectedCSV, err := manifests.NewPackageServerCSV(
manifests.WithName(csv.Name),
manifests.WithNamespace(csv.Namespace),
manifests.WithImage(image),
manifests.WithRunFlags(flags),
manifests.WithRunFlags(runFlags),
)
if err != nil {
return false, err
Expand Down
42 changes: 39 additions & 3 deletions pkg/package-server-manager/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,20 @@ package controllers
import (
"context"
"fmt"
"strings"
"sync"

"github.com/go-logr/logr"

configv1 "github.com/openshift/api/config/v1"
libcrypto "github.com/openshift/library-go/pkg/crypto"
"github.com/openshift/operator-framework-olm/pkg/manifests"
olmapiserver "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/apiserver"
olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

Expand Down Expand Up @@ -85,6 +89,22 @@ func (r *PackageServerCSVReconciler) Reconcile(ctx context.Context, req ctrl.Req
flags = append(flags, "--interval", r.Interval)
}

var apiServer configv1.APIServer
if err := r.Client.Get(ctx, types.NamespacedName{Name: "cluster"}, &apiServer); err != nil {
if !apierrors.IsNotFound(err) {
return ctrl.Result{}, err
}
} else {
minVersion, cipherSuites := olmapiserver.GetSecurityProfileConfig(apiServer.Spec.TLSSecurityProfile)
minVersionStr := libcrypto.TLSVersionToNameOrDie(minVersion)
cipherSuitesStr := strings.Join(libcrypto.CipherSuitesToNamesOrDie(cipherSuites), ",")
flags = append(flags,
"--tls-min-version", minVersionStr,
"--tls-cipher-suites", cipherSuitesStr,
)
log.Info("applying cluster TLS security profile to packageserver", "minVersion", minVersionStr, "cipherSuites", cipherSuitesStr)
}

required, err := manifests.NewPackageServerCSV(
manifests.WithName(r.Name),
manifests.WithNamespace(r.Namespace),
Expand All @@ -96,7 +116,7 @@ func (r *PackageServerCSVReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, err
}
res, err := controllerutil.CreateOrUpdate(ctx, r.Client, required, func() error {
return reconcileCSV(r.Log, r.Image, r.Interval, required, highAvailabilityMode)
return reconcileCSV(r.Log, r.Image, r.Interval, flags, required, highAvailabilityMode)
})

log.Info("reconciliation result", "res", res)
Expand All @@ -123,12 +143,12 @@ func ensureRBAC(client client.Client, ctx context.Context, namespace string, log
return nil
}

func reconcileCSV(log logr.Logger, image string, interval string, csv *olmv1alpha1.ClusterServiceVersion, highAvailabilityMode bool) error {
func reconcileCSV(log logr.Logger, image string, interval string, flags []string, csv *olmv1alpha1.ClusterServiceVersion, highAvailabilityMode bool) error {
if csv.ObjectMeta.CreationTimestamp.IsZero() {
log.Info("attempting to create the packageserver csv")
}

modified, err := ensureCSV(log, image, interval, csv, highAvailabilityMode)
modified, err := ensureCSV(log, image, interval, flags, csv, highAvailabilityMode)
if err != nil {
return fmt.Errorf("error ensuring CSV: %v", err)
}
Expand Down Expand Up @@ -158,10 +178,26 @@ func (r *PackageServerCSVReconciler) infrastructureHandler(_ context.Context, ob
}
}

func (r *PackageServerCSVReconciler) apiServerHandler(_ context.Context, obj client.Object) []reconcile.Request {
if obj.GetName() != "cluster" {
return nil
}
r.Log.Info("requeueing the packageserver deployment after encountering APIServer TLS profile change")
return []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: r.Name,
Namespace: r.Namespace,
},
},
}
}

// SetupWithManager sets up the controller with the Manager.
func (r *PackageServerCSVReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&olmv1alpha1.ClusterServiceVersion{}).
Watches(&configv1.Infrastructure{}, handler.EnqueueRequestsFromMapFunc(r.infrastructureHandler)).
Watches(&configv1.APIServer{}, handler.EnqueueRequestsFromMapFunc(r.apiServerHandler)).
Complete(r)
}
2 changes: 1 addition & 1 deletion pkg/package-server-manager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestEnsureCSV(t *testing.T) {
tc := tc

t.Run(tc.name, func(t *testing.T) {
gotBool, gotErr := ensureCSV(logger, image, interval, tc.inputCSV, tc.highlyAvailable)
gotBool, gotErr := ensureCSV(logger, image, interval, nil, tc.inputCSV, tc.highlyAvailable)
require.EqualValues(t, tc.want.expectedBool, gotBool)
require.EqualValues(t, tc.want.expectedErr, gotErr)
require.EqualValues(t, tc.inputCSV.Spec, tc.expectedCSV.Spec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ spec:
verbs:
- get
- list
- apiGroups:
- "config.openshift.io"
resources:
- apiservers
resourceNames:
- cluster
verbs:
- get
deployments:
- name: packageserver
{{- include "packageserver.deployment-spec" . | nindent 8 }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@ spec:
verbs:
- get
- list
- apiGroups:
- "config.openshift.io"
resources:
- apiservers
resourceNames:
- cluster
verbs:
- get
deployments:
- name: packageserver
spec:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ import (
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/util/workqueue"

configv1client "github.com/openshift/client-go/config/clientset/versioned"
libcrypto "github.com/openshift/library-go/pkg/crypto"
operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client"
olminformers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/informers/externalversions"
olmapiserver "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/apiserver"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/openshiftconfig"
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
"github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apiserver"
genericpackageserver "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apiserver/generic"
"github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/provider"
apidiscovery "k8s.io/client-go/discovery"
)

const DefaultWakeupInterval = 12 * time.Hour
Expand Down Expand Up @@ -202,19 +207,13 @@ func (o *PackageServerOptions) Run(ctx context.Context) error {
string(genericfeatures.UnauthenticatedHTTP2DOSMitigation): true,
})

// Grab the config for the API server
config, err := o.Config(ctx)
if err != nil {
return err
}
config.GenericConfig.EnableMetrics = true

// Set up the client config
// Set up the client config before calling Config() so it can be used to
// apply the cluster TLS security profile to the serving options.
var clientConfig *rest.Config
var err error
if len(o.Kubeconfig) > 0 {
loadingRules := &clientcmd.ClientConfigLoadingRules{ExplicitPath: o.Kubeconfig}
loader := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{})

clientConfig, err = loader.ClientConfig()
} else {
clientConfig, err = rest.InClusterConfig()
Expand All @@ -223,6 +222,23 @@ func (o *PackageServerOptions) Run(ctx context.Context) error {
return fmt.Errorf("unable to construct lister client config: %v", err)
}

// If --tls-min-version was not supplied (e.g. no PSM-injected flags yet), fall
// back to a direct GET of the cluster APIServer CR so the packageserver still
// honours the cluster TLS security profile on first boot or during upgrades.
if o.SecureServing.MinTLSVersion == "" {
if err := applyClusterTLSProfile(ctx, clientConfig, o.SecureServing); err != nil {
return fmt.Errorf("failed to apply cluster TLS profile to serving options: %w", err)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// Grab the config for the API server
var config *apiserver.Config
config, err = o.Config(ctx)
if err != nil {
return err
}
config.GenericConfig.EnableMetrics = true

kubeClient, err := kubernetes.NewForConfig(clientConfig)
if err != nil {
return fmt.Errorf("unable to construct lister client: %v", err)
Expand Down Expand Up @@ -326,3 +342,58 @@ func (op *Operator) syncOLMConfig(obj interface{}) error {

return nil
}

// applyClusterTLSProfile fetches the cluster-wide APIServer TLS security profile
// and applies it to the SecureServingOptions. It is a no-op on non-OpenShift clusters.
// This is the fallback path used when --tls-min-version is not provided via flags
// (i.e. before the PSM has had a chance to inject them).
func applyClusterTLSProfile(ctx context.Context, config *rest.Config, serving *genericoptions.SecureServingOptionsWithLoopback) error {
const lookupTimeout = 30 * time.Second
profileCtx, cancel := context.WithTimeout(ctx, lookupTimeout)
defer cancel()

profileConfig := rest.CopyConfig(config)
if profileConfig.Timeout == 0 || profileConfig.Timeout > lookupTimeout {
profileConfig.Timeout = lookupTimeout
}

kubeClient, err := kubernetes.NewForConfig(profileConfig)
if err != nil {
return fmt.Errorf("failed to create kubernetes client: %w", err)
}
cfgClient, err := configv1client.NewForConfig(profileConfig)
if err != nil {
return fmt.Errorf("failed to create config client: %w", err)
}
return applyClusterTLSProfileWithClients(profileCtx, kubeClient.Discovery(), cfgClient, serving)
}

// applyClusterTLSProfileWithClients is the testable core of applyClusterTLSProfile.
// It applies the cluster-wide APIServer TLS security profile to the SecureServingOptions,
// but only for fields not already set by explicit flags (--tls-min-version / --tls-cipher-suites).
// It is a no-op on non-OpenShift clusters.
func applyClusterTLSProfileWithClients(ctx context.Context, discovery apidiscovery.DiscoveryInterface, cfgClient configv1client.Interface, serving *genericoptions.SecureServingOptionsWithLoopback) error {
available, err := openshiftconfig.IsAPIAvailable(discovery)
if err != nil {
return fmt.Errorf("failed to check OpenShift config API: %w", err)
}
if !available {
return nil
}

apiServer, err := cfgClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get APIServer config: %w", err)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

minVersion, cipherSuites := olmapiserver.GetSecurityProfileConfig(apiServer.Spec.TLSSecurityProfile)
// Only override fields not already set by explicit flags.
if serving.MinTLSVersion == "" {
serving.MinTLSVersion = libcrypto.TLSVersionToNameOrDie(minVersion)
}
if len(serving.CipherSuites) == 0 {
serving.CipherSuites = libcrypto.CipherSuitesToNamesOrDie(cipherSuites)
}
log.Infof("Applying cluster TLS security profile: minVersion=%s cipherSuites=%v", serving.MinTLSVersion, serving.CipherSuites)
return nil
}
Loading