Skip to content

Commit

Permalink
Merge pull request #565 from chiragkyal/feature/external-certificate-…
Browse files Browse the repository at this point in the history
…reference

CFE-1020: feature:route external certificate reference
  • Loading branch information
openshift-merge-bot[bot] authored May 15, 2024
2 parents 56ab14f + 2f06f7c commit 4d9b8c4
Show file tree
Hide file tree
Showing 53 changed files with 2,628 additions and 3,066 deletions.
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ require (
github.com/gocarina/gocsv v0.0.0-20190927101021-3ecffd272576
github.com/google/go-cmp v0.6.0
github.com/haproxytech/config-parser/v4 v4.0.0-rc1
github.com/openshift/api v0.0.0-20240202140003-8b34b9854c7f
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084
github.com/openshift/library-go v0.0.0-20230120202744-256994f916c4
github.com/openshift/api v0.0.0-20240424142232-29a704bf5aa2
github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8
github.com/openshift/library-go v0.0.0-20240426144148-0690e4a4602d
github.com/prometheus/client_golang v1.16.0
github.com/prometheus/client_model v0.4.0
github.com/prometheus/common v0.44.0
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,12 @@ github.com/onsi/ginkgo/v2 v2.13.0 h1:0jY9lJquiL8fcf3M4LAXN5aMlS/b2BV86HFFPCPMgE4
github.com/onsi/ginkgo/v2 v2.13.0/go.mod h1:TE309ZR8s5FsKKpuB1YAQYBzCaAfUgatB/xlT/ETL/o=
github.com/onsi/gomega v1.29.0 h1:KIA/t2t5UBzoirT4H9tsML45GEbo3ouUnBHsCfD2tVg=
github.com/onsi/gomega v1.29.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
github.com/openshift/api v0.0.0-20240202140003-8b34b9854c7f h1:yJ8mEpEW2aJo+97wguYL+ehCsJe/pbeKjFsbdMuuEJI=
github.com/openshift/api v0.0.0-20240202140003-8b34b9854c7f/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE=
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084/go.mod h1:M3h9m001PWac3eAudGG3isUud6yBjr5XpzLYLLTlHKo=
github.com/openshift/library-go v0.0.0-20230120202744-256994f916c4 h1:cFYg18OROQMHlrGWL9HpV1elDKbnRFLz/ED5VvP3qvw=
github.com/openshift/library-go v0.0.0-20230120202744-256994f916c4/go.mod h1:wrrjvk/CK+nte9Wxend9/K6apy6Zep28lzM27/aJar8=
github.com/openshift/api v0.0.0-20240424142232-29a704bf5aa2 h1:U1BsjJoTsvYjymeMseC8apZnvCgExIIRolpc/xJ7jhM=
github.com/openshift/api v0.0.0-20240424142232-29a704bf5aa2/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8 h1:HGfbllzRcrJBSiwzNjBCs7sExLUxC5/1evnvlNGB0Cg=
github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8/go.mod h1:+VvvaMSTUhOt+rBq7NwRLSNxq06hTeRCBqm0j0PQEq8=
github.com/openshift/library-go v0.0.0-20240426144148-0690e4a4602d h1:PVCZvkSfUEwiMEYQ5AL+FIN4HpUUkpduUSkC/1U43H4=
github.com/openshift/library-go v0.0.0-20240426144148-0690e4a4602d/go.mod h1:lFwyRj0XjUf25Da3Q00y+KuaxCWTJ6YzYPDX1+96nco=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down
5 changes: 5 additions & 0 deletions pkg/cmd/infra/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ type RouterSelection struct {
// WatchEndpoints when true will watch Endpoints instead of
// EndpointSlices.
WatchEndpoints bool

// AllowExternalCertificates when true enables RouteSecretManager plugin and the external certificate validation.
// The cluster-ingress-operator sets it if RouteExternalCertificate feature-gate is enabled.
AllowExternalCertificates bool
}

// Bind sets the appropriate labels
Expand Down Expand Up @@ -107,6 +111,7 @@ func (o *RouterSelection) Bind(flag *pflag.FlagSet) {
flag.MarkDeprecated("enable-ingress", "Ingress resources are now synchronized to routes automatically.")
flag.StringVar(&o.ListenAddr, "listen-addr", env("ROUTER_LISTEN_ADDR", ""), "The name of an interface to listen on to expose metrics and health checking. If not specified, will not listen. Overrides stats port.")
flag.BoolVar(&o.WatchEndpoints, "watch-endpoints", isTrue(env("ROUTER_WATCH_ENDPOINTS", "")), "Watch Endpoints instead of the EndpointSlice resource.")
flag.BoolVar(&o.AllowExternalCertificates, "allow-external-certificates", isTrue(env("ROUTER_ENABLE_EXTERNAL_CERTIFICATE", "")), "Enable RouteSecretManager plugin and validation of external certificates.")
}

// RouteUpdate updates the route before it is seen by the cache.
Expand Down
13 changes: 13 additions & 0 deletions pkg/cmd/infra/router/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
routelisters "github.com/openshift/client-go/route/listers/route/v1"
"github.com/openshift/library-go/pkg/crypto"
"github.com/openshift/library-go/pkg/proc"
"github.com/openshift/library-go/pkg/route/secretmanager"

"github.com/openshift/router/pkg/router"
"github.com/openshift/router/pkg/router/controller"
Expand Down Expand Up @@ -718,6 +719,10 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
if err != nil {
return err
}
authorizationClient, err := authorizationclient.NewForConfig(config)
if err != nil {
return err
}

var cfgManager templateplugin.ConfigManager
var blueprintPlugin router.Plugin
Expand Down Expand Up @@ -746,6 +751,8 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
return err
}

