From 19337f05fba83cce6728da9c774303b6888aa890 Mon Sep 17 00:00:00 2001 From: Andrew Louis Date: Tue, 10 Apr 2018 14:14:36 -0400 Subject: [PATCH 1/3] Introduce ConfigMap updating helpers into e2e/framework and retain default nginx-configuration state between tests Group sublogic --- test/e2e/framework/framework.go | 70 ++++++++++++++++++++++++++ test/e2e/settings/no_auth_locations.go | 15 ++++-- test/e2e/settings/proxy_protocol.go | 34 ++++++------- test/e2e/settings/server_tokens.go | 21 ++++++-- 4 files changed, 114 insertions(+), 26 deletions(-) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 2d8831d2f..b85c12b73 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -259,6 +259,76 @@ func (f *Framework) matchNginxConditions(name string, matcher func(cfg string) b } } +// GetNginxConfigMap gets ingress-nginx's nginx-configuration map +func (f *Framework) GetNginxConfigMap() (*v1.ConfigMap, error) { + if f.KubeClientSet == nil { + return nil, fmt.Errorf("KubeClientSet not initialized") + } + + config, err := f.KubeClientSet.CoreV1().ConfigMaps("ingress-nginx").Get("nginx-configuration", metav1.GetOptions{}) + if err != nil { + return nil, err + } + + time.Sleep(1 * time.Second) + return config, err +} + +// GetNginxConfigMapData gets ingress-nginx's nginx-configuration map's data +func (f *Framework) GetNginxConfigMapData() (map[string]string, error) { + config, err := f.GetNginxConfigMap() + + if err != nil { + return nil, err + } + + if config == nil { + return nil, fmt.Errorf("Unable to get nginx-configuration configMap") + } + + if config.Data == nil { + config.Data = map[string]string{} + } + + return config.Data, err +} + +// SetNginxConfigMapData sets ingress-nginx's nginx-configuration configMap data +func (f *Framework) SetNginxConfigMapData(cmData map[string]string) error { + // Needs to do a Get and Set, Update will not take just the Data field + // or a configMap that is not the very last revision + config, err := f.GetNginxConfigMap() + if err != nil { + return err + } + if config == nil { + return fmt.Errorf("Unable to get nginx-configuration configMap") + } + + config.Data = cmData + + _, err = f.KubeClientSet.CoreV1().ConfigMaps("ingress-nginx").Update(config) + if err != nil { + return err + } + + time.Sleep(1 * time.Second) + + return err +} + +// UpdateNginxConfigMapData updates single field in ingress-nginx's nginx-configuration map data +func (f *Framework) UpdateNginxConfigMapData(key string, value string) error { + config, err := f.GetNginxConfigMapData() + if err != nil { + return err + } + + config[key] = value + + return f.SetNginxConfigMapData(config) +} + // 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 *appsv1beta1.Deployment) error) error { deployment, err := kubeClientSet.AppsV1beta1().Deployments(namespace).Get(name, metav1.GetOptions{}) diff --git a/test/e2e/settings/no_auth_locations.go b/test/e2e/settings/no_auth_locations.go index b46863f57..e7a421424 100644 --- a/test/e2e/settings/no_auth_locations.go +++ b/test/e2e/settings/no_auth_locations.go @@ -41,6 +41,7 @@ var _ = framework.IngressNginxDescribe("No Auth locations", func() { secretName := "test-secret" host := "no-auth-locations" noAuthPath := "/noauth" + var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() @@ -51,16 +52,24 @@ var _ = framework.IngressNginxDescribe("No Auth locations", func() { Expect(s).NotTo(BeNil()) Expect(s.ObjectMeta).NotTo(BeNil()) - updateConfigmap(setting, noAuthPath, f.KubeClientSet) - bi := buildBasicAuthIngressWithSecondPath(host, f.Namespace.Name, s.Name, noAuthPath) ing, err := f.EnsureIngress(bi) Expect(err).NotTo(HaveOccurred()) Expect(ing).NotTo(BeNil()) + + if defaultNginxConfigMapData == nil { + defaultNginxConfigMapData, err = f.GetNginxConfigMapData() + Expect(err).NotTo(HaveOccurred()) + Expect(defaultNginxConfigMapData).NotTo(BeNil()) + } + + err = f.UpdateNginxConfigMapData(setting, noAuthPath) + Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { - updateConfigmap(setting, "/.well-known/acme-challenge", f.KubeClientSet) + err := f.SetNginxConfigMapData(defaultNginxConfigMapData) + Expect(err).NotTo(HaveOccurred()) }) It("should return status code 401 when accessing '/' unauthentication", func() { diff --git a/test/e2e/settings/proxy_protocol.go b/test/e2e/settings/proxy_protocol.go index 1cfb5c0cd..2b81afca6 100644 --- a/test/e2e/settings/proxy_protocol.go +++ b/test/e2e/settings/proxy_protocol.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "net" "strings" - "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -29,7 +28,6 @@ import ( "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -37,20 +35,32 @@ var _ = framework.IngressNginxDescribe("Proxy Protocol", func() { f := framework.NewDefaultFramework("proxy-protocol") setting := "use-proxy-protocol" + var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() Expect(err).NotTo(HaveOccurred()) + + if defaultNginxConfigMapData == nil { + defaultNginxConfigMapData, err = f.GetNginxConfigMapData() + Expect(err).NotTo(HaveOccurred()) + Expect(defaultNginxConfigMapData).NotTo(BeNil()) + } + + err = f.UpdateNginxConfigMapData(setting, "false") + Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { - updateConfigmap(setting, "false", f.KubeClientSet) + err := f.SetNginxConfigMapData(defaultNginxConfigMapData) + Expect(err).NotTo(HaveOccurred()) }) It("should respect port passed by the PROXY Protocol", func() { host := "proxy-protocol" - updateConfigmap(setting, "true", f.KubeClientSet) + err := f.UpdateNginxConfigMapData(setting, "true") + Expect(err).NotTo(HaveOccurred()) ing, err := f.EnsureIngress(&v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -111,19 +121,3 @@ var _ = framework.IngressNginxDescribe("Proxy Protocol", func() { Expect(body).Should(ContainSubstring(fmt.Sprintf("x-forwarded-for=192.168.0.1"))) }) }) - -func updateConfigmap(k, v string, c kubernetes.Interface) { - By(fmt.Sprintf("updating configuration configmap setting %v to '%v'", k, v)) - config, err := c.CoreV1().ConfigMaps("ingress-nginx").Get("nginx-configuration", metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - Expect(config).NotTo(BeNil()) - - if config.Data == nil { - config.Data = map[string]string{} - } - - config.Data[k] = v - _, err = c.CoreV1().ConfigMaps("ingress-nginx").Update(config) - Expect(err).NotTo(HaveOccurred()) - time.Sleep(1 * time.Second) -} diff --git a/test/e2e/settings/server_tokens.go b/test/e2e/settings/server_tokens.go index 40ca96590..f8c21c847 100644 --- a/test/e2e/settings/server_tokens.go +++ b/test/e2e/settings/server_tokens.go @@ -31,18 +31,32 @@ import ( var _ = framework.IngressNginxDescribe("Server Tokens", func() { f := framework.NewDefaultFramework("server-tokens") serverTokens := "server-tokens" + var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() Expect(err).NotTo(HaveOccurred()) + + if defaultNginxConfigMapData == nil { + defaultNginxConfigMapData, err = f.GetNginxConfigMapData() + Expect(err).NotTo(HaveOccurred()) + Expect(defaultNginxConfigMapData).NotTo(BeNil()) + } }) AfterEach(func() { - updateConfigmap(serverTokens, "false", f.KubeClientSet) + err := f.SetNginxConfigMapData(defaultNginxConfigMapData) + Expect(err).NotTo(HaveOccurred()) }) +<<<<<<< HEAD It("should not exist Server header in the response", func() { updateConfigmap(serverTokens, "false", f.KubeClientSet) +======= + It("should not exists Server header in the response", func() { + err := f.UpdateNginxConfigMapData(serverTokens, "false") + Expect(err).NotTo(HaveOccurred()) +>>>>>>> Introduce ConfigMap updating helpers into e2e/framework and retain default nginx-configuration state between tests ing, err := f.EnsureIngress(&v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -83,8 +97,9 @@ var _ = framework.IngressNginxDescribe("Server Tokens", func() { Expect(err).NotTo(HaveOccurred()) }) - It("should exist Server header in the response when is enabled", func() { - updateConfigmap(serverTokens, "true", f.KubeClientSet) + It("should exists Server header in the response when is enabled", func() { + err := f.UpdateNginxConfigMapData(serverTokens, "true") + Expect(err).NotTo(HaveOccurred()) ing, err := f.EnsureIngress(&v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ From 444914b76457499b45440b7e0c660ee693dc6914 Mon Sep 17 00:00:00 2001 From: Andrew Louis Date: Mon, 16 Apr 2018 15:30:35 -0400 Subject: [PATCH 2/3] Move the resetting logic into framework Stylistic fixes based on feedback --- test/e2e/framework/framework.go | 31 ++++++++++++++++---------- test/e2e/settings/no_auth_locations.go | 15 +++---------- test/e2e/settings/proxy_protocol.go | 9 -------- test/e2e/settings/server_tokens.go | 14 ------------ 4 files changed, 22 insertions(+), 47 deletions(-) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index b85c12b73..2af16d1df 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -61,6 +61,9 @@ type Framework struct { // should abort, the AfterSuite hook should run all Cleanup actions. cleanupHandle CleanupActionHandle + // Map to hold the default nginx-configuration configmap data before a test run + DefaultNginxConfigMapData map[string]string + NginxHTTPURL string NginxHTTPSURL string } @@ -69,7 +72,8 @@ type Framework struct { // you (you can write additional before/after each functions). func NewDefaultFramework(baseName string) *Framework { f := &Framework{ - BaseName: baseName, + BaseName: baseName, + DefaultNginxConfigMapData: nil, } BeforeEach(f.BeforeEach) @@ -101,6 +105,13 @@ func (f *Framework) BeforeEach() { By("Building NGINX HTTPS URL") f.NginxHTTPSURL, err = f.GetNginxURL(HTTPS) Expect(err).NotTo(HaveOccurred()) + + By("Persist nginx-configuration configMap Data") + if f.DefaultNginxConfigMapData == nil { + f.DefaultNginxConfigMapData, err = f.GetNginxConfigMapData() + Expect(err).NotTo(HaveOccurred()) + Expect(f.DefaultNginxConfigMapData).NotTo(BeNil()) + } } // AfterEach deletes the namespace, after reading its events. @@ -114,6 +125,10 @@ func (f *Framework) AfterEach() { By("Waiting for test namespace to no longer exist") err = WaitForNoPodsInNamespace(f.KubeClientSet, f.Namespace.Name) Expect(err).NotTo(HaveOccurred()) + + By("Reset nginx-configuration configMap to default state") + err = f.SetNginxConfigMapData(f.DefaultNginxConfigMapData) + Expect(err).NotTo(HaveOccurred()) } // IngressNginxDescribe wrapper function for ginkgo describe. Adds namespacing. @@ -259,8 +274,7 @@ func (f *Framework) matchNginxConditions(name string, matcher func(cfg string) b } } -// GetNginxConfigMap gets ingress-nginx's nginx-configuration map -func (f *Framework) GetNginxConfigMap() (*v1.ConfigMap, error) { +func (f *Framework) getNginxConfigMap() (*v1.ConfigMap, error) { if f.KubeClientSet == nil { return nil, fmt.Errorf("KubeClientSet not initialized") } @@ -270,22 +284,15 @@ func (f *Framework) GetNginxConfigMap() (*v1.ConfigMap, error) { return nil, err } - time.Sleep(1 * time.Second) return config, err } // GetNginxConfigMapData gets ingress-nginx's nginx-configuration map's data func (f *Framework) GetNginxConfigMapData() (map[string]string, error) { - config, err := f.GetNginxConfigMap() - + config, err := f.getNginxConfigMap() if err != nil { return nil, err } - - if config == nil { - return nil, fmt.Errorf("Unable to get nginx-configuration configMap") - } - if config.Data == nil { config.Data = map[string]string{} } @@ -297,7 +304,7 @@ func (f *Framework) GetNginxConfigMapData() (map[string]string, error) { func (f *Framework) SetNginxConfigMapData(cmData map[string]string) error { // Needs to do a Get and Set, Update will not take just the Data field // or a configMap that is not the very last revision - config, err := f.GetNginxConfigMap() + config, err := f.getNginxConfigMap() if err != nil { return err } diff --git a/test/e2e/settings/no_auth_locations.go b/test/e2e/settings/no_auth_locations.go index e7a421424..ae801a840 100644 --- a/test/e2e/settings/no_auth_locations.go +++ b/test/e2e/settings/no_auth_locations.go @@ -41,7 +41,6 @@ var _ = framework.IngressNginxDescribe("No Auth locations", func() { secretName := "test-secret" host := "no-auth-locations" noAuthPath := "/noauth" - var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() @@ -52,24 +51,16 @@ var _ = framework.IngressNginxDescribe("No Auth locations", func() { Expect(s).NotTo(BeNil()) Expect(s.ObjectMeta).NotTo(BeNil()) + err = f.UpdateNginxConfigMapData(setting, noAuthPath) + Expect(err).NotTo(HaveOccurred()) + bi := buildBasicAuthIngressWithSecondPath(host, f.Namespace.Name, s.Name, noAuthPath) ing, err := f.EnsureIngress(bi) Expect(err).NotTo(HaveOccurred()) Expect(ing).NotTo(BeNil()) - - if defaultNginxConfigMapData == nil { - defaultNginxConfigMapData, err = f.GetNginxConfigMapData() - Expect(err).NotTo(HaveOccurred()) - Expect(defaultNginxConfigMapData).NotTo(BeNil()) - } - - err = f.UpdateNginxConfigMapData(setting, noAuthPath) - Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { - err := f.SetNginxConfigMapData(defaultNginxConfigMapData) - Expect(err).NotTo(HaveOccurred()) }) It("should return status code 401 when accessing '/' unauthentication", func() { diff --git a/test/e2e/settings/proxy_protocol.go b/test/e2e/settings/proxy_protocol.go index 2b81afca6..94ce2b4b5 100644 --- a/test/e2e/settings/proxy_protocol.go +++ b/test/e2e/settings/proxy_protocol.go @@ -35,25 +35,16 @@ var _ = framework.IngressNginxDescribe("Proxy Protocol", func() { f := framework.NewDefaultFramework("proxy-protocol") setting := "use-proxy-protocol" - var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() Expect(err).NotTo(HaveOccurred()) - if defaultNginxConfigMapData == nil { - defaultNginxConfigMapData, err = f.GetNginxConfigMapData() - Expect(err).NotTo(HaveOccurred()) - Expect(defaultNginxConfigMapData).NotTo(BeNil()) - } - err = f.UpdateNginxConfigMapData(setting, "false") Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { - err := f.SetNginxConfigMapData(defaultNginxConfigMapData) - Expect(err).NotTo(HaveOccurred()) }) It("should respect port passed by the PROXY Protocol", func() { diff --git a/test/e2e/settings/server_tokens.go b/test/e2e/settings/server_tokens.go index f8c21c847..c5420ffff 100644 --- a/test/e2e/settings/server_tokens.go +++ b/test/e2e/settings/server_tokens.go @@ -31,32 +31,18 @@ import ( var _ = framework.IngressNginxDescribe("Server Tokens", func() { f := framework.NewDefaultFramework("server-tokens") serverTokens := "server-tokens" - var defaultNginxConfigMapData map[string]string = nil BeforeEach(func() { err := f.NewEchoDeployment() Expect(err).NotTo(HaveOccurred()) - - if defaultNginxConfigMapData == nil { - defaultNginxConfigMapData, err = f.GetNginxConfigMapData() - Expect(err).NotTo(HaveOccurred()) - Expect(defaultNginxConfigMapData).NotTo(BeNil()) - } }) AfterEach(func() { - err := f.SetNginxConfigMapData(defaultNginxConfigMapData) - Expect(err).NotTo(HaveOccurred()) }) -<<<<<<< HEAD - It("should not exist Server header in the response", func() { - updateConfigmap(serverTokens, "false", f.KubeClientSet) -======= It("should not exists Server header in the response", func() { err := f.UpdateNginxConfigMapData(serverTokens, "false") Expect(err).NotTo(HaveOccurred()) ->>>>>>> Introduce ConfigMap updating helpers into e2e/framework and retain default nginx-configuration state between tests ing, err := f.EnsureIngress(&v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ From 882a99c1ec59d26dfa3c0eb98b15f60221d58ae7 Mon Sep 17 00:00:00 2001 From: Andrew Louis Date: Wed, 18 Apr 2018 11:33:51 -0400 Subject: [PATCH 3/3] Fix leaky test --- test/e2e/lua/dynamic_configuration.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index 95aff13fb..15c964cfe 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -210,6 +210,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() { return err }) Expect(err).NotTo(HaveOccurred()) + time.Sleep(5 * time.Second) By("Making a first request") host := "foo.com"