diff --git a/manifests/03-rbac-role-cluster.yaml b/manifests/03-rbac-role-cluster.yaml index 1fe654cbcc..ab646e7c6a 100644 --- a/manifests/03-rbac-role-cluster.yaml +++ b/manifests/03-rbac-role-cluster.yaml @@ -18,6 +18,7 @@ rules: - config.openshift.io resources: - infrastructures + - ingresses verbs: - get - list diff --git a/manifests/03-rbac-role-ns-console.yaml b/manifests/03-rbac-role-ns-console.yaml index b850befa12..95778c68f8 100644 --- a/manifests/03-rbac-role-ns-console.yaml +++ b/manifests/03-rbac-role-ns-console.yaml @@ -63,6 +63,7 @@ rules: - route.openshift.io resources: - routes + - routes/custom-host verbs: - get - list diff --git a/manifests/03-rbac-role-ns-openshift-config.yaml b/manifests/03-rbac-role-ns-openshift-config.yaml index 8c55ad4ce5..45e04629e1 100644 --- a/manifests/03-rbac-role-ns-openshift-config.yaml +++ b/manifests/03-rbac-role-ns-openshift-config.yaml @@ -8,6 +8,7 @@ rules: - "" resources: - configmaps + - secrets verbs: - get - list diff --git a/pkg/api/api.go b/pkg/api/api.go index c7657d7634..ed863d1e27 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -14,6 +14,7 @@ const ( OpenShiftConsoleOperator = "console-operator" OpenShiftConsoleConfigMapName = "console-config" OpenShiftConsolePublicConfigMapName = "console-public" + OpenshiftConsoleCustomRouteName = "console-custom" ServiceCAConfigMapName = "service-ca" DefaultIngressCertConfigMapName = "default-ingress-cert" OpenShiftConsoleDeploymentName = OpenShiftConsoleName diff --git a/pkg/console/controllers/route/controller.go b/pkg/console/controllers/route/controller.go index 6cb224a85a..67630ba34e 100644 --- a/pkg/console/controllers/route/controller.go +++ b/pkg/console/controllers/route/controller.go @@ -4,12 +4,16 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/pem" "errors" "fmt" "net/http" + "strings" "time" // k8s + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -21,6 +25,7 @@ import ( // openshift operatorsv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" + configclientv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" operatorclientv1 "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1" v1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" routeclientv1 "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" @@ -31,7 +36,6 @@ import ( "github.com/openshift/console-operator/pkg/api" customerrors "github.com/openshift/console-operator/pkg/console/errors" "github.com/openshift/console-operator/pkg/console/status" - "github.com/openshift/console-operator/pkg/console/subresource/route" routesub "github.com/openshift/console-operator/pkg/console/subresource/route" ) @@ -43,8 +47,10 @@ const ( type RouteSyncController struct { // clients operatorConfigClient operatorclientv1.ConsoleInterface + ingressClient configclientv1.IngressInterface routeClient routeclientv1.RoutesGetter configMapClient coreclientv1.ConfigMapsGetter + secretClient coreclientv1.SecretsGetter // names targetNamespace string routeName string @@ -57,10 +63,13 @@ type RouteSyncController struct { } func NewRouteSyncController( + // top level config + configClient configclientv1.ConfigV1Interface, // clients operatorConfigClient operatorclientv1.ConsoleInterface, routev1Client routeclientv1.RoutesGetter, configMapClient coreclientv1.ConfigMapsGetter, + secretClient coreclientv1.SecretsGetter, // informers operatorConfigInformer v1.ConsoleInformer, routeInformer routesinformersv1.RouteInformer, @@ -74,8 +83,10 @@ func NewRouteSyncController( ) *RouteSyncController { ctrl := &RouteSyncController{ operatorConfigClient: operatorConfigClient, + ingressClient: configClient.Ingresses(), routeClient: routev1Client, configMapClient: configMapClient, + secretClient: secretClient, targetNamespace: targetNamespace, routeName: routeName, // events @@ -110,59 +121,184 @@ func (c *RouteSyncController) sync() error { return nil case operatorsv1.Removed: klog.V(4).Infoln("console is in a removed state: deleting route") - return c.removeRoute() + if err = c.removeRoute(api.OpenshiftConsoleCustomRouteName); err != nil { + return err + } + return c.removeRoute(api.OpenShiftConsoleName) default: return fmt.Errorf("unknown state: %v", operatorConfig.Spec.ManagementState) } updatedOperatorConfig := operatorConfig.DeepCopy() - _, _, errReason, err := c.SyncRoute(updatedOperatorConfig) - status.HandleProgressingOrDegraded(updatedOperatorConfig, "RouteSync", errReason, err) - status.SyncStatus(c.ctx, c.operatorConfigClient, updatedOperatorConfig) + defaultRouteErrReason, defaultRouteErr := c.SyncDefaultRoute(updatedOperatorConfig) + status.HandleProgressingOrDegraded(updatedOperatorConfig, "DefaultRouteSync", defaultRouteErrReason, defaultRouteErr) + if defaultRouteErr != nil { + return defaultRouteErr + } + + customRouteErrReason, customRouteErr := c.SyncCustomRoute(updatedOperatorConfig) + status.HandleProgressingOrDegraded(updatedOperatorConfig, "CustomRouteSync", customRouteErrReason, customRouteErr) + + return err +} +func (c *RouteSyncController) removeRoute(routeName string) error { + err := c.routeClient.Routes(c.targetNamespace).Delete(c.ctx, routeName, metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } return err } -func (c *RouteSyncController) removeRoute() error { - klog.V(2).Info("deleting console route") - defer klog.V(2).Info("finished deleting console route") - return c.routeClient.Routes(c.targetNamespace).Delete(c.ctx, route.Stub().Name, metav1.DeleteOptions{}) +func (c *RouteSyncController) SyncDefaultRoute(operatorConfig *operatorsv1.Console) (string, error) { + requiredDefaultRoute := routesub.DefaultRoute(operatorConfig) + + defaultRoute, _, defaultRouteError := routesub.ApplyRoute(c.routeClient, c.recorder, requiredDefaultRoute) + if defaultRouteError != nil { + return "FailedDefaultRouteApply", defaultRouteError + } + + if len(routesub.GetCanonicalHost(defaultRoute)) == 0 { + return "FailedDefaultRouteHost", customerrors.NewSyncError(fmt.Sprintf("default route is not available at canonical host %s", defaultRoute.Status.Ingress)) + } + + if !routesub.IsCustomRouteSet(operatorConfig) { + c.CheckRouteHealth(operatorConfig, defaultRoute) + } + + return "", defaultRouteError +} + +// Custom route sync needs to: +// 1. validate if the reference for secret with TLS certificate and key is defined in operator config(in case a non-openshift cluster domain is used) +// 2. if secret is defined, verify the TLS certificate and key +// 4. create the custom console route, if custom TLS certificate and key are defined use them +// 5. apply the custom route +func (c *RouteSyncController) SyncCustomRoute(operatorConfig *operatorsv1.Console) (string, error) { + if !routesub.IsCustomRouteSet(operatorConfig) { + return "", nil + } + + customSecret, configErr := c.ValidateCustomRouteConfig(operatorConfig) + if configErr != nil { + return "InvalidCustomRouteConfig", configErr + } + + customTLSCert, secretValidationErr := ValidateCustomCertSecret(customSecret) + if secretValidationErr != nil { + return "InvalidCustomTLSSecret", secretValidationErr + } + + requiredCustomRoute := routesub.CustomRoute(operatorConfig, customTLSCert) + customRoute, _, customRouteError := routesub.ApplyRoute(c.routeClient, c.recorder, requiredCustomRoute) + if customRouteError != nil { + return "FailedCustomRouteApply", customRouteError + } + + if len(routesub.GetCanonicalHost(customRoute)) == 0 { + return "FailedCustomRouteHost", customerrors.NewSyncError(fmt.Sprintf("custom route is not available at canonical host %s", customRoute.Status.Ingress)) + } + + if routesub.IsCustomRouteSet(operatorConfig) { + c.CheckRouteHealth(operatorConfig, customRoute) + } + + return "", customRouteError } -// apply route -// - be sure to test that we don't trigger an infinite loop by stomping on the -// default host name set by the server, or any other values. The ApplyRoute() -// logic will have to be sound. -// - update to ApplyRoute() once the logic is settled -func (c *RouteSyncController) SyncRoute(operatorConfig *operatorsv1.Console) (consoleRoute *routev1.Route, isNew bool, reason string, err error) { - // ensure we have a route. any error returned is a non-404 error - - rt, rtIsNew, rtErr := routesub.GetOrCreate(c.ctx, c.routeClient, routesub.DefaultRoute(operatorConfig)) - if rtErr != nil { - return nil, false, "FailedCreate", rtErr - } - // Check if the console is reachable - c.CheckRouteHealth(operatorConfig, rt) - - // we will not proceed until the route is valid. this eliminates complexity with the - // configmap, secret & oauth client as they can be certain they have a host if we pass this point. - host := routesub.GetCanonicalHost(rt) - if len(host) == 0 { - return nil, false, "FailedHost", customerrors.NewSyncError(fmt.Sprintf("route is not available at canonical host %s", rt.Status.Ingress)) - } - - if validatedRoute, changed := routesub.Validate(rt); changed { - // if validation changed the route, issue an update - if _, err := c.routeClient.Routes(api.TargetNamespace).Update(c.ctx, validatedRoute, metav1.UpdateOptions{}); err != nil { - // error is unexpected, this is a real error - return nil, false, "InvalidRouteCorrection", err +func (c *RouteSyncController) ValidateCustomRouteConfig(operatorConfig *operatorsv1.Console) (*corev1.Secret, error) { + // get ingress + ingress, err := c.ingressClient.Get(c.ctx, api.ConfigResourceName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + // Check if the custom hostname has cluster domain suffix, which indicates + // if a secret that contains TLS certificate and key needs to exist in the + // `openshift-config` namespace and referenced in the operator config. + // If the suffix matches the cluster domain, then the secret is optional. + // If the suffix doesn't matches the cluster domain, then the secret is mandatory. + if !routesub.IsCustomRouteSecretSet(operatorConfig) { + if !strings.HasSuffix(operatorConfig.Spec.Route.Hostname, ingress.Spec.Domain) { + return nil, fmt.Errorf("secret reference for custom route TLS secret is not defined") } - // abort sync, route changed, let it settle & retry - return nil, true, "InvalidRoute", customerrors.NewSyncError("route is invalid, correcting route state") + return nil, nil + } + + return c.secretClient.Secrets(api.OpenShiftConfigNamespace).Get(c.ctx, operatorConfig.Spec.Route.Secret.Name, metav1.GetOptions{}) +} + +// 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 +func ValidateCustomCertSecret(customCertSecret *corev1.Secret) (*routesub.CustomTLSCert, error) { + if customCertSecret == nil { + return nil, nil + } + if customCertSecret.Type != corev1.SecretTypeTLS { + return nil, fmt.Errorf("custom cert secret is not in %q type, instead uses %q type", corev1.SecretTypeTLS, customCertSecret.Type) + } + + customTLS := &routesub.CustomTLSCert{} + cert, certExist := customCertSecret.Data["tls.crt"] + if !certExist { + return nil, fmt.Errorf("custom cert secret data doesn't contain 'tls.crt' entry") } - // only return the route if it is valid with a host - return rt, rtIsNew, "", rtErr + + certificateVerifyErr := certificateVerifier(cert) + if certificateVerifyErr != nil { + return nil, fmt.Errorf("failed to verify custom certificate PEM: " + certificateVerifyErr.Error()) + } + customTLS.Certificate = string(cert) + + key, keyExist := customCertSecret.Data["tls.key"] + if !keyExist { + return nil, fmt.Errorf("custom cert secret data doesn't contain 'tls.key' entry") + } + + privateKeyVerifyErr := privateKeyVerifier(key) + if privateKeyVerifyErr != nil { + return nil, fmt.Errorf("failed to verify custom key PEM: " + privateKeyVerifyErr.Error()) + } + customTLS.Key = string(key) + + return customTLS, nil +} + +func certificateVerifier(customCert []byte) error { + block, _ := pem.Decode([]byte(customCert)) + if block == nil { + return fmt.Errorf("failed to decode certificate PEM") + } + certificate, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return err + } + now := time.Now() + if now.After(certificate.NotAfter) { + return fmt.Errorf("custom TLS certificate is expired") + } + if now.Before(certificate.NotBefore) { + return fmt.Errorf("custom TLS certificate is not valid yet") + } + return nil +} + +func privateKeyVerifier(customKey []byte) error { + block, _ := pem.Decode([]byte(customKey)) + if block == nil { + return fmt.Errorf("failed to decode key PEM") + } + if _, err := x509.ParsePKCS8PrivateKey(block.Bytes); err != nil { + if _, err = x509.ParsePKCS1PrivateKey(block.Bytes); err != nil { + if _, err = x509.ParseECPrivateKey(block.Bytes); err != nil { + return fmt.Errorf("block %s is not valid key PEM", block.Type) + } + } + } + return nil } func (c *RouteSyncController) Run(workers int, stopCh <-chan struct{}) { @@ -209,36 +345,39 @@ func (c *RouteSyncController) newEventHandler() cache.ResourceEventHandler { } } -func (c *RouteSyncController) CheckRouteHealth(opConfig *operatorsv1.Console, rt *routev1.Route) { +func (c *RouteSyncController) CheckRouteHealth(operatorConfig *operatorsv1.Console, route *routev1.Route) { status.HandleDegraded(func() (conf *operatorsv1.Console, prefix string, reason string, err error) { prefix = "RouteHealth" caPool, err := c.getCA() if err != nil { - return opConfig, prefix, "FailedLoadCA", fmt.Errorf("failed to read CA to check route health: %v", err) + return operatorConfig, prefix, "FailedLoadCA", fmt.Errorf("failed to read CA to check route health: %v", err) } client := clientWithCA(caPool) - url := "https://" + rt.Spec.Host + "/health" + if len(route.Spec.Host) == 0 { + return operatorConfig, prefix, "RouteHostError", fmt.Errorf("route does not have host specified") + } + url := "https://" + route.Spec.Host + "/health" req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { - return opConfig, prefix, "FailedRequest", fmt.Errorf("failed to build request to route (%s): %v", url, err) + return operatorConfig, prefix, "FailedRequest", fmt.Errorf("failed to build request to route (%s): %v", url, err) } resp, err := client.Do(req) if err != nil { - return opConfig, prefix, "FailedGet", fmt.Errorf("failed to GET route (%s): %v", url, err) + return operatorConfig, prefix, "FailedGet", fmt.Errorf("failed to GET route (%s): %v", url, err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return opConfig, prefix, "StatusError", fmt.Errorf("route not yet available, %s returns '%s'", url, resp.Status) + return operatorConfig, prefix, "StatusError", fmt.Errorf("route not yet available, %s returns '%s'", url, resp.Status) } - return opConfig, prefix, "", nil + return operatorConfig, prefix, "", nil }()) - status.HandleAvailable(opConfig, "Route", "FailedAdmittedIngress", func() error { - if !routesub.IsAdmitted(rt) { + status.HandleAvailable(operatorConfig, "Route", "FailedAdmittedIngress", func() error { + if !routesub.IsAdmitted(route) { return errors.New("console route is not admitted") } return nil diff --git a/pkg/console/controllers/route/controller_test.go b/pkg/console/controllers/route/controller_test.go new file mode 100644 index 0000000000..3bdcd5874c --- /dev/null +++ b/pkg/console/controllers/route/controller_test.go @@ -0,0 +1,202 @@ +package route + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "testing" + + "github.com/go-test/deep" + + // k8s + corev1 "k8s.io/api/core/v1" + + // console-operator + routesub "github.com/openshift/console-operator/pkg/console/subresource/route" +) + +const ( + validCertificate = `-----BEGIN CERTIFICATE----- +MIICRzCCAfGgAwIBAgIJAIydTIADd+yqMA0GCSqGSIb3DQEBCwUAMH4xCzAJBgNV +BAYTAkdCMQ8wDQYDVQQIDAZMb25kb24xDzANBgNVBAcMBkxvbmRvbjEYMBYGA1UE +CgwPR2xvYmFsIFNlY3VyaXR5MRYwFAYDVQQLDA1JVCBEZXBhcnRtZW50MRswGQYD +VQQDDBJ0ZXN0LWNlcnRpZmljYXRlLTIwIBcNMTcwNDI2MjMyNDU4WhgPMjExNzA0 +MDIyMzI0NThaMH4xCzAJBgNVBAYTAkdCMQ8wDQYDVQQIDAZMb25kb24xDzANBgNV +BAcMBkxvbmRvbjEYMBYGA1UECgwPR2xvYmFsIFNlY3VyaXR5MRYwFAYDVQQLDA1J +VCBEZXBhcnRtZW50MRswGQYDVQQDDBJ0ZXN0LWNlcnRpZmljYXRlLTIwXDANBgkq +hkiG9w0BAQEFAANLADBIAkEAuiRet28DV68Dk4A8eqCaqgXmymamUEjW/DxvIQqH +3lbhtm8BwSnS9wUAajSLSWiq3fci2RbRgaSPjUrnbOHCLQIDAQABo1AwTjAdBgNV +HQ4EFgQU0vhI4OPGEOqT+VAWwxdhVvcmgdIwHwYDVR0jBBgwFoAU0vhI4OPGEOqT ++VAWwxdhVvcmgdIwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAANBALNeJGDe +nV5cXbp9W1bC12Tc8nnNXn4ypLE2JTQAvyp51zoZ8hQoSnRVx/VCY55Yu+br8gQZ ++tW+O/PoE7B3tuY= +-----END CERTIFICATE-----` + validKey = `-----BEGIN RSA PRIVATE KEY----- +MIIBVgIBADANBgkqhkiG9w0BAQEFAASCAUAwggE8AgEAAkEAuiRet28DV68Dk4A8 +eqCaqgXmymamUEjW/DxvIQqH3lbhtm8BwSnS9wUAajSLSWiq3fci2RbRgaSPjUrn +bOHCLQIDAQABAkEArDR1g9IqD3aUImNikDgAngbzqpAokOGyMoxeavzpEaFOgCzi +gi7HF7yHRmZkUt8CzdEvnHSqRjFuaaB0gGA+AQIhAOc8Z1h8ElLRSqaZGgI3jCTp +Izx9HNY//U5NGrXD2+ttAiEAzhOqkqI4+nDab7FpiD7MXI6fO549mEXeVBPvPtsS +OcECIQCIfkpOm+ZBBpO3JXaJynoqK4gGI6ALA/ik6LSUiIlfPQIhAISjd9hlfZME +bDQT1r8Q3Gx+h9LRqQeHgPBQ3F5ylqqBAiBaJ0hkYvrIdWxNlcLqD3065bJpHQ4S +WQkuZUQN1M/Xvg== +-----END RSA PRIVATE KEY-----` + invalidCertificate = ` +-----BEGIN CERTIFICATE----- +MIIEBDCCAuygAwIBAgIDAjppMA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNVBAYTAlVT +WHPbqCRiOwY1nQ2pM714A5AuTHhdUDqB1O6gyHA43LL5Z/qHQF1hwFGPa4NrzQU6 +yuGnBXj8ytqU0CwIPX4WecigUCAkVDNx +-----END CERTIFICATE-----` + invalidKey = ` +-----BEGIN RSA PRIVATE KEY----- +MIIJKQIBAAKCAgEAw2jtDhf4X2W8182vtAiwXUk/Zr7mruiiFt3y4l7YRBXaazmI +eIWaEkvN9O90gL09Cx5jgq6mP1pjCzHsEFhnICziFd1r+M3cMeb4EAqwMZ84 +-----END RSA PRIVATE KEY-----` +) + +type certificateData struct { + keyPEM []byte + certificatePEM []byte + certificate *tls.Certificate +} + +func newCertificateData(certificatePEM string, keyPEM string) *certificateData { + certificate, err := tls.X509KeyPair([]byte(certificatePEM), []byte(keyPEM)) + if err != nil { + panic(fmt.Sprintf("Unable to initialize certificate: %v", err)) + } + certs, err := x509.ParseCertificates(certificate.Certificate[0]) + if err != nil { + panic(fmt.Sprintf("Unable to initialize certificate leaf: %v", err)) + } + certificate.Leaf = certs[0] + return &certificateData{ + keyPEM: []byte(keyPEM), + certificatePEM: []byte(certificatePEM), + certificate: &certificate, + } +} + +func TestValidateCustomCertSecret(t *testing.T) { + type args struct { + secret *corev1.Secret + } + type want struct { + customTLSCert *routesub.CustomTLSCert + err error + } + tests := []struct { + name string + args args + want want + }{ + { + name: "Test valid custom cert secret", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(validCertificate), + corev1.TLSPrivateKeyKey: []byte(validKey), + }, + }, + }, + want: want{ + customTLSCert: &routesub.CustomTLSCert{ + Certificate: validCertificate, + Key: validKey, + }, + err: nil, + }, + }, + { + name: "Test custom cert secret with invalid type", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeSSHAuth, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(validCertificate), + corev1.TLSPrivateKeyKey: []byte(validKey), + }, + }, + }, + want: want{ + customTLSCert: nil, + err: fmt.Errorf("custom cert secret is not in %q type, instead uses %q type", corev1.SecretTypeTLS, corev1.SecretTypeSSHAuth), + }, + }, + { + name: "Test custom cert secret missing 'tls.key' data field", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(validCertificate), + }, + }, + }, + want: want{ + customTLSCert: nil, + err: fmt.Errorf("custom cert secret data doesn't contain '%s' entry", corev1.TLSPrivateKeyKey), + }, + }, + { + name: "Test custom cert secret missing 'tls.crt' data field", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSPrivateKeyKey: []byte(validKey), + }, + }, + }, + want: want{ + customTLSCert: nil, + err: fmt.Errorf("custom cert secret data doesn't contain '%s' entry", corev1.TLSCertKey), + }, + }, + { + name: "Test custom cert secret with invalid TLS cert", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(invalidCertificate), + corev1.TLSPrivateKeyKey: []byte(validKey), + }, + }, + }, + want: want{ + customTLSCert: nil, + err: fmt.Errorf("failed to verify custom certificate PEM: asn1: syntax error: data truncated"), + }, + }, + { + name: "Test custom cert secret with invalid TLS key", + args: args{ + secret: &corev1.Secret{ + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(validCertificate), + corev1.TLSPrivateKeyKey: []byte(invalidKey), + }, + }, + }, + want: want{ + customTLSCert: nil, + err: fmt.Errorf("failed to verify custom key PEM: block RSA PRIVATE KEY is not valid key PEM"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + customTLSCert, err := ValidateCustomCertSecret(tt.args.secret) + if diff := deep.Equal(customTLSCert, tt.want.customTLSCert); diff != nil { + t.Error(diff) + } + if diff := deep.Equal(err, tt.want.err); diff != nil { + t.Error(diff) + } + }) + } +} diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index e2832bbb4f..4fa53baf5c 100644 --- a/pkg/console/operator/sync_v400.go +++ b/pkg/console/operator/sync_v400.go @@ -25,7 +25,6 @@ import ( "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" // operator - customerrors "github.com/openshift/console-operator/pkg/console/errors" "github.com/openshift/console-operator/pkg/console/metrics" "github.com/openshift/console-operator/pkg/console/status" @@ -47,7 +46,12 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, // track changes, may trigger ripples & update operator config or console config status toUpdate := false - rt, rtErr := co.routeClient.Routes(api.TargetNamespace).Get(co.ctx, api.OpenShiftConsoleName, metav1.GetOptions{}) + routeName := api.OpenShiftConsoleName + if routesub.IsCustomRouteSet(updatedOperatorConfig) { + routeName = api.OpenshiftConsoleCustomRouteName + } + + route, routeErr := co.routeClient.Routes(api.TargetNamespace).Get(co.ctx, routeName, metav1.GetOptions{}) // TODO: this controller is no longer responsible for syncing the route. // however, the route is essential for several of the components below. // - is it appropraite for SyncLoopRefresh InProgress to be used here? @@ -56,12 +60,12 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, // at the same resource. // - RouteSyncController is responsible for updates // - ConsoleOperatorController (future ConsoleDeploymentController) is responsible for reads only. - status.HandleProgressingOrDegraded(updatedOperatorConfig, "SyncLoopRefresh", "InProgress", rtErr) - if rtErr != nil { - return rtErr + status.HandleProgressingOrDegraded(updatedOperatorConfig, "SyncLoopRefresh", "InProgress", routeErr) + if routeErr != nil { + return routeErr } - cm, cmChanged, cmErrReason, cmErr := co.SyncConfigMap(set.Operator, set.Console, set.Infrastructure, rt) + cm, cmChanged, cmErrReason, cmErr := co.SyncConfigMap(set.Operator, set.Console, set.Infrastructure, route) toUpdate = toUpdate || cmChanged status.HandleProgressingOrDegraded(updatedOperatorConfig, "ConfigMapSync", cmErrReason, cmErr) if cmErr != nil { @@ -101,14 +105,14 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, return secErr } - oauthClient, oauthChanged, oauthErrReason, oauthErr := co.SyncOAuthClient(set.Operator, sec, rt) + oauthClient, oauthChanged, oauthErrReason, oauthErr := co.SyncOAuthClient(set.Operator, sec, route) toUpdate = toUpdate || oauthChanged status.HandleProgressingOrDegraded(updatedOperatorConfig, "OAuthClientSync", oauthErrReason, oauthErr) if oauthErr != nil { return oauthErr } - actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(set.Operator, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, rt, set.Proxy, customLogoCanMount) + actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment(set.Operator, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, set.Proxy, customLogoCanMount) toUpdate = toUpdate || depChanged status.HandleProgressingOrDegraded(updatedOperatorConfig, "DeploymentSync", depErrReason, depErr) if depErr != nil { @@ -150,7 +154,7 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, // if we survive the gauntlet, we need to update the console config with the // public hostname so that the world can know the console is ready to roll klog.V(4).Infoln("sync_v400: updating console status") - consoleURL := getConsoleURL(rt) + consoleURL := getConsoleURL(route) if consoleURL == "" { err := customerrors.NewSyncError("waiting on route host") @@ -214,11 +218,10 @@ func (co *consoleOperator) SyncDeployment( defaultIngressCertConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, - rt *routev1.Route, proxyConfig *configv1.Proxy, canMountCustomLogo bool) (consoleDeployment *appsv1.Deployment, changed bool, reason string, err error) { - requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, rt, proxyConfig, canMountCustomLogo) + requiredDeployment := deploymentsub.DefaultDeployment(operatorConfig, cm, serviceCAConfigMap, defaultIngressCertConfigMap, trustedCAConfigMap, sec, proxyConfig, canMountCustomLogo) expectedGeneration := getDeploymentGeneration(co) genChanged := operatorConfig.ObjectMeta.Generation != operatorConfig.Status.ObservedGeneration @@ -281,7 +284,7 @@ func (co *consoleOperator) SyncConfigMap( operatorConfig *operatorv1.Console, consoleConfig *configv1.Console, infrastructureConfig *configv1.Infrastructure, - consoleRoute *routev1.Route) (consoleConfigMap *corev1.ConfigMap, changed bool, reason string, err error) { + activeConsoleRoute *routev1.Route) (consoleConfigMap *corev1.ConfigMap, changed bool, reason string, err error) { managedConfig, mcErr := co.configMapClient.ConfigMaps(api.OpenShiftConfigManagedNamespace).Get(co.ctx, api.OpenShiftConsoleConfigMapName, metav1.GetOptions{}) if mcErr != nil && !apierrors.IsNotFound(mcErr) { @@ -303,7 +306,7 @@ func (co *consoleOperator) SyncConfigMap( return nil, false, "FailedGetMonitoringSharedConfig", mscErr } - defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, monitoringSharedConfig, infrastructureConfig, consoleRoute, useDefaultCAFile) + defaultConfigmap, _, err := configmapsub.DefaultConfigMap(operatorConfig, consoleConfig, managedConfig, monitoringSharedConfig, infrastructureConfig, activeConsoleRoute, useDefaultCAFile) if err != nil { return nil, false, "FailedConsoleConfigBuilder", err } diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index 72ce516d1b..bc93d43a5b 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -90,10 +90,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle options.FieldSelector = fields.OneTermEqualSelector("metadata.name", api.OAuthClientName).String() } - tweakListOptionsForRoute := func(options *metav1.ListOptions) { - options.FieldSelector = fields.OneTermEqualSelector("metadata.name", api.OpenShiftConsoleRouteName).String() - } - kubeInformersNamespaced := informers.NewSharedInformerFactoryWithOptions( kubeClient, resync, @@ -121,7 +117,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle routesClient, resync, routesinformers.WithNamespace(api.TargetNamespace), - routesinformers.WithTweakListOptions(tweakListOptionsForRoute), ) // oauthclients are not namespaced @@ -232,16 +227,19 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle ) consoleRouteController := route.NewRouteSyncController( - // operator config + // top level config + configClient.ConfigV1(), + // clients operatorConfigClient.OperatorV1().Consoles(), routesClient.RouteV1(), kubeClient.CoreV1(), + kubeClient.CoreV1(), // route operatorConfigInformers.Operator().V1().Consoles(), routesInformersNamespaced.Route().V1().Routes(), // names api.OpenShiftConsoleNamespace, - api.OpenShiftConsoleName, + api.OpenShiftConsoleRouteName, // events recorder, // context diff --git a/pkg/console/subresource/configmap/configmap.go b/pkg/console/subresource/configmap/configmap.go index ac28e5b355..f19fd9873f 100644 --- a/pkg/console/subresource/configmap/configmap.go +++ b/pkg/console/subresource/configmap/configmap.go @@ -40,11 +40,11 @@ func DefaultConfigMap( managedConfig *corev1.ConfigMap, monitoringSharedConfig *corev1.ConfigMap, infrastructureConfig *configv1.Infrastructure, - rt *routev1.Route, + activeConsoleRoute *routev1.Route, useDefaultCAFile bool) (consoleConfigmap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) { defaultBuilder := &consoleserver.ConsoleServerCLIConfigBuilder{} - defaultConfig, err := defaultBuilder.Host(rt.Spec.Host). + defaultConfig, err := defaultBuilder.Host(activeConsoleRoute.Spec.Host). LogoutURL(defaultLogoutURL). Brand(DEFAULT_BRAND). DocURL(DEFAULT_DOC_URL). @@ -55,7 +55,7 @@ func DefaultConfigMap( extractedManagedConfig := extractYAML(managedConfig) userDefinedBuilder := &consoleserver.ConsoleServerCLIConfigBuilder{} - userDefinedConfig, err := userDefinedBuilder.Host(rt.Spec.Host). + userDefinedConfig, err := userDefinedBuilder.Host(activeConsoleRoute.Spec.Host). LogoutURL(consoleConfig.Spec.Authentication.LogoutRedirect). Brand(operatorConfig.Spec.Customization.Brand). DocURL(operatorConfig.Spec.Customization.DocumentationBaseURL). diff --git a/pkg/console/subresource/deployment/deployment.go b/pkg/console/subresource/deployment/deployment.go index beeeb46a3c..3d1e25c6b6 100644 --- a/pkg/console/subresource/deployment/deployment.go +++ b/pkg/console/subresource/deployment/deployment.go @@ -16,7 +16,6 @@ import ( // openshift configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" - routev1 "github.com/openshift/api/route/v1" "github.com/openshift/console-operator/pkg/api" "github.com/openshift/console-operator/pkg/console/subresource/util" ) @@ -64,7 +63,7 @@ type volumeConfig struct { mappedKeys map[string]string } -func DefaultDeployment(operatorConfig *operatorv1.Console, cm *corev1.ConfigMap, serviceCAConfigMap *corev1.ConfigMap, defaultIngressCertConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, rt *routev1.Route, proxyConfig *configv1.Proxy, canMountCustomLogo bool) *appsv1.Deployment { +func DefaultDeployment(operatorConfig *operatorv1.Console, cm *corev1.ConfigMap, serviceCAConfigMap *corev1.ConfigMap, defaultIngressCertConfigMap *corev1.ConfigMap, trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, proxyConfig *configv1.Proxy, canMountCustomLogo bool) *appsv1.Deployment { labels := util.LabelsForConsole() meta := util.SharedMeta() meta.Labels = labels diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index f9593aa0e4..86e3e1120f 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -11,7 +11,6 @@ import ( configv1 "github.com/openshift/api/config/v1" operatorsv1 "github.com/openshift/api/operator/v1" - v1 "github.com/openshift/api/route/v1" "github.com/openshift/console-operator/pkg/api" "github.com/openshift/console-operator/pkg/console/subresource/configmap" "github.com/openshift/console-operator/pkg/console/subresource/util" @@ -30,7 +29,6 @@ func TestDefaultDeployment(t *testing.T) { dica *corev1.ConfigMap tca *corev1.ConfigMap sec *corev1.Secret - rt *v1.Route proxy *configv1.Proxy canMountCustomLogo bool } @@ -176,10 +174,6 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - rt: &v1.Route{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{}, - }, proxy: proxyConfig, }, want: &appsv1.Deployment{ @@ -242,10 +236,6 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - rt: &v1.Route{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{}, - }, proxy: proxyConfig, }, want: &appsv1.Deployment{ @@ -294,7 +284,7 @@ func TestDefaultDeployment(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if diff := deep.Equal(DefaultDeployment(tt.args.config, tt.args.cm, tt.args.dica, tt.args.cm, tt.args.tca, tt.args.sec, tt.args.rt, tt.args.proxy, tt.args.canMountCustomLogo), tt.want); diff != nil { + if diff := deep.Equal(DefaultDeployment(tt.args.config, tt.args.cm, tt.args.dica, tt.args.cm, tt.args.tca, tt.args.sec, tt.args.proxy, tt.args.canMountCustomLogo), tt.want); diff != nil { t.Error(diff) } }) diff --git a/pkg/console/subresource/route/route.go b/pkg/console/subresource/route/route.go index c47e6b9acb..a7f0798bf5 100644 --- a/pkg/console/subresource/route/route.go +++ b/pkg/console/subresource/route/route.go @@ -1,9 +1,9 @@ package route import ( - // kube "context" + // kube corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -14,7 +14,10 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" + "github.com/openshift/console-operator/pkg/api" "github.com/openshift/console-operator/pkg/console/subresource/util" ) @@ -24,6 +27,38 @@ const ( defaultIngressController = "default" ) +// holds information about custom TLS certificate and its key +type CustomTLSCert struct { + Certificate string + Key string +} + +func ApplyRoute(client routeclient.RoutesGetter, recorder events.Recorder, required *routev1.Route) (*routev1.Route, bool, error) { + existing, err := client.Routes(required.Namespace).Get(context.TODO(), required.Name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + requiredCopy := required.DeepCopy() + actual, err := client.Routes(requiredCopy.Namespace).Create(context.TODO(), resourcemerge.WithCleanLabelsAndAnnotations(requiredCopy).(*routev1.Route), metav1.CreateOptions{}) + return actual, true, err + } + if err != nil { + return nil, false, err + } + + existingCopy := existing.DeepCopy() + modified := resourcemerge.BoolPtr(false) + resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta) + specSame := equality.Semantic.DeepEqual(existingCopy.Spec, required.Spec) + + if specSame && !*modified { + klog.V(4).Infof("%s route exists and is in the correct state", existingCopy.ObjectMeta.Name) + return existingCopy, false, nil + } + + existingCopy.Spec = required.Spec + actual, err := client.Routes(required.Namespace).Update(context.TODO(), existingCopy, metav1.UpdateOptions{}) + return actual, true, err +} + // ensures route exists. // handles 404 with a create // returns any other error @@ -34,6 +69,7 @@ func GetOrCreate(ctx context.Context, client routeclient.RoutesGetter, required isNew = true route, err = client.Routes(required.Namespace).Create(ctx, required, metav1.CreateOptions{}) } + if err != nil { return nil, isNew, err } @@ -41,53 +77,36 @@ func GetOrCreate(ctx context.Context, client routeclient.RoutesGetter, required } func DefaultRoute(cr *operatorv1.Console) *routev1.Route { - route := Stub() + route := DefaultStub() route.Spec = routev1.RouteSpec{ To: toService(), Port: port(), - TLS: tls(), + TLS: tls(nil), WildcardPolicy: wildcard(), } util.AddOwnerRef(route, util.OwnerRefFrom(cr)) return route } -func Stub() *routev1.Route { +func DefaultStub() *routev1.Route { meta := util.SharedMeta() return &routev1.Route{ ObjectMeta: meta, } } -// we can't just blindly apply the route, we need the route.Spec.Host -// and we don't want to trigger a sync loop. -// TODO: evaluate metadata.annotations to see what will affect our route in -// an undesirable way: -// - https://docs.openshift.com/container-platform/3.9/architecture/networking/routes.html#alternateBackends -func Validate(route *routev1.Route) (*routev1.Route, bool) { - changed := false - - if toServiceSame := equality.Semantic.DeepEqual(route.Spec.To, toService()); !toServiceSame { - changed = true - route.Spec.To = toService() - } - - if portSame := equality.Semantic.DeepEqual(route.Spec.Port, port()); !portSame { - changed = true - route.Spec.Port = port() - } - - if tlsSame := equality.Semantic.DeepEqual(route.Spec.TLS, tls()); !tlsSame { - changed = true - route.Spec.TLS = tls() - } - - if wildcardSame := equality.Semantic.DeepEqual(route.Spec.WildcardPolicy, wildcard()); !wildcardSame { - changed = true - route.Spec.WildcardPolicy = wildcard() +func CustomRoute(cr *operatorv1.Console, tlsConfig *CustomTLSCert) *routev1.Route { + route := DefaultStub() + route.ObjectMeta.Name = api.OpenshiftConsoleCustomRouteName + route.Spec = routev1.RouteSpec{ + Host: cr.Spec.Route.Hostname, + To: toService(), + Port: port(), + TLS: tls(tlsConfig), + WildcardPolicy: wildcard(), } - - return route, changed + util.AddOwnerRef(route, util.OwnerRefFrom(cr)) + return route } func routeMeta() metav1.ObjectMeta { @@ -110,11 +129,16 @@ func port() *routev1.RoutePort { } } -func tls() *routev1.TLSConfig { - return &routev1.TLSConfig{ +func tls(tlsConfig *CustomTLSCert) *routev1.TLSConfig { + tls := &routev1.TLSConfig{ Termination: routev1.TLSTerminationReencrypt, InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, } + if tlsConfig != nil { + tls.Certificate = tlsConfig.Certificate + tls.Key = tlsConfig.Key + } + return tls } func wildcard() routev1.WildcardPolicyType { @@ -160,3 +184,12 @@ func isIngressAdmitted(ingress routev1.RouteIngress) bool { } return admitted } + +func IsCustomRouteSet(operatorConfig *operatorv1.Console) bool { + return len(operatorConfig.Spec.Route.Hostname) != 0 +} + +// Check if reference for secret holding custom TLS certificate and key is set +func IsCustomRouteSecretSet(operatorConfig *operatorv1.Console) bool { + return len(operatorConfig.Spec.Route.Secret.Name) != 0 +} diff --git a/pkg/console/subresource/route/route_test.go b/pkg/console/subresource/route/route_test.go index eba06879fe..2f416c3740 100644 --- a/pkg/console/subresource/route/route_test.go +++ b/pkg/console/subresource/route/route_test.go @@ -106,7 +106,7 @@ func TestStub(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if diff := deep.Equal(Stub(), tt.want); diff != nil { + if diff := deep.Equal(DefaultStub(), tt.want); diff != nil { t.Error(diff) } })