secretManager := secretmanager.NewManager(kc, nil)

pluginCfg := templateplugin.TemplatePluginConfig{
WorkingDir: o.WorkingDir,
TemplatePath: o.TemplateFile,
Expand Down Expand Up @@ -773,6 +780,9 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
HTTPResponseHeaders: o.HTTPResponseHeaders,
HTTPRequestHeaders: o.HTTPRequestHeaders,
}
if o.AllowExternalCertificates {
pluginCfg.SecretManager = secretManager
}

svcFetcher := templateplugin.NewListWatchServiceLookup(kc.CoreV1(), o.ResyncInterval, o.Namespace)
templatePlugin, err := templateplugin.NewTemplatePlugin(pluginCfg, svcFetcher)
Expand Down Expand Up @@ -804,6 +814,9 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
if o.ExtendedValidation {
plugin = controller.NewExtendedValidator(plugin, recorder)
}
if o.AllowExternalCertificates {
plugin = controller.NewRouteSecretManager(plugin, recorder, secretManager, kc.CoreV1(), authorizationClient.SubjectAccessReviews())
}
plugin = controller.NewUniqueHost(plugin, o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)
plugin = controller.NewHostAdmitter(plugin, o.RouteAdmissionFunc(), o.AllowWildcardRoutes, o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)

Expand Down
223 changes: 223 additions & 0 deletions pkg/router/controller/route_secret_manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
package controller

import (
"context"
"fmt"

routev1 "github.com/openshift/api/route/v1"
"github.com/openshift/library-go/pkg/route/secretmanager"
"github.com/openshift/router/pkg/router"
"github.com/openshift/router/pkg/router/routeapihelpers"
kapi "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apimachinery/pkg/watch"
authorizationclient "k8s.io/client-go/kubernetes/typed/authorization/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
)

// RouteSecretManager implements the router.Plugin interface to register
// or unregister route with secretManger if externalCertificate is used.
// It also reads the referenced secret to update in-memory tls.Certificate and tls.Key
type RouteSecretManager struct {
// plugin is the next plugin in the chain.
plugin router.Plugin
// recorder is an interface for indicating route status.
recorder RouteStatusRecorder

secretManager secretmanager.SecretManager
secretsGetter corev1client.SecretsGetter
sarClient authorizationclient.SubjectAccessReviewInterface
}

// NewRouteSecretManager creates a new instance of RouteSecretManager.
// It wraps the provided plugin and adds secret management capabilities.
func NewRouteSecretManager(plugin router.Plugin, recorder RouteStatusRecorder, secretManager secretmanager.SecretManager, secretsGetter corev1client.SecretsGetter, sarClient authorizationclient.SubjectAccessReviewInterface) *RouteSecretManager {
return &RouteSecretManager{
plugin: plugin,
recorder: recorder,
secretManager: secretManager,
secretsGetter: secretsGetter,
sarClient: sarClient,
}
}

