Skip to content

Commit

Permalink
Improve path rule (#8623)
Browse files Browse the repository at this point in the history
* Improve path rule

* Add nginx configuration tests

* Revert framework changes

* Add test to patched directives

* Fix root conf test

* Add comment in new function
  • Loading branch information
rikatz committed May 26, 2022
1 parent 4dfb3f2 commit bd1eb04
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 19 deletions.
4 changes: 2 additions & 2 deletions internal/ingress/inspector/rules.go
Expand Up @@ -22,8 +22,8 @@ import (
)

var (
invalidAliasDirective = regexp.MustCompile(`\s*alias\s*.*;`)
invalidRootDirective = regexp.MustCompile(`\s*root\s*.*;`)
invalidAliasDirective = regexp.MustCompile(`(?s)\s*alias\s*.*;`)
invalidRootDirective = regexp.MustCompile(`(?s)\s*root\s*.*;`)
invalidEtcDir = regexp.MustCompile(`/etc/(passwd|shadow|group|nginx|ingress-controller)`)
invalidSecretsDir = regexp.MustCompile(`/var/run/secrets`)
invalidByLuaDirective = regexp.MustCompile(`.*_by_lua.*`)
Expand Down
5 changes: 5 additions & 0 deletions internal/ingress/inspector/rules_test.go
Expand Up @@ -35,6 +35,11 @@ func TestCheckRegex(t *testing.T) {
wantErr: true,
value: " alias blabla/lala ;",
},
{
name: "must refuse invalid alias with line break",
wantErr: true,
value: "alias #\n/lalala/1/;",
},
{
name: "must refuse invalid attempt to call /etc",
wantErr: true,
Expand Down
1 change: 1 addition & 0 deletions test/e2e/e2e.go
Expand Up @@ -40,6 +40,7 @@ import (
_ "k8s.io/ingress-nginx/test/e2e/leaks"
_ "k8s.io/ingress-nginx/test/e2e/loadbalance"
_ "k8s.io/ingress-nginx/test/e2e/lua"
_ "k8s.io/ingress-nginx/test/e2e/nginx"
_ "k8s.io/ingress-nginx/test/e2e/security"
_ "k8s.io/ingress-nginx/test/e2e/servicebackend"
_ "k8s.io/ingress-nginx/test/e2e/settings"
Expand Down
16 changes: 12 additions & 4 deletions test/e2e/framework/deployment.go
Expand Up @@ -150,8 +150,9 @@ http {
f.NGINXWithConfigDeployment(SlowEchoService, cfg)
}

// NGINXWithConfigDeployment creates an NGINX deployment using a configmap containing the nginx.conf configuration
func (f *Framework) NGINXWithConfigDeployment(name string, cfg string) {
// NGINXDeployment creates a new simple NGINX Deployment using NGINX base image
// and passing the desired configuration
func (f *Framework) NGINXDeployment(name string, cfg string, waitendpoint bool) {
cfgMap := map[string]string{
"nginx.conf": cfg,
}
Expand Down Expand Up @@ -213,8 +214,15 @@ func (f *Framework) NGINXWithConfigDeployment(name string, cfg string) {

f.EnsureService(service)

err = WaitForEndpoints(f.KubeClientSet, DefaultTimeout, name, f.Namespace, 1)
assert.Nil(ginkgo.GinkgoT(), err, "waiting for endpoints to become ready")
if waitendpoint {
err = WaitForEndpoints(f.KubeClientSet, DefaultTimeout, name, f.Namespace, 1)
assert.Nil(ginkgo.GinkgoT(), err, "waiting for endpoints to become ready")
}
}

// NGINXWithConfigDeployment creates an NGINX deployment using a configmap containing the nginx.conf configuration
func (f *Framework) NGINXWithConfigDeployment(name string, cfg string) {
f.NGINXDeployment(name, cfg, true)
}

// NewGRPCBinDeployment creates a new deployment of the
Expand Down
71 changes: 58 additions & 13 deletions test/e2e/framework/framework.go
Expand Up @@ -88,8 +88,22 @@ func NewDefaultFramework(baseName string) *Framework {
return f
}

// BeforeEach gets a client and makes a namespace.
func (f *Framework) BeforeEach() {
// NewSimpleFramework makes a new framework that allows the usage of a namespace
// for arbitraty tests.
func NewSimpleFramework(baseName string) *Framework {
defer ginkgo.GinkgoRecover()

f := &Framework{
BaseName: baseName,
}

ginkgo.BeforeEach(f.CreateEnvironment)
ginkgo.AfterEach(f.DestroyEnvironment)

return f
}

func (f *Framework) CreateEnvironment() {
var err error

if f.KubeClientSet == nil {
Expand All @@ -107,6 +121,21 @@ func (f *Framework) BeforeEach() {

f.Namespace, err = CreateKubeNamespace(f.BaseName, f.KubeClientSet)
assert.Nil(ginkgo.GinkgoT(), err, "creating namespace")
}

func (f *Framework) DestroyEnvironment() {
go func() {
defer ginkgo.GinkgoRecover()
err := DeleteKubeNamespace(f.KubeClientSet, f.Namespace)
assert.Nil(ginkgo.GinkgoT(), err, "deleting namespace %v", f.Namespace)
}()
}

// BeforeEach gets a client and makes a namespace.
func (f *Framework) BeforeEach() {
var err error

f.CreateEnvironment()

f.IngressClass, err = CreateIngressClass(f.Namespace, f.KubeClientSet)
assert.Nil(ginkgo.GinkgoT(), err, "creating IngressClass")
Expand All @@ -122,13 +151,7 @@ func (f *Framework) BeforeEach() {

// AfterEach deletes the namespace, after reading its events.
func (f *Framework) AfterEach() {
defer func(kubeClient kubernetes.Interface, ns string) {
go func() {
defer ginkgo.GinkgoRecover()
err := DeleteKubeNamespace(kubeClient, ns)
assert.Nil(ginkgo.GinkgoT(), err, "deleting namespace %v", f.Namespace)
}()
}(f.KubeClientSet, f.Namespace)
defer f.DestroyEnvironment()

defer func(kubeClient kubernetes.Interface, ingressclass string) {
go func() {
Expand Down Expand Up @@ -425,26 +448,35 @@ func (f *Framework) DeleteNGINXPod(grace int64) {
assert.Nil(ginkgo.GinkgoT(), err, "while waiting for ingress nginx pod to come up again")
}

// HTTPDumbTestClient returns a new httpexpect client without BaseURL.
func (f *Framework) HTTPDumbTestClient() *httpexpect.Expect {
return f.newTestClient(nil, false)
}

// HTTPTestClient returns a new httpexpect client for end-to-end HTTP testing.
func (f *Framework) HTTPTestClient() *httpexpect.Expect {
return f.newTestClient(nil)
return f.newTestClient(nil, true)
}

// HTTPTestClientWithTLSConfig returns a new httpexpect client for end-to-end
// HTTP testing with a custom TLS configuration.
func (f *Framework) HTTPTestClientWithTLSConfig(config *tls.Config) *httpexpect.Expect {
return f.newTestClient(config)
return f.newTestClient(config, true)
}

func (f *Framework) newTestClient(config *tls.Config) *httpexpect.Expect {
func (f *Framework) newTestClient(config *tls.Config, setIngressURL bool) *httpexpect.Expect {
if config == nil {
config = &tls.Config{
InsecureSkipVerify: true,
}
}
var baseURL string
if setIngressURL {
baseURL = f.GetURL(HTTP)
}

return httpexpect.WithConfig(httpexpect.Config{
BaseURL: f.GetURL(HTTP),
BaseURL: baseURL,
Client: &http.Client{
Transport: &http.Transport{
TLSClientConfig: config,
Expand Down Expand Up @@ -485,6 +517,19 @@ func (f *Framework) WaitForNginxListening(port int) {
assert.Nil(ginkgo.GinkgoT(), err, "waiting for ingress controller pod listening on port 80")
}

// WaitForPod waits for a specific Pod to be ready, using a label selector
func (f *Framework) WaitForPod(selector string, timeout time.Duration, shouldFail bool) {
err := waitForPodsReady(f.KubeClientSet, timeout, 1, f.Namespace, metav1.ListOptions{
LabelSelector: selector,
})

if shouldFail {
assert.NotNil(ginkgo.GinkgoT(), err, "waiting for pods to be ready")
} else {
assert.Nil(ginkgo.GinkgoT(), err, "waiting for pods to be ready")
}
}

// UpdateDeployment runs the given updateFunc on the deployment and waits for it to be updated
func UpdateDeployment(kubeClientSet kubernetes.Interface, namespace string, name string, replicas int, updateFunc func(d *appsv1.Deployment) error) error {
deployment, err := kubeClientSet.AppsV1().Deployments(namespace).Get(context.TODO(), name, metav1.GetOptions{})
Expand Down
132 changes: 132 additions & 0 deletions test/e2e/nginx/nginx.go
@@ -0,0 +1,132 @@
/*
Copyright 2022 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 nginx

import (
"fmt"
"net/http"
"time"

"github.com/onsi/ginkgo"

"k8s.io/ingress-nginx/test/e2e/framework"
)

var (
cfgOK = `#
events {
worker_connections 1024;
multi_accept on;
}
http {
default_type 'text/plain';
client_max_body_size 0;
server {
access_log on;
access_log /dev/stdout;
listen 80;
location / {
content_by_lua_block {
ngx.print("ok")
}
}
}
}
`

cfgAlias = `#
events {
worker_connections 1024;
multi_accept on;
}
http {
default_type 'text/plain';
client_max_body_size 0;
server {
access_log on;
access_log /dev/stdout;
listen 80;
location / {
alias /www/html;
}
}
}
`

cfgRoot = `#
events {
worker_connections 1024;
multi_accept on;
}
http {
default_type 'text/plain';
client_max_body_size 0;
root /srv/www;
server {
access_log on;
access_log /dev/stdout;
listen 80;
}
}
`
)

var _ = framework.DescribeSetting("nginx-configuration", func() {
f := framework.NewSimpleFramework("nginxconfiguration")

ginkgo.It("start nginx with default configuration", func() {

f.NGINXWithConfigDeployment("default-nginx", cfgOK)
f.WaitForPod("app=default-nginx", 60*time.Second, false)
framework.Sleep(5 * time.Second)

f.HTTPDumbTestClient().
GET("/").
WithURL(fmt.Sprintf("http://default-nginx.%s", f.Namespace)).
Expect().
Status(http.StatusOK).Body().Contains("ok")
})

ginkgo.It("fails when using alias directive", func() {

f.NGINXDeployment("alias-nginx", cfgAlias, false)
// This should fail with a crashloopback because our NGINX does not have
// alias directive!
f.WaitForPod("app=alias-nginx", 60*time.Second, true)

})

ginkgo.It("fails when using root directive", func() {

f.NGINXDeployment("root-nginx", cfgRoot, false)
// This should fail with a crashloopback because our NGINX does not have
// root directive!
f.WaitForPod("app=root-nginx", 60*time.Second, true)

})
})

0 comments on commit bd1eb04

Please sign in to comment.