Support all ingress versions (#128)

* Support all ingress versions

Signed-off-by: Mingfei Huang <mingfei.huang@getcruise.com>

Author: max0ne

charts/k-rail/Chart.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 name: k-rail
 description: Kubernetes security tool for policy enforcement
 home: https://github.com/cruise-automation/k-rail
-version: v3.6.0
+version: v3.6.1
 maintainers:
   - name: cruise-automation
     url: https://cruise-automation.github.io/k-rail/

policies/ingress/disallow_nginx_snippet.go

@@ -40,10 +40,7 @@ func (p PolicyDisallowNGINXSnippet) Validate(ctx context.Context, config policie
 		return resourceViolations, nil
 	}
 
-	if ingressResource.IngressExt.Annotations == nil {
-		return resourceViolations, nil
-	}
-	for key := range ingressResource.IngressExt.Annotations {
+	for key := range ingressResource.GetAnnotations() {
 		if nginxSnippetAnnotationRegex.MatchString(key) {
 			resourceViolations = append(resourceViolations, policies.ResourceViolation{
 				Namespace:    ar.Namespace,

policies/ingress/disallow_nginx_snippet_test.go

@@ -20,8 +20,10 @@ import (
 
 	admissionv1 "k8s.io/api/admission/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
+	networkingv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/cruise-automation/k-rail/v3/policies"
 )
@@ -30,14 +32,19 @@ func TestPolicyDisallowNGINXSnippet(t *testing.T) {
 	ctx := context.Background()
 
 	tests := []struct {
-		name       string
-		ingressExt *extensionsv1beta1.Ingress
+		name    string
+		ingress interface {
+			GetObjectKind() schema.ObjectKind
+		}
 		violations int
 	}{
 		{
 			name:       "deny 1",
 			violations: 1,
-			ingressExt: &extensionsv1beta1.Ingress{
+			ingress: &extensionsv1beta1.Ingress{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "extensions/v1beta1",
+				},
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
 						"nginx.ingress.kubernetes.io/server-snippet": "i'm malicious",
@@ -48,7 +55,25 @@ func TestPolicyDisallowNGINXSnippet(t *testing.T) {
 		{
 			name:       "deny 2",
 			violations: 2,
-			ingressExt: &extensionsv1beta1.Ingress{
+			ingress: &extensionsv1beta1.Ingress{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "networking.k8s.io/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"nginx.ingress.kubernetes.io/server-snippet": "i'm malicious",
+						"nginx.ingress.kubernetes.io/auth-snippet":   "me too",
+					},
+				},
+			},
+		},
+		{
+			name:       "deny 3",
+			violations: 2,
+			ingress: &networkingv1.Ingress{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "networking.k8s.io/v1",
+				},
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
 						"nginx.ingress.kubernetes.io/server-snippet": "i'm malicious",
@@ -60,7 +85,7 @@ func TestPolicyDisallowNGINXSnippet(t *testing.T) {
 		{
 			name:       "allow",
 			violations: 0,
-			ingressExt: &extensionsv1beta1.Ingress{
+			ingress: &extensionsv1beta1.Ingress{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
 						"foo": "bar",
@@ -71,16 +96,16 @@ func TestPolicyDisallowNGINXSnippet(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var ar = &admissionv1.AdmissionRequest{}
-
-			if tt.ingressExt != nil {
-				raw, _ := json.Marshal(tt.ingressExt)
-				ar = &admissionv1.AdmissionRequest{
-					Namespace: "namespace",
-					Name:      "name",
-					Object:    runtime.RawExtension{Raw: raw},
-					Resource:  metav1.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "ingresses"},
-				}
+			raw, _ := json.Marshal(tt.ingress)
+			ar := &admissionv1.AdmissionRequest{
+				Namespace: "namespace",
+				Name:      "name",
+				Object:    runtime.RawExtension{Raw: raw},
+				Resource: metav1.GroupVersionResource{
+					Group:    tt.ingress.GetObjectKind().GroupVersionKind().Group,
+					Version:  tt.ingress.GetObjectKind().GroupVersionKind().Version,
+					Resource: "ingresses",
+				},
 			}
 
 			v := PolicyDisallowNGINXSnippet{}

policies/ingress/require_ingress_exemption.go

@@ -38,20 +38,7 @@ func (p PolicyRequireIngressExemption) Validate(ctx context.Context, config poli
 	violationText := "Require Ingress Exemption: Using certain Ingress classes requires an exemption"
 
 	for _, ingressClass := range config.PolicyRequireIngressExemptionClasses {
-		for annotation, value := range ingressResource.IngressExt.ObjectMeta.GetAnnotations() {
-			if annotation == "kubernetes.io/ingress.class" {
-				if value == ingressClass {
-					resourceViolations = append(resourceViolations, policies.ResourceViolation{
-						Namespace:    ar.Namespace,
-						ResourceName: ingressResource.ResourceName,
-						ResourceKind: ingressResource.ResourceKind,
-						Violation:    violationText,
-						Policy:       p.Name(),
-					})
-				}
-			}
-		}
-		for annotation, value := range ingressResource.IngressNet.ObjectMeta.GetAnnotations() {
+		for annotation, value := range ingressResource.GetAnnotations() {
 			if annotation == "kubernetes.io/ingress.class" {
 				if value == ingressClass {
 					resourceViolations = append(resourceViolations, policies.ResourceViolation{

policies/ingress/require_ingress_exemption_test.go

@@ -100,7 +100,7 @@ func TestPolicyRequireIngressExemption(t *testing.T) {
 					Namespace: "namespace",
 					Name:      "name",
 					Object:    runtime.RawExtension{Raw: raw},
-					Resource:  metav1.GroupVersionResource{Group: "networking", Version: "v1beta1", Resource: "ingresses"},
+					Resource:  metav1.GroupVersionResource{Group: "networking.k8s.io", Version: "v1beta1", Resource: "ingresses"},
 				}
 			}
 

policies/ingress/unique_ingress_host.go

@@ -86,36 +86,8 @@ func (p PolicyRequireUniqueHost) Validate(ctx context.Context, config policies.C
 
 	violationText := "Requires Unique Ingress Host: Ingress Host should not point to multiple namespaces. Host already in:"
 
-	for _, rule := range ingressResource.IngressExt.Spec.Rules {
-		ingressNamespacesMap, err := p.CheckIngressNamespaces(ctx, rule.Host)
-		if err != nil {
-			log.Error(err)
-			return nil, nil
-		}
-		foundNamespace := false
-		_, ok := ingressNamespacesMap[ar.Namespace]
-		if ok {
-			foundNamespace = true
-		}
-		if (len(ingressNamespacesMap) == 0) || (len(ingressNamespacesMap) == 1 && foundNamespace) {
-			return resourceViolations, nil
-		} else {
-			namespacesStr := ""
-			for k := range ingressNamespacesMap {
-				namespacesStr = namespacesStr + " " + k
-			}
-			resourceViolations = append(resourceViolations, policies.ResourceViolation{
-				Namespace:    ar.Namespace,
-				ResourceName: ingressResource.ResourceName,
-				ResourceKind: ingressResource.ResourceKind,
-				Violation:    violationText + ": " + namespacesStr,
-				Policy:       p.Name(),
-			})
-		}
-	}
-
-	for _, rule := range ingressResource.IngressNet.Spec.Rules {
-		ingressNamespacesMap, err := p.CheckIngressNamespaces(ctx, rule.Host)
+	for _, host := range ingressResource.GetHosts() {
+		ingressNamespacesMap, err := p.CheckIngressNamespaces(ctx, host)
 		if err != nil {
 			log.Error(err)
 			return nil, nil

resource/ingress.go

@@ -17,16 +17,18 @@ import (
 
 	admissionv1 "k8s.io/api/admission/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
+	networkingv1 "k8s.io/api/networking/v1"
 	networkingv1beta1 "k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // IngressResource contains the information needed for processing by a Policy
 type IngressResource struct {
-	IngressExt   extensionsv1beta1.Ingress
-	IngressNet   networkingv1beta1.Ingress
-	ResourceName string
-	ResourceKind string
+	IngressExt        extensionsv1beta1.Ingress
+	IngressNetV1Beta1 networkingv1beta1.Ingress
+	IngressNetV1      networkingv1.Ingress
+	ResourceName      string
+	ResourceKind      string
 }
 
 // GetIngressResource extracts and IngressResource from an AdmissionRequest
@@ -49,17 +51,54 @@ func decodeIngressResource(ar *admissionv1.AdmissionRequest) *IngressResource {
 			ResourceName: GetResourceName(ing.ObjectMeta),
 			ResourceKind: "Ingress",
 		}
-	case metav1.GroupVersionResource{Group: "networking", Version: "v1beta1", Resource: "ingresses"}:
+	case metav1.GroupVersionResource{Group: "networking.k8s.io", Version: "v1beta1", Resource: "ingresses"}:
 		ing := networkingv1beta1.Ingress{}
 		if err := decodeObject(ar.Object.Raw, &ing); err != nil {
 			return nil
 		}
 		return &IngressResource{
-			IngressNet:   ing,
+			IngressNetV1Beta1: ing,
+			ResourceName:      GetResourceName(ing.ObjectMeta),
+			ResourceKind:      "Ingress",
+		}
+	case metav1.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "ingresses"}:
+		ing := networkingv1.Ingress{}
+		if err := decodeObject(ar.Object.Raw, &ing); err != nil {
+			return nil
+		}
+		return &IngressResource{
+			IngressNetV1: ing,
 			ResourceName: GetResourceName(ing.ObjectMeta),
 			ResourceKind: "Ingress",
 		}
 	default:
 		return nil
 	}
 }
+
+// GetAnnotations returns ingress annotations, across all available ingress versions
+func (ir IngressResource) GetAnnotations() map[string]string {
+	if ir.IngressExt.Annotations != nil {
+		return ir.IngressExt.Annotations
+	} else if ir.IngressNetV1Beta1.Annotations != nil {
+		return ir.IngressNetV1Beta1.Annotations
+	} else if ir.IngressNetV1.Annotations != nil {
+		return ir.IngressNetV1.Annotations
+	}
+	return nil
+}
+
+// GetHosts returns list of all hosts in ingress spec, across all available ingress versions
+func (ir IngressResource) GetHosts() []string {
+	hosts := []string{}
+	for _, rule := range ir.IngressExt.Spec.Rules {
+		hosts = append(hosts, rule.Host)
+	}
+	for _, rule := range ir.IngressNetV1Beta1.Spec.Rules {
+		hosts = append(hosts, rule.Host)
+	}
+	for _, rule := range ir.IngressNetV1.Spec.Rules {
+		hosts = append(hosts, rule.Host)
+	}
+	return hosts
+}