func (p *RouteSecretManager) HandleNode(eventType watch.EventType, node *kapi.Node) error {
return p.plugin.HandleNode(eventType, node)
}

func (p *RouteSecretManager) HandleEndpoints(eventType watch.EventType, endpoints *kapi.Endpoints) error {
return p.plugin.HandleEndpoints(eventType, endpoints)
}

func (p *RouteSecretManager) HandleNamespaces(namespaces sets.String) error {
return p.plugin.HandleNamespaces(namespaces)
}

func (p *RouteSecretManager) Commit() error {
return p.plugin.Commit()
}

// HandleRoute manages the registration, unregistration, and validation of routes with external certificates.
// For Added events, it validates the route's external certificate configuration and registers it with the secret manager.
// For Modified events, it first unregisters the route if it's already registered and then revalidates and registers it again.
// For Deleted events, it unregisters the route if it's registered.
// Additionally, it delegates the handling of the event to the next plugin in the chain after performing the necessary actions.
func (p *RouteSecretManager) HandleRoute(eventType watch.EventType, route *routev1.Route) error {
log.V(10).Info("HandleRoute: RouteSecretManager", "eventType", eventType)

switch eventType {
case watch.Added:
// register with secret monitor
if hasExternalCertificate(route) {
if err := p.validateAndRegister(route); err != nil {
return err
}
}

// For Modified events always unregister and reregister the route even if the TLS configuration did not change.
// Since the `HandleRoute()` method does not carry the old route spec,
// and there's no definite way to compare old and new TLS configurations,
// assume that the TLS configuration is always updated, necessitating re-registration.
// Additionally, always creating a new `secretHandler` ensures that there are no stale route specs
// in the next plugin chain, especially when the referenced secret is updated or deleted.
// This prevents sending outdated routes to subsequent plugins, preserving expected functionality.
// TODO: Refer https://github.com/openshift/router/pull/565#discussion_r1596441128 for possible ways to improve the logic.
case watch.Modified:
// unregister associated secret monitor, if registered
if p.secretManager.IsRouteRegistered(route.Namespace, route.Name) {
if err := p.secretManager.UnregisterRoute(route.Namespace, route.Name); err != nil {
log.Error(err, "failed to unregister route")
return err
}
}
// register with secret monitor
if hasExternalCertificate(route) {
if err := p.validateAndRegister(route); err != nil {
return err
}
}

case watch.Deleted:
// unregister associated secret monitor, if registered
if p.secretManager.IsRouteRegistered(route.Namespace, route.Name) {
if err := p.secretManager.UnregisterRoute(route.Namespace, route.Name); err != nil {
log.Error(err, "failed to unregister route")
return err
}
}
default:
return fmt.Errorf("invalid eventType %v", eventType)
}

// call next plugin
return p.plugin.HandleRoute(eventType, route)
}

// validateAndRegister validates the route's externalCertificate configuration and registers it with the secret manager.
// It also updates the in-memory TLS certificate and key after reading from secret informer's cache.
func (p *RouteSecretManager) validateAndRegister(route *routev1.Route) error {
fldPath := field.NewPath("spec").Child("tls").Child("externalCertificate")
// validate
if err := routeapihelpers.ValidateTLSExternalCertificate(route, fldPath, p.sarClient, p.secretsGetter).ToAggregate(); err != nil {
log.Error(err, "skipping route due to invalid externalCertificate configuration", "namespace", route.Namespace, "route", route.Name)
p.recorder.RecordRouteRejection(route, "ExternalCertificateValidationFailed", err.Error())
p.plugin.HandleRoute(watch.Deleted, route)
return err
}

// register route with secretManager
handler := p.generateSecretHandler(route)
if err := p.secretManager.RegisterRoute(context.TODO(), route.Namespace, route.Name, route.Spec.TLS.ExternalCertificate.Name, handler); err != nil {
log.Error(err, "failed to register route")
return err
}
// read referenced secret
secret, err := p.secretManager.GetSecret(context.TODO(), route.Namespace, route.Name)
if err != nil {
log.Error(err, "failed to get referenced secret")
return err
}

// Update the tls.Certificate and tls.Key fields of the route with the data from the referenced secret.
// Since externalCertificate does not contain the CACertificate, tls.CACertificate will not be updated.
// NOTE that this update is only performed in-memory and will not reflect in the actual route resource stored in etcd, because
// the router does not make kube-client calls to directly update route resources.
route.Spec.TLS.Certificate = string(secret.Data["tls.crt"])
route.Spec.TLS.Key = string(secret.Data["tls.key"])

return nil
}

