diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index d3ea7dd07..0ebe4cc70 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -372,18 +372,14 @@ func (n *NGINXController) getDefaultUpstream() *ingress.Backend { // getBackendServers returns a list of Upstream and Server to be used by the // backend. An upstream can be used in multiple servers if the namespace, // service name and port are the same. -func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]*ingress.Backend, []*ingress.Server) { +func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*ingress.Backend, []*ingress.Server) { du := n.getDefaultUpstream() upstreams := n.createUpstreams(ingresses, du) servers := n.createServers(ingresses, upstreams, du) for _, ing := range ingresses { ingKey := k8s.MetaNamespaceKey(ing) - - anns, err := n.store.GetIngressAnnotations(ingKey) - if err != nil { - glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) - } + anns := ing.ParsedAnnotations for _, rule := range ing.Spec.Rules { host := rule.Host @@ -631,17 +627,12 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] // createUpstreams creates the NGINX upstreams (Endpoints) for each Service // referenced in Ingress rules. -func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingress.Backend) map[string]*ingress.Backend { +func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.Backend) map[string]*ingress.Backend { upstreams := make(map[string]*ingress.Backend) upstreams[defUpstreamName] = du for _, ing := range data { - ingKey := k8s.MetaNamespaceKey(ing) - - anns, err := n.store.GetIngressAnnotations(ingKey) - if err != nil { - glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) - } + anns := ing.ParsedAnnotations var defBackend string if ing.Spec.Backend != nil { @@ -878,7 +869,7 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string) ([]ingres // createServers builds a map of host name to Server structs from a map of // already computed Upstream structs. Each Server is configured with at least // one root location, which uses a default backend if left unspecified. -func (n *NGINXController) createServers(data []*extensions.Ingress, +func (n *NGINXController) createServers(data []*ingress.Ingress, upstreams map[string]*ingress.Backend, du *ingress.Backend) map[string]*ingress.Server { @@ -932,11 +923,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // initialize all other servers for _, ing := range data { ingKey := k8s.MetaNamespaceKey(ing) - - anns, err := n.store.GetIngressAnnotations(ingKey) - if err != nil { - glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) - } + anns := ing.ParsedAnnotations // default upstream name un := du.Name @@ -1015,11 +1002,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // configure default location, alias, and SSL for _, ing := range data { ingKey := k8s.MetaNamespaceKey(ing) - - anns, err := n.store.GetIngressAnnotations(ingKey) - if err != nil { - glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) - } + anns := ing.ParsedAnnotations for _, rule := range ing.Spec.Rules { host := rule.Host @@ -1126,7 +1109,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress, // If a match is found, we know that this server should back the alternative backend and add the alternative backend // to a backend's alternative list. // If no match is found, then the serverless backend is deleted. -func mergeAlternativeBackends(ing *extensions.Ingress, upstreams map[string]*ingress.Backend, +func mergeAlternativeBackends(ing *ingress.Ingress, upstreams map[string]*ingress.Backend, servers map[string]*ingress.Server) { // merge catch-all alternative backends @@ -1183,7 +1166,7 @@ func mergeAlternativeBackends(ing *extensions.Ingress, upstreams map[string]*ing // extractTLSSecretName returns the name of the Secret containing a SSL // certificate for the given host name, or an empty string. -func extractTLSSecretName(host string, ing *extensions.Ingress, +func extractTLSSecretName(host string, ing *ingress.Ingress, getLocalSSLCert func(string) (*ingress.SSLCert, error)) string { if ing == nil { diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 5edd51d0c..10342db7f 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -20,9 +20,10 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" - "k8s.io/apimachinery/pkg/util/intstr" "testing" + "k8s.io/apimachinery/pkg/util/intstr" + extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-nginx/internal/ingress" @@ -30,31 +31,33 @@ import ( func TestMergeAlternativeBackends(t *testing.T) { testCases := map[string]struct { - ingress *extensions.Ingress + ingress *ingress.Ingress upstreams map[string]*ingress.Backend servers map[string]*ingress.Server expNumAlternativeBackends int expNumLocations int }{ "alternative backend has no server and embeds into matching real backend": { - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "example", - }, - Spec: extensions.IngressSpec{ - Rules: []extensions.IngressRule{ - { - Host: "example.com", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/", - Backend: extensions.IngressBackend{ - ServiceName: "http-svc-canary", - ServicePort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: 80, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "example.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, }, }, }, @@ -93,43 +96,45 @@ func TestMergeAlternativeBackends(t *testing.T) { 1, }, "merging a alternative backend matches with the correct host": { - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "example", - }, - Spec: extensions.IngressSpec{ - Rules: []extensions.IngressRule{ - { - Host: "foo.bar", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/", - Backend: extensions.IngressBackend{ - ServiceName: "foo-http-svc-canary", - ServicePort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: 80, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "foo.bar", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "foo-http-svc-canary", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, }, }, }, }, }, }, - }, - { - Host: "example.com", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/", - Backend: extensions.IngressBackend{ - ServiceName: "http-svc-canary", - ServicePort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: 80, + { + Host: "example.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, }, }, }, @@ -209,7 +214,7 @@ func TestMergeAlternativeBackends(t *testing.T) { func TestExtractTLSSecretName(t *testing.T) { testCases := map[string]struct { host string - ingress *extensions.Ingress + ingress *ingress.Ingress fn func(string) (*ingress.SSLCert, error) expName string }{ @@ -223,7 +228,7 @@ func TestExtractTLSSecretName(t *testing.T) { }, "empty ingress": { "foo.bar", - &extensions.Ingress{}, + &ingress.Ingress{}, func(string) (*ingress.SSLCert, error) { return nil, nil }, @@ -231,17 +236,19 @@ func TestExtractTLSSecretName(t *testing.T) { }, "ingress tls, nil secret": { "foo.bar", - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extensions.IngressSpec{ - TLS: []extensions.IngressTLS{ - {SecretName: "demo"}, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", }, - Rules: []extensions.IngressRule{ - { - Host: "foo.bar", + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + {SecretName: "demo"}, + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar", + }, }, }, }, @@ -253,17 +260,19 @@ func TestExtractTLSSecretName(t *testing.T) { }, "ingress tls, no host, matching cert cn": { "foo.bar", - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extensions.IngressSpec{ - TLS: []extensions.IngressTLS{ - {SecretName: "demo"}, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", }, - Rules: []extensions.IngressRule{ - { - Host: "foo.bar", + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + {SecretName: "demo"}, + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar", + }, }, }, }, @@ -277,19 +286,21 @@ func TestExtractTLSSecretName(t *testing.T) { }, "ingress tls, no host, wildcard cert with matching cn": { "foo.bar", - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extensions.IngressSpec{ - TLS: []extensions.IngressTLS{ - { - SecretName: "demo", - }, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", }, - Rules: []extensions.IngressRule{ - { - Host: "test.foo.bar", + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + { + SecretName: "demo", + }, + }, + Rules: []extensions.IngressRule{ + { + Host: "test.foo.bar", + }, }, }, }, @@ -303,20 +314,22 @@ func TestExtractTLSSecretName(t *testing.T) { }, "ingress tls, hosts, matching cert cn": { "foo.bar", - &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extensions.IngressSpec{ - TLS: []extensions.IngressTLS{ - { - Hosts: []string{"foo.bar", "example.com"}, - SecretName: "demo", - }, + &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", }, - Rules: []extensions.IngressRule{ - { - Host: "foo.bar", + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + { + Hosts: []string{"foo.bar", "example.com"}, + SecretName: "demo", + }, + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar", + }, }, }, }, diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 751246c01..0870eda03 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -74,10 +74,7 @@ type Storer interface { GetIngress(key string) (*extensions.Ingress, error) // ListIngresses returns a list of all Ingresses in the store. - ListIngresses() []*extensions.Ingress - - // GetIngressAnnotations returns the parsed annotations of an Ingress matching key. - GetIngressAnnotations(key string) (*annotations.Ingress, error) + ListIngresses() []*ingress.Ingress // GetLocalSSLCert returns the local copy of a SSLCert GetLocalSSLCert(name string) (*ingress.SSLCert, error) @@ -644,15 +641,22 @@ func (s k8sStore) GetIngress(key string) (*extensions.Ingress, error) { } // ListIngresses returns the list of Ingresses -func (s k8sStore) ListIngresses() []*extensions.Ingress { +func (s k8sStore) ListIngresses() []*ingress.Ingress { // filter ingress rules - var ingresses []*extensions.Ingress + var ingresses []*ingress.Ingress for _, item := range s.listers.Ingress.List() { ing := item.(*extensions.Ingress) if !class.IsValid(ing) { continue } + ingKey := k8s.MetaNamespaceKey(ing) + anns, err := s.getIngressAnnotations(ingKey) + if err != nil { + glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err) + + } + for ri, rule := range ing.Spec.Rules { if rule.HTTP == nil { continue @@ -665,14 +669,17 @@ func (s k8sStore) ListIngresses() []*extensions.Ingress { } } - ingresses = append(ingresses, ing) + ingresses = append(ingresses, &ingress.Ingress{ + Ingress: *ing, + ParsedAnnotations: anns, + }) } return ingresses } -// GetIngressAnnotations returns the parsed annotations of an Ingress matching key. -func (s k8sStore) GetIngressAnnotations(key string) (*annotations.Ingress, error) { +// getIngressAnnotations returns the parsed annotations of an Ingress matching key. +func (s k8sStore) getIngressAnnotations(key string) (*annotations.Ingress, error) { ia, err := s.listers.IngressAnnotation.ByKey(key) if err != nil { return &annotations.Ingress{}, err diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 5b6e0738f..a3ccd8cd6 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -730,7 +730,8 @@ func newStore(t *testing.T) *k8sStore { return &k8sStore{ listers: &Lister{ // add more listers if needed - Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, + Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, + IngressAnnotation: IngressAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)}, }, sslStore: NewSSLCertTracker(), filesystem: fs, diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 1d463616a..ca36c355e 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -34,7 +34,6 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" - extensions "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" @@ -796,9 +795,9 @@ type ingressInformation struct { } func getIngressInformation(i, p interface{}) *ingressInformation { - ing, ok := i.(*extensions.Ingress) + ing, ok := i.(*ingress.Ingress) if !ok { - glog.Errorf("expected an '*extensions.Ingress' type but %T was returned", i) + glog.Errorf("expected an '*ingress.Ingress' type but %T was returned", i) return &ingressInformation{} } diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index f6f9ee36f..d941f54ed 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -30,7 +30,6 @@ import ( pool "gopkg.in/go-playground/pool.v3" apiv1 "k8s.io/api/core/v1" - extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/wait" @@ -41,6 +40,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" + "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -57,7 +57,7 @@ type Sync interface { type ingressLister interface { // ListIngresses returns the list of Ingresses - ListIngresses() []*extensions.Ingress + ListIngresses() []*ingress.Ingress } // Config ... @@ -371,7 +371,7 @@ func (s *statusSync) updateStatus(newIngressPoint []apiv1.LoadBalancerIngress) { batch.WaitAll() } -func runUpdate(ing *extensions.Ingress, status []apiv1.LoadBalancerIngress, +func runUpdate(ing *ingress.Ingress, status []apiv1.LoadBalancerIngress, client clientset.Interface) pool.WorkFunc { return func(wu pool.WorkUnit) (interface{}, error) { if wu.IsCancelled() { diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index c4f223ec5..463f830b9 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testclient "k8s.io/client-go/kubernetes/fake" + "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" @@ -231,25 +232,27 @@ func buildExtensionsIngresses() []extensions.Ingress { type testIngressLister struct { } -func (til *testIngressLister) ListIngresses() []*extensions.Ingress { - var ingresses []*extensions.Ingress - ingresses = append(ingresses, &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo_ingress_non_01", - Namespace: apiv1.NamespaceDefault, - }}) +func (til *testIngressLister) ListIngresses() []*ingress.Ingress { + var ingresses []*ingress.Ingress + ingresses = append(ingresses, &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo_ingress_non_01", + Namespace: apiv1.NamespaceDefault, + }}}) - ingresses = append(ingresses, &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo_ingress_1", - Namespace: apiv1.NamespaceDefault, - }, - Status: extensions.IngressStatus{ - LoadBalancer: apiv1.LoadBalancerStatus{ - Ingress: buildLoadBalancerIngressByIP(), + ingresses = append(ingresses, &ingress.Ingress{ + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo_ingress_1", + Namespace: apiv1.NamespaceDefault, }, - }, - }) + Status: extensions.IngressStatus{ + LoadBalancer: apiv1.LoadBalancerStatus{ + Ingress: buildLoadBalancerIngressByIP(), + }, + }, + }}) return ingresses } diff --git a/internal/ingress/types.go b/internal/ingress/types.go index 59ff3cbec..3256df169 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -20,6 +20,7 @@ import ( apiv1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations" "k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity" "k8s.io/ingress-nginx/internal/ingress/annotations/auth" @@ -209,7 +210,7 @@ type Location struct { // uses the default backend. IsDefBackend bool `json:"isDefBackend"` // Ingress returns the ingress from which this location was generated - Ingress *extensions.Ingress `json:"ingress"` + Ingress *Ingress `json:"ingress"` // Backend describes the name of the backend to use. Backend string `json:"backend"` // Service describes the referenced services from the ingress @@ -331,3 +332,9 @@ type ProxyProtocol struct { Decode bool `json:"decode"` Encode bool `json:"encode"` } + +// Ingress holds the definition of an Ingress plus its annotations +type Ingress struct { + extensions.Ingress + ParsedAnnotations *annotations.Ingress +}