Improve parsing of annotations and use of Ingress wrapper

This commit is contained in:
Manuel Alejandro de Brito Fontes 2018-11-30 19:56:11 -03:00
parent ccd7b890fd
commit 67808c0ed8
No known key found for this signature in database
GPG key ID: 786136016A8BA02A
13 changed files with 278 additions and 334 deletions

View file

@ -18,16 +18,16 @@ package store
import (
"k8s.io/client-go/tools/cache"
"k8s.io/ingress-nginx/internal/ingress/annotations"
"k8s.io/ingress-nginx/internal/ingress"
)
// IngressAnnotationsLister makes a Store that lists annotations in Ingress rules.
type IngressAnnotationsLister struct {
// IngressWithAnnotationsLister makes a Store that lists Ingress rules with annotations already parsed
type IngressWithAnnotationsLister struct {
cache.Store
}
// ByKey returns the Ingress annotations matching key in the local Ingress annotations Store.
func (il IngressAnnotationsLister) ByKey(key string) (*annotations.Ingress, error) {
// ByKey returns the Ingress with annotations matching key in the local store or an error
func (il IngressWithAnnotationsLister) ByKey(key string) (*ingress.Ingress, error) {
i, exists, err := il.GetByKey(key)
if err != nil {
return nil, err
@ -35,5 +35,5 @@ func (il IngressAnnotationsLister) ByKey(key string) (*annotations.Ingress, erro
if !exists {
return nil, NotExistsError(key)
}
return i.(*annotations.Ingress), nil
return i.(*ingress.Ingress), nil
}

View file

@ -73,9 +73,6 @@ type Storer interface {
// GetServiceEndpoints returns the Endpoints of a Service matching key.
GetServiceEndpoints(key string) (*corev1.Endpoints, error)
// GetIngress returns the Ingress matching key.
GetIngress(key string) (*extensions.Ingress, error)
// ListIngresses returns a list of all Ingresses in the store.
ListIngresses() []*ingress.Ingress
@ -132,13 +129,13 @@ type Informer struct {
// Lister contains object listers (stores).
type Lister struct {
Ingress IngressLister
Service ServiceLister
Endpoint EndpointLister
Secret SecretLister
ConfigMap ConfigMapLister
IngressAnnotation IngressAnnotationsLister
Pod PodLister
Ingress IngressLister
Service ServiceLister
Endpoint EndpointLister
Secret SecretLister
ConfigMap ConfigMapLister
IngressWithAnnotation IngressWithAnnotationsLister
Pod PodLister
}
// NotExistsError is returned when an object does not exist in a local store.
@ -261,7 +258,7 @@ func New(checkOCSP bool,
// k8sStore fulfills resolver.Resolver interface
store.annotations = annotations.NewAnnotationExtractor(store)
store.listers.IngressAnnotation.Store = cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)
store.listers.IngressWithAnnotation.Store = cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)
// create informers factory, enable and assign required informers
infFactory := informers.NewSharedInformerFactoryWithOptions(client, resyncPeriod,
@ -343,7 +340,7 @@ func New(checkOCSP bool,
}
recorder.Eventf(ing, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name))
store.listers.IngressAnnotation.Delete(ing)
store.listers.IngressWithAnnotation.Delete(ing)
key := k8s.MetaNamespaceKey(ing)
store.secretIngressMap.Delete(key)
@ -392,7 +389,7 @@ func New(checkOCSP bool,
if ings := store.secretIngressMap.Reference(key); len(ings) > 0 {
glog.Infof("secret %v was added and it is used in ingress annotations. Parsing...", key)
for _, ingKey := range ings {
ing, err := store.GetIngress(ingKey)
ing, err := store.getIngress(ingKey)
if err != nil {
glog.Errorf("could not find Ingress %v in local store", ingKey)
continue
@ -419,7 +416,7 @@ func New(checkOCSP bool,
if ings := store.secretIngressMap.Reference(key); len(ings) > 0 {
glog.Infof("secret %v was updated and it is used in ingress annotations. Parsing...", key)
for _, ingKey := range ings {
ing, err := store.GetIngress(ingKey)
ing, err := store.getIngress(ingKey)
if err != nil {
glog.Errorf("could not find Ingress %v in local store", ingKey)
continue
@ -458,7 +455,7 @@ func New(checkOCSP bool,
if ings := store.secretIngressMap.Reference(key); len(ings) > 0 {
glog.Infof("secret %v was deleted and it is used in ingress annotations. Parsing...", key)
for _, ingKey := range ings {
ing, err := store.GetIngress(ingKey)
ing, err := store.getIngress(ingKey)
if err != nil {
glog.Errorf("could not find Ingress %v in local store", ingKey)
continue
@ -525,10 +522,10 @@ func New(checkOCSP bool,
store.setConfig(cm)
}
ings := store.listers.IngressAnnotation.List()
ings := store.listers.IngressWithAnnotation.List()
for _, ingKey := range ings {
key := k8s.MetaNamespaceKey(ingKey)
ing, err := store.GetIngress(key)
ing, err := store.getIngress(key)
if err != nil {
glog.Errorf("could not find Ingress %v in local store: %v", key, err)
continue
@ -597,9 +594,24 @@ func (s *k8sStore) extractAnnotations(ing *extensions.Ingress) {
key := k8s.MetaNamespaceKey(ing)
glog.V(3).Infof("updating annotations information for ingress %v", key)
anns := s.annotations.Extract(ing)
copyIng := ing.DeepCopy()
err := s.listers.IngressAnnotation.Update(anns)
for ri, rule := range copyIng.Spec.Rules {
if rule.HTTP == nil {
continue
}
for pi, path := range rule.HTTP.Paths {
if path.Path == "" {
copyIng.Spec.Rules[ri].HTTP.Paths[pi].Path = "/"
}
}
}
err := s.listers.IngressWithAnnotation.Update(&ingress.Ingress{
Ingress: *copyIng,
ParsedAnnotations: s.annotations.Extract(copyIng),
})
if err != nil {
glog.Error(err)
}
@ -697,59 +709,28 @@ func (s k8sStore) GetService(key string) (*corev1.Service, error) {
return s.listers.Service.ByKey(key)
}
// GetIngress returns the Ingress matching key.
func (s k8sStore) GetIngress(key string) (*extensions.Ingress, error) {
return s.listers.Ingress.ByKey(key)
// getIngress returns the Ingress matching key.
func (s k8sStore) getIngress(key string) (*extensions.Ingress, error) {
ing, err := s.listers.IngressWithAnnotation.ByKey(key)
if err != nil {
return nil, err
}
return &ing.Ingress, nil
}
// ListIngresses returns the list of Ingresses
func (s k8sStore) ListIngresses() []*ingress.Ingress {
// filter ingress rules
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
}
for pi, path := range rule.HTTP.Paths {
if path.Path == "" {
ing.Spec.Rules[ri].HTTP.Paths[pi].Path = "/"
}
}
}
ingresses = append(ingresses, &ingress.Ingress{
Ingress: *ing,
ParsedAnnotations: anns,
})
ingresses := make([]*ingress.Ingress, 0)
for _, item := range s.listers.IngressWithAnnotation.List() {
ing := item.(*ingress.Ingress)
ingresses = append(ingresses, ing)
}
return ingresses
}
// 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
}
return ia, nil
}
// GetLocalSSLCert returns the local copy of a SSLCert
func (s k8sStore) GetLocalSSLCert(key string) (*ingress.SSLCert, error) {
return s.sslStore.ByKey(key)

View file

@ -38,6 +38,7 @@ import (
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/ingress-nginx/internal/file"
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/test/e2e/framework"
@ -86,14 +87,6 @@ func TestStore(t *testing.T) {
storer.Run(stopCh)
key := fmt.Sprintf("%v/anything", ns)
ing, err := storer.GetIngress(key)
if err == nil {
t.Errorf("expected an error but none returned")
}
if ing != nil {
t.Errorf("expected an Ingres but none returned")
}
ls, err := storer.GetLocalSSLCert(key)
if err == nil {
t.Errorf("expected an error but none returned")
@ -753,9 +746,9 @@ func newStore(t *testing.T) *k8sStore {
return &k8sStore{
listers: &Lister{
// add more listers if needed
Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
IngressAnnotation: IngressAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)},
Pod: PodLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
IngressWithAnnotation: IngressWithAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)},
Pod: PodLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
},
sslStore: NewSSLCertTracker(),
filesystem: fs,
@ -833,58 +826,43 @@ func TestUpdateSecretIngressMap(t *testing.T) {
func TestListIngresses(t *testing.T) {
s := newStore(t)
ingEmptyClass := &extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-1",
Namespace: "testns",
},
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
ingressToIgnore := &ingress.Ingress{
Ingress: extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-2",
Namespace: "testns",
Annotations: map[string]string{
"kubernetes.io/ingress.class": "something",
},
},
Rules: []extensions.IngressRule{
{
Host: "foo.bar",
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
},
},
},
}
s.listers.Ingress.Add(ingEmptyClass)
s.listers.IngressWithAnnotation.Add(ingressToIgnore)
ingressToIgnore := &extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-2",
Namespace: "testns",
Annotations: map[string]string{
"kubernetes.io/ingress.class": "something",
ingressWithoutPath := &ingress.Ingress{
Ingress: extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-3",
Namespace: "testns",
},
},
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
},
},
}
s.listers.Ingress.Add(ingressToIgnore)
ingressWithoutPath := &extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-3",
Namespace: "testns",
},
Spec: extensions.IngressSpec{
Rules: []extensions.IngressRule{
{
Host: "foo.bar",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Backend: extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
Spec: extensions.IngressSpec{
Rules: []extensions.IngressRule{
{
Host: "foo.bar",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Backend: extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
},
},
},
},
@ -894,28 +872,30 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.Ingress.Add(ingressWithoutPath)
s.listers.IngressWithAnnotation.Add(ingressWithoutPath)
ingressWithNginxClass := &extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-4",
Namespace: "testns",
Annotations: map[string]string{
"kubernetes.io/ingress.class": "nginx",
ingressWithNginxClass := &ingress.Ingress{
Ingress: extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-4",
Namespace: "testns",
Annotations: map[string]string{
"kubernetes.io/ingress.class": "nginx",
},
},
},
Spec: extensions.IngressSpec{
Rules: []extensions.IngressRule{
{
Host: "foo.bar",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Path: "/demo",
Backend: extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
Spec: extensions.IngressSpec{
Rules: []extensions.IngressRule{
{
Host: "foo.bar",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Path: "/demo",
Backend: extensions.IngressBackend{
ServiceName: "demo",
ServicePort: intstr.FromInt(80),
},
},
},
},
@ -925,7 +905,7 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.Ingress.Add(ingressWithNginxClass)
s.listers.IngressWithAnnotation.Add(ingressWithNginxClass)
ingresses := s.ListIngresses()
if s := len(ingresses); s != 3 {