Skip to content

Commit

Permalink
Add ingress class filtering to ingress status updating
Browse files Browse the repository at this point in the history 
Fixes projectcontour#2388
Updates projectcontour#403

Move annotations to use upstream `ObjectMetaAccessor` instead of `k8s.Object`, removes an import cycle.

Copy `KindOf` from k8s to `annotations_test.go` for use in the tests only, also prevents and import cycle.

Move `ParseTimeout` from `k8s` to `annotation` for now as well, more import cycles.

Move ingress class matching code from `dag` to `annotation`, so it's only in one place.

Add ingress class filtering to `k8s.IngressStatusUpdater` `OnAdd` and `OnUpdate`.

Signed-off-by: Nick Young <ynick@vmware.com>
  • Loading branch information
@youngnick
youngnick committed Apr 7, 2020
1 parent 2cd68e2 commit c47550c
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 64 deletions.
38 changes: 30 additions & 8 deletions internal/annotation/annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ import (
"time"

envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth"
"github.com/projectcontour/contour/internal/k8s"
"k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// DEFAULT_INGRESS_CLASS is the Contour default.
const DEFAULT_INGRESS_CLASS = "contour"

// IsKnown checks if an annotation is one Contour knows about.
func IsKnown(key string) bool {
// We should know about everything with a Contour prefix.
Expand Down Expand Up @@ -101,7 +104,7 @@ func ValidForKind(kind string, key string) bool {
// CompatAnnotation checks the Object for the given annotation, first with the
// "projectcontour.io/" prefix, and then with the "contour.heptio.com/" prefix
// if that is not found.
func CompatAnnotation(o k8s.Object, key string) string {
func CompatAnnotation(o metav1.ObjectMetaAccessor, key string) string {
a := o.GetObjectMeta().GetAnnotations()

if val, ok := a["projectcontour.io/"+key]; ok {
Expand Down Expand Up @@ -184,15 +187,15 @@ func NumRetries(i *v1beta1.Ingress) uint32 {

// PerTryTimeout returns the duration envoy will wait per retry cycle.
func PerTryTimeout(i *v1beta1.Ingress) time.Duration {
return k8s.ParseTimeout(CompatAnnotation(i, "per-try-timeout"))
return ParseTimeout(CompatAnnotation(i, "per-try-timeout"))
}

// IngressClass returns the first matching ingress class for the following
// annotations:
// 1. projectcontour.io/ingress.class
// 2. contour.heptio.com/ingress.class
// 3. kubernetes.io/ingress.class
func IngressClass(o k8s.Object) string {
func IngressClass(o metav1.ObjectMetaAccessor) string {
a := o.GetObjectMeta().GetAnnotations()
if class, ok := a["projectcontour.io/ingress.class"]; ok {
return class
Expand All @@ -206,6 +209,25 @@ func IngressClass(o k8s.Object) string {
return ""
}

// MatchesIngressClass checks that the passed object has an ingress class that matches
// either the passed ingress-class string, or DEFAULT_INGRESS_CLASS if it's empty.
func MatchesIngressClass(o metav1.ObjectMetaAccessor, ic string) bool {

switch IngressClass(o) {
case ic:
// Handles ic == "" and ic == "custom".
return true
case DEFAULT_INGRESS_CLASS:
// ic == "" implicitly matches the default too.
if ic == "" {
return true
}
}

return false

}

// MinProtoVersion returns the TLS protocol version specified by an ingress annotation
// or default if non present.
func MinProtoVersion(version string) envoy_api_v2_auth.TlsParameters_TlsProtocol {
Expand All @@ -226,7 +248,7 @@ func MinProtoVersion(version string) envoy_api_v2_auth.TlsParameters_TlsProtocol
// 2. contour.heptio.com/max-connections
//
// '0' is returned if the annotation is absent or unparseable.
func MaxConnections(o k8s.Object) uint32 {
func MaxConnections(o metav1.ObjectMetaAccessor) uint32 {
return parseUInt32(CompatAnnotation(o, "max-connections"))
}

Expand All @@ -236,7 +258,7 @@ func MaxConnections(o k8s.Object) uint32 {
// 2. contour.heptio.com/max-pending-requests
//
// '0' is returned if the annotation is absent or unparseable.
func MaxPendingRequests(o k8s.Object) uint32 {
func MaxPendingRequests(o metav1.ObjectMetaAccessor) uint32 {
return parseUInt32(CompatAnnotation(o, "max-pending-requests"))
}

Expand All @@ -246,7 +268,7 @@ func MaxPendingRequests(o k8s.Object) uint32 {
// 2. contour.heptio.com/max-requests
//
// '0' is returned if the annotation is absent or unparseable.
func MaxRequests(o k8s.Object) uint32 {
func MaxRequests(o metav1.ObjectMetaAccessor) uint32 {
return parseUInt32(CompatAnnotation(o, "max-requests"))
}

Expand All @@ -256,6 +278,6 @@ func MaxRequests(o k8s.Object) uint32 {
// 2. contour.heptio.com/max-retries
//
// '0' is returned if the annotation is absent or unparseable.
func MaxRetries(o k8s.Object) uint32 {
func MaxRetries(o metav1.ObjectMetaAccessor) uint32 {
return parseUInt32(CompatAnnotation(o, "max-retries"))
}
125 changes: 118 additions & 7 deletions internal/annotation/annotations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
ingressroutev1 "github.com/projectcontour/contour/apis/contour/v1beta1"
projectcontour "github.com/projectcontour/contour/apis/projectcontour/v1"
"github.com/projectcontour/contour/internal/assert"
"github.com/projectcontour/contour/internal/k8s"
v1 "k8s.io/api/core/v1"
"k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -388,7 +387,7 @@ func TestAnnotationKindValidation(t *testing.T) {
valid bool
}
tests := map[string]struct {
obj k8s.Object
obj metav1.ObjectMetaAccessor
annotations map[string]status
}{
"service": {
Expand Down Expand Up @@ -436,10 +435,10 @@ func TestAnnotationKindValidation(t *testing.T) {
// Trivially check that everything specified in the global
// table is valid.
for _, kind := range []string{
k8s.KindOf(&v1.Service{}),
k8s.KindOf(&v1beta1.Ingress{}),
k8s.KindOf(&ingressroutev1.IngressRoute{}),
k8s.KindOf(&projectcontour.HTTPProxy{}),
kindOf(&v1.Service{}),
kindOf(&v1beta1.Ingress{}),
kindOf(&ingressroutev1.IngressRoute{}),
kindOf(&projectcontour.HTTPProxy{}),
} {
for key := range annotationsByKind[kind] {
t.Run(fmt.Sprintf("%s is known and valid for %s", key, kind),
Expand All @@ -455,15 +454,127 @@ func TestAnnotationKindValidation(t *testing.T) {
t.Run(name, func(t *testing.T) {
for k, s := range tc.annotations {
assert.Equal(t, s.known, IsKnown(k))
assert.Equal(t, s.valid, ValidForKind(k8s.KindOf(tc.obj), k))
assert.Equal(t, s.valid, ValidForKind(kindOf(tc.obj), k))
}
})
}
}

func TestMatchIngressClass(t *testing.T) {

// This is a matrix test, we are testing the annotation parser
// across various annotations, with two options:
// ingress class is empty
// ingress class is not empty.
tests := map[string]struct {
fixture metav1.ObjectMetaAccessor
// these are results for empty and "contour" ingress class
// respectively.
want []bool
}{
"ingress nginx kubernetes.io/ingress.class": {
fixture: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"kubernetes.io/ingress.class": "nginx",
},
},
},
want: []bool{false, false},
},
"ingress nginx contour.heptio.com/ingress.class": {
fixture: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"contour.heptio.com/ingress.class": "nginx",
},
},
},
want: []bool{false, false},
},
"ingress contour kubernetes.io/ingress.class": {
fixture: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"kubernetes.io/ingress.class": DEFAULT_INGRESS_CLASS,
},
},
},
want: []bool{true, true},
},
"ingress contour contour.heptio.com/ingress.class": {
fixture: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"contour.heptio.com/ingress.class": DEFAULT_INGRESS_CLASS,
},
},
},
want: []bool{true, true},
},
"no annotation": {
fixture: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "noannotation",
Namespace: "default",
},
},
want: []bool{true, false},
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
cases := []string{"", DEFAULT_INGRESS_CLASS}
for i := 0; i < len(cases); i++ {
got := MatchesIngressClass(tc.fixture, cases[i])
if tc.want[i] != got {
t.Errorf("IngressClass match(%v): expected %v, got %v", tc.fixture, tc.want[i], got)
}
}

})
}
}
func backend(name string, port intstr.IntOrString) *v1beta1.IngressBackend {
return &v1beta1.IngressBackend{
ServiceName: name,
ServicePort: port,
}
}

// kindOf returns the kind string for the given Kubernetes object.
//
// The API machinery doesn't populate the metav1.TypeMeta field for
// objects, so we have to use a type assertion to detect kinds that
// we care about.
// TODO(youngnick): This is a straight copy from internal/k8s/kind.go
// Needs to be moved to a separate module somewhere.
func kindOf(obj interface{}) string {
switch obj.(type) {
case *v1.Secret:
return "Secret"
case *v1.Service:
return "Service"
case *v1beta1.Ingress:
return "Ingress"
case *ingressroutev1.IngressRoute:
return "IngressRoute"
case *projectcontour.HTTPProxy:
return "HTTPProxy"
case *ingressroutev1.TLSCertificateDelegation:
return "TLSCertificateDelegation"
case *projectcontour.TLSCertificateDelegation:
return "TLSCertificateDelegation"
default:
return ""
}
}
5 changes: 4 additions & 1 deletion internal/k8s/timeout.go → internal/annotation/timeout.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package k8s
package annotation

import "time"

// TODO(youngnick): This needs to move to another package, but we need to be careful
// about the import graph, this must stay a leaf node.

// ParseTimeout parses timeouts we pass in various places in a standard way.
func ParseTimeout(timeout string) time.Duration {
if timeout == "" {
Expand Down
34 changes: 11 additions & 23 deletions internal/dag/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ import (
serviceapis "sigs.k8s.io/service-apis/api/v1alpha1"
)

// DEFAULT_INGRESS_CLASS is the Contour default.
const DEFAULT_INGRESS_CLASS = "contour"

// A KubernetesCache holds Kubernetes objects and associated configuration and produces
// DAG values.
type KubernetesCache struct {
Expand Down Expand Up @@ -60,30 +57,21 @@ type KubernetesCache struct {
// matchesIngressClass returns true if the given Kubernetes object
// belongs to the Ingress class that this cache is using.
func (kc *KubernetesCache) matchesIngressClass(obj k8s.Object) bool {
objectClass := annotation.IngressClass(obj)

switch objectClass {
case kc.IngressClass:
// Handles kc.IngressClass == "" and kc.IngressClass == "custom".
return true
case DEFAULT_INGRESS_CLASS:
// kc.IngressClass == "" implicitly matches the default too.
if kc.IngressClass == "" {
return true
}
}
if !annotation.MatchesIngressClass(obj, kc.IngressClass) {
kind := k8s.KindOf(obj)
om := obj.GetObjectMeta()

// Any other ingress class fails to match.
kind := k8s.KindOf(obj)
om := obj.GetObjectMeta()
kc.WithField("name", om.GetName()).
WithField("namespace", om.GetNamespace()).
WithField("kind", kind).
WithField("ingress.class", annotation.IngressClass(obj)).
Debug("ignoring object with unmatched ingress class")
return false
}

kc.WithField("name", om.GetName()).
WithField("namespace", om.GetNamespace()).
WithField("kind", kind).
WithField("ingress.class", objectClass).
Debug("ignoring object with unmatched ingress class")
return true

return false
}

// Insert inserts obj into the KubernetesCache.
Expand Down
13 changes: 7 additions & 6 deletions internal/dag/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

ingressroutev1 "github.com/projectcontour/contour/apis/contour/v1beta1"
projcontour "github.com/projectcontour/contour/apis/projectcontour/v1"
"github.com/projectcontour/contour/internal/annotation"
"github.com/sirupsen/logrus"
v1 "k8s.io/api/core/v1"
"k8s.io/api/networking/v1beta1"
Expand Down Expand Up @@ -585,7 +586,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"kubernetes.io/ingress.class": DEFAULT_INGRESS_CLASS,
"kubernetes.io/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand All @@ -597,7 +598,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "incorrect",
Namespace: "default",
Annotations: map[string]string{
"contour.heptio.com/ingress.class": DEFAULT_INGRESS_CLASS,
"contour.heptio.com/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand Down Expand Up @@ -642,7 +643,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "kuard",
Namespace: "default",
Annotations: map[string]string{
"contour.heptio.com/ingress.class": DEFAULT_INGRESS_CLASS,
"contour.heptio.com/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand All @@ -654,7 +655,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "kuard",
Namespace: "default",
Annotations: map[string]string{
"kubernetes.io/ingress.class": DEFAULT_INGRESS_CLASS,
"kubernetes.io/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand Down Expand Up @@ -699,7 +700,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "kuard",
Namespace: "default",
Annotations: map[string]string{
"contour.heptio.com/ingress.class": DEFAULT_INGRESS_CLASS,
"contour.heptio.com/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand All @@ -711,7 +712,7 @@ func TestKubernetesCacheInsert(t *testing.T) {
Name: "kuard",
Namespace: "default",
Annotations: map[string]string{
"kubernetes.io/ingress.class": DEFAULT_INGRESS_CLASS,
"kubernetes.io/ingress.class": annotation.DEFAULT_INGRESS_CLASS,
},
},
},
Expand Down

0 comments on commit c47550c

Please sign in to comment.