Skip to content

NO-JIRA: Revert "Merge pull request #1170 from logonoff/http2" #1194

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
logonoff wants to merge 1 commit into
base: main
Choose a base branch
from
Open

NO-JIRA: Revert "Merge pull request #1170 from logonoff/http2" #1194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 0 additions & 22 deletions manifests/03-rbac-role-ns-openshift-ingress-operator.yaml
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions manifests/04-rbac-rolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,6 @@ subjects:
---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: console-operator
namespace: openshift-ingress-operator
annotations:
include.release.openshift.io/hypershift: "true"
include.release.openshift.io/ibm-cloud-managed: "true"
include.release.openshift.io/self-managed-high-availability: "true"
include.release.openshift.io/single-node-developer: "true"
capability.openshift.io/name: Console
roleRef:
kind: Role
name: console-operator
apiGroup: rbac.authorization.k8s.io
subjects:
- kind: ServiceAccount
name: console-operator
namespace: openshift-console-operator
---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: console-operator
namespace: openshift-config
Expand Down
2 changes: 0 additions & 2 deletions pkg/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const (
ConsoleContainerPort = 443
ConsoleContainerPortName = "https"
ConsoleContainerTargetPort = 8443
ConsoleHTTP2CertSecretName = "console-http2-cert"
ConsoleServingCertName = "console-serving-cert"
DefaultIngressCertConfigMapName = "default-ingress-cert"
DownloadsPort = 8080
Expand Down Expand Up @@ -55,7 +54,6 @@ const (
// ingress instance named "default" is the OOTB ingresscontroller
// this is an implicit stable API
DefaultIngressController = "default"
IngressCASecretName = "router-ca"
IngressControllerNamespace = "openshift-ingress-operator"

OAuthClientName = OpenShiftConsoleName
Expand Down
90 changes: 7 additions & 83 deletions pkg/console/controllers/route/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
coreinformersv1 "k8s.io/client-go/informers/core/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/klog/v2"

Expand All @@ -29,7 +28,6 @@ import (
routesinformersv1 "github.com/openshift/client-go/route/informers/externalversions/route/v1"
routev1listers "github.com/openshift/client-go/route/listers/route/v1"
"github.com/openshift/library-go/pkg/controller/factory"
libcrypto "github.com/openshift/library-go/pkg/crypto"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/v1helpers"
"github.com/openshift/library-go/pkg/route/routeapihelpers"
Expand All @@ -48,13 +46,10 @@ type RouteSyncController struct {
operatorClient v1helpers.OperatorClient
routeClient routeclientv1.RoutesGetter
routeLister routev1listers.RouteLister
secretClient corev1client.SecretsGetter
operatorConfigLister operatorv1listers.ConsoleLister
ingressConfigLister configlistersv1.IngressLister
ingressControllerLister operatorv1listers.IngressControllerLister
secretLister corev1listers.SecretLister
consoleSecretLister corev1listers.SecretLister
ingressCASecretLister corev1listers.SecretLister
infrastructureConfigLister configlistersv1.InfrastructureLister
clusterVersionLister configlistersv1.ClusterVersionLister
}
Expand All @@ -67,14 +62,11 @@ func NewRouteSyncController(
// clients
operatorClient v1helpers.OperatorClient,
routev1Client routeclientv1.RoutesGetter,
secretClient corev1client.SecretsGetter,
// informers
operatorConfigInformer v1.ConsoleInformer,
ingressControllerInformer v1.IngressControllerInformer,
secretInformer coreinformersv1.SecretInformer,
consoleSecretInformer coreinformersv1.SecretInformer,
routeInformer routesinformersv1.RouteInformer,
ingressCASecretInformer coreinformersv1.SecretInformer,
// events
recorder events.Recorder,
) factory.Controller {
Expand All @@ -87,21 +79,14 @@ func NewRouteSyncController(
ingressControllerLister: ingressControllerInformer.Lister(),
routeClient: routev1Client,
routeLister: routeInformer.Lister(),
secretClient: secretClient,
secretLister: secretInformer.Lister(),
infrastructureConfigLister: configInformer.Config().V1().Infrastructures().Lister(),
clusterVersionLister: configInformer.Config().V1().ClusterVersions().Lister(),
}
if consoleSecretInformer != nil {
ctrl.consoleSecretLister = consoleSecretInformer.Lister()
}
if ingressCASecretInformer != nil {
ctrl.ingressCASecretLister = ingressCASecretInformer.Lister()
}

configV1Informers := configInformer.Config().V1()

controllerBuilder := factory.New().
return factory.New().
WithFilteredEventsInformers( // configs
util.IncludeNamesFilter(api.ConfigResourceName),
configV1Informers.Consoles().Informer(),
Expand All @@ -114,22 +99,7 @@ func NewRouteSyncController(
ingressControllerInformer.Informer(),
).WithInformers( // routes — watch all routes in namespace for additional route discovery
routeInformer.Informer(),
).ResyncEvery(time.Minute).WithSync(ctrl.Sync)

if consoleSecretInformer != nil {
controllerBuilder = controllerBuilder.WithFilteredEventsInformers(
util.IncludeNamesFilter(api.ConsoleHTTP2CertSecretName),
consoleSecretInformer.Informer(),
)
}
if ingressCASecretInformer != nil {
controllerBuilder = controllerBuilder.WithFilteredEventsInformers(
util.IncludeNamesFilter(api.IngressCASecretName),
ingressCASecretInformer.Informer(),
)
}

return controllerBuilder.
).ResyncEvery(time.Minute).WithSync(ctrl.Sync).
ToController(fmt.Sprintf("%sRouteController", strings.Title(routeName)), recorder.WithComponentSuffix(fmt.Sprintf("%s-route-controller", routeName)))
}

Expand All @@ -151,11 +121,6 @@ func (c *RouteSyncController) Sync(ctx context.Context, controllerContext factor
if err = c.removeRoute(ctx, routesub.GetCustomRouteName(c.routeName)); err != nil {
return err
}
if c.routeName == api.OpenShiftConsoleRouteName {
if err := c.removeHTTP2CertSecret(ctx); err != nil {
return err
}
}
return c.removeRoute(ctx, c.routeName)
default:
return fmt.Errorf("unknown state: %v", updatedOperatorConfig.Spec.ManagementState)
Expand Down Expand Up @@ -240,17 +205,6 @@ func (c *RouteSyncController) removeRoute(ctx context.Context, routeName string)
return err
}

func (c *RouteSyncController) removeHTTP2CertSecret(ctx context.Context) error {
if c.secretClient == nil {
return nil
}
err := c.secretClient.Secrets(api.OpenShiftConsoleNamespace).Delete(ctx, api.ConsoleHTTP2CertSecretName, metav1.DeleteOptions{})
if apierrors.IsNotFound(err) {
return nil
}
return err
}

func (c *RouteSyncController) SyncDefaultRoute(ctx context.Context, routeConfig *routesub.RouteConfig, ingressConfig *configv1.Ingress, controllerContext factory.SyncContext) (*routev1.Route, string, error) {
customTLSSecret, configErr := c.GetDefaultRouteTLSSecret(ctx, routeConfig)
if configErr != nil {
Expand All @@ -261,16 +215,6 @@ func (c *RouteSyncController) SyncDefaultRoute(ctx context.Context, routeConfig
return nil, "InvalidCustomTLSSecret", secretValidationErr
}

if customTLSCert == nil && c.routeName == api.OpenShiftConsoleRouteName && c.secretClient != nil && c.consoleSecretLister != nil {
hostname := routesub.GetDefaultRouteHost(c.routeName, ingressConfig)
ca := c.loadIngressCA()
http2Cert, err := routesub.EnsureHTTP2Cert(ctx, c.secretClient, c.consoleSecretLister, hostname, ca)
if err != nil {
return nil, "FailedHTTP2Cert", err
}
customTLSCert = http2Cert
}

requiredDefaultRoute := routeConfig.DefaultRoute(customTLSCert, ingressConfig)

defaultRoute, _, defaultRouteError := routesub.ApplyRoute(c.routeClient, requiredDefaultRoute)
Expand Down Expand Up @@ -380,31 +324,11 @@ func (c *RouteSyncController) ValidateCustomRouteConfig(ctx context.Context, rou
return nil
}

// loadIngressCA attempts to load the ingress controller's CA for signing the
// HTTP/2 cert. Returns nil if unavailable (falls back to self-signed).
func (c *RouteSyncController) loadIngressCA() *libcrypto.CA {
if c.ingressCASecretLister == nil {
return nil
}
secret, err := c.ingressCASecretLister.Secrets(api.IngressControllerNamespace).Get(api.IngressCASecretName)
if err != nil {
if apierrors.IsNotFound(err) {
klog.V(4).Infof("ingress CA secret %s/%s not found, falling back to self-signed HTTP/2 cert", api.IngressControllerNamespace, api.IngressCASecretName)
} else {
klog.Warningf("failed to get ingress CA secret %s/%s, falling back to self-signed HTTP/2 cert: %v", api.IngressControllerNamespace, api.IngressCASecretName, err)
}
return nil
}
ca, err := routesub.LoadCAFromSecret(secret)
if err != nil {
klog.Warningf("failed to parse ingress CA secret %s/%s, falling back to self-signed HTTP/2 cert: %v", api.IngressControllerNamespace, api.IngressCASecretName, err)
return nil
}
return ca
}

// ValidateCustomCertSecret validates the TLS certificate and key in a Secret.
// Returns the parsed cert/key pair, or nil if the secret is nil.
// Validate secret that holds custom TLS certificate and key.
// Secret has to contain `tls.crt` and `tls.key` data keys
// where the certificate and key are stored and both need
// to be in valid format.
// Return the custom TLS certificate and key
Comment on lines +327 to +331

@coderabbitai coderabbitai Bot Jul 21, 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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="pkg/console/controllers/route/controller.go"

echo "--- line count ---"
wc -l "$FILE"

echo "--- relevant slice ---"
sed -n '315,345p' "$FILE" | cat -n

Repository: openshift/console-operator

Length of output: 1921

Start the Godoc comment with ValidateCustomCertSecret. The exported function’s doc comment should begin with its identifier so GoDoc picks it up.

🤖 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/console/controllers/route/controller.go` around lines 327 - 331, Update
the Godoc comment for the exported custom certificate validation function to
begin exactly with ValidateCustomCertSecret, while preserving the existing
explanation of the required tls.crt and tls.key data and validation behavior.

Source: Coding guidelines

func ValidateCustomCertSecret(customCertSecret *corev1.Secret) (*routesub.CustomTLSCert, error) {
if customCertSecret == nil {
return nil, nil
Expand Down
131 changes: 0 additions & 131 deletions pkg/console/controllers/route/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,18 @@
package route

import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"testing"
"time"

"github.com/go-test/deep"

// k8s
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubefake "k8s.io/client-go/kubernetes/fake"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"

// console-operator
"github.com/openshift/console-operator/pkg/api"
routesub "github.com/openshift/console-operator/pkg/console/subresource/route"
"github.com/openshift/library-go/pkg/crypto"
)

const (
Expand Down Expand Up @@ -208,126 +200,3 @@ func TestValidateCustomCertSecret(t *testing.T) {
})
}
}

func TestRemoveHTTP2CertSecret(t *testing.T) {
t.Run("secret exists and is deleted", func(t *testing.T) {
existingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: api.ConsoleHTTP2CertSecretName,
Namespace: api.OpenShiftConsoleNamespace,
},
Type: corev1.SecretTypeTLS,
}
fakeClient := kubefake.NewSimpleClientset(existingSecret)
ctrl := &RouteSyncController{secretClient: fakeClient.CoreV1()}

err := ctrl.removeHTTP2CertSecret(context.Background())
if err != nil {
t.Fatalf("expected no error, got: %v", err)
}

_, getErr := fakeClient.CoreV1().Secrets(api.OpenShiftConsoleNamespace).Get(context.Background(), api.ConsoleHTTP2CertSecretName, metav1.GetOptions{})
if getErr == nil {
t.Error("expected secret to be deleted")
}
})

t.Run("secret does not exist", func(t *testing.T) {
fakeClient := kubefake.NewSimpleClientset()
ctrl := &RouteSyncController{secretClient: fakeClient.CoreV1()}

err := ctrl.removeHTTP2CertSecret(context.Background())
if err != nil {
t.Fatalf("expected no error for non-existent secret, got: %v", err)
}
})

t.Run("secretClient is nil", func(t *testing.T) {
ctrl := &RouteSyncController{secretClient: nil}

err := ctrl.removeHTTP2CertSecret(context.Background())
if err != nil {
t.Fatalf("expected no error when secretClient is nil, got: %v", err)
}
})
}

func TestLoadIngressCA(t *testing.T) {
t.Run("ingressCASecretLister is nil", func(t *testing.T) {
ctrl := &RouteSyncController{ingressCASecretLister: nil}
ca := ctrl.loadIngressCA()
if ca != nil {
t.Error("expected nil CA when lister is nil")
}
})

t.Run("secret not found", func(t *testing.T) {
lister := newControllerFakeSecretLister(t)
ctrl := &RouteSyncController{ingressCASecretLister: lister}
ca := ctrl.loadIngressCA()
if ca != nil {
t.Error("expected nil CA when secret not found")
}
})

t.Run("secret has invalid PEM", func(t *testing.T) {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: api.IngressCASecretName,
Namespace: api.IngressControllerNamespace,
},
Data: map[string][]byte{
"tls.crt": []byte("not-valid-pem"),
"tls.key": []byte("not-valid-pem"),
},
}
lister := newControllerFakeSecretLister(t, secret)
ctrl := &RouteSyncController{ingressCASecretLister: lister}
ca := ctrl.loadIngressCA()
if ca != nil {
t.Error("expected nil CA for invalid PEM")
}
})

t.Run("secret has valid CA", func(t *testing.T) {
caConfig, err := crypto.MakeSelfSignedCAConfigForDuration("test-ingress-ca", 24*time.Hour)
if err != nil {
t.Fatalf("failed to create test CA: %v", err)
}
certPEM, keyPEM, err := caConfig.GetPEMBytes()
if err != nil {
t.Fatalf("failed to get PEM bytes: %v", err)
}

secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: api.IngressCASecretName,
Namespace: api.IngressControllerNamespace,
},
Data: map[string][]byte{
"tls.crt": certPEM,
"tls.key": keyPEM,
},
}
lister := newControllerFakeSecretLister(t, secret)
ctrl := &RouteSyncController{ingressCASecretLister: lister}
ca := ctrl.loadIngressCA()
if ca == nil {
t.Fatal("expected non-nil CA")
}
if ca.Config.Certs[0].Subject.CommonName != "test-ingress-ca" {
t.Errorf("expected CN=test-ingress-ca, got CN=%s", ca.Config.Certs[0].Subject.CommonName)
}
})
}

func newControllerFakeSecretLister(t *testing.T, secrets ...*corev1.Secret) corev1listers.SecretLister {
t.Helper()
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
for _, s := range secrets {
if err := indexer.Add(s.DeepCopy()); err != nil {
t.Fatalf("failed to add secret to indexer: %v", err)
}
}
return corev1listers.NewSecretLister(indexer)
}
Loading