Add admission controller e2e test

This commit is contained in:
Manuel Alejandro de Brito Fontes 2020-09-25 18:45:13 -03:00
parent 4e3e5ebb94
commit 7722fa38aa
15 changed files with 295 additions and 53 deletions

View file

@ -24,7 +24,7 @@ webhooks:
failurePolicy: Fail
sideEffects: None
admissionReviewVersions:
- v1beta1
- v1
clientConfig:
service:
namespace: {{ .Release.Namespace }}

View file

@ -19,7 +19,7 @@ package controller
import (
"fmt"
"k8s.io/api/admission/v1beta1"
admissionv1 "k8s.io/api/admission/v1"
networking "k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
@ -56,9 +56,9 @@ var (
// HandleAdmission populates the admission Response
// with Allowed=false if the Object is an ingress that would prevent nginx to reload the configuration
// with Allowed=true otherwise
func (ia *IngressAdmission) HandleAdmission(ar *v1beta1.AdmissionReview) {
func (ia *IngressAdmission) HandleAdmission(ar *admissionv1.AdmissionReview) {
if ar.Request == nil {
ar.Response = &v1beta1.AdmissionResponse{
ar.Response = &admissionv1.AdmissionResponse{
Allowed: false,
}
@ -68,7 +68,7 @@ func (ia *IngressAdmission) HandleAdmission(ar *v1beta1.AdmissionReview) {
if ar.Request.Resource != networkingV1Beta1Resource && ar.Request.Resource != networkingV1Resource {
err := fmt.Errorf("rejecting admission review because the request does not contains an Ingress resource but %s with name %s in namespace %s",
ar.Request.Resource.String(), ar.Request.Name, ar.Request.Namespace)
ar.Response = &v1beta1.AdmissionResponse{
ar.Response = &admissionv1.AdmissionResponse{
UID: ar.Request.UID,
Allowed: false,
Result: &metav1.Status{Message: err.Error()},
@ -83,7 +83,7 @@ func (ia *IngressAdmission) HandleAdmission(ar *v1beta1.AdmissionReview) {
klog.Errorf("failed to decode ingress %s in namespace %s: %s, refusing it",
ar.Request.Name, ar.Request.Namespace, err.Error())
ar.Response = &v1beta1.AdmissionResponse{
ar.Response = &admissionv1.AdmissionResponse{
UID: ar.Request.UID,
Allowed: false,
@ -99,7 +99,7 @@ func (ia *IngressAdmission) HandleAdmission(ar *v1beta1.AdmissionReview) {
if err := ia.Checker.CheckIngress(&ingress); err != nil {
klog.Errorf("failed to generate configuration for ingress %s in namespace %s: %s, refusing it",
ar.Request.Name, ar.Request.Namespace, err.Error())
ar.Response = &v1beta1.AdmissionResponse{
ar.Response = &admissionv1.AdmissionResponse{
UID: ar.Request.UID,
Allowed: false,
Result: &metav1.Status{Message: err.Error()},
@ -113,7 +113,7 @@ func (ia *IngressAdmission) HandleAdmission(ar *v1beta1.AdmissionReview) {
klog.Infof("successfully validated configuration, accepting ingress %s in namespace %s",
ar.Request.Name, ar.Request.Namespace)
ar.Response = &v1beta1.AdmissionResponse{
ar.Response = &admissionv1.AdmissionResponse{
UID: ar.Request.UID,
Allowed: true,
}

View file

@ -20,7 +20,7 @@ import (
"fmt"
"testing"
"k8s.io/api/admission/v1beta1"
admissionv1 "k8s.io/api/admission/v1"
networking "k8s.io/api/networking/v1beta1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/json"
@ -53,8 +53,8 @@ func TestHandleAdmission(t *testing.T) {
adm := &IngressAdmission{
Checker: failTestChecker{t: t},
}
review := &v1beta1.AdmissionReview{
Request: &v1beta1.AdmissionRequest{
review := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: v1.GroupVersionResource{Group: "", Version: "v1", Resource: "pod"},
},
}

View file

@ -21,7 +21,7 @@ import (
"io/ioutil"
"net/http"
"k8s.io/api/admission/v1beta1"
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/json"
@ -36,7 +36,7 @@ var (
// AdmissionController checks if an object
// is allowed in the cluster
type AdmissionController interface {
HandleAdmission(*v1beta1.AdmissionReview)
HandleAdmission(*admissionv1.AdmissionReview)
}
// AdmissionControllerServer implements an HTTP server
@ -71,8 +71,8 @@ func (acs *AdmissionControllerServer) ServeHTTP(w http.ResponseWriter, r *http.R
}
}
func parseAdmissionReview(decoder runtime.Decoder, r io.Reader) (*v1beta1.AdmissionReview, error) {
review := &v1beta1.AdmissionReview{}
func parseAdmissionReview(decoder runtime.Decoder, r io.Reader) (*admissionv1.AdmissionReview, error) {
review := &admissionv1.AdmissionReview{}
data, err := ioutil.ReadAll(r)
if err != nil {
return nil, err
@ -81,7 +81,7 @@ func parseAdmissionReview(decoder runtime.Decoder, r io.Reader) (*v1beta1.Admiss
return review, err
}
func writeAdmissionReview(w io.Writer, ar *v1beta1.AdmissionReview) error {
func writeAdmissionReview(w io.Writer, ar *admissionv1.AdmissionReview) error {
e := json.NewEncoder(w)
return e.Encode(ar)
}

View file

@ -24,13 +24,13 @@ import (
"strings"
"testing"
"k8s.io/api/admission/v1beta1"
admissionv1 "k8s.io/api/admission/v1"
)
type testAdmissionHandler struct{}
func (testAdmissionHandler) HandleAdmission(ar *v1beta1.AdmissionReview) {
ar.Response = &v1beta1.AdmissionResponse{
func (testAdmissionHandler) HandleAdmission(ar *admissionv1.AdmissionReview) {
ar.Response = &admissionv1.AdmissionResponse{
Allowed: true,
}
}
@ -56,7 +56,7 @@ func (errorWriter) WriteHeader(statusCode int) {}
func TestServer(t *testing.T) {
w := httptest.NewRecorder()
b := bytes.NewBuffer(nil)
writeAdmissionReview(b, &v1beta1.AdmissionReview{})
writeAdmissionReview(b, &admissionv1.AdmissionReview{})
// Happy path
r := httptest.NewRequest("GET", "http://test.ns.svc", b)

View file

@ -38,6 +38,8 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/annotations/proxy"
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
"k8s.io/ingress-nginx/internal/ingress/controller/store"
"k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/internal/nginx"
"k8s.io/klog/v2"
@ -126,7 +128,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
return nil
}
ings := n.store.ListIngresses(nil)
ings := n.store.ListIngresses()
hosts, servers, pcfg := n.getConfiguration(ings)
n.metricCollector.SetSSLExpireTime(servers)
@ -225,24 +227,31 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error {
}
}
k8s.SetDefaultNGINXPathType(ing)
allIngresses := n.store.ListIngresses()
filter := func(toCheck *ingress.Ingress) bool {
return toCheck.ObjectMeta.Namespace == ing.ObjectMeta.Namespace &&
toCheck.ObjectMeta.Name == ing.ObjectMeta.Name
}
k8s.SetDefaultNGINXPathType(ing)
ings := n.store.ListIngresses(filter)
ings := store.FilterIngresses(allIngresses, filter)
ings = append(ings, &ingress.Ingress{
Ingress: *ing,
ParsedAnnotations: annotations.NewAnnotationExtractor(n.store).Extract(ing),
})
_, _, pcfg := n.getConfiguration(ings)
cfg := n.store.GetBackendConfiguration()
cfg.Resolver = n.resolver
_, servers, pcfg := n.getConfiguration(ings)
err := checkOverlap(ing, allIngresses, servers)
if err != nil {
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
return err
}
content, err := n.generateTemplate(cfg, *pcfg)
if err != nil {
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
@ -252,11 +261,11 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error {
err = n.testTemplate(content)
if err != nil {
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
} else {
n.metricCollector.IncCheckCount(ing.ObjectMeta.Namespace, ing.Name)
return err
}
return err
n.metricCollector.IncCheckCount(ing.ObjectMeta.Namespace, ing.Name)
return nil
}
func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Protocol) []ingress.L4Service {
@ -1519,3 +1528,79 @@ func externalNamePorts(name string, svc *apiv1.Service) *apiv1.ServicePort {
TargetPort: intstr.FromInt(port),
}
}
func checkOverlap(ing *networking.Ingress, ingresses []*ingress.Ingress, servers []*ingress.Server) error {
for _, rule := range ing.Spec.Rules {
if rule.HTTP == nil {
continue
}
if rule.Host == "" {
rule.Host = defServerName
}
for _, path := range rule.HTTP.Paths {
if path.Path == "" {
path.Path = rootLocation
}
existingIngresses := ingressForHostPath(rule.Host, path.Path, servers)
// no previous ingress
if len(existingIngresses) == 0 {
continue
}
// same ingress
skipValidation := false
for _, existing := range existingIngresses {
if existing.ObjectMeta.Namespace == ing.ObjectMeta.Namespace && existing.ObjectMeta.Name == ing.ObjectMeta.Name {
return nil
}
}
if skipValidation {
continue
}
// path overlap. Check if one of the ingresses has a canary annotation
isCanaryEnabled, annotationErr := parser.GetBoolAnnotation("canary", ing)
for _, existing := range existingIngresses {
isExistingCanaryEnabled, existingAnnotationErr := parser.GetBoolAnnotation("canary", existing)
if isCanaryEnabled && isExistingCanaryEnabled {
return fmt.Errorf(`host "%s" and path "%s" is already defined in ingress %s/%s`, rule.Host, path.Path, existing.Namespace, existing.Name)
}
if annotationErr == errors.ErrMissingAnnotations && existingAnnotationErr == existingAnnotationErr {
return fmt.Errorf(`host "%s" and path "%s" is already defined in ingress %s/%s`, rule.Host, path.Path, existing.Namespace, existing.Name)
}
}
// no overlap
return nil
}
}
return nil
}
func ingressForHostPath(hostname, path string, servers []*ingress.Server) []*networking.Ingress {
ingresses := make([]*networking.Ingress, 0)
for _, server := range servers {
if hostname != server.Hostname {
continue
}
for _, location := range server.Locations {
if location.Path != path {
continue
}
ingresses = append(ingresses, &location.Ingress.Ingress)
}
}
return ingresses
}

View file

@ -78,10 +78,14 @@ func (fakeIngressStore) GetServiceEndpoints(key string) (*corev1.Endpoints, erro
return nil, fmt.Errorf("test error")
}
func (fis fakeIngressStore) ListIngresses(store.IngressFilterFunc) []*ingress.Ingress {
func (fis fakeIngressStore) ListIngresses() []*ingress.Ingress {
return fis.ingresses
}
func (fis fakeIngressStore) FilterIngresses(ingresses []*ingress.Ingress, filterFunc store.IngressFilterFunc) []*ingress.Ingress {
return ingresses
}
func (fakeIngressStore) GetRunningControllerPodsCount() int {
return 0
}

View file

@ -19,6 +19,7 @@ package store
import (
networking "k8s.io/api/networking/v1beta1"
"k8s.io/client-go/tools/cache"
"k8s.io/ingress-nginx/internal/ingress"
)
// IngressLister makes a Store that lists Ingress.
@ -37,3 +38,16 @@ func (il IngressLister) ByKey(key string) (*networking.Ingress, error) {
}
return i.(*networking.Ingress), nil
}
// FilterIngresses returns the list of Ingresses
func FilterIngresses(ingresses []*ingress.Ingress, filterFunc IngressFilterFunc) []*ingress.Ingress {
afterFilter := make([]*ingress.Ingress, 0)
for _, ingress := range ingresses {
if !filterFunc(ingress) {
afterFilter = append(afterFilter, ingress)
}
}
sortIngressSlice(afterFilter)
return afterFilter
}

View file

@ -79,7 +79,7 @@ type Storer interface {
GetServiceEndpoints(key string) (*corev1.Endpoints, error)
// ListIngresses returns a list of all Ingresses in the store.
ListIngresses(IngressFilterFunc) []*ingress.Ingress
ListIngresses() []*ingress.Ingress
// GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods.
GetRunningControllerPodsCount() int
@ -804,20 +804,7 @@ func (s *k8sStore) getIngress(key string) (*networkingv1beta1.Ingress, error) {
return &ing.Ingress, nil
}
// ListIngresses returns the list of Ingresses
func (s *k8sStore) ListIngresses(filter IngressFilterFunc) []*ingress.Ingress {
// filter ingress rules
ingresses := make([]*ingress.Ingress, 0)
for _, item := range s.listers.IngressWithAnnotation.List() {
ing := item.(*ingress.Ingress)
if filter != nil && filter(ing) {
continue
}
ingresses = append(ingresses, ing)
}
func sortIngressSlice(ingresses []*ingress.Ingress) {
// sort Ingresses using the CreationTimestamp field
sort.SliceStable(ingresses, func(i, j int) bool {
ir := ingresses[i].CreationTimestamp
@ -830,6 +817,18 @@ func (s *k8sStore) ListIngresses(filter IngressFilterFunc) []*ingress.Ingress {
}
return ir.Before(&jr)
})
}
// ListIngresses returns the list of Ingresses
func (s *k8sStore) ListIngresses() []*ingress.Ingress {
// filter ingress rules
ingresses := make([]*ingress.Ingress, 0)
for _, item := range s.listers.IngressWithAnnotation.List() {
ing := item.(*ingress.Ingress)
ingresses = append(ingresses, ing)
}
sortIngressSlice(ingresses)
return ingresses
}

View file

@ -952,7 +952,7 @@ func TestListIngresses(t *testing.T) {
}
s.listers.IngressWithAnnotation.Add(ingressWithNginxClass)
ingresses := s.ListIngresses(nil)
ingresses := s.ListIngresses()
if s := len(ingresses); s != 3 {
t.Errorf("Expected 3 Ingresses but got %v", s)

View file

@ -37,7 +37,6 @@ import (
"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/controller/store"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/internal/task"
)
@ -55,7 +54,7 @@ type Syncer interface {
type ingressLister interface {
// ListIngresses returns the list of Ingresses
ListIngresses(store.IngressFilterFunc) []*ingress.Ingress
ListIngresses() []*ingress.Ingress
}
// Config ...
@ -236,7 +235,7 @@ func sliceToStatus(endpoints []string) []apiv1.LoadBalancerIngress {
// updateStatus changes the status information of Ingress rules
func (s *statusSync) updateStatus(newIngressPoint []apiv1.LoadBalancerIngress) {
ings := s.IngressLister.ListIngresses(nil)
ings := s.IngressLister.ListIngresses()
p := pool.NewLimited(10)
defer p.Close()

View file

@ -30,7 +30,6 @@ import (
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/class"
"k8s.io/ingress-nginx/internal/ingress/controller/store"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/internal/task"
)
@ -234,7 +233,7 @@ func buildExtensionsIngresses() []networking.Ingress {
type testIngressLister struct {
}
func (til *testIngressLister) ListIngresses(store.IngressFilterFunc) []*ingress.Ingress {
func (til *testIngressLister) ListIngresses() []*ingress.Ingress {
var ingresses []*ingress.Ingress
ingresses = append(ingresses, &ingress.Ingress{
Ingress: networking.Ingress{

View file

@ -0,0 +1,34 @@
# TODO: remove the need to use fullnameOverride
fullnameOverride: nginx-ingress
controller:
image:
repository: ingress-controller/controller
tag: 1.0.0-dev
digest:
containerPort:
http: "1080"
https: "1443"
extraArgs:
http-port: "1080"
https-port: "1443"
# e2e tests do not require information about ingress status
update-status: "false"
scope:
enabled: true
config:
worker-processes: "1"
service:
type: NodePort
admissionWebhooks:
enabled: true
defaultBackend:
enabled: false
rbac:
create: true
scope: true

View file

@ -0,0 +1,108 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package admission
import (
"context"
"fmt"
"os/exec"
"strings"
"github.com/onsi/ginkgo"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/test/e2e/framework"
)
var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() {
f := framework.NewDefaultFramework("admission")
ginkgo.BeforeEach(func() {
f.NewEchoDeployment()
f.NewSlowEchoDeployment()
})
ginkgo.It("should not allow overlaps of host and paths without canary annotations", func() {
host := "admission-test"
firstIngress := framework.NewSingleIngress("first-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
_, err := f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
assert.Nil(ginkgo.GinkgoT(), err, "creating ingress")
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("server_name %v", host))
})
secondIngress := framework.NewSingleIngress("second-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
err = uninstallChart(f)
assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
})
ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
host := "admission-test"
firstIngress := framework.NewSingleIngress("first-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
_, err := f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
assert.Nil(ginkgo.GinkgoT(), err, "creating ingress")
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("server_name %v", host))
})
canaryAnnotations := map[string]string{
"nginx.ingress.kubernetes.io/canary": "true",
"nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader",
}
secondIngress := framework.NewSingleIngress("second-ingress", "/", host, f.Namespace, framework.SlowEchoService, 80, canaryAnnotations)
_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should not return an error using a canary annotation")
err = uninstallChart(f)
assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
})
ginkgo.It("should return an error if there is an error validating the ingress definition", func() {
host := "admission-test"
annotations := map[string]string{
"nginx.ingress.kubernetes.io/configuration-snippet": "something invalid",
}
firstIngress := framework.NewSingleIngress("first-ingress", "/", host, f.Namespace, framework.EchoService, 80, annotations)
_, err := f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
err = uninstallChart(f)
assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
})
})
func uninstallChart(f *framework.Framework) error {
cmd := exec.Command("helm", "uninstall", "--namespace", f.Namespace, "nginx-ingress")
_, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("unexpected error uninstalling ingress-nginx release: %v", err)
}
return nil
}

View file

@ -25,12 +25,12 @@ import (
"k8s.io/component-base/logs"
// required
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/ingress-nginx/test/e2e/framework"
// tests to run
_ "k8s.io/ingress-nginx/test/e2e/admission"
_ "k8s.io/ingress-nginx/test/e2e/annotations"
_ "k8s.io/ingress-nginx/test/e2e/dbg"
_ "k8s.io/ingress-nginx/test/e2e/defaultbackend"