diff --git a/deploy/chart/templates/_packageserver.clusterserviceversion.yaml b/deploy/chart/templates/_packageserver.clusterserviceversion.yaml index 5739dff461..38658adbb2 100644 --- a/deploy/chart/templates/_packageserver.clusterserviceversion.yaml +++ b/deploy/chart/templates/_packageserver.clusterserviceversion.yaml @@ -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 }} diff --git a/deploy/upstream/quickstart/olm.yaml b/deploy/upstream/quickstart/olm.yaml index 718ff64d55..1b8c3d0970 100644 --- a/deploy/upstream/quickstart/olm.yaml +++ b/deploy/upstream/quickstart/olm.yaml @@ -264,6 +264,14 @@ spec: verbs: - get - list + - apiGroups: + - "config.openshift.io" + resources: + - apiservers + resourceNames: + - cluster + verbs: + - get deployments: - name: packageserver spec: diff --git a/pkg/package-server/server/server.go b/pkg/package-server/server/server.go index 81ea882c78..a6e8a3216c 100644 --- a/pkg/package-server/server/server.go +++ b/pkg/package-server/server/server.go @@ -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 @@ -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() @@ -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) + } + } + + // 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) @@ -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) + } + + 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 +} diff --git a/pkg/package-server/server/server_test.go b/pkg/package-server/server/server_test.go new file mode 100644 index 0000000000..27a7e1fefb --- /dev/null +++ b/pkg/package-server/server/server_test.go @@ -0,0 +1,132 @@ +package server + +import ( + "context" + "testing" + + apiconfigv1 "github.com/openshift/api/config/v1" + configfake "github.com/openshift/client-go/config/clientset/versioned/fake" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + genericoptions "k8s.io/apiserver/pkg/server/options" + fakediscovery "k8s.io/client-go/discovery/fake" + k8sfake "k8s.io/client-go/kubernetes/fake" +) + +// clusterAPIServer returns a minimal APIServer singleton with the given TLS profile. +func clusterAPIServer(profile *apiconfigv1.TLSSecurityProfile) *apiconfigv1.APIServer { + return &apiconfigv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: apiconfigv1.APIServerSpec{TLSSecurityProfile: profile}, + } +} + +func newServing() *genericoptions.SecureServingOptionsWithLoopback { + return genericoptions.NewSecureServingOptions().WithLoopback() +} + +// nonOpenShiftDiscovery returns a fake discovery that advertises only core k8s +// groups — no config.openshift.io — simulating a vanilla Kubernetes cluster. +func nonOpenShiftDiscovery() *fakediscovery.FakeDiscovery { + k8sClient := k8sfake.NewSimpleClientset() + disc := k8sClient.Discovery().(*fakediscovery.FakeDiscovery) + // Set a non-empty resource list so ServerGroups doesn't return an empty + // list (which ServerSupportsVersion treats as "all supported"). + disc.Resources = []*metav1.APIResourceList{ + {GroupVersion: "v1"}, + } + return disc +} + +// TestApplyClusterTLSProfileWithClients_NonOpenShift verifies that the function +// is a no-op when the OpenShift config API is not present (vanilla Kubernetes). +func TestApplyClusterTLSProfileWithClients_NonOpenShift(t *testing.T) { + cfgClient := configfake.NewSimpleClientset() + serving := newServing() + + err := applyClusterTLSProfileWithClients(context.Background(), nonOpenShiftDiscovery(), cfgClient, serving) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if serving.MinTLSVersion != "" { + t.Errorf("expected MinTLSVersion to be unset, got %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) != 0 { + t.Errorf("expected CipherSuites to be unset, got %v", serving.CipherSuites) + } +} + +// TestApplyClusterTLSProfileWithClients_IntermediateProfile verifies that the +// Intermediate TLS profile populates MinTLSVersion and CipherSuites. +func TestApplyClusterTLSProfileWithClients_IntermediateProfile(t *testing.T) { + apiServer := clusterAPIServer(&apiconfigv1.TLSSecurityProfile{ + Type: apiconfigv1.TLSProfileIntermediateType, + }) + cfgClient := configfake.NewSimpleClientset(apiServer) + serving := newServing() + + err := applyClusterTLSProfileWithClients(context.Background(), cfgClient.Discovery(), cfgClient, serving) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if serving.MinTLSVersion == "" { + t.Error("expected MinTLSVersion to be set for Intermediate profile") + } + if len(serving.CipherSuites) == 0 { + t.Error("expected CipherSuites to be set for Intermediate profile") + } +} + +// TestApplyClusterTLSProfileWithClients_ModernProfile verifies that the Modern +// profile sets MinTLSVersion to VersionTLS13. +func TestApplyClusterTLSProfileWithClients_ModernProfile(t *testing.T) { + apiServer := clusterAPIServer(&apiconfigv1.TLSSecurityProfile{ + Type: apiconfigv1.TLSProfileModernType, + }) + cfgClient := configfake.NewSimpleClientset(apiServer) + serving := newServing() + + err := applyClusterTLSProfileWithClients(context.Background(), cfgClient.Discovery(), cfgClient, serving) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if serving.MinTLSVersion != "VersionTLS13" { + t.Errorf("expected MinTLSVersion=VersionTLS13 for Modern profile, got %q", serving.MinTLSVersion) + } +} + +// TestApplyClusterTLSProfileWithClients_FlagsTakePrecedence verifies that +// explicitly set flags are not overwritten by the cluster profile. +func TestApplyClusterTLSProfileWithClients_FlagsTakePrecedence(t *testing.T) { + apiServer := clusterAPIServer(&apiconfigv1.TLSSecurityProfile{ + Type: apiconfigv1.TLSProfileModernType, + }) + cfgClient := configfake.NewSimpleClientset(apiServer) + serving := newServing() + // Simulate user-supplied flags. + serving.MinTLSVersion = "VersionTLS12" + serving.CipherSuites = []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"} + + err := applyClusterTLSProfileWithClients(context.Background(), cfgClient.Discovery(), cfgClient, serving) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if serving.MinTLSVersion != "VersionTLS12" { + t.Errorf("user-supplied MinTLSVersion should not be overwritten, got %q", serving.MinTLSVersion) + } + if len(serving.CipherSuites) != 1 || serving.CipherSuites[0] != "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" { + t.Errorf("user-supplied CipherSuites should not be overwritten, got %v", serving.CipherSuites) + } +} + +// TestApplyClusterTLSProfileWithClients_MissingAPIServerCR verifies that a +// missing singleton APIServer CR propagates as an error (fail-closed). +func TestApplyClusterTLSProfileWithClients_MissingAPIServerCR(t *testing.T) { + // config client advertises the API group but has no APIServer object + cfgClient := configfake.NewSimpleClientset() + serving := newServing() + + err := applyClusterTLSProfileWithClients(context.Background(), cfgClient.Discovery(), cfgClient, serving) + if err == nil { + t.Fatal("expected an error when the APIServer CR is missing, got nil") + } +}