// generateSecretHandler creates ResourceEventHandlerFuncs to handle Add, Update, and Delete events on secrets.
// AddFunc: Invoked when a new secret is added. It logs the addition of the secret.
// UpdateFunc: Invoked when an existing secret is updated. It performs validation of the route's external certificate configuration.
// If the validation fails, it records the route rejection, and triggers the deletion of the route by calling the HandleRoute method with a watch.Deleted event.
// If the validation succeeds, it updates the route's TLS certificate and key with the new secret data and calls the next plugin's HandleRoute method with a watch.Modified event, and then the next plugin's Commit() method.
// DeleteFunc: Invoked when the secret is deleted. It unregisters the associated route, records the route rejection, and triggers the deletion of the route by calling the HandleRoute method with a watch.Deleted event.
func (p *RouteSecretManager) generateSecretHandler(route *routev1.Route) cache.ResourceEventHandlerFuncs {
// secret handler
return cache.ResourceEventHandlerFuncs{

// AddFunc is intentionally left empty (only logs the event) because this handler is generated only after ensuring the existence of the secret.
// By leaving this empty, we prevent unnecessary triggering for the addition of the secret again. Additionally, GetSecret() method is called
// immediately after registering with the secretManager, to read the secret from the cache.
AddFunc: func(obj interface{}) {
secret := obj.(*kapi.Secret)
log.V(4).Info("secret added for route", "namespace", route.Namespace, "secret", secret.Name, "route", route.Name)
},

UpdateFunc: func(old interface{}, new interface{}) {
secretOld := old.(*kapi.Secret)
secretNew := new.(*kapi.Secret)
log.V(4).Info("secret updated for route", "namespace", route.Namespace, "secret", secretNew.Name, "old-version", secretOld.ResourceVersion, "new-version", secretNew.ResourceVersion, "route", route.Name)

// re-validate
fldPath := field.NewPath("spec").Child("tls").Child("externalCertificate")
if err := routeapihelpers.ValidateTLSExternalCertificate(route, fldPath, p.sarClient, p.secretsGetter).ToAggregate(); err != nil {
log.Error(err, "skipping route due to invalid externalCertificate configuration", "namespace", route.Namespace, "route", route.Name)
p.recorder.RecordRouteRejection(route, "ExternalCertificateValidationFailed", err.Error())
p.plugin.HandleRoute(watch.Deleted, route)
return
}

// read referenced secret (updated data)
secret, err := p.secretManager.GetSecret(context.TODO(), route.Namespace, route.Name)
if err != nil {
log.Error(err, "failed to get referenced secret")
p.recorder.RecordRouteRejection(route, "ExternalCertificateGetFailed", err.Error())
p.plugin.HandleRoute(watch.Deleted, route)
return
}

// update tls.Certificate and tls.Key
route.Spec.TLS.Certificate = string(secret.Data["tls.crt"])
route.Spec.TLS.Key = string(secret.Data["tls.key"])

// call the next plugin with watch.Modified
p.plugin.HandleRoute(watch.Modified, route)
// commit the changes
p.plugin.Commit()
},

DeleteFunc: func(obj interface{}) {
secret := obj.(*kapi.Secret)
msg := fmt.Sprintf("secret %s deleted for route %s/%s", secret.Name, route.Namespace, route.Name)
log.V(4).Info(msg)

// unregister associated secret monitor
if err := p.secretManager.UnregisterRoute(route.Namespace, route.Name); err != nil {
log.Error(err, "failed to unregister route")
}

p.recorder.RecordRouteRejection(route, "ExternalCertificateSecretDeleted", msg)
p.plugin.HandleRoute(watch.Deleted, route)
},
}
}

func hasExternalCertificate(route *routev1.Route) bool {
tls := route.Spec.TLS
return tls != nil && tls.ExternalCertificate != nil && len(tls.ExternalCertificate.Name) > 0
}
Loading

0 comments on commit 4d9b8c4

Please sign in to comment.