diff --git a/pkg/cmd/infra/router/f5.go b/pkg/cmd/infra/router/f5.go index de7e48983afc..032b3595125b 100644 --- a/pkg/cmd/infra/router/f5.go +++ b/pkg/cmd/infra/router/f5.go @@ -251,7 +251,7 @@ func (o *F5RouterOptions) Run() error { factory := o.RouterSelection.NewFactory(routeclient, projectclient.Project().Projects(), kc) watchNodes := (len(o.InternalAddress) != 0 && len(o.VxlanGateway) != 0) - controller := factory.Create(plugin, watchNodes, o.EnableIngress) + controller := factory.Create(plugin, watchNodes) controller.Run() select {} diff --git a/pkg/cmd/infra/router/router.go b/pkg/cmd/infra/router/router.go index 3bec63aedd69..cc5159b25f13 100644 --- a/pkg/cmd/infra/router/router.go +++ b/pkg/cmd/infra/router/router.go @@ -60,8 +60,6 @@ type RouterSelection struct { ExtendedValidation bool - EnableIngress bool - ListenAddr string } @@ -82,8 +80,9 @@ func (o *RouterSelection) Bind(flag *pflag.FlagSet) { flag.StringSliceVar(&o.AllowedDomains, "allowed-domains", envVarAsStrings("ROUTER_ALLOWED_DOMAINS", "", ","), "List of comma separated domains to allow in routes. If specified, only the domains in this list will be allowed routes. Note that domains in the denied list take precedence over the ones in the allowed list") flag.BoolVar(&o.AllowWildcardRoutes, "allow-wildcard-routes", isTrue(cmdutil.Env("ROUTER_ALLOW_WILDCARD_ROUTES", "")), "Allow wildcard host names for routes") flag.BoolVar(&o.DisableNamespaceOwnershipCheck, "disable-namespace-ownership-check", isTrue(cmdutil.Env("ROUTER_DISABLE_NAMESPACE_OWNERSHIP_CHECK", "")), "Disables the namespace ownership checks for a route host with different paths or for overlapping host names in the case of wildcard routes. Please be aware that if namespace ownership checks are disabled, routes in a different namespace can use this mechanism to 'steal' sub-paths for existing domains. This is only safe if route creation privileges are restricted, or if all the users can be trusted.") - flag.BoolVar(&o.EnableIngress, "enable-ingress", isTrue(cmdutil.Env("ROUTER_ENABLE_INGRESS", "")), "Enable configuration via ingress resources") flag.BoolVar(&o.ExtendedValidation, "extended-validation", isTrue(cmdutil.Env("EXTENDED_VALIDATION", "true")), "If set, then an additional extended validation step is performed on all routes admitted in by this router. Defaults to true and enables the extended validation checks.") + flag.Bool("enable-ingress", false, "Enable configuration via ingress resources.") + flag.MarkDeprecated("enable-ingress", "Ingress resources are now synchronized to routes automatically.") flag.StringVar(&o.ListenAddr, "listen-addr", cmdutil.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.") } @@ -96,14 +95,10 @@ func (o *RouterSelection) RouteSelectionFunc() controller.RouteHostFunc { if !o.OverrideHostname && len(route.Spec.Host) > 0 { return route.Spec.Host } - // GetNameForHost returns the ingress name for a generated route, and the route route - // name otherwise. When a route and ingress in the same namespace share a name, the - // route and the ingress' rules should receive the same generated host. - nameForHost := controller.GetNameForHost(route.Name) s, err := variable.ExpandStrict(o.HostnameTemplate, func(key string) (string, bool) { switch key { case "name": - return nameForHost, true + return route.Name, true case "namespace": return route.Namespace, true default: diff --git a/pkg/cmd/infra/router/template.go b/pkg/cmd/infra/router/template.go index 4488ae63b8f0..24724bf11ba2 100644 --- a/pkg/cmd/infra/router/template.go +++ b/pkg/cmd/infra/router/template.go @@ -431,7 +431,7 @@ func (o *TemplateRouterOptions) Run() error { plugin = controller.NewHostAdmitter(plugin, o.RouteAdmissionFunc(), o.AllowWildcardRoutes, o.RouterSelection.DisableNamespaceOwnershipCheck, recorder) factory := o.RouterSelection.NewFactory(routeclient, projectclient.Project().Projects(), kc) - controller := factory.Create(plugin, false, o.EnableIngress) + controller := factory.Create(plugin, false) controller.Run() proc.StartReaper() diff --git a/pkg/cmd/openshift-controller-manager/controller/config.go b/pkg/cmd/openshift-controller-manager/controller/config.go index 83007b79a5e3..499220919218 100644 --- a/pkg/cmd/openshift-controller-manager/controller/config.go +++ b/pkg/cmd/openshift-controller-manager/controller/config.go @@ -63,9 +63,10 @@ type OpenshiftControllerConfig struct { ServiceServingCertsControllerOptions ServiceServingCertsControllerOptions - SDNControllerConfig SDNControllerConfig - UnidlingControllerConfig UnidlingControllerConfig - IngressIPControllerConfig IngressIPControllerConfig + SDNControllerConfig SDNControllerConfig + UnidlingControllerConfig UnidlingControllerConfig + IngressIPControllerConfig IngressIPControllerConfig + IngressToRouteControllerConfig IngressToRouteControllerConfig ClusterQuotaReconciliationControllerConfig ClusterQuotaReconciliationControllerConfig @@ -98,6 +99,7 @@ func (c *OpenshiftControllerConfig) GetControllerInitializers() (map[string]Init ret["openshift.io/sdn"] = c.SDNControllerConfig.RunController ret["openshift.io/unidling"] = c.UnidlingControllerConfig.RunController ret["openshift.io/ingress-ip"] = c.IngressIPControllerConfig.RunController + ret["openshift.io/ingress-to-route"] = c.IngressToRouteControllerConfig.RunController ret["openshift.io/resourcequota"] = RunResourceQuotaManager ret["openshift.io/cluster-quota-reconciliation"] = c.ClusterQuotaReconciliationControllerConfig.RunController diff --git a/pkg/cmd/openshift-controller-manager/controller/ingress.go b/pkg/cmd/openshift-controller-manager/controller/ingress.go new file mode 100644 index 000000000000..74c00edee00c --- /dev/null +++ b/pkg/cmd/openshift-controller-manager/controller/ingress.go @@ -0,0 +1,36 @@ +package controller + +import ( + coreclient "k8s.io/client-go/kubernetes/typed/core/v1" + + routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" + "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" + "github.com/openshift/origin/pkg/route/controller/ingress" +) + +type IngressToRouteControllerConfig struct{} + +func (c *IngressToRouteControllerConfig) RunController(ctx ControllerContext) (bool, error) { + clientConfig := ctx.ClientBuilder.ConfigOrDie(bootstrappolicy.InfraIngressToRouteControllerServiceAccountName) + coreClient, err := coreclient.NewForConfig(clientConfig) + if err != nil { + return false, err + } + routeClient, err := routeclient.NewForConfig(clientConfig) + if err != nil { + return false, err + } + + controller := ingress.NewController( + coreClient, + routeClient, + ctx.ExternalKubeInformers.Extensions().V1beta1().Ingresses(), + ctx.ExternalKubeInformers.Core().V1().Secrets(), + ctx.ExternalKubeInformers.Core().V1().Services(), + ctx.RouteInformers.Route().V1().Routes(), + ) + + go controller.Run(5, ctx.Stop) + + return true, nil +} diff --git a/pkg/cmd/openshift-controller-manager/controller/interfaces.go b/pkg/cmd/openshift-controller-manager/controller/interfaces.go index 45ceba6cd79a..5e56acaf2200 100644 --- a/pkg/cmd/openshift-controller-manager/controller/interfaces.go +++ b/pkg/cmd/openshift-controller-manager/controller/interfaces.go @@ -12,6 +12,7 @@ import ( kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" "k8s.io/kubernetes/pkg/controller" + routeinformer "github.com/openshift/client-go/route/informers/externalversions" appinformer "github.com/openshift/origin/pkg/apps/generated/informers/internalversion" appsclientinternal "github.com/openshift/origin/pkg/apps/generated/internalclientset" authorizationinformer "github.com/openshift/origin/pkg/authorization/generated/informers/internalversion" @@ -46,6 +47,7 @@ type ControllerContext struct { TemplateInformers templateinformer.SharedInformerFactory QuotaInformers quotainformer.SharedInformerFactory AuthorizationInformers authorizationinformer.SharedInformerFactory + RouteInformers routeinformer.SharedInformerFactory SecurityInformers securityinformer.SharedInformerFactory GenericResourceInformer GenericResourceInformer diff --git a/pkg/cmd/openshift-controller-manager/controller_manager.go b/pkg/cmd/openshift-controller-manager/controller_manager.go index b717f3ea2a96..98ce08d35ffc 100644 --- a/pkg/cmd/openshift-controller-manager/controller_manager.go +++ b/pkg/cmd/openshift-controller-manager/controller_manager.go @@ -185,6 +185,7 @@ func newControllerContext( NetworkInformers: informers.GetNetworkInformers(), QuotaInformers: informers.GetQuotaInformers(), SecurityInformers: informers.GetSecurityInformers(), + RouteInformers: informers.GetRouteInformers(), TemplateInformers: informers.GetTemplateInformers(), GenericResourceInformer: informers.ToGenericInformer(), Stop: stopCh, diff --git a/pkg/cmd/server/bootstrappolicy/controller_policy.go b/pkg/cmd/server/bootstrappolicy/controller_policy.go index dc981d07a6bf..11229c98317d 100644 --- a/pkg/cmd/server/bootstrappolicy/controller_policy.go +++ b/pkg/cmd/server/bootstrappolicy/controller_policy.go @@ -35,6 +35,7 @@ const ( InfraPersistentVolumeRecyclerControllerServiceAccountName = "pv-recycler-controller" InfraResourceQuotaControllerServiceAccountName = "resourcequota-controller" InfraDefaultRoleBindingsControllerServiceAccountName = "default-rolebindings-controller" + InfraIngressToRouteControllerServiceAccountName = "ingress-to-route-controller" // template instance controller watches for TemplateInstance object creation // and instantiates templates as a result. @@ -296,6 +297,18 @@ func init() { }, }) + // ingress-to-route-controller + addControllerRole(rbac.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraIngressToRouteControllerServiceAccountName}, + Rules: []rbac.PolicyRule{ + rbac.NewRule("get", "list", "watch").Groups(kapiGroup).Resources("secrets", "services").RuleOrDie(), + rbac.NewRule("get", "list", "watch").Groups(extensionsGroup).Resources("ingress").RuleOrDie(), + rbac.NewRule("get", "list", "watch", "create", "update", "patch", "delete").Groups(routeGroup).Resources("routes").RuleOrDie(), + rbac.NewRule("create", "update").Groups(routeGroup).Resources("routes/custom-host").RuleOrDie(), + eventsRule(), + }, + }) + // pv-recycler-controller addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraPersistentVolumeRecyclerControllerServiceAccountName}, diff --git a/pkg/cmd/server/origin/master_config.go b/pkg/cmd/server/origin/master_config.go index 90935ecd83e4..5651d1c70c14 100644 --- a/pkg/cmd/server/origin/master_config.go +++ b/pkg/cmd/server/origin/master_config.go @@ -27,6 +27,7 @@ import ( rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation" rbacauthorizer "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" + routeinformer "github.com/openshift/client-go/route/informers/externalversions" userinformer "github.com/openshift/client-go/user/informers/externalversions" appinformer "github.com/openshift/origin/pkg/apps/generated/informers/internalversion" authorizationinformer "github.com/openshift/origin/pkg/authorization/generated/informers/internalversion" @@ -96,6 +97,7 @@ type MasterConfig struct { InternalKubeInformers kinternalinformers.SharedInformerFactory ClientGoKubeInformers kubeclientgoinformers.SharedInformerFactory AuthorizationInformers authorizationinformer.SharedInformerFactory + RouteInformers routeinformer.SharedInformerFactory QuotaInformers quotainformer.SharedInformerFactory SecurityInformers securityinformer.SharedInformerFactory } @@ -112,6 +114,7 @@ type InformerAccess interface { GetOauthInformers() oauthinformer.SharedInformerFactory GetQuotaInformers() quotainformer.SharedInformerFactory GetSecurityInformers() securityinformer.SharedInformerFactory + GetRouteInformers() routeinformer.SharedInformerFactory GetUserInformers() userinformer.SharedInformerFactory GetTemplateInformers() templateinformer.SharedInformerFactory ToGenericInformer() GenericResourceInformer @@ -226,6 +229,7 @@ func BuildMasterConfig( AuthorizationInformers: informers.GetAuthorizationInformers(), QuotaInformers: informers.GetQuotaInformers(), SecurityInformers: informers.GetSecurityInformers(), + RouteInformers: informers.GetRouteInformers(), } for name, hook := range authenticatorPostStartHooks { diff --git a/pkg/route/controller/ingress/ingress.go b/pkg/route/controller/ingress/ingress.go new file mode 100644 index 000000000000..13e045a9835d --- /dev/null +++ b/pkg/route/controller/ingress/ingress.go @@ -0,0 +1,693 @@ +package ingress + +import ( + "fmt" + "sync" + "time" + + "github.com/golang/glog" + + "k8s.io/api/core/v1" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/json" + utilrand "k8s.io/apimachinery/pkg/util/rand" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + coreinformers "k8s.io/client-go/informers/core/v1" + extensionsinformers "k8s.io/client-go/informers/extensions/v1beta1" + kv1core "k8s.io/client-go/kubernetes/typed/core/v1" + corelisters "k8s.io/client-go/listers/core/v1" + extensionslisters "k8s.io/client-go/listers/extensions/v1beta1" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/workqueue" + "k8s.io/kubernetes/pkg/api/legacyscheme" + + routev1 "github.com/openshift/api/route/v1" + routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" + routeinformers "github.com/openshift/client-go/route/informers/externalversions/route/v1" + routelisters "github.com/openshift/client-go/route/listers/route/v1" +) + +// Controller ensures that zero or more routes exist to match any supported ingress. The +// controller creates a controller owner reference from the route to the parent ingress, +// allowing users to orphan their ingress. All owned routes have specific spec fields +// managed (those attributes present on the ingress), while any other fields may be +// modified by the user. +// +// Invariants: +// +// 1. For every ingress path rule with a non-empty backend statement, a route should +// exist that points to that backend. +// 2. For every TLS hostname that has a corresponding path rule and points to a secret +// that exists, a route should exist with a valid TLS config from that secret. +// 3. For every service referenced by the ingress path rule, the route should have +// a target port based on the service. +// 4. A route owned by an ingress that is not described by any of the three invariants +// above should be deleted. +// +// The controller also relies on the use of expectations to remind itself whether there +// are route creations it has not yet observed, which prevents the controller from +// creating more objects than it needs. The expectations are reset when the ingress +// object is modified. It is possible that expectations could leak if an ingress is +// deleted and its deletion is not observed by the cache, but such leaks are only expected +// if there is a bug in the informer cache which must be fixed anyway. +// +// Unsupported attributes: +// +// * the ingress class attribute +// * nginx annotations +// * the empty backend +// * paths with empty backends +// * creating a dynamic route spec.host +// +type Controller struct { + eventRecorder record.EventRecorder + + client routeclient.RoutesGetter + + ingressLister extensionslisters.IngressLister + secretLister corelisters.SecretLister + routeLister routelisters.RouteLister + serviceLister corelisters.ServiceLister + + // syncs are the items that must return true before the queue can be processed + syncs []cache.InformerSynced + + // queue is the list of namespace keys that must be synced. + queue workqueue.RateLimitingInterface + + // expectations track upcoming route creations that we have not yet observed + expectations *expectations + // expectationDelay controls how long the controller waits to observe its + // own creates. Exposed only for testing. + expectationDelay time.Duration +} + +// expectations track an upcoming change to a named resource related +// to an ingress. This is a thread safe object but callers assume +// responsibility for ensuring expectations do not leak. +type expectations struct { + lock sync.Mutex + expect map[queueKey]sets.String +} + +// newExpectations returns a tracking object for upcoming events +// that the controller may expect to happen. +func newExpectations() *expectations { + return &expectations{ + expect: make(map[queueKey]sets.String), + } +} + +// Expect that an event will happen in the future for the given ingress +// and a named resource related to that ingress. +func (e *expectations) Expect(namespace, ingressName, name string) { + e.lock.Lock() + defer e.lock.Unlock() + key := queueKey{namespace: namespace, name: ingressName} + set, ok := e.expect[key] + if !ok { + set = sets.NewString() + e.expect[key] = set + } + set.Insert(name) +} + +// Satisfied clears the expectation for the given resource name on an +// ingress. +func (e *expectations) Satisfied(namespace, ingressName, name string) { + e.lock.Lock() + defer e.lock.Unlock() + key := queueKey{namespace: namespace, name: ingressName} + set := e.expect[key] + set.Delete(name) + if set.Len() == 0 { + delete(e.expect, key) + } +} + +// Expecting returns true if the provided ingress is still waiting to +// see changes. +func (e *expectations) Expecting(namespace, ingressName string) bool { + e.lock.Lock() + defer e.lock.Unlock() + key := queueKey{namespace: namespace, name: ingressName} + return e.expect[key].Len() > 0 +} + +// Clear indicates that all expectations for the given ingress should +// be cleared. +func (e *expectations) Clear(namespace, ingressName string) { + e.lock.Lock() + defer e.lock.Unlock() + key := queueKey{namespace: namespace, name: ingressName} + delete(e.expect, key) +} + +type queueKey struct { + namespace string + name string +} + +// NewController instantiates a Controller +func NewController(eventsClient kv1core.EventsGetter, client routeclient.RoutesGetter, ingresses extensionsinformers.IngressInformer, secrets coreinformers.SecretInformer, services coreinformers.ServiceInformer, routes routeinformers.RouteInformer) *Controller { + broadcaster := record.NewBroadcaster() + broadcaster.StartLogging(glog.Infof) + // TODO: remove the wrapper when every clients have moved to use the clientset. + broadcaster.StartRecordingToSink(&kv1core.EventSinkImpl{Interface: eventsClient.Events("")}) + recorder := broadcaster.NewRecorder(legacyscheme.Scheme, v1.EventSource{Component: "ingress-to-route-controller"}) + + c := &Controller{ + eventRecorder: recorder, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ingress-to-route"), + + expectations: newExpectations(), + expectationDelay: 2 * time.Second, + + client: client, + + ingressLister: ingresses.Lister(), + secretLister: secrets.Lister(), + routeLister: routes.Lister(), + serviceLister: services.Lister(), + + syncs: []cache.InformerSynced{ + ingresses.Informer().HasSynced, + secrets.Informer().HasSynced, + routes.Informer().HasSynced, + services.Informer().HasSynced, + }, + } + + // any change to a secret of type TLS in the namespace + secrets.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: func(obj interface{}) bool { + switch t := obj.(type) { + case *v1.Secret: + return t.Type == v1.SecretTypeTLS + } + return true + }, + Handler: cache.ResourceEventHandlerFuncs{ + AddFunc: c.processNamespace, + DeleteFunc: c.processNamespace, + UpdateFunc: func(oldObj, newObj interface{}) { + c.processNamespace(newObj) + }, + }, + }) + + // any change to a service in the namespace + services.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: c.processNamespace, + DeleteFunc: c.processNamespace, + UpdateFunc: func(oldObj, newObj interface{}) { + c.processNamespace(newObj) + }, + }) + + // any change to a route that has the controller relationship to an Ingress + routes.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: func(obj interface{}) bool { + switch t := obj.(type) { + case *routev1.Route: + _, ok := hasIngressOwnerRef(t.OwnerReferences) + return ok + } + return true + }, + Handler: cache.ResourceEventHandlerFuncs{ + AddFunc: c.processRoute, + DeleteFunc: c.processRoute, + UpdateFunc: func(oldObj, newObj interface{}) { + c.processRoute(newObj) + }, + }, + }) + + // changes to ingresses + ingresses.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: c.processIngress, + DeleteFunc: c.processIngress, + UpdateFunc: func(oldObj, newObj interface{}) { + c.processIngress(newObj) + }, + }) + + return c +} + +func (c *Controller) processNamespace(obj interface{}) { + switch t := obj.(type) { + case metav1.Object: + ns := t.GetNamespace() + if len(ns) == 0 { + utilruntime.HandleError(fmt.Errorf("object %T has no namespace", obj)) + return + } + c.queue.Add(queueKey{namespace: ns}) + default: + utilruntime.HandleError(fmt.Errorf("couldn't get key for object %T", obj)) + } +} + +func (c *Controller) processRoute(obj interface{}) { + switch t := obj.(type) { + case *routev1.Route: + ingressName, ok := hasIngressOwnerRef(t.OwnerReferences) + if !ok { + return + } + c.expectations.Satisfied(t.Namespace, ingressName, t.Name) + c.queue.Add(queueKey{namespace: t.Namespace, name: ingressName}) + default: + utilruntime.HandleError(fmt.Errorf("couldn't get key for object %T", obj)) + } +} + +func (c *Controller) processIngress(obj interface{}) { + switch t := obj.(type) { + case *extensionsv1beta1.Ingress: + // when we see a change to an ingress, reset our expectations + // this also allows periodic purging of the expectation list in the event + // we miss one or more events. + c.expectations.Clear(t.Namespace, t.Name) + c.queue.Add(queueKey{namespace: t.Namespace, name: t.Name}) + default: + utilruntime.HandleError(fmt.Errorf("couldn't get key for object %T", obj)) + } +} + +// Run begins watching and syncing. +func (c *Controller) Run(workers int, stopCh <-chan struct{}) { + defer utilruntime.HandleCrash() + defer c.queue.ShutDown() + + glog.Infof("Starting controller") + + if !cache.WaitForCacheSync(stopCh, c.syncs...) { + utilruntime.HandleError(fmt.Errorf("timed out waiting for caches to sync")) + return + } + + for i := 0; i < workers; i++ { + go wait.Until(c.worker, time.Second, stopCh) + } + + <-stopCh + glog.Infof("Shutting down controller") +} + +func (c *Controller) worker() { + for c.processNext() { + } + glog.V(4).Infof("Worker stopped") +} + +func (c *Controller) processNext() bool { + key, quit := c.queue.Get() + if quit { + return false + } + defer c.queue.Done(key) + + glog.V(5).Infof("processing %v begin", key) + err := c.sync(key.(queueKey)) + c.handleNamespaceErr(err, key) + glog.V(5).Infof("processing %v end", key) + + return true +} + +func (c *Controller) handleNamespaceErr(err error, key interface{}) { + if err == nil { + c.queue.Forget(key) + return + } + + glog.V(4).Infof("Error syncing %v: %v", key, err) + c.queue.AddRateLimited(key) +} + +func (c *Controller) sync(key queueKey) error { + // sync all ingresses in the namespace + if len(key.name) == 0 { + ingresses, err := c.ingressLister.Ingresses(key.namespace).List(labels.Everything()) + if err != nil { + return err + } + for _, ingress := range ingresses { + c.queue.Add(queueKey{namespace: ingress.Namespace, name: ingress.Name}) + } + return nil + } + // if we are waiting to observe the result of route creations, simply delay + if c.expectations.Expecting(key.namespace, key.name) { + c.queue.AddAfter(key, c.expectationDelay) + glog.V(5).Infof("Ingress %s/%s has unsatisfied expectations", key.namespace, key.name) + return nil + } + + ingress, err := c.ingressLister.Ingresses(key.namespace).Get(key.name) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + + // find all matching routes + routes, err := c.routeLister.Routes(key.namespace).List(labels.Everything()) + if err != nil { + return err + } + old := routes[:0] + for _, route := range routes { + ingressName, ok := hasIngressOwnerRef(route.OwnerReferences) + if !ok || ingressName != ingress.Name { + continue + } + old = append(old, route) + } + + // walk the ingress and identify whether any of the child routes need to be updated, deleted, + // or created, as efficiently as possible. + var creates, updates []*routev1.Route + for _, rule := range ingress.Spec.Rules { + if rule.HTTP == nil { + continue + } + if len(rule.Host) == 0 { + continue + } + for _, path := range rule.HTTP.Paths { + if len(path.Backend.ServiceName) == 0 { + continue + } + + var existing *routev1.Route + old, existing = splitForPathAndHost(old, rule.Host, path.Path) + if existing == nil { + if r := newRouteForIngress(ingress, &rule, &path, c.secretLister, c.serviceLister); r != nil { + creates = append(creates, r) + } + continue + } + + if routeMatchesIngress(existing, ingress, &rule, &path, c.secretLister, c.serviceLister) { + continue + } + + if r := newRouteForIngress(ingress, &rule, &path, c.secretLister, c.serviceLister); r != nil { + // merge the relevant spec pieces + preserveRouteAttributesFromExisting(r, existing) + updates = append(updates, r) + } else { + // the route cannot be fully calculated, delete it + old = append(old, existing) + } + } + } + + var errs []error + + // add the new routes + for _, route := range creates { + if err := createRouteWithName(c.client, ingress, route, c.expectations); err != nil { + errs = append(errs, err) + } + } + + // update any existing routes in place + for _, route := range updates { + data, err := json.Marshal(&route.Spec) + if err != nil { + return err + } + data = []byte(fmt.Sprintf(`[{"op":"replace","path":"/spec","value":%s}]`, data)) + _, err = c.client.Routes(route.Namespace).Patch(route.Name, types.JSONPatchType, data) + if err != nil { + errs = append(errs, err) + } + } + + // purge any previously managed routes + for _, route := range old { + if err := c.client.Routes(route.Namespace).Delete(route.Name, nil); err != nil && !errors.IsNotFound(err) { + errs = append(errs, err) + } + } + + if len(errs) > 0 { + return utilerrors.NewAggregate(errs) + } + return nil +} + +func hasIngressOwnerRef(owners []metav1.OwnerReference) (string, bool) { + for _, ref := range owners { + if ref.Kind != "Ingress" || ref.APIVersion != "extensions/v1beta1" || ref.Controller == nil || !*ref.Controller { + continue + } + return ref.Name, true + } + return "", false +} + +func newRouteForIngress( + ingress *extensionsv1beta1.Ingress, + rule *extensionsv1beta1.IngressRule, + path *extensionsv1beta1.HTTPIngressPath, + secretLister corelisters.SecretLister, + serviceLister corelisters.ServiceLister, +) *routev1.Route { + var tlsConfig *routev1.TLSConfig + if name, ok := referencesSecret(ingress, rule.Host); ok { + secret, err := secretLister.Secrets(ingress.Namespace).Get(name) + if err != nil { + // secret doesn't exist yet, wait + return nil + } + if secret.Type != v1.SecretTypeTLS { + // secret is the wrong type + return nil + } + if _, ok := secret.Data[v1.TLSCertKey]; !ok { + return nil + } + if _, ok := secret.Data[v1.TLSPrivateKeyKey]; !ok { + return nil + } + tlsConfig = &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Certificate: string(secret.Data[v1.TLSCertKey]), + Key: string(secret.Data[v1.TLSPrivateKeyKey]), + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + } + } + + targetPort := targetPortForService(ingress.Namespace, path, serviceLister) + if targetPort == nil { + // no valid target port + return nil + } + + t := true + return &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: ingress.Name + "-", + Namespace: ingress.Namespace, + Labels: ingress.Labels, + Annotations: ingress.Annotations, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "extensions/v1beta1", Kind: "Ingress", Controller: &t, Name: ingress.Name, UID: ingress.UID}, + }, + }, + Spec: routev1.RouteSpec{ + Host: rule.Host, + Path: path.Path, + To: routev1.RouteTargetReference{ + Name: path.Backend.ServiceName, + }, + Port: &routev1.RoutePort{ + TargetPort: *targetPort, + }, + TLS: tlsConfig, + }, + } +} + +func preserveRouteAttributesFromExisting(r, existing *routev1.Route) { + r.Name = existing.Name + r.GenerateName = "" + r.Spec.To.Weight = existing.Spec.To.Weight + if r.Spec.TLS != nil && existing.Spec.TLS != nil { + r.Spec.TLS.CACertificate = existing.Spec.TLS.CACertificate + r.Spec.TLS.DestinationCACertificate = existing.Spec.TLS.DestinationCACertificate + r.Spec.TLS.InsecureEdgeTerminationPolicy = existing.Spec.TLS.InsecureEdgeTerminationPolicy + } +} + +func routeMatchesIngress( + route *routev1.Route, + ingress *extensionsv1beta1.Ingress, + rule *extensionsv1beta1.IngressRule, + path *extensionsv1beta1.HTTPIngressPath, + secretLister corelisters.SecretLister, + serviceLister corelisters.ServiceLister, +) bool { + match := route.Spec.Host == rule.Host && + route.Spec.Path == path.Path && + route.Spec.To.Name == path.Backend.ServiceName && + route.Spec.Port != nil && + route.Spec.WildcardPolicy == routev1.WildcardPolicyNone && + len(route.Spec.AlternateBackends) == 0 + if !match { + return false + } + + targetPort := targetPortForService(ingress.Namespace, path, serviceLister) + if targetPort == nil || *targetPort != route.Spec.Port.TargetPort { + // not valid + return false + } + + var secret *v1.Secret + if name, ok := referencesSecret(ingress, rule.Host); ok { + secret, _ = secretLister.Secrets(ingress.Namespace).Get(name) + if secret == nil { + return false + } + } + if !secretMatchesRoute(secret, route.Spec.TLS) { + return false + } + return true +} + +func targetPortForService(namespace string, path *extensionsv1beta1.HTTPIngressPath, serviceLister corelisters.ServiceLister) *intstr.IntOrString { + service, err := serviceLister.Services(namespace).Get(path.Backend.ServiceName) + if err != nil { + // service doesn't exist yet, wait + return nil + } + if path.Backend.ServicePort.Type == intstr.String { + expect := path.Backend.ServicePort.StrVal + for _, port := range service.Spec.Ports { + if port.Name == expect { + return &port.TargetPort + } + } + } else { + for _, port := range service.Spec.Ports { + expect := path.Backend.ServicePort.IntVal + if port.Port == expect { + return &port.TargetPort + } + } + } + return nil +} + +func secretMatchesRoute(secret *v1.Secret, tlsConfig *routev1.TLSConfig) bool { + if secret == nil { + return tlsConfig == nil + } + if secret.Type != v1.SecretTypeTLS { + return tlsConfig == nil + } + if _, ok := secret.Data[v1.TLSCertKey]; !ok { + return false + } + if _, ok := secret.Data[v1.TLSPrivateKeyKey]; !ok { + return false + } + if tlsConfig == nil { + return false + } + return tlsConfig.Termination == routev1.TLSTerminationEdge && + tlsConfig.Certificate == string(secret.Data[v1.TLSCertKey]) && + tlsConfig.Key == string(secret.Data[v1.TLSPrivateKeyKey]) +} + +func splitForPathAndHost(routes []*routev1.Route, host, path string) ([]*routev1.Route, *routev1.Route) { + for i, route := range routes { + if route.Spec.Host == host && route.Spec.Path == path { + last := len(routes) - 1 + routes[i], routes[last] = routes[last], route + return routes[:last], route + } + } + return routes, nil +} + +func referencesSecret(ingress *extensionsv1beta1.Ingress, host string) (string, bool) { + for _, tls := range ingress.Spec.TLS { + for _, tlsHost := range tls.Hosts { + if tlsHost == host { + return tls.SecretName, true + } + } + } + return "", false +} + +// createRouteWithName performs client side name generation so we can set a predictable expectation. +// If we fail multiple times in a row we will return an error. +// TODO: future optimization, check the local cache for the name first +func createRouteWithName(client routeclient.RoutesGetter, ingress *extensionsv1beta1.Ingress, route *routev1.Route, expect *expectations) error { + base := route.GenerateName + var lastErr error + // only retry a limited number of times + for i := 0; i < 3; i++ { + if len(base) > 0 { + route.GenerateName = "" + route.Name = generateRouteName(base) + } + + // Set the expectation before we talk to the server in order to + // prevent racing with the route cache. + expect.Expect(ingress.Namespace, ingress.Name, route.Name) + + _, err := client.Routes(route.Namespace).Create(route) + if err == nil { + return nil + } + + // We either collided with another randomly generated name, or another + // error between us and the server prevented observing the success + // of the result. In either case we are not expecting a new route. This + // is safe because expectations are an optimization to avoid churn rather + // than to prevent true duplicate creation. + expect.Satisfied(ingress.Namespace, ingress.Name, route.Name) + + // if we aren't generating names (or if we got any other type of error) + // return right away + if len(base) == 0 || !errors.IsAlreadyExists(err) { + return err + } + lastErr = err + } + return lastErr +} + +const ( + maxNameLength = 63 + randomLength = 5 + maxGeneratedNameLength = maxNameLength - randomLength +) + +func generateRouteName(base string) string { + if len(base) > maxGeneratedNameLength { + base = base[:maxGeneratedNameLength] + } + return fmt.Sprintf("%s%s", base, utilrand.String(randomLength)) +} diff --git a/pkg/route/controller/ingress/ingress_test.go b/pkg/route/controller/ingress/ingress_test.go new file mode 100644 index 000000000000..50686350dc43 --- /dev/null +++ b/pkg/route/controller/ingress/ingress_test.go @@ -0,0 +1,1627 @@ +package ingress + +import ( + "reflect" + "testing" + + v1 "k8s.io/api/core/v1" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" + corelisters "k8s.io/client-go/listers/core/v1" + extensionslisters "k8s.io/client-go/listers/extensions/v1beta1" + clientgotesting "k8s.io/client-go/testing" + "k8s.io/client-go/util/workqueue" + + routev1 "github.com/openshift/api/route/v1" + "github.com/openshift/client-go/route/clientset/versioned/fake" + routelisters "github.com/openshift/client-go/route/listers/route/v1" +) + +type routeLister struct { + Err error + Items []*routev1.Route +} + +func (r *routeLister) List(selector labels.Selector) (ret []*routev1.Route, err error) { + return r.Items, r.Err +} +func (r *routeLister) Routes(namespace string) routelisters.RouteNamespaceLister { + return &nsRouteLister{r: r, ns: namespace} +} + +type nsRouteLister struct { + r *routeLister + ns string +} + +func (r *nsRouteLister) List(selector labels.Selector) (ret []*routev1.Route, err error) { + return r.r.Items, r.r.Err +} +func (r *nsRouteLister) Get(name string) (*routev1.Route, error) { + for _, s := range r.r.Items { + if s.Name == name && r.ns == s.Namespace { + return s, nil + } + } + return nil, errors.NewNotFound(schema.GroupResource{}, name) +} + +type ingressLister struct { + Err error + Items []*extensionsv1beta1.Ingress +} + +func (r *ingressLister) List(selector labels.Selector) (ret []*extensionsv1beta1.Ingress, err error) { + return r.Items, r.Err +} +func (r *ingressLister) Ingresses(namespace string) extensionslisters.IngressNamespaceLister { + return &nsIngressLister{r: r, ns: namespace} +} + +type nsIngressLister struct { + r *ingressLister + ns string +} + +func (r *nsIngressLister) List(selector labels.Selector) (ret []*extensionsv1beta1.Ingress, err error) { + return r.r.Items, r.r.Err +} +func (r *nsIngressLister) Get(name string) (*extensionsv1beta1.Ingress, error) { + for _, s := range r.r.Items { + if s.Name == name && r.ns == s.Namespace { + return s, nil + } + } + return nil, errors.NewNotFound(schema.GroupResource{}, name) +} + +type serviceLister struct { + Err error + Items []*v1.Service +} + +func (r *serviceLister) List(selector labels.Selector) (ret []*v1.Service, err error) { + return r.Items, r.Err +} +func (r *serviceLister) Services(namespace string) corelisters.ServiceNamespaceLister { + return &nsServiceLister{r: r, ns: namespace} +} + +func (r *serviceLister) GetPodServices(pod *v1.Pod) ([]*v1.Service, error) { + panic("unsupported") +} + +type nsServiceLister struct { + r *serviceLister + ns string +} + +func (r *nsServiceLister) List(selector labels.Selector) (ret []*v1.Service, err error) { + return r.r.Items, r.r.Err +} +func (r *nsServiceLister) Get(name string) (*v1.Service, error) { + for _, s := range r.r.Items { + if s.Name == name && r.ns == s.Namespace { + return s, nil + } + } + return nil, errors.NewNotFound(schema.GroupResource{}, name) +} + +type secretLister struct { + Err error + Items []*v1.Secret +} + +func (r *secretLister) List(selector labels.Selector) (ret []*v1.Secret, err error) { + return r.Items, r.Err +} +func (r *secretLister) Secrets(namespace string) corelisters.SecretNamespaceLister { + return &nsSecretLister{r: r, ns: namespace} +} + +type nsSecretLister struct { + r *secretLister + ns string +} + +func (r *nsSecretLister) List(selector labels.Selector) (ret []*v1.Secret, err error) { + return r.r.Items, r.r.Err +} +func (r *nsSecretLister) Get(name string) (*v1.Secret, error) { + for _, s := range r.r.Items { + if s.Name == name && r.ns == s.Namespace { + return s, nil + } + } + return nil, errors.NewNotFound(schema.GroupResource{}, name) +} + +const complexIngress = ` +apiVersion: extensions/v1beta1 +kind: Ingress +metadata: + name: test-1 + namespace: test +spec: + rules: + - host: 1.ingress-test.com + http: + paths: + - path: /test + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - path: /other + backend: + serviceName: ingress-endpoint-2 + servicePort: 80 + - host: 2.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - host: 3.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 +` + +func TestController_stabilizeAfterCreate(t *testing.T) { + obj, _, err := scheme.Codecs.UniversalDeserializer().Decode([]byte(complexIngress), nil, nil) + if err != nil { + t.Fatal(err) + } + ingress := obj.(*extensionsv1beta1.Ingress) + + i := &ingressLister{ + Items: []*extensionsv1beta1.Ingress{ + ingress, + }, + } + r := &routeLister{} + s := &secretLister{} + svc := &serviceLister{Items: []*v1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-endpoint-1", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Port: 80, + TargetPort: intstr.FromInt(8080), + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-endpoint-2", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "80-tcp", + Port: 80, + TargetPort: intstr.FromInt(8080), + }, + }, + }, + }, + }} + + var names []string + kc := &fake.Clientset{} + kc.AddReactor("*", "routes", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + switch a := action.(type) { + case clientgotesting.CreateAction: + obj := a.GetObject().DeepCopyObject() + m := obj.(metav1.Object) + if len(m.GetName()) == 0 { + m.SetName(m.GetGenerateName()) + } + names = append(names, m.GetName()) + return true, obj, nil + } + return true, nil, nil + }) + + c := &Controller{ + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ingress-to-route-test"), + client: kc.Route(), + ingressLister: i, + routeLister: r, + secretLister: s, + serviceLister: svc, + expectations: newExpectations(), + } + defer c.queue.ShutDown() + + // load the ingresses for the namespace + if err := c.sync(queueKey{namespace: "test"}); err != nil { + t.Errorf("Controller.sync() error = %v", err) + } + if c.queue.Len() != 1 { + t.Fatalf("Controller.sync() unexpected queue: %#v", c.queue.Len()) + } + actions := kc.Actions() + if len(actions) != 0 { + t.Fatalf("Controller.sync() unexpected actions: %#v", actions) + } + + // process the ingress + key, _ := c.queue.Get() + expectKey := queueKey{namespace: ingress.Namespace, name: ingress.Name} + if key.(queueKey) != expectKey { + t.Fatalf("incorrect key: %v", key) + } + if err := c.sync(key.(queueKey)); err != nil { + t.Fatalf("Controller.sync() error = %v", err) + } + c.queue.Done(key) + if c.queue.Len() != 0 { + t.Fatalf("Controller.sync() unexpected queue: %#v", c.queue.Len()) + } + actions = kc.Actions() + if len(actions) == 0 { + t.Fatalf("Controller.sync() unexpected actions: %#v", actions) + } + if !c.expectations.Expecting("test", "test-1") { + t.Fatalf("Controller.sync() should be holding an expectation: %#v", c.expectations.expect) + } + + for _, action := range actions { + switch action.GetVerb() { + case "create": + switch o := action.(clientgotesting.CreateAction).GetObject().(type) { + case *routev1.Route: + r.Items = append(r.Items, o) + c.processRoute(o) + default: + t.Fatalf("Unexpected create: %T", o) + } + default: + t.Fatalf("Unexpected action: %#v", action) + } + } + if c.queue.Len() != 1 { + t.Fatalf("Controller.sync() unexpected queue: %#v", c.queue.Len()) + } + if c.expectations.Expecting("test", "test-1") { + t.Fatalf("Controller.sync() should have cleared all expectations: %#v", c.expectations.expect) + } + c.expectations.Expect("test", "test-1", names[0]) + + // waiting for a single expected route, will do nothing + key, _ = c.queue.Get() + if err := c.sync(key.(queueKey)); err != nil { + t.Errorf("Controller.sync() error = %v", err) + } + c.queue.Done(key) + actions = kc.Actions() + if len(actions) == 0 { + t.Fatalf("Controller.sync() unexpected actions: %#v", actions) + } + if c.queue.Len() != 1 { + t.Fatalf("Controller.sync() unexpected queue: %#v", c.queue.Len()) + } + c.expectations.Satisfied("test", "test-1", names[0]) + + // steady state, nothing has changed + key, _ = c.queue.Get() + if err := c.sync(key.(queueKey)); err != nil { + t.Errorf("Controller.sync() error = %v", err) + } + c.queue.Done(key) + actions = kc.Actions() + if len(actions) == 0 { + t.Fatalf("Controller.sync() unexpected actions: %#v", actions) + } + if c.queue.Len() != 0 { + t.Fatalf("Controller.sync() unexpected queue: %#v", c.queue.Len()) + } +} + +func newTestExpectations(fn func(*expectations)) *expectations { + e := newExpectations() + fn(e) + return e +} + +func TestController_sync(t *testing.T) { + services := &serviceLister{Items: []*v1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "service-1", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Port: 80, + TargetPort: intstr.FromInt(8080), + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "service-2", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "80-tcp", + Port: 80, + TargetPort: intstr.FromInt(8080), + }, + }, + }, + }, + }} + secrets := &secretLister{Items: []*v1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-0", + Namespace: "test", + }, + Type: v1.SecretTypeOpaque, + Data: map[string][]byte{ + v1.TLSCertKey: []byte(`cert`), + v1.TLSPrivateKeyKey: []byte(`key`), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-1", + Namespace: "test", + }, + Type: v1.SecretTypeTLS, + Data: map[string][]byte{ + v1.TLSCertKey: []byte(`cert`), + v1.TLSPrivateKeyKey: []byte(`key`), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-1a", + Namespace: "test", + }, + Type: v1.SecretTypeTLS, + Data: map[string][]byte{ + v1.TLSCertKey: []byte(`cert`), + v1.TLSPrivateKeyKey: []byte(`key2`), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-2", + Namespace: "test", + }, + Type: v1.SecretTypeTLS, + Data: map[string][]byte{ + v1.TLSPrivateKeyKey: []byte(`key`), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-3", + Namespace: "test", + }, + Type: v1.SecretTypeTLS, + Data: map[string][]byte{ + v1.TLSCertKey: []byte(``), + v1.TLSPrivateKeyKey: []byte(``), + }, + }, + }} + boolTrue := true + type fields struct { + i extensionslisters.IngressLister + r routelisters.RouteLister + s corelisters.SecretLister + svc corelisters.ServiceLister + } + tests := []struct { + name string + fields fields + args queueKey + expects *expectations + wantErr bool + wantCreates []*routev1.Route + wantPatches []clientgotesting.PatchActionImpl + wantDeletes []clientgotesting.DeleteActionImpl + wantQueue []queueKey + wantExpectation *expectations + wantExpects []queueKey + }{ + { + name: "no changes", + fields: fields{i: &ingressLister{}, r: &routeLister{}}, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "sync namespace - no ingress", + fields: fields{i: &ingressLister{}, r: &routeLister{}}, + args: queueKey{namespace: "test"}, + }, + { + name: "sync namespace - two ingress", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "2", + Namespace: "test", + }, + }, + }}, + r: &routeLister{}, + }, + args: queueKey{namespace: "test"}, + wantQueue: []queueKey{ + {namespace: "test", name: "1"}, + {namespace: "test", name: "2"}, + }, + }, + { + name: "ignores incomplete ingress - no host", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/deep", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "ignores incomplete ingress - no service", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/deep", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "ignores incomplete ingress - no paths", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{}, + }, + }, + }, + }, + }, + }}, + r: &routeLister{}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "create route", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/deep", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantExpects: []queueKey{{namespace: "test", name: "1"}}, + wantCreates: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/deep", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + }, + }, + }, + }, + { + name: "create route - blocked by expectation", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/deep", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{}, + }, + expects: newTestExpectations(func(e *expectations) { + e.Expect("test", "1", "route-test-1") + }), + args: queueKey{namespace: "test", name: "1"}, + wantQueue: []queueKey{{namespace: "test", name: "1"}}, + // preserves the expectations unchanged + wantExpectation: newTestExpectations(func(e *expectations) { + e.Expect("test", "1", "route-test-1") + }), + }, + { + name: "update route", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(80), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantPatches: []clientgotesting.PatchActionImpl{ + { + Name: "1-abcdef", + Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080}}}]`), + }, + }, + }, + { + name: "no-op", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "no-op - ignore partially owned resource", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + // this route is identical to the ingress + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + // this route should be left as is because controller is not true + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-empty", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1"}}, + }, + Spec: routev1.RouteSpec{}, + }, + // this route should be ignored because it doesn't match the ingress name + { + ObjectMeta: metav1.ObjectMeta{ + Name: "2-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "2", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "update ingress with missing secret ref", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-4"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + Name: "1-abcdef", + }, + }, + }, + { + name: "update ingress to not reference secret", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com1"}, SecretName: "secret-1"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Key: "key", + Certificate: "cert", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantPatches: []clientgotesting.PatchActionImpl{ + { + Name: "1-abcdef", + Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080}}}]`), + }, + }, + }, + { + name: "update route - tls config missing", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-1"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantPatches: []clientgotesting.PatchActionImpl{ + { + Name: "1-abcdef", + Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080},"tls":{"termination":"edge","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}}]`), + }, + }, + }, + { + name: "update route - secret values changed", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-1a"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Key: "key", + Certificate: "cert", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantPatches: []clientgotesting.PatchActionImpl{ + { + Name: "1-abcdef", + Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080},"tls":{"termination":"edge","certificate":"cert","key":"key2"}}}]`), + }, + }, + }, + { + name: "no-op - has TLS", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-1"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Key: "key", + Certificate: "cert", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "no-op - has secret with empty keys", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-3"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Key: "", + Certificate: "", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "no-op - termination policy has been changed by the user", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-1"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + Key: "key", + Certificate: "cert", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + { + name: "delete route when referenced secret is not TLS", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-0"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Key: "key", + Certificate: "cert", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + Name: "1-abcdef", + }, + }, + }, + { + name: "delete route when referenced secret is not valid", + fields: fields{ + i: &ingressLister{Items: []*extensionsv1beta1.Ingress{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1", + Namespace: "test", + }, + Spec: extensionsv1beta1.IngressSpec{ + TLS: []extensionsv1beta1.IngressTLS{ + {Hosts: []string{"test.com"}, SecretName: "secret-2"}, + }, + Rules: []extensionsv1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: extensionsv1beta1.IngressRuleValue{ + HTTP: &extensionsv1beta1.HTTPIngressRuleValue{ + Paths: []extensionsv1beta1.HTTPIngressPath{ + { + Path: "/", Backend: extensionsv1beta1.IngressBackend{ + ServiceName: "service-1", + ServicePort: intstr.FromString("http"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{ + Host: "test.com", + Path: "/", + To: routev1.RouteTargetReference{ + Name: "service-1", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + Key: "key", + Certificate: "", + }, + }, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + wantDeletes: []clientgotesting.DeleteActionImpl{ + { + Name: "1-abcdef", + }, + }, + }, + { + name: "ignore route when parent ingress no longer exists (gc will handle)", + fields: fields{ + i: &ingressLister{}, + r: &routeLister{Items: []*routev1.Route{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "1-abcdef", + Namespace: "test", + OwnerReferences: []metav1.OwnerReference{{APIVersion: "extensions/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + }, + Spec: routev1.RouteSpec{}, + }, + }}, + }, + args: queueKey{namespace: "test", name: "1"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var names []string + kc := &fake.Clientset{} + kc.AddReactor("*", "routes", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + switch a := action.(type) { + case clientgotesting.CreateAction: + obj := a.GetObject().DeepCopyObject() + m := obj.(metav1.Object) + if len(m.GetName()) == 0 { + m.SetName(m.GetGenerateName()) + } + names = append(names, m.GetName()) + return true, obj, nil + } + return true, nil, nil + }) + + c := &Controller{ + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ingress-to-route-test"), + client: kc.Route(), + ingressLister: tt.fields.i, + routeLister: tt.fields.r, + secretLister: tt.fields.s, + serviceLister: tt.fields.svc, + expectations: tt.expects, + } + // default these + if c.expectations == nil { + c.expectations = newExpectations() + } + if c.secretLister == nil { + c.secretLister = secrets + } + if c.serviceLister == nil { + c.serviceLister = services + } + + if err := c.sync(tt.args); (err != nil) != tt.wantErr { + t.Errorf("Controller.sync() error = %v, wantErr %v", err, tt.wantErr) + } + + c.queue.ShutDown() + var hasQueue []queueKey + for { + key, shutdown := c.queue.Get() + if shutdown { + break + } + hasQueue = append(hasQueue, key.(queueKey)) + } + if !reflect.DeepEqual(tt.wantQueue, hasQueue) { + t.Errorf("unexpected queue: %s", diff.ObjectReflectDiff(tt.wantQueue, hasQueue)) + } + + wants := tt.wantExpectation + if wants == nil { + wants = newTestExpectations(func(e *expectations) { + for _, key := range tt.wantExpects { + for _, routeName := range names { + e.Expect(key.namespace, key.name, routeName) + } + } + }) + } + if !reflect.DeepEqual(wants, c.expectations) { + t.Errorf("unexpected expectations: %s", diff.ObjectReflectDiff(wants.expect, c.expectations.expect)) + } + + actions := kc.Actions() + + for i := range tt.wantCreates { + if i > len(actions)-1 { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + if actions[i].GetVerb() != "create" { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + action := actions[i].(clientgotesting.CreateAction) + if action.GetNamespace() != tt.args.namespace { + t.Errorf("unexpected action[%d]: %#v", i, action) + } + obj := action.GetObject() + if tt.wantCreates[i].Name == "" { + tt.wantCreates[i].Name = names[0] + names = names[1:] + } + if !reflect.DeepEqual(tt.wantCreates[i], obj) { + t.Errorf("unexpected create: %s", diff.ObjectReflectDiff(tt.wantCreates[i], obj)) + } + } + actions = actions[len(tt.wantCreates):] + + for i := range tt.wantPatches { + if i > len(actions)-1 { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + if actions[i].GetVerb() != "patch" { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + action := actions[i].(clientgotesting.PatchAction) + if action.GetNamespace() != tt.args.namespace || action.GetName() != tt.wantPatches[i].Name { + t.Errorf("unexpected action[%d]: %#v", i, action) + } + if !reflect.DeepEqual(string(action.GetPatch()), string(tt.wantPatches[i].Patch)) { + t.Errorf("unexpected action[%d]: %s", i, string(action.GetPatch())) + } + } + actions = actions[len(tt.wantPatches):] + + for i := range tt.wantDeletes { + if i > len(actions)-1 { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + if actions[i].GetVerb() != "delete" { + t.Fatalf("Controller.sync() unexpected actions: %#v", kc.Actions()) + } + action := actions[i].(clientgotesting.DeleteAction) + if action.GetName() != tt.wantDeletes[i].Name || action.GetNamespace() != tt.args.namespace { + t.Errorf("unexpected action[%d]: %#v", i, action) + } + } + actions = actions[len(tt.wantDeletes):] + + if len(actions) != 0 { + t.Fatalf("Controller.sync() unexpected actions: %#v", actions) + } + }) + } +} diff --git a/pkg/router/controller/extended_validator.go b/pkg/router/controller/extended_validator.go index 93de90d59d8a..4b11262d4a97 100644 --- a/pkg/router/controller/extended_validator.go +++ b/pkg/router/controller/extended_validator.go @@ -2,7 +2,6 @@ package controller import ( "fmt" - "reflect" "github.com/golang/glog" "k8s.io/apimachinery/pkg/util/sets" @@ -22,9 +21,6 @@ type ExtendedValidator struct { // recorder is an interface for indicating route rejections. recorder RejectionRecorder - - // invalidRoutes is a map of invalid routes previously encountered. - invalidRoutes map[string]routeapi.Route } // NewExtendedValidator creates a plugin wrapper that ensures only routes that @@ -32,9 +28,8 @@ type ExtendedValidator struct { // Recorder is an interface for indicating why a route was rejected. func NewExtendedValidator(plugin router.Plugin, recorder RejectionRecorder) *ExtendedValidator { return &ExtendedValidator{ - plugin: plugin, - recorder: recorder, - invalidRoutes: make(map[string]routeapi.Route), + plugin: plugin, + recorder: recorder, } } @@ -52,13 +47,6 @@ func (p *ExtendedValidator) HandleEndpoints(eventType watch.EventType, endpoints func (p *ExtendedValidator) HandleRoute(eventType watch.EventType, route *routeapi.Route) error { // Check if previously seen route and its Spec is unchanged. routeName := routeNameKey(route) - old, ok := p.invalidRoutes[routeName] - if ok && reflect.DeepEqual(old.Spec, route.Spec) { - // Route spec was unchanged and it is already marked in - // error, we don't need to do anything more. - return fmt.Errorf("invalid route configuration") - } - if errs := validation.ExtendedValidateRoute(route); len(errs) > 0 { errmsg := "" for i := 0; i < len(errs); i++ { @@ -67,6 +55,7 @@ func (p *ExtendedValidator) HandleRoute(eventType watch.EventType, route *routea glog.Errorf("Skipping route %s due to invalid configuration: %s", routeName, errmsg) p.recorder.RecordRouteRejection(route, "ExtendedValidationFailed", errmsg) + p.plugin.HandleRoute(watch.Deleted, route) return fmt.Errorf("invalid route configuration") } diff --git a/pkg/router/controller/factory/factory.go b/pkg/router/controller/factory/factory.go index 7f2ab7cb4622..58e7345fbf64 100644 --- a/pkg/router/controller/factory/factory.go +++ b/pkg/router/controller/factory/factory.go @@ -17,7 +17,6 @@ import ( "k8s.io/apimachinery/pkg/watch" kcache "k8s.io/client-go/tools/cache" kapi "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/extensions" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" projectclient "github.com/openshift/origin/pkg/project/generated/internalclientset/typed/project/internalversion" @@ -65,12 +64,10 @@ func NewDefaultRouterControllerFactory(rc routeclientset.Interface, pc projectcl // Create begins listing and watching against the API server for the desired route and endpoint // resources. It spawns child goroutines that cannot be terminated. -func (f *RouterControllerFactory) Create(plugin router.Plugin, watchNodes, enableIngress bool) *routercontroller.RouterController { +func (f *RouterControllerFactory) Create(plugin router.Plugin, watchNodes bool) *routercontroller.RouterController { rc := &routercontroller.RouterController{ - Plugin: plugin, - WatchNodes: watchNodes, - EnableIngress: enableIngress, - IngressTranslator: routercontroller.NewIngressTranslator(f.KClient.Core()), + Plugin: plugin, + WatchNodes: watchNodes, NamespaceLabels: f.NamespaceLabels, FilteredNamespaceNames: make(sets.String), @@ -107,10 +104,6 @@ func (f *RouterControllerFactory) initInformers(rc *routercontroller.RouterContr if rc.WatchNodes { f.createNodesSharedInformer(rc) } - if rc.EnableIngress { - f.createIngressesSharedInformer(rc) - f.createSecretsSharedInformer(rc) - } // Start informers for _, informer := range f.informers { @@ -134,10 +127,6 @@ func (f *RouterControllerFactory) registerInformerEventHandlers(rc *routercontro if rc.WatchNodes { f.registerSharedInformerEventHandlers(&kapi.Node{}, rc.HandleNode) } - if rc.EnableIngress { - f.registerSharedInformerEventHandlers(&extensions.Ingress{}, rc.HandleIngress) - f.registerSharedInformerEventHandlers(&kapi.Secret{}, rc.HandleSecret) - } } func (f *RouterControllerFactory) informerStoreList(obj runtime.Object) []interface{} { @@ -191,16 +180,6 @@ func (f *RouterControllerFactory) processExistingItems(rc *routercontroller.Rout rc.HandleNode(watch.Added, item.(*kapi.Node)) } } - - if rc.EnableIngress { - for _, item := range f.informerStoreList(&extensions.Ingress{}) { - rc.HandleIngress(watch.Added, item.(*extensions.Ingress)) - } - - for _, item := range f.informerStoreList(&kapi.Secret{}) { - rc.HandleSecret(watch.Added, item.(*kapi.Secret)) - } - } } func (f *RouterControllerFactory) setSelectors(options *v1.ListOptions) { @@ -249,25 +228,6 @@ func (f *RouterControllerFactory) createRoutesSharedInformer(rc *routercontrolle f.informers[objType] = informer } -func (f *RouterControllerFactory) createIngressesSharedInformer(rc *routercontroller.RouterController) { - // The same filtering is applied to ingress as is applied to routes - lw := &kcache.ListWatch{ - ListFunc: func(options v1.ListOptions) (runtime.Object, error) { - f.setSelectors(&options) - return f.KClient.Extensions().Ingresses(f.Namespace).List(options) - }, - WatchFunc: func(options v1.ListOptions) (watch.Interface, error) { - f.setSelectors(&options) - return f.KClient.Extensions().Ingresses(f.Namespace).Watch(options) - }, - } - ig := &extensions.Ingress{} - objType := reflect.TypeOf(ig) - indexers := kcache.Indexers{kcache.NamespaceIndex: kcache.MetaNamespaceIndexFunc} - informer := kcache.NewSharedIndexInformer(lw, ig, f.ResyncInterval, indexers) - f.informers[objType] = informer -} - func (f *RouterControllerFactory) createNodesSharedInformer(rc *routercontroller.RouterController) { // Use stock node informer as we don't need namespace/labels/fields filtering on nodes ifactory := informerfactory.NewSharedInformerFactory(f.KClient, f.ResyncInterval) @@ -276,22 +236,6 @@ func (f *RouterControllerFactory) createNodesSharedInformer(rc *routercontroller f.informers[objType] = informer } -func (f *RouterControllerFactory) createSecretsSharedInformer(rc *routercontroller.RouterController) { - lw := &kcache.ListWatch{ - ListFunc: func(options v1.ListOptions) (runtime.Object, error) { - return f.KClient.Core().Secrets(f.Namespace).List(options) - }, - WatchFunc: func(options v1.ListOptions) (watch.Interface, error) { - return f.KClient.Core().Secrets(f.Namespace).Watch(options) - }, - } - sc := &kapi.Secret{} - objType := reflect.TypeOf(sc) - indexers := kcache.Indexers{kcache.NamespaceIndex: kcache.MetaNamespaceIndexFunc} - informer := kcache.NewSharedIndexInformer(lw, sc, f.ResyncInterval, indexers) - f.informers[objType] = informer -} - func (f *RouterControllerFactory) createNamespacesSharedInformer(rc *routercontroller.RouterController) { lw := &kcache.ListWatch{ ListFunc: func(options v1.ListOptions) (runtime.Object, error) { diff --git a/pkg/router/controller/host_admitter.go b/pkg/router/controller/host_admitter.go index b54c73e0d730..4ffb065a6c8c 100644 --- a/pkg/router/controller/host_admitter.go +++ b/pkg/router/controller/host_admitter.go @@ -125,6 +125,7 @@ func (p *HostAdmitter) HandleRoute(eventType watch.EventType, route *routeapi.Ro if err := p.admitter(route); err != nil { glog.V(4).Infof("Route %s not admitted: %s", routeNameKey(route), err.Error()) p.recorder.RecordRouteRejection(route, "RouteNotAdmitted", err.Error()) + p.plugin.HandleRoute(watch.Deleted, route) return err } diff --git a/pkg/router/controller/ingress.go b/pkg/router/controller/ingress.go deleted file mode 100644 index 8a1d88633412..000000000000 --- a/pkg/router/controller/ingress.go +++ /dev/null @@ -1,574 +0,0 @@ -package controller - -import ( - "crypto/md5" - "fmt" - "strings" - "sync" - - "github.com/golang/glog" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/watch" - kapi "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/extensions" - kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" - - routeapi "github.com/openshift/origin/pkg/route/apis/route" -) - -// ingressRouteEvents maps an ingress key to route events -// generated from the ingress. It is intended to be the -// return type used by the ingress and secret event -// translation methods. -type ingressRouteEvents struct { - ingressKey string - routeEvents []routeEvent -} - -type routeEvent struct { - eventType watch.EventType - route *routeapi.Route -} - -type ingressRouteMap map[string]*routeapi.Route - -type ingressMap map[string]*extensions.Ingress - -// IngressTranslator converts secret and ingress events into route events. -// -// - Caches ingresses to enable: -// - Identification of which secrets are referenced by ingresses -// - Generation of route events in response to a secret event -// - Route deletion when an ingress rule is removed -// -// - Caches secrets to minimize the lookups required to generate route events from an ingress -// - Secrets will be read into the cache via Get() the first time they are referenced -// - Only secrets referenced by an ingress cached by the router will themselves be cached -// - Secrets will be updated by calls to TranslateSecretEvent -type IngressTranslator struct { - // Synchronize access to the maps - lock sync.Mutex - - // Tracks ingresses seen by the router. Key is [namespace]/[ingress name]. - ingressMap ingressMap - - // Caches tls from secrets referenced by ingresses. Key is - // [namespace]/[secret name]. Entries should be added to the map - // for all referenced secrets - not just those that could be - // retrieved and read - to ensure that a retrieval failure can be - // incurred only once per secret rather than once per reference. - tlsMap map[string]*referencedTLS - - // Enables ingress handling to lookup referenced secrets - client kcoreclient.SecretsGetter - - // If non-nil, ingresses will only be translated into routes if - // they are in an allowed namespace - allowedNamespaces sets.String -} - -// referencedTLS records the ingress keys ([namespace]/[name]) of ingresses that reference a -// given tls configuration. Intended for use in a map indexed by secret key. -type referencedTLS struct { - ingressKeys sets.String - tls *cachedTLS -} - -// cachedTLS caches the cert and key data read from an ingress-referenced secret -type cachedTLS struct { - cert string - privateKey string -} - -// getRouteTLS converts cached tls to route tls -func (ctls *cachedTLS) getRouteTLS() *routeapi.TLSConfig { - // Defaults will be used for route tls options that cannot - // be configured from an ingress resource: - // - // - Termination - // - CACertificate - // - DestinationCACertificate - // - InsecureEdgeTerminationPolicy - // - // TODO Until ingress is extended to support the - // necessary configuration (directly or via - // annotations), it may be desirable to set defaults - // via router options. - return &routeapi.TLSConfig{ - // From SetDefaults_TLSConfig - Termination: routeapi.TLSTerminationEdge, - - InsecureEdgeTerminationPolicy: routeapi.InsecureEdgeTerminationPolicyAllow, - - Certificate: ctls.cert, - Key: ctls.privateKey, - } -} - -// NewIngressTranslator creates a new cache for the given client -func NewIngressTranslator(kc kcoreclient.SecretsGetter) *IngressTranslator { - return &IngressTranslator{ - ingressMap: make(map[string]*extensions.Ingress), - tlsMap: make(map[string]*referencedTLS), - client: kc, - } -} - -// TranslateIngressEvent converts an ingress event into route events. -func (it *IngressTranslator) TranslateIngressEvent(eventType watch.EventType, ingress *extensions.Ingress) []ingressRouteEvents { - it.lock.Lock() - defer it.lock.Unlock() - - // Do not return events for ingresses outside the set of allowed namespaces - if it.allowedNamespaces != nil && !it.allowedNamespaces.Has(ingress.Namespace) { - return []ingressRouteEvents{} - } - - events := it.unsafeTranslateIngressEvent(eventType, ingress) - return []ingressRouteEvents{events} -} - -// TranslateSecretEvent converts the given secret event into route events. -func (it *IngressTranslator) TranslateSecretEvent(eventType watch.EventType, secret *kapi.Secret) (events []ingressRouteEvents) { - key := getResourceKey(secret.ObjectMeta) - - events = []ingressRouteEvents{} - - it.lock.Lock() - defer it.lock.Unlock() - - refTLS := it.tlsMap[key] - if refTLS == nil { - // If the secret has not been cached, no referencing ingresses - // have been seen and the event can be ignored. - return - } - - switch eventType { - case watch.Added, watch.Modified: - newTLS, err := tlsFromSecret(secret) - // If an error was encountered, newTLS will be nil and tls - // previously cached for the secret will be removed. - if err != nil { - glog.V(4).Info(err) - } - - // Avoid returning events for tls that hasn't changed - if newTLS == refTLS.tls { - return - } - - refTLS.tls = newTLS - case watch.Deleted: - // Clear the tls but don't remove the cache entry. It's - // desirable to keep track of secrets referenced by - // ingresses even where no tls data exists to ensure that - // the tls cache can be updated by future secret events and - // that secret retrieval is only attempted the first time it - // is referenced rather than for every ingress. - refTLS.tls = nil - } - - // Generate route events for ingresses that reference this secret - for key := range refTLS.ingressKeys { - ingress := it.ingressMap[key] - routeEvents := it.unsafeTranslateIngressEvent(watch.Modified, ingress) - events = append(events, routeEvents) - } - - return -} - -// UpdateNamespaces sets which namespaces ingress objects are allowed from and updates the cache accordingly. -func (it *IngressTranslator) UpdateNamespaces(namespaces sets.String) { - it.lock.Lock() - defer it.lock.Unlock() - - it.allowedNamespaces = namespaces - - for _, ingress := range it.ingressMap { - if !namespaces.Has(ingress.Namespace) { - it.dereferenceIngress(ingress) - } - } -} - -// unsafeTranslateIngressEvent converts an ingress event into route events without -// applying a lock so that it can be called by both of the Translate methods. -func (it *IngressTranslator) unsafeTranslateIngressEvent(eventType watch.EventType, ingress *extensions.Ingress) ingressRouteEvents { - ingressKey := getResourceKey(ingress.ObjectMeta) - - // Get a reference to an existing ingress before updating the cache - oldIngress := it.ingressMap[ingressKey] - - // Update cache state - it.handleIngressEvent(eventType, ingress, oldIngress) - - routeEvents := it.generateRouteEvents(eventType, ingress, oldIngress) - - return ingressRouteEvents{ - ingressKey: ingressKey, - routeEvents: routeEvents, - } -} - -// handleIngressEvent updates the cache of the translator in response to the provided ingress event. -func (it *IngressTranslator) handleIngressEvent(eventType watch.EventType, ingress, oldIngress *extensions.Ingress) { - switch eventType { - case watch.Added, watch.Modified: - key := getResourceKey(ingress.ObjectMeta) - it.ingressMap[key] = ingress - it.cacheTLS(ingress.Spec.TLS, ingress.Namespace, key) - case watch.Deleted: - it.dereferenceIngress(oldIngress) - } -} - -// generateRouteEvents computes route events implied by an ingress event. The old ingress is used in computing deletions. -func (it *IngressTranslator) generateRouteEvents(eventType watch.EventType, ingress, oldIngress *extensions.Ingress) []routeEvent { - routeEvents := []routeEvent{} - - // Compute the routes for the ingress - var routeNames sets.String - var routes []*routeapi.Route - if eventType == watch.Deleted { - // A deleted ingress implies no routes - } else { - // Process ingress into routes even if tls configuration was not cached. So long as the default insecure - // edge termination policy is 'allow', there's no point in refusing to serve routes for which tls - // configuration may be missing. - // - // TODO Revisit this if/when edge termination policy becomes configurable. - routes, routeNames = ingressToRoutes(ingress) - } - - if oldIngress != nil { - // Diff the routes from an existing ingress and the new ingress to ensure: - // - // - rule deletion results in a deletion event for a route that was - // generated from the rule - // - // - a deleted ingress (indicated by an empty route map) results in deletion - // events for all routes generated from the previous ingress. - oldRoutes, _ := ingressToRoutes(oldIngress) - for _, oldRoute := range oldRoutes { - if !routeNames.Has(oldRoute.Name) { - // Not necessary to add TLS to routes marked for deletion. - routeEvents = append(routeEvents, routeEvent{ - eventType: watch.Deleted, - route: oldRoute, - }) - } - } - } - - // Add events for all routes in the route map. Assume tls has been cached. - for _, route := range routes { - tls := it.tlsForHost(ingress.Spec.TLS, ingress.Namespace, route.Spec.Host) - if tls != nil { - route.Spec.TLS = tls.getRouteTLS() - } - routeEvents = append(routeEvents, routeEvent{ - eventType: eventType, - route: route, - }) - } - - return routeEvents -} - -// cacheTLS attempts to populate the tls cache for all secrets referenced by an ingress. -func (it *IngressTranslator) cacheTLS(ingressTLS []extensions.IngressTLS, namespace, ingressKey string) bool { - success := true - - for _, tls := range ingressTLS { - secretKey := getKey(namespace, tls.SecretName) - - var refTLS *referencedTLS - - if refTLS = it.tlsMap[secretKey]; refTLS == nil { - // Attempt to retrieve a secret the first time it is referenced - - refTLS = &referencedTLS{ - ingressKeys: sets.String{}, - } - - it.tlsMap[secretKey] = refTLS - - // Attempt to retrieve the secret and read tls configuration from it. - // If retrieval or reading fails, the tls for the cache entry will be - // nil and only be populated in response to a subsequent event for the - // secret that provides valid tls data. - // - // TODO should initial retrieval be retried to minimize the chances - // that a temporary connectivity failure delays route availability - // until the next sync event (when the secret will next be seen)? - if secret, err := it.client.Secrets(namespace).Get(tls.SecretName, metav1.GetOptions{}); err != nil { - glog.V(4).Infof("Error retrieving secret %v: %v", secretKey, err) - } else { - newTLS, err := tlsFromSecret(secret) - // If an error was encountered, newTLS will be nil - if err != nil { - glog.V(4).Info(err) - } - refTLS.tls = newTLS - } - } - - // Indicate that the secret is referenced by this ingress - refTLS.ingressKeys.Insert(ingressKey) - - if refTLS.tls == nil { - glog.V(4).Infof("Unable to source TLS configuration for ingress %v from secret %v", ingressKey, secretKey) - success = false - } - } - - return success -} - -// dereferenceIngress ensures the given ingress is removed from the cache. -func (it *IngressTranslator) dereferenceIngress(ingress *extensions.Ingress) { - if ingress != nil { - key := getResourceKey(ingress.ObjectMeta) - delete(it.ingressMap, key) - it.dereferenceTLS(ingress.Spec.TLS, ingress.Namespace, key) - } -} - -// dereferenceTLS removes references from ingress tls to cached tls. -func (it *IngressTranslator) dereferenceTLS(ingressTLS []extensions.IngressTLS, namespace, ingressKey string) { - for _, tls := range ingressTLS { - secretKey := getKey(namespace, tls.SecretName) - if refTLS, ok := it.tlsMap[secretKey]; ok { - refTLS.ingressKeys.Delete(ingressKey) - - // TLS that is no longer referenced can be deleted - if len(refTLS.ingressKeys) == 0 { - delete(it.tlsMap, secretKey) - } - } - } -} - -// tlsForHost attempts to retrieve ingress tls configuration for the given host -func (it *IngressTranslator) tlsForHost(ingressTLS []extensions.IngressTLS, namespace, host string) (match *cachedTLS) { - for _, tls := range ingressTLS { - for _, tlsHost := range tls.Hosts { - if len(host) == 0 { - // Only match an empty host to tls without hosts defined - if len(tlsHost) > 0 { - continue - } - } else if !matchesHost(tlsHost, host) { - continue - } - - key := getKey(namespace, tls.SecretName) - refTLS := it.tlsMap[key] - if refTLS == nil { - // A cache entry should exist for each secret referenced by an ingress, - // so this condition indicates a serious problem. - glog.Errorf("Secret %v missing from the ingress translator cache", key) - continue - } - - // Pick the first tls that matches the host. Continue iterating over all tls - // to allow logging of subsequent matches. - if match == nil { - match = refTLS.tls - } else { - if len(host) == 0 { - host = "" - } - glog.Warningf("More than one tls configuration matches host: %s", host) - } - } - } - return -} - -// tlsFromSecret attempts to read tls config from a secret. If an error is -// encountered, nil config will be returned. -func tlsFromSecret(secret *kapi.Secret) (*cachedTLS, error) { - // TODO validate that the cert and key are within reasonable size limits - - var cert, privateKey string - msgs := []string{} - - // Read the cert - rawCert, ok := secret.Data[kapi.TLSCertKey] - if ok && len(rawCert) > 0 { - cert = string(rawCert) - } else { - msgs = append(msgs, "invalid cert") - } - - // Read the key - rawPrivateKey, ok := secret.Data[kapi.TLSPrivateKeyKey] - if ok && len(rawPrivateKey) > 0 { - privateKey = string(rawPrivateKey) - } else { - msgs = append(msgs, "invalid private key") - } - - // Return tls only if both fields were loaded without error - if len(msgs) == 0 { - return &cachedTLS{ - cert: cert, - privateKey: privateKey, - }, nil - } else { - secretKey := getResourceKey(secret.ObjectMeta) - return nil, fmt.Errorf("Unable to read TLS configuration from secret %v: %v", secretKey, strings.Join(msgs, " and ")) - } -} - -// ingressToRoutes generates routes implied by the provided ingress. -// -// TLS configuration is intended to be applied separately to avoid -// coupling route generation to cache state. -func ingressToRoutes(ingress *extensions.Ingress) (routes []*routeapi.Route, routeNames sets.String) { - routeNames = sets.String{} - routes = []*routeapi.Route{} - - if ingress.Spec.Backend != nil { - key := getResourceKey(ingress.ObjectMeta) - glog.V(4).Infof("The default backend defined for ingress %v will be ignored. Default backends are not compatible with the OpenShift router.", key) - } - - for _, rule := range ingress.Spec.Rules { - if rule.HTTP == nil { - continue - } - for _, path := range rule.HTTP.Paths { - name := generateRouteName(ingress.Name, rule.Host, path.Path) - if routeNames.Has(name) { - glog.V(4).Infof("Ingress %s has more than one rule for host '%s' and path '%s'", - ingress.Name, rule.Host, path.Path) - continue - } - - route := &routeapi.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - // Reuse the values from ingress - Namespace: ingress.Namespace, - CreationTimestamp: ingress.CreationTimestamp, - // Append a hash of the name to the ingress uid to ensure both uniqueness - // and consistent sorting. - UID: types.UID(fmt.Sprintf("%s-%x", ingress.UID, md5.Sum([]byte(name)))), - }, - Spec: routeapi.RouteSpec{ - Host: rule.Host, - Path: path.Path, - To: routeapi.RouteTargetReference{ - Name: path.Backend.ServiceName, - }, - Port: &routeapi.RoutePort{ - TargetPort: path.Backend.ServicePort, - }, - }, - } - - // The router depends on defaults being set on resource creation. Manually - // set the same defaults for a generated route to ensure compatibility. - // - // TODO Consider round tripping the route through the api - // conversion to simplify maintenance of defaults. - - // From SetDefaults_RouteSpec - route.Spec.WildcardPolicy = routeapi.WildcardPolicyNone - - // From SetDefaults_RouteTargetReference - route.Spec.To.Kind = "Service" - route.Spec.To.Weight = new(int32) - *route.Spec.To.Weight = 100 - - routeNames.Insert(name) - routes = append(routes, route) - } - } - return -} - -// matchHost checks whether a pattern (which can represent a wildcard -// domain of the form *.[subdomain]) matches the given host. -func matchesHost(pattern, host string) bool { - if len(pattern) == 0 || len(host) == 0 { - return false - } - - patternParts := strings.Split(pattern, ".") - hostParts := strings.Split(host, ".") - - if len(patternParts) != len(hostParts) { - return false - } - - for i, patternPart := range patternParts { - if i == 0 && patternPart == "*" { - continue - } - if patternPart != hostParts[i] { - return false - } - } - return true -} - -func getKey(namespace, name string) string { - return fmt.Sprintf("%v/%v", namespace, name) -} - -// generateRouteName returns a stable route name for an ingress name, -// host and path. It's fine if the host or path happen to be empty. -func generateRouteName(name, host, path string) string { - // Routes generated from ingress rules contain '/' to prevent name - // clashes with user-defined routes. '/' is not permitted in a resource - // name submitted to the api, but generated routes are only intended to - // be used in the context of the router controller. - // - // Hash the path to ensure compatibility with inclusion in haproxy configuration - return fmt.Sprintf("ingress/%s/%s/%x", name, host, md5.Sum([]byte(path))) -} - -// IsGeneratedRoute indicates whether the given route name was generated from an ingress. -func IsGeneratedRouteName(name string) bool { - // The name of a route generated from an ingress rule contains '/' - // to prevent name clashes with user-defined routes. See generateRouteName - return strings.Index(name, "/") != -1 -} - -// GetNameForHost returns the name of the ingress if the route name was -// generated from a path, otherwise it returns the name as given. -func GetNameForHost(name string) string { - if IsGeneratedRouteName(name) { - // Use the ingress name embedded in the route name for the purposes - // of generating a host. The name of routes generated from ingress - // rules will be 'ingress/[name]/[host]/[path]' (see generateRouteName). - nameParts := strings.Split(name, "/") - return nameParts[1] - } - return name -} - -// GetSafeRouteName returns a name that is safe for use in an HAproxy config. -func GetSafeRouteName(name string) string { - if IsGeneratedRouteName(name) { - // The name of a route generated from an ingress path will contain '/', which - // isn't compatible with HAproxy or F5. - return strings.Replace(name, "/", ":", -1) - } - return name -} - -// TODO this should probably be removed and replaced with an upstream keyfunc -// getResourceKey returns a string of the form [namespace]/[name] for -// the given resource. This is a common way of ensuring a key for a -// resource that is unique across the cluster. -func getResourceKey(obj metav1.ObjectMeta) string { - return fmt.Sprintf("%s/%s", obj.Namespace, obj.Name) -} diff --git a/pkg/router/controller/ingress_test.go b/pkg/router/controller/ingress_test.go deleted file mode 100644 index 882ded16f5ea..000000000000 --- a/pkg/router/controller/ingress_test.go +++ /dev/null @@ -1,705 +0,0 @@ -package controller - -import ( - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/watch" - kapi "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/extensions" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" -) - -func TestIngressTranslator_TranslateIngressEvent(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - ingress := getTestIngress() - - testCases := map[string]struct { - allowedNamespaces sets.String - eventsExpected bool - }{ - "Events returned for ingress when allowed namespaces unset": { - eventsExpected: true, - }, - - "Events returned for ingress in allowed namespace": { - allowedNamespaces: sets.NewString(ingress.Namespace), - eventsExpected: true, - }, - "Events not generated for ingress not in allowed namespace": { - allowedNamespaces: sets.String{}, - }, - } - for testName, tc := range testCases { - it.allowedNamespaces = tc.allowedNamespaces - events := it.TranslateIngressEvent(watch.Added, ingress) - if tc.eventsExpected && len(events) == 0 { - t.Fatalf("%v: Events should have been returned", testName) - } - if !tc.eventsExpected && len(events) > 0 { - t.Fatalf("%v: Events should not have been returned", testName) - } - } -} - -func TestIngressTranslator_TranslateSecretEvent(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - // Cache an ingress that will reference cached tls - ingress := getTestIngress() - ingressKey := getResourceKey(ingress.ObjectMeta) - it.ingressMap[ingressKey] = ingress - - tls := &cachedTLS{ - cert: "my-cert", - privateKey: "my-private-key", - } - - secretName := "my-secret" - secretKey := getKey(ingress.Namespace, secretName) - secret := getTestSecret(ingress.Namespace, secretName, tls.cert, tls.privateKey) - - testCases := map[string]struct { - eventType watch.EventType - referencedTLS *referencedTLS - eventCount int - tlsDeleted bool - }{ - "Secret not in the cache returns no events": { - eventType: watch.Added, - }, - "Unchanged tls returns no events": { - eventType: watch.Added, - referencedTLS: &referencedTLS{ - ingressKeys: sets.String{}, - tls: tls, - }, - }, - "Changed secret returns events for affected ingresses": { - eventType: watch.Added, - referencedTLS: &referencedTLS{ - ingressKeys: sets.NewString(ingressKey), - tls: tls, - }, - eventCount: 1, - }, - "Deleted secret removes tls from cache entry and returns events for affected ingresses": { - eventType: watch.Deleted, - referencedTLS: &referencedTLS{ - ingressKeys: sets.NewString(ingressKey), - tls: tls, - }, - eventCount: 1, - tlsDeleted: true, - }, - } - for testName, tc := range testCases { - it.tlsMap[secretKey] = tc.referencedTLS - events := it.TranslateSecretEvent(tc.eventType, secret) - eventCount := len(events) - if tc.eventCount != eventCount { - t.Fatalf("%v: expected %d events, got %v", testName, tc.eventCount, eventCount) - } - if tc.tlsDeleted && tc.referencedTLS.tls != nil { - t.Fatalf("%v: expected cached tls to be removed", testName) - } - } -} - -func TestIngressTranslator_UpdateNamespaces(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - ingress := getTestIngress() - ingressKey := getResourceKey(ingress.ObjectMeta) - it.ingressMap[ingressKey] = ingress - - disallowed := getTestIngress() - disallowed.Namespace = "not-allowed" - disallowedKey := getResourceKey(disallowed.ObjectMeta) - it.ingressMap[disallowedKey] = disallowed - - namespaces := sets.NewString(ingress.Namespace) - it.UpdateNamespaces(namespaces) - - if !namespaces.Equal(it.allowedNamespaces) { - t.Fatal("Allowed namespaces not set") - } - if it.ingressMap[ingressKey] == nil { - t.Fatal("Allowed ingress removed") - } - if it.ingressMap[disallowedKey] != nil { - t.Fatal("Disallowed ingress not removed") - } -} - -func TestIngressTranslator_unsafeTranslateIngressEvent(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - ingress := getTestIngress() - events := it.TranslateIngressEvent(watch.Added, ingress) - - if len(events) != 1 { - t.Fatal("expected route events for one ingress to be generated") - } - - ingressRouteEvents := events[0] - if ingressRouteEvents.ingressKey != getResourceKey(ingress.ObjectMeta) { - t.Fatal("expected the ingress key to be set") - } - - routeEvents := ingressRouteEvents.routeEvents - if len(routeEvents) != 1 { - t.Fatal("expected a single route event to have been generated") - } -} - -func TestIngressTranslator_handleIngressEvents(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - ingress := getTestIngress() - ingressKey := getResourceKey(ingress.ObjectMeta) - secretName := "my-secret" - ingress.Spec.TLS = []extensions.IngressTLS{ - { - SecretName: secretName, - }, - } - secretKey := getKey(ingress.Namespace, secretName) - - testCases := map[string]struct { - eventType watch.EventType - ingress *extensions.Ingress - oldIngress *extensions.Ingress - cachedIngress bool - cachedSecret bool - }{ - "Should be safe to attempt deletion of an ingress that isn't in the cache": { - eventType: watch.Deleted, - ingress: ingress, - }, - "Ingress addition should cache the ingress and referenced tls": { - eventType: watch.Added, - ingress: ingress, - cachedIngress: true, - cachedSecret: true, - }, - // This test depends on the addition test to prime the cache - "Ingress deletion should remove ingress and tls from the cache": { - eventType: watch.Deleted, - ingress: ingress, - oldIngress: ingress, - }, - } - for testName, tc := range testCases { - it.handleIngressEvent(tc.eventType, tc.ingress, tc.oldIngress) - if tc.cachedIngress { - if it.ingressMap[ingressKey] != tc.ingress { - t.Fatalf("%v: ingress not cached", testName) - } - } else if it.ingressMap[ingressKey] != nil { - t.Fatalf("%v: ingress not removed from the cache", testName) - } - if tc.cachedSecret { - if it.tlsMap[secretKey] == nil { - t.Fatalf("%v: tls not cached", testName) - } - } else if it.tlsMap[secretKey] != nil { - t.Fatalf("%v: tls not removed from the cache", testName) - } - } -} - -func TestIngressTranslator_generateRouteEvents(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - ingress := getTestIngress() - - // Create an ingress with 2 rules to support validating that rule - // removal results in a deletion event. - twoRuleIngress := getTestIngress() - twoRuleIngress.Spec.Rules = append(ingress.Spec.Rules, extensions.IngressRule{ - Host: "my.host", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/my-path2", - Backend: extensions.IngressBackend{ - ServiceName: "my-service", - ServicePort: intstr.FromString("80"), - }, - }, - }, - }, - }, - }) - - // Add tls to the cache to enable validation of tls addition to generated routes - hostName := "my.host" - secretName := "my-secret" - secretKey := getKey(ingress.Namespace, secretName) - cert := "my-cert" - it.tlsMap[secretKey] = &referencedTLS{ - ingressKeys: sets.String{}, - tls: &cachedTLS{ - cert: cert, - privateKey: "my-key", - }, - } - - // Add tls to the ingresses - tls := []extensions.IngressTLS{ - { - Hosts: []string{hostName}, - SecretName: secretName, - }, - } - ingress.Spec.TLS = tls - twoRuleIngress.Spec.TLS = tls - - type expectedEvent struct { - eventType watch.EventType - // Path is used as a shallow check that the expected route is - // generated. More comprehensive validation of route - // generation is the responsibility of testing for - // ingressToRoutes. - path string - } - - testCases := map[string]struct { - eventType watch.EventType - ingress *extensions.Ingress - oldIngress *extensions.Ingress - expected []expectedEvent - }{ - "Should generate a route addition event for each of the 2 rules": { - eventType: watch.Added, - ingress: twoRuleIngress, - expected: []expectedEvent{ - { - eventType: watch.Added, - path: twoRuleIngress.Spec.Rules[0].HTTP.Paths[0].Path, - }, - { - eventType: watch.Added, - path: twoRuleIngress.Spec.Rules[1].HTTP.Paths[0].Path, - }, - }, - }, - "Should generate a route addition and a route deletion for the missing rule": { - eventType: watch.Modified, - ingress: ingress, - oldIngress: twoRuleIngress, - expected: []expectedEvent{ - { - eventType: watch.Deleted, - path: twoRuleIngress.Spec.Rules[1].HTTP.Paths[0].Path, - }, - { - eventType: watch.Modified, - path: ingress.Spec.Rules[0].HTTP.Paths[0].Path, - }, - }, - }, - "Should generate a route deletion": { - eventType: watch.Deleted, - ingress: ingress, - oldIngress: ingress, - expected: []expectedEvent{ - { - eventType: watch.Deleted, - path: ingress.Spec.Rules[0].HTTP.Paths[0].Path, - }, - }, - }, - "Rule-less ingress should generate no events": { - eventType: watch.Added, - ingress: &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "empty-ingress", - Namespace: "my-namespace", - }, - }, - }, - } - for testName, tc := range testCases { - events := it.generateRouteEvents(tc.eventType, tc.ingress, tc.oldIngress) - expectedCount := len(tc.expected) - if expectedCount != len(events) { - t.Fatalf("%v: Expected %d route events to be generated", testName, expectedCount) - } - for i, expected := range tc.expected { - if events[i].eventType != expected.eventType { - t.Fatalf("%v: Expected event of type %v, got %v", testName, expected.eventType, events[i].eventType) - } - if events[i].route.Spec.Path != expected.path { - t.Fatalf("%v: Expected path to be %v, got %v", testName, expected.path, events[i].route.Spec.Path) - } - // Cert is used for a shallow check of tls config being set. More comprehensive - // validation is the responsibility of testing for addRouteTLS. - if expected.eventType != watch.Deleted && events[i].route.Spec.TLS.Certificate != cert { - t.Fatalf("%v: TLS not applied to route", testName) - } - } - } -} - -func TestIngressTranslator_cacheTLS(t *testing.T) { - secretName := "my-secret" - namespace := "my-namespace" - - cert := "my-cert" - privateKey := "my-key" - - // Ensure a secret can be retrieved via the client - secret := getTestSecret(namespace, secretName, cert, privateKey) - objects := []runtime.Object{secret} - client := fake.NewSimpleClientset(objects...) - - testCases := map[string]struct { - secretName string - alreadyCached bool - success bool - }{ - "Should create cache entry even if secret cannot be read": { - secretName: "unknown-secret", - }, - "Should cache tls if secret read successfully": { - secretName: secretName, - success: true, - }, - "Should retrieve cached tls if available": { - secretName: "already-cached", - alreadyCached: true, - success: true, - }, - } - for testName, tc := range testCases { - it := NewIngressTranslator(client.Core()) - ingressKey := getKey(namespace, "my-ingress") - - secretKey := getKey(namespace, tc.secretName) - - if tc.alreadyCached { - it.tlsMap[secretKey] = &referencedTLS{ - ingressKeys: sets.String{}, - tls: &cachedTLS{}, - } - } - - ingressTLS := []extensions.IngressTLS{ - { - SecretName: tc.secretName, - }, - } - success := it.cacheTLS(ingressTLS, namespace, ingressKey) - - if success != tc.success { - t.Fatalf("%v: Expected success to be %v, got %v", testName, tc.success, success) - } - - if refTLS := it.tlsMap[secretKey]; refTLS == nil { - t.Fatalf("%v: TLS entry is missing", testName) - } else { - if !refTLS.ingressKeys.Has(ingressKey) { - t.Fatalf("%v: Ingress key not referenced by the TLS cache entry", testName) - } - if success && refTLS.tls == nil { - t.Fatalf("%v: TLS data missing", testName) - } - } - } -} - -func TestIngressTranslator_dereferenceTLS(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - namespace := "my-namespace" - ingress1Key := getKey(namespace, "my-ingress1") - ingress2Key := getKey(namespace, "my-ingress2") - secretName := "my-secret" - secretKey := getKey(namespace, secretName) - - // Add cache entry - refTLS := &referencedTLS{ - ingressKeys: sets.NewString(ingress1Key, ingress2Key), - } - it.tlsMap[secretKey] = refTLS - - ingressTLS := []extensions.IngressTLS{ - { - SecretName: secretName, - }, - } - - // Cache entry should exist but only be referenced by the second ingress - it.dereferenceTLS(ingressTLS, namespace, ingress1Key) - expectedKeys := sets.NewString(ingress2Key) - if _, ok := it.tlsMap[secretKey]; !ok { - t.Fatalf("TLS cache entry removed while still referenced") - } else if !refTLS.ingressKeys.Equal(expectedKeys) { - t.Fatalf("Expected cache entry to have ingress keys of %v but got %v", expectedKeys, refTLS.ingressKeys) - } - - // Cache entry should be removed because it is no longer referenced - it.dereferenceTLS(ingressTLS, namespace, ingress2Key) - if _, ok := it.tlsMap[secretKey]; ok { - t.Fatalf("TLS cache entry not removed") - } -} - -func TestIngressTranslator_tlsForHost(t *testing.T) { - client := fake.NewSimpleClientset() - it := NewIngressTranslator(client.Core()) - - namespace := "my-namespace" - secretName1 := "my-secret" - secretKey1 := getKey(namespace, secretName1) - secretName2 := "other-secret" - secretKey2 := getKey(namespace, secretName2) - - firstCert := "my-cert" - - // Cache the tls - it.tlsMap[secretKey1] = &referencedTLS{ - ingressKeys: sets.String{}, - tls: &cachedTLS{ - cert: firstCert, - privateKey: "my-private-key", - }, - } - it.tlsMap[secretKey2] = &referencedTLS{ - ingressKeys: sets.String{}, - tls: &cachedTLS{ - cert: "other-cert", - privateKey: "my-private-key", - }, - } - - defaultHost := "foo" - - testCases := map[string]struct { - tlsHost string - host string - success bool - }{ - "Unmatched host returns no tls": { - tlsHost: defaultHost, - host: "bar", - }, - "Matched host returns tls": { - tlsHost: defaultHost, - host: defaultHost, - success: true, - }, - "Empty host matches tls with empty host": { - tlsHost: "", - host: "", - success: true, - }, - "Empty host without tls with empty host returns no tls": { - tlsHost: defaultHost, - host: "", - }, - } - for testName, tc := range testCases { - // Create tls items that match on the same host to validate - // that the first matching tls wins. - ingressTLS := []extensions.IngressTLS{ - { - Hosts: []string{tc.tlsHost}, - SecretName: secretName1, - }, - { - Hosts: []string{tc.tlsHost}, - SecretName: secretName2, - }, - } - tls := it.tlsForHost(ingressTLS, namespace, tc.host) - // Validate that only the first secret is ever returned - if tc.success && (tls == nil || tls.cert != firstCert) { - t.Fatalf("%v: tls not returned as expected", testName) - } - if !tc.success && tls != nil { - t.Fatalf("%v: tls unexpectedly returned", testName) - } - } -} - -func TestTLSFromSecret(t *testing.T) { - testCases := map[string]struct { - cert string - privateKey string - error bool - }{ - "Invalid cert and private key should result in an error": { - error: true, - }, - "Valid cert and private key should return a tls object": { - cert: "my-cert", - privateKey: "my-key", - }, - } - for testName, tc := range testCases { - secret := getTestSecret("my-namespace", "my-secret", tc.cert, tc.privateKey) - tls, err := tlsFromSecret(secret) - if tc.error && err == nil { - t.Fatalf("%v: Error not returned", testName) - } - if tls == nil { - if !tc.error { - t.Fatalf("%v: tls not returned", testName) - } - } else { - if tls.cert != tc.cert { - t.Fatalf("%v: expected tls cert to be %v, got %v", testName, tc.cert, tls.cert) - } - if tls.privateKey != tc.privateKey { - t.Fatalf("%v: expected tls private key to be %v, got %v", testName, tc.privateKey, tls.privateKey) - } - } - } -} - -func TestIngressToRoutes(t *testing.T) { - noRuleValueIngress := getTestIngress() - noRuleValueIngress.Spec.Rules[0].HTTP = nil - - testCases := map[string]struct { - ingress *extensions.Ingress - expectedCount int - }{ - "Ingress without rule value generates no routes": { - ingress: noRuleValueIngress, - }, - "Ingress with one path generates one route": { - ingress: getTestIngress(), - expectedCount: 1, - }, - } - for testName, tc := range testCases { - routes, routeNames := ingressToRoutes(tc.ingress) - routeCount := len(routes) - if routeCount != tc.expectedCount { - t.Fatalf("%v: Expected %v route(s) to be generated from ingress, got %v", testName, tc.expectedCount, routeCount) - } - nameCount := len(routeNames) - if nameCount != tc.expectedCount { - t.Fatalf("%v: Expected %v route(s) to be generated from ingress, got %v", testName, tc.expectedCount, nameCount) - } - } -} - -func TestMatchesHost(t *testing.T) { - testCases := map[string]struct { - pattern string - host string - matches bool - }{ - "zero length pattern or host does not match": {}, - "differing number of path components does not match": { - pattern: "foo.bar", - host: "bar", - }, - "wildcard pattern with differing path components does not match": { - pattern: "*.foo.bar", - host: "foo.baz", - }, - "single component pattern and host match": { - pattern: "foo", - host: "foo", - matches: true, - }, - "multiple component pattern and host match": { - pattern: "foo.bar", - host: "foo.bar", - matches: true, - }, - "multiple component wildcard pattern and host match": { - pattern: "*.bar.baz", - host: "foo.bar.baz", - matches: true, - }, - } - for testName, tc := range testCases { - matches := matchesHost(tc.pattern, tc.host) - if matches != tc.matches { - t.Fatalf("%v: expected match to be %v, got %v", testName, tc.matches, matches) - } - } -} - -func TestGetNameForHost(t *testing.T) { - host := "foo" - testCases := map[string]struct { - name string - nameForHost string - }{ - "Route name is returned": { - name: host, - nameForHost: host, - }, - "Ingress name is returned": { - name: generateRouteName(host, "", ""), - nameForHost: host, - }, - } - for testName, tc := range testCases { - nameForHost := GetNameForHost(tc.name) - if nameForHost != tc.nameForHost { - t.Fatalf("%v: expected %v, got %v", testName, tc.nameForHost, nameForHost) - } - } -} - -func getTestIngress() *extensions.Ingress { - return &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-ingress", - Namespace: "my-namespace", - }, - Spec: extensions.IngressSpec{ - Rules: []extensions.IngressRule{ - { - Host: "my.host", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/my-path", - Backend: extensions.IngressBackend{ - ServiceName: "my-service", - ServicePort: intstr.FromString("80"), - }, - }, - }, - }, - }, - }, - }, - }, - } -} - -func getTestSecret(namespace, name, cert, privateKey string) *kapi.Secret { - return &kapi.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Data: map[string][]byte{ - "tls.crt": []byte(cert), - "tls.key": []byte(privateKey), - }, - Type: kapi.SecretTypeOpaque, - } -} diff --git a/pkg/router/controller/router_controller.go b/pkg/router/controller/router_controller.go index 91df6c8a6052..9e790e1d840a 100644 --- a/pkg/router/controller/router_controller.go +++ b/pkg/router/controller/router_controller.go @@ -14,7 +14,6 @@ import ( utilwait "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" kapi "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/extensions" projectclient "github.com/openshift/origin/pkg/project/generated/internalclientset/typed/project/internalversion" routeapi "github.com/openshift/origin/pkg/route/apis/route" @@ -44,9 +43,6 @@ type RouterController struct { ProjectRetries int WatchNodes bool - - EnableIngress bool - IngressTranslator *IngressTranslator } // Run begins watching and syncing. @@ -138,13 +134,6 @@ func (c *RouterController) processNamespace(eventType watch.EventType, ns *kapi. func (c *RouterController) UpdateNamespaces() { namespaces := c.FilteredNamespaceNames - // The ingress translator synchronizes access to its cache with a - // lock, so calls to it are made outside of the controller lock to - // avoid unintended interaction. - if c.EnableIngress { - c.IngressTranslator.UpdateNamespaces(namespaces) - } - glog.V(4).Infof("Updating watched namespaces: %v", namespaces) if err := c.Plugin.HandleNamespaces(namespaces); err != nil { utilruntime.HandleError(err) @@ -234,36 +223,6 @@ func (c *RouterController) HandleEndpoints(eventType watch.EventType, obj interf c.Commit() } -// HandleIngress handles a single Ingress event and synchronizes the router backend. -func (c *RouterController) HandleIngress(eventType watch.EventType, obj interface{}) { - ingress := obj.(*extensions.Ingress) - // The ingress translator synchronizes access to its cache with a - // lock, so calls to it are made outside of the controller lock to - // avoid unintended interaction. - events := c.IngressTranslator.TranslateIngressEvent(eventType, ingress) - - c.lock.Lock() - defer c.lock.Unlock() - - c.processIngressEvents(events) - c.Commit() -} - -// HandleSecret handles a single Secret event and synchronizes the router backend. -func (c *RouterController) HandleSecret(eventType watch.EventType, obj interface{}) { - secret := obj.(*kapi.Secret) - // The ingress translator synchronizes access to its cache with a - // lock, so calls to it are made outside of the controller lock to - // avoid unintended interaction. - events := c.IngressTranslator.TranslateSecretEvent(eventType, secret) - - c.lock.Lock() - defer c.lock.Unlock() - - c.processIngressEvents(events) - c.Commit() -} - // Commit notifies the plugin that it is safe to commit state. func (c *RouterController) Commit() { if c.firstSyncDone { @@ -288,16 +247,6 @@ func (c *RouterController) processRoute(eventType watch.EventType, route *routea } } -// processIngressEvents logs and propagates the route events resulting from an ingress or secret event -func (c *RouterController) processIngressEvents(events []ingressRouteEvents) { - for _, ingressEvent := range events { - glog.V(4).Infof("Processing Ingress: %s", ingressEvent.ingressKey) - for _, routeEvent := range ingressEvent.routeEvents { - c.processRoute(routeEvent.eventType, routeEvent.route) - } - } -} - func (c *RouterController) handleFirstSync() { c.lock.Lock() defer c.lock.Unlock() diff --git a/pkg/router/controller/status.go b/pkg/router/controller/status.go index 9ab30dde5c88..5e8cbfdf6fd9 100644 --- a/pkg/router/controller/status.go +++ b/pkg/router/controller/status.go @@ -72,16 +72,12 @@ var nowFn = getRfc3339Timestamp // HandleRoute attempts to admit the provided route on watch add / modifications. func (a *StatusAdmitter) HandleRoute(eventType watch.EventType, route *routeapi.Route) error { - if IsGeneratedRouteName(route.Name) { - // Can't record status for ingress resources - } else { - switch eventType { - case watch.Added, watch.Modified: - performIngressConditionUpdate("admit", a.lease, a.tracker, a.client, route, a.routerName, a.routerCanonicalHostname, routeapi.RouteIngressCondition{ - Type: routeapi.RouteAdmitted, - Status: kapi.ConditionTrue, - }) - } + switch eventType { + case watch.Added, watch.Modified: + performIngressConditionUpdate("admit", a.lease, a.tracker, a.client, route, a.routerName, a.routerCanonicalHostname, routeapi.RouteIngressCondition{ + Type: routeapi.RouteAdmitted, + Status: kapi.ConditionTrue, + }) } return a.plugin.HandleRoute(eventType, route) } @@ -104,10 +100,6 @@ func (a *StatusAdmitter) Commit() error { // RecordRouteRejection attempts to update the route status with a reason for a route being rejected. func (a *StatusAdmitter) RecordRouteRejection(route *routeapi.Route, reason, message string) { - if IsGeneratedRouteName(route.Name) { - // Can't record status for ingress resources - return - } performIngressConditionUpdate("reject", a.lease, a.tracker, a.client, route, a.routerName, a.routerCanonicalHostname, routeapi.RouteIngressCondition{ Type: routeapi.RouteAdmitted, Status: kapi.ConditionFalse, @@ -364,7 +356,6 @@ func (t *SimpleContentionTracker) flush() { removed++ continue } - } } if t.contentions > 0 && len(t.message) > 0 { diff --git a/pkg/router/controller/unique_host.go b/pkg/router/controller/unique_host.go index a7ee960e2e4d..328767bfcb77 100644 --- a/pkg/router/controller/unique_host.go +++ b/pkg/router/controller/unique_host.go @@ -97,6 +97,7 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout if len(host) == 0 { glog.V(4).Infof("Route %s has no host value", routeName) p.recorder.RecordRouteRejection(route, "NoHostValue", "no host value was defined for the route") + p.plugin.HandleRoute(watch.Deleted, route) return nil } route.Spec.Host = host @@ -112,6 +113,7 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout err := fmt.Errorf("host name validation errors: %s", strings.Join(errMessages, ", ")) p.recorder.RecordRouteRejection(route, "InvalidHost", err.Error()) + p.plugin.HandleRoute(watch.Deleted, route) return err } @@ -130,6 +132,7 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout glog.V(4).Infof("Route %s cannot take %s from %s", routeName, host, routeNameKey(oldest)) err := fmt.Errorf("route %s already exposes %s and is older", oldest.Name, host) p.recorder.RecordRouteRejection(route, "HostAlreadyClaimed", err.Error()) + p.plugin.HandleRoute(watch.Deleted, route) return err } added = true @@ -173,6 +176,7 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout glog.V(4).Infof("Route %s cannot take %s from %s", routeName, host, routeNameKey(oldest)) err := fmt.Errorf("a route in another namespace holds %s and is older than %s", host, route.Name) p.recorder.RecordRouteRejection(route, "HostAlreadyClaimed", err.Error()) + p.plugin.HandleRoute(watch.Deleted, route) return err } diff --git a/pkg/router/f5/plugin.go b/pkg/router/f5/plugin.go index c7d097525f30..c7f3417d4916 100644 --- a/pkg/router/f5/plugin.go +++ b/pkg/router/f5/plugin.go @@ -10,7 +10,6 @@ import ( kapi "k8s.io/kubernetes/pkg/apis/core" routeapi "github.com/openshift/origin/pkg/route/apis/route" - "github.com/openshift/origin/pkg/router/controller" "github.com/openshift/origin/pkg/util/netutils" ) @@ -326,8 +325,7 @@ func (p *F5Plugin) HandleEndpoints(eventType watch.EventType, // routeName returns a string that can be used as a rule name in F5 BIG-IP and // is distinct for the given route. func routeName(route routeapi.Route) string { - name := controller.GetSafeRouteName(route.Name) - return fmt.Sprintf("openshift_route_%s_%s", route.Namespace, name) + return fmt.Sprintf("openshift_route_%s_%s", route.Namespace, route.Name) } // In order to map OpenShift routes to F5 objects, we must divide routes into diff --git a/pkg/router/template/router.go b/pkg/router/template/router.go index fea48638971a..1e04167c5f26 100644 --- a/pkg/router/template/router.go +++ b/pkg/router/template/router.go @@ -21,7 +21,6 @@ import ( cmdutil "github.com/openshift/origin/pkg/cmd/util" routeapi "github.com/openshift/origin/pkg/route/apis/route" - "github.com/openshift/origin/pkg/router/controller" "github.com/openshift/origin/pkg/router/template/limiter" ) @@ -556,7 +555,7 @@ func (r *templateRouter) DeleteEndpoints(id string) { // routeKey generates route key. This allows templates to use this key without having to create a separate method func routeKey(route *routeapi.Route) string { - return routeKeyFromParts(route.Namespace, controller.GetSafeRouteName(route.Name)) + return routeKeyFromParts(route.Namespace, route.Name) } func routeKeyFromParts(namespace, name string) string { diff --git a/test/end-to-end/router_test.go b/test/end-to-end/router_test.go index 57c85e0c1fca..1ee91905cf8c 100644 --- a/test/end-to-end/router_test.go +++ b/test/end-to-end/router_test.go @@ -1275,17 +1275,16 @@ u3YLAbyW/lHhOCiZu2iAI8AbmXem9lW6Tr7p/97s0w== // Constants used to default createAndStartRouterContainerExtended const ( defaultBindPortsAfterSync = false - defaultEnableIngress = false defaultNamespaceLabels = "" ) // createAndStartRouterContainer is responsible for deploying the router image in docker. It assumes that all router images // will use a command line flag that can take --master which points to the master url func createAndStartRouterContainer(dockerCli *dockerClient.Client, masterIp string, routerStatsPort int, reloadInterval int) (containerId string, err error) { - return createAndStartRouterContainerExtended(dockerCli, masterIp, routerStatsPort, reloadInterval, defaultBindPortsAfterSync, defaultEnableIngress, defaultNamespaceLabels) + return createAndStartRouterContainerExtended(dockerCli, masterIp, routerStatsPort, reloadInterval, defaultBindPortsAfterSync, defaultNamespaceLabels) } -func createAndStartRouterContainerExtended(dockerCli *dockerClient.Client, masterIp string, routerStatsPort int, reloadInterval int, bindPortsAfterSync, enableIngress bool, namespaceLabels string) (containerId string, err error) { +func createAndStartRouterContainerExtended(dockerCli *dockerClient.Client, masterIp string, routerStatsPort int, reloadInterval int, bindPortsAfterSync bool, namespaceLabels string) (containerId string, err error) { ports := []string{"80", "443"} if routerStatsPort > 0 { ports = append(ports, fmt.Sprintf("%d", routerStatsPort)) @@ -1322,7 +1321,6 @@ func createAndStartRouterContainerExtended(dockerCli *dockerClient.Client, maste fmt.Sprintf("STATS_PASSWORD=%s", statsPassword), fmt.Sprintf("DEFAULT_CERTIFICATE=%s\n%s", defaultCert, defaultKey), fmt.Sprintf("ROUTER_BIND_PORTS_AFTER_SYNC=%s", strconv.FormatBool(bindPortsAfterSync)), - fmt.Sprintf("ROUTER_ENABLE_INGRESS=%s", strconv.FormatBool(enableIngress)), fmt.Sprintf("NAMESPACE_LABELS=%s", namespaceLabels), } @@ -1644,7 +1642,7 @@ func TestRouterBindsPortsAfterSync(t *testing.T) { bindPortsAfterSync := true reloadInterval := 1 - routerId, err := createAndStartRouterContainerExtended(dockerCli, fakeMasterAndPod.MasterHttpAddr, statsPort, reloadInterval, bindPortsAfterSync, defaultEnableIngress, defaultNamespaceLabels) + routerId, err := createAndStartRouterContainerExtended(dockerCli, fakeMasterAndPod.MasterHttpAddr, statsPort, reloadInterval, bindPortsAfterSync, defaultNamespaceLabels) if err != nil { t.Fatalf("Error starting container %s : %v", getRouterImage(), err) } @@ -1709,7 +1707,7 @@ func TestRouterBindsPortsAfterSync(t *testing.T) { type routerIntegrationTest func(*testing.T, *tr.TestHttpService) -func runRouterTest(t *testing.T, rit routerIntegrationTest, enableIngress bool, namespaceNames *[]string) { +func runRouterTest(t *testing.T, rit routerIntegrationTest, namespaceNames *[]string) { namespaceLabels, namespaceListResponse := getNamespaceConfig(t, namespaceNames) //create a server which will act as a user deployed application that @@ -1733,7 +1731,7 @@ func runRouterTest(t *testing.T, rit routerIntegrationTest, enableIngress bool, reloadInterval := 1 routerId, err := createAndStartRouterContainerExtended( - dockerCli, fakeMasterAndPod.MasterHttpAddr, statsPort, reloadInterval, defaultBindPortsAfterSync, enableIngress, namespaceLabels) + dockerCli, fakeMasterAndPod.MasterHttpAddr, statsPort, reloadInterval, defaultBindPortsAfterSync, namespaceLabels) if err != nil { t.Fatalf("Error starting container %s : %v", getRouterImage(), err) @@ -1894,8 +1892,7 @@ func ingressConfiguredRouter(t *testing.T, fakeMasterAndPod *tr.TestHttpService) // TestRouterIngress validates that an ingress resource can configure a router to expose a tls route. func TestIngressConfiguredRouter(t *testing.T) { - enableIngress := true // Enable namespace filtering to allow validation of compatibility with ingress. namespaceNames := []string{defaultNamespace} - runRouterTest(t, ingressConfiguredRouter, enableIngress, &namespaceNames) + runRouterTest(t, ingressConfiguredRouter, &namespaceNames) } diff --git a/test/extended/router/metrics.go b/test/extended/router/metrics.go index 346eda6d96c3..7055b6084415 100644 --- a/test/extended/router/metrics.go +++ b/test/extended/router/metrics.go @@ -110,8 +110,8 @@ var _ = g.Describe("[Conformance][Area:Networking][Feature:Router]", func() { o.Expect(err).NotTo(o.HaveOccurred()) g.By("checking for the expected metrics") - routeLabels := labels{"backend": "http", "namespace": ns, "route": "weightedroute"} - serverLabels := labels{"namespace": ns, "route": "weightedroute"} + routeLabels := promLabels{"backend": "http", "namespace": ns, "route": "weightedroute"} + serverLabels := promLabels{"namespace": ns, "route": "weightedroute"} var metrics map[string]*dto.MetricFamily times := 10 p := expfmt.TextParser{} @@ -222,10 +222,10 @@ var _ = g.Describe("[Conformance][Area:Networking][Feature:Router]", func() { }) }) -type labels map[string]string +type promLabels map[string]string -func (l labels) With(name, value string) labels { - n := make(labels) +func (l promLabels) With(name, value string) promLabels { + n := make(promLabels) for k, v := range l { n[k] = v } @@ -242,7 +242,7 @@ func findEnvVar(vars []kapi.EnvVar, key string) string { return "" } -func findMetricsWithLabels(f *dto.MetricFamily, labels map[string]string) []*dto.Metric { +func findMetricsWithLabels(f *dto.MetricFamily, promLabels map[string]string) []*dto.Metric { var result []*dto.Metric if f == nil { return result @@ -250,14 +250,14 @@ func findMetricsWithLabels(f *dto.MetricFamily, labels map[string]string) []*dto for _, m := range f.Metric { matched := map[string]struct{}{} for _, l := range m.Label { - if expect, ok := labels[l.GetName()]; ok { + if expect, ok := promLabels[l.GetName()]; ok { if expect != l.GetValue() { break } matched[l.GetName()] = struct{}{} } } - if len(matched) != len(labels) { + if len(matched) != len(promLabels) { continue } result = append(result, m) @@ -265,25 +265,25 @@ func findMetricsWithLabels(f *dto.MetricFamily, labels map[string]string) []*dto return result } -func findCountersWithLabels(f *dto.MetricFamily, labels map[string]string) []float64 { +func findCountersWithLabels(f *dto.MetricFamily, promLabels map[string]string) []float64 { var result []float64 - for _, m := range findMetricsWithLabels(f, labels) { + for _, m := range findMetricsWithLabels(f, promLabels) { result = append(result, m.Counter.GetValue()) } return result } -func findGaugesWithLabels(f *dto.MetricFamily, labels map[string]string) []float64 { +func findGaugesWithLabels(f *dto.MetricFamily, promLabels map[string]string) []float64 { var result []float64 - for _, m := range findMetricsWithLabels(f, labels) { + for _, m := range findMetricsWithLabels(f, promLabels) { result = append(result, m.Gauge.GetValue()) } return result } -func findMetricLabels(f *dto.MetricFamily, labels map[string]string, match string) []string { +func findMetricLabels(f *dto.MetricFamily, promLabels map[string]string, match string) []string { var result []string - for _, m := range findMetricsWithLabels(f, labels) { + for _, m := range findMetricsWithLabels(f, promLabels) { for _, l := range m.Label { if l.GetName() == match { result = append(result, l.GetValue()) diff --git a/test/extended/router/router.go b/test/extended/router/router.go new file mode 100644 index 000000000000..e614ddadf8be --- /dev/null +++ b/test/extended/router/router.go @@ -0,0 +1,111 @@ +package images + +import ( + "net/http" + "time" + + g "github.com/onsi/ginkgo" + o "github.com/onsi/gomega" + + kapierrs "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/wait" + + routev1 "github.com/openshift/api/route/v1" + routeclientset "github.com/openshift/client-go/route/clientset/versioned" + exutil "github.com/openshift/origin/test/extended/util" + "github.com/openshift/origin/test/extended/util/url" +) + +var _ = g.Describe("[Conformance][Area:Networking][Feature:Router]", func() { + defer g.GinkgoRecover() + var ( + host, ns string + oc *exutil.CLI + + configPath = exutil.FixturePath("testdata", "ingress.yaml") + ) + + // this hook must be registered before the framework namespace teardown + // hook + g.AfterEach(func() { + if g.CurrentGinkgoTestDescription().Failed { + exutil.DumpPodLogsStartingWithInNamespace("router", "default", oc.AsAdmin()) + selector, err := labels.Parse("router=router") + if err != nil { + panic(err) + } + exutil.DumpPodsCommand(oc.AdminKubeClient(), "default", selector, "cat /var/lib/haproxy/router/routes.json /var/lib//var/lib/haproxy/conf/haproxy.config") + } + }) + + oc = exutil.NewCLI("router-stress", exutil.KubeConfigPath()) + + g.BeforeEach(func() { + _, err := oc.AdminAppsClient().Apps().DeploymentConfigs("default").Get("router", metav1.GetOptions{}) + if kapierrs.IsNotFound(err) { + g.Skip("no router installed on the cluster") + return + } + o.Expect(err).NotTo(o.HaveOccurred()) + + // wait for the router endpoints to show up + err = wait.PollImmediate(2*time.Second, 120*time.Second, func() (bool, error) { + epts, err := oc.AdminKubeClient().CoreV1().Endpoints("default").Get("router", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + if len(epts.Subsets) == 0 || len(epts.Subsets[0].Addresses) == 0 { + return false, nil + } + host = epts.Subsets[0].Addresses[0].IP + return true, nil + }) + o.Expect(err).NotTo(o.HaveOccurred()) + + ns = oc.KubeFramework().Namespace.Name + }) + + g.Describe("The HAProxy router", func() { + g.It("should respond with 503 to unrecognized hosts", func() { + t := url.NewTester(oc.AdminKubeClient(), ns) + defer t.Close() + t.Within( + time.Minute, + url.Expect("GET", "https://www.google.com").Through(host).SkipTLSVerification().HasStatusCode(503), + url.Expect("GET", "http://www.google.com").Through(host).HasStatusCode(503), + ) + }) + + g.It("should serve routes that were created from an ingress", func() { + g.By("deploying an ingress rule") + err := oc.Run("create").Args("-f", configPath).Execute() + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("waiting for the ingress rule to be converted to routes") + client := routeclientset.NewForConfigOrDie(oc.AdminConfig()) + var r []routev1.Route + err = wait.Poll(time.Second, time.Minute, func() (bool, error) { + routes, err := client.Route().Routes(ns).List(metav1.ListOptions{}) + if err != nil { + return false, err + } + r = routes.Items + return len(routes.Items) == 4, nil + }) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("verifying the router reports the correct behavior") + t := url.NewTester(oc.AdminKubeClient(), ns) + defer t.Close() + t.Within( + 3*time.Minute, + url.Expect("GET", "http://1.ingress-test.com/test").Through(host).HasStatusCode(200), + url.Expect("GET", "http://1.ingress-test.com/other/deep").Through(host).HasStatusCode(200), + url.Expect("GET", "http://1.ingress-test.com/").Through(host).HasStatusCode(503), + url.Expect("GET", "http://2.ingress-test.com/").Through(host).HasStatusCode(200), + url.Expect("GET", "https://3.ingress-test.com/").Through(host).SkipTLSVerification().HasStatusCode(200), + url.Expect("GET", "http://3.ingress-test.com/").Through(host).RedirectsTo("https://3.ingress-test.com/", http.StatusFound), + ) + }) + }) +}) diff --git a/test/extended/testdata/bindata.go b/test/extended/testdata/bindata.go index 42f4a0e86fbe..da5c0c00600a 100644 --- a/test/extended/testdata/bindata.go +++ b/test/extended/testdata/bindata.go @@ -137,6 +137,7 @@ // test/extended/testdata/image_ecosystem/perl-hotdeploy/perl.json // test/extended/testdata/imagestream-jenkins-slave-pods.yaml // test/extended/testdata/imagestreamtag-jenkins-slave-pods.yaml +// test/extended/testdata/ingress.yaml // test/extended/testdata/jenkins-plugin/build-job-clone.xml // test/extended/testdata/jenkins-plugin/build-job-slave.xml // test/extended/testdata/jenkins-plugin/build-job.xml @@ -7231,6 +7232,127 @@ func testExtendedTestdataImagestreamtagJenkinsSlavePodsYaml() (*asset, error) { return a, nil } +var _testExtendedTestdataIngressYaml = []byte(`kind: List +apiVersion: v1 +items: +# an ingress that should be captured as individual routes +- apiVersion: extensions/v1beta1 + kind: Ingress + metadata: + name: test + spec: + tls: + - hosts: + - 3.ingress-test.com + secretName: ingress-endpoint-secret + rules: + - host: 1.ingress-test.com + http: + paths: + - path: /test + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - path: /other + backend: + serviceName: ingress-endpoint-2 + servicePort: 80 + - host: 2.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - host: 3.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 +# an empty secret +- apiVersion: v1 + kind: Secret + metadata: + name: ingress-endpoint-secret + type: kubernetes.io/tls + stringData: + tls.key: "" + tls.crt: "" +# a service to be routed to +- apiVersion: v1 + kind: Service + metadata: + name: ingress-endpoint-1 + spec: + selector: + app: ingress-endpoint-1 + ports: + - port: 80 + targetPort: 8080 +# a service to be routed to +- apiVersion: v1 + kind: Service + metadata: + name: ingress-endpoint-2 + spec: + selector: + app: ingress-endpoint-2 + ports: + - port: 80 + targetPort: 8080 +# a pod that serves a response +- apiVersion: v1 + kind: Pod + metadata: + name: ingress-endpoint-1 + labels: + app: ingress-endpoint-1 + spec: + terminationGracePeriodSeconds: 1 + containers: + - name: test + image: openshift/hello-openshift + ports: + - containerPort: 8080 + name: http + - containerPort: 100 + protocol: UDP +# a pod that serves a response +- apiVersion: v1 + kind: Pod + metadata: + name: ingress-endpoint-2 + labels: + app: ingress-endpoint-2 + spec: + terminationGracePeriodSeconds: 1 + containers: + - name: test + image: openshift/hello-openshift + ports: + - containerPort: 8080 + name: http + - containerPort: 100 + protocol: UDP +`) + +func testExtendedTestdataIngressYamlBytes() ([]byte, error) { + return _testExtendedTestdataIngressYaml, nil +} + +func testExtendedTestdataIngressYaml() (*asset, error) { + bytes, err := testExtendedTestdataIngressYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "test/extended/testdata/ingress.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _testExtendedTestdataJenkinsPluginBuildJobCloneXml = []byte(` @@ -31457,6 +31579,7 @@ var _bindata = map[string]func() (*asset, error){ "test/extended/testdata/image_ecosystem/perl-hotdeploy/perl.json": testExtendedTestdataImage_ecosystemPerlHotdeployPerlJson, "test/extended/testdata/imagestream-jenkins-slave-pods.yaml": testExtendedTestdataImagestreamJenkinsSlavePodsYaml, "test/extended/testdata/imagestreamtag-jenkins-slave-pods.yaml": testExtendedTestdataImagestreamtagJenkinsSlavePodsYaml, + "test/extended/testdata/ingress.yaml": testExtendedTestdataIngressYaml, "test/extended/testdata/jenkins-plugin/build-job-clone.xml": testExtendedTestdataJenkinsPluginBuildJobCloneXml, "test/extended/testdata/jenkins-plugin/build-job-slave.xml": testExtendedTestdataJenkinsPluginBuildJobSlaveXml, "test/extended/testdata/jenkins-plugin/build-job.xml": testExtendedTestdataJenkinsPluginBuildJobXml, @@ -31952,6 +32075,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ }}, "imagestream-jenkins-slave-pods.yaml": &bintree{testExtendedTestdataImagestreamJenkinsSlavePodsYaml, map[string]*bintree{}}, "imagestreamtag-jenkins-slave-pods.yaml": &bintree{testExtendedTestdataImagestreamtagJenkinsSlavePodsYaml, map[string]*bintree{}}, + "ingress.yaml": &bintree{testExtendedTestdataIngressYaml, map[string]*bintree{}}, "jenkins-plugin": &bintree{nil, map[string]*bintree{ "build-job-clone.xml": &bintree{testExtendedTestdataJenkinsPluginBuildJobCloneXml, map[string]*bintree{}}, "build-job-slave.xml": &bintree{testExtendedTestdataJenkinsPluginBuildJobSlaveXml, map[string]*bintree{}}, diff --git a/test/extended/testdata/ingress.yaml b/test/extended/testdata/ingress.yaml new file mode 100644 index 000000000000..9e1a67cc3499 --- /dev/null +++ b/test/extended/testdata/ingress.yaml @@ -0,0 +1,104 @@ +kind: List +apiVersion: v1 +items: +# an ingress that should be captured as individual routes +- apiVersion: extensions/v1beta1 + kind: Ingress + metadata: + name: test + spec: + tls: + - hosts: + - 3.ingress-test.com + secretName: ingress-endpoint-secret + rules: + - host: 1.ingress-test.com + http: + paths: + - path: /test + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - path: /other + backend: + serviceName: ingress-endpoint-2 + servicePort: 80 + - host: 2.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 + - host: 3.ingress-test.com + http: + paths: + - path: / + backend: + serviceName: ingress-endpoint-1 + servicePort: 80 +# an empty secret +- apiVersion: v1 + kind: Secret + metadata: + name: ingress-endpoint-secret + type: kubernetes.io/tls + stringData: + tls.key: "" + tls.crt: "" +# a service to be routed to +- apiVersion: v1 + kind: Service + metadata: + name: ingress-endpoint-1 + spec: + selector: + app: ingress-endpoint-1 + ports: + - port: 80 + targetPort: 8080 +# a service to be routed to +- apiVersion: v1 + kind: Service + metadata: + name: ingress-endpoint-2 + spec: + selector: + app: ingress-endpoint-2 + ports: + - port: 80 + targetPort: 8080 +# a pod that serves a response +- apiVersion: v1 + kind: Pod + metadata: + name: ingress-endpoint-1 + labels: + app: ingress-endpoint-1 + spec: + terminationGracePeriodSeconds: 1 + containers: + - name: test + image: openshift/hello-openshift + ports: + - containerPort: 8080 + name: http + - containerPort: 100 + protocol: UDP +# a pod that serves a response +- apiVersion: v1 + kind: Pod + metadata: + name: ingress-endpoint-2 + labels: + app: ingress-endpoint-2 + spec: + terminationGracePeriodSeconds: 1 + containers: + - name: test + image: openshift/hello-openshift + ports: + - containerPort: 8080 + name: http + - containerPort: 100 + protocol: UDP diff --git a/test/extended/util/framework.go b/test/extended/util/framework.go index 53a44c36dbce..f631e309b6b4 100644 --- a/test/extended/util/framework.go +++ b/test/extended/util/framework.go @@ -277,6 +277,23 @@ func DumpPodLogs(pods []kapiv1.Pod, oc *CLI) { } } +// DumpPodsCommand runs the provided command in every pod identified by selector in the provided namespace. +func DumpPodsCommand(c kclientset.Interface, ns string, selector labels.Selector, cmd string) { + podList, err := c.CoreV1().Pods(ns).List(metav1.ListOptions{LabelSelector: selector.String()}) + o.Expect(err).NotTo(o.HaveOccurred()) + + values := make(map[string]string) + for _, pod := range podList.Items { + stdout, err := e2e.RunHostCmdWithRetries(pod.Namespace, pod.Name, cmd, e2e.StatefulSetPoll, e2e.StatefulPodTimeout) + o.Expect(err).NotTo(o.HaveOccurred()) + values[pod.Name] = stdout + } + for name, stdout := range values { + stdout = strings.TrimSuffix(stdout, "\n") + e2e.Logf(name + ": " + strings.Join(strings.Split(stdout, "\n"), fmt.Sprintf("\n%s: ", name))) + } +} + // GetMasterThreadDump will get a golang thread stack dump func GetMasterThreadDump(oc *CLI) { out, err := oc.AsAdmin().Run("get").Args("--raw", "/debug/pprof/goroutine?debug=2").Output() diff --git a/test/integration/router_without_haproxy_test.go b/test/integration/router_without_haproxy_test.go index 637780efac62..331ffb9cd4b2 100644 --- a/test/integration/router_without_haproxy_test.go +++ b/test/integration/router_without_haproxy_test.go @@ -378,7 +378,7 @@ func launchRateLimitedRouter(t *testing.T, routeclient routeinternalclientset.In } factory := controllerfactory.NewDefaultRouterControllerFactory(routeclient, projectclient.Project().Projects(), kc) - ctrl := factory.Create(plugin, false, false) + ctrl := factory.Create(plugin, false) ctrl.Run() return templatePlugin @@ -411,7 +411,7 @@ func launchRouter(routeclient routeinternalclientset.Interface, projectclient pr return nil }) factory := routerSelection.NewFactory(routeclient, projectclient.Project().Projects(), kc) - ctrl := factory.Create(plugin, false, false) + ctrl := factory.Create(plugin, false) ctrl.Run() return templatePlugin diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml index 619d72c9767e..87c63e51c830 100644 --- a/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml @@ -937,6 +937,21 @@ items: - kind: ServiceAccount name: service-ingress-ip-controller namespace: openshift-infra +- apiVersion: rbac.authorization.k8s.io/v1beta1 + kind: ClusterRoleBinding + metadata: + annotations: + rbac.authorization.kubernetes.io/autoupdate: "true" + creationTimestamp: null + name: system:openshift:controller:ingress-to-route-controller + roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:openshift:controller:ingress-to-route-controller + subjects: + - kind: ServiceAccount + name: ingress-to-route-controller + namespace: openshift-infra - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRoleBinding metadata: diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml index d6542a6eabd5..b1750d4325b3 100644 --- a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml @@ -3341,6 +3341,59 @@ items: - create - patch - update +- apiVersion: rbac.authorization.k8s.io/v1beta1 + kind: ClusterRole + metadata: + annotations: + authorization.openshift.io/system-only: "true" + rbac.authorization.kubernetes.io/autoupdate: "true" + creationTimestamp: null + name: system:openshift:controller:ingress-to-route-controller + rules: + - apiGroups: + - "" + resources: + - secrets + - services + verbs: + - get + - list + - watch + - apiGroups: + - extensions + resources: + - ingress + verbs: + - get + - list + - watch + - apiGroups: + - route.openshift.io + resources: + - routes + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - route.openshift.io + resources: + - routes/custom-host + verbs: + - create + - update + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch + - update - apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRole metadata: diff --git a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml index 211b540cb55b..1ce8e9d994e0 100644 --- a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml @@ -3655,6 +3655,64 @@ items: - create - patch - update +- apiVersion: v1 + kind: ClusterRole + metadata: + annotations: + authorization.openshift.io/system-only: "true" + openshift.io/reconcile-protect: "false" + creationTimestamp: null + name: system:openshift:controller:ingress-to-route-controller + rules: + - apiGroups: + - "" + attributeRestrictions: null + resources: + - secrets + - services + verbs: + - get + - list + - watch + - apiGroups: + - extensions + attributeRestrictions: null + resources: + - ingress + verbs: + - get + - list + - watch + - apiGroups: + - route.openshift.io + attributeRestrictions: null + resources: + - routes + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - route.openshift.io + attributeRestrictions: null + resources: + - routes/custom-host + verbs: + - create + - update + - apiGroups: + - "" + attributeRestrictions: null + resources: + - events + verbs: + - create + - patch + - update - apiVersion: v1 kind: ClusterRole metadata: @@ -7119,6 +7177,22 @@ items: namespace: openshift-infra userNames: - system:serviceaccount:openshift-infra:service-ingress-ip-controller +- apiVersion: v1 + groupNames: null + kind: ClusterRoleBinding + metadata: + annotations: + openshift.io/reconcile-protect: "false" + creationTimestamp: null + name: system:openshift:controller:ingress-to-route-controller + roleRef: + name: system:openshift:controller:ingress-to-route-controller + subjects: + - kind: ServiceAccount + name: ingress-to-route-controller + namespace: openshift-infra + userNames: + - system:serviceaccount:openshift-infra:ingress-to-route-controller - apiVersion: v1 groupNames: null kind: ClusterRoleBinding