Skip to content

Commit

Permalink
Fix MergeableIngress unit test. Fix typos
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dean-Coakley
Dean-Coakley committed Aug 24, 2018
1 parent dd0ab26 commit fe6bf81
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 42 deletions.
21 changes: 8 additions & 13 deletions nginx-controller/nginx/configurator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ func (cnf *Configurator) AddOrUpdateDHParam(content string) (string, error) {

// AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource
func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error {
err := cnf.addOrUpdateIngress(ingEx)
if err != nil {
if err := cnf.addOrUpdateIngress(ingEx); err != nil {
return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}
if err = cnf.nginx.Reload(); err != nil {
if err := cnf.nginx.Reload(); err != nil {
return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}
return nil
Expand All @@ -83,17 +82,16 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error {

// AddOrUpdateMergableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types
func (cnf *Configurator) AddOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error {
err := cnf.addOrUpdateMergableIngress(mergeableIngs)
if err != nil {
if err := cnf.addOrUpdateMergeableIngress(mergeableIngs); err != nil {
return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}
if err = cnf.nginx.Reload(); err != nil {
if err := cnf.nginx.Reload(); err != nil {
return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}
return nil
}

func (cnf *Configurator) addOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error {
func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) error {
nginxCfg := cnf.generateNginxCfgForMergeableIngresses(mergeableIngs)
name := objectMetaToFileName(&mergeableIngs.Master.Ingress.ObjectMeta)
content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg)
Expand Down Expand Up @@ -881,10 +879,7 @@ func upstreamMapToSlice(upstreams map[string]Upstream) []Upstream {
func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) error {
cnf.addOrUpdateSecret(secret)

kind, err := GetSecretKind(secret)
if err != nil {
return fmt.Errorf("Error secret is unknown")
}
kind, _ := GetSecretKind(secret)
if cnf.isPlus() && kind == JWK {
return nil
}
Expand Down Expand Up @@ -990,7 +985,7 @@ func (cnf *Configurator) UpdateEndpoints(ingEx *IngressEx) error {

// UpdateEndpointsMergeableIngress updates endpoints in NGINX configuration for a mergeable Ingress resource
func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngs *MergeableIngresses) error {
err := cnf.addOrUpdateMergableIngress(mergeableIngs)
err := cnf.addOrUpdateMergeableIngress(mergeableIngs)
if err != nil {
return fmt.Errorf("Error adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}
Expand Down Expand Up @@ -1154,7 +1149,7 @@ func (cnf *Configurator) UpdateConfig(config *Config, ingExes []*IngressEx, merg
}
}
for _, mergeableIng := range mergeableIngs {
if err := cnf.addOrUpdateMergableIngress(mergeableIng); err != nil {
if err := cnf.addOrUpdateMergeableIngress(mergeableIng); err != nil {
return err
}
}
Expand Down
111 changes: 82 additions & 29 deletions nginx-controller/nginx/configurator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,16 +482,39 @@ func createExpectedConfigForMergeableCafeIngress() IngressNginxConfig {

}

func createTestConfigurator() *Configurator {
templateExecutor, _ := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080)
func createTestConfigurator() (*Configurator, error) {
templateExecutor, err := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080)
if err != nil {
return nil, err
}
ngxc := NewNginxController("/etc/nginx", true)
apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true)
if err != nil {
return nil, err
}
return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil
}

func createTestConfiguratorInvalidIngressTemplate() (*Configurator, error) {
templateExecutor, err := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080)
if err != nil {
return nil, err
}
invalidIngressTemplate := "{{.Upstreams.This.Field.Does.Not.Exist}}"
if err := templateExecutor.UpdateIngressTemplate(&invalidIngressTemplate); err != nil {
return nil, err
}
ngxc := NewNginxController("/etc/nginx", true)
apiCtrl, _ := plus.NewNginxAPIController(&http.Client{}, "", true)
return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor)
return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil
}

func TestGenerateNginxCfg(t *testing.T) {
cafeIngressEx := createCafeIngressEx()
cnf := createTestConfigurator()
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}
expected := createExpectedConfigForCafeIngressEx()

pems := map[string]string{
Expand All @@ -514,7 +537,10 @@ func TestGenerateNginxCfgForJWT(t *testing.T) {
cafeIngressEx.Ingress.Annotations["nginx.com/jwt-login-url"] = "https://login.example.com"
cafeIngressEx.JWTKey = &api_v1.Secret{}

cnf := createTestConfigurator()
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}
expected := createExpectedConfigForCafeIngressEx()
expected.Servers[0].JWTAuth = &JWTAuth{
Key: "/etc/nginx/secrets/default-cafe-jwk",
Expand Down Expand Up @@ -547,7 +573,10 @@ func TestGenerateNginxCfgForMergeableIngresses(t *testing.T) {
mergeableIngresses := createMergeableCafeIngress()
expected := createExpectedConfigForMergeableCafeIngress()

cnf := createTestConfigurator()
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}

result := cnf.generateNginxCfgForMergeableIngresses(mergeableIngresses)

Expand Down Expand Up @@ -604,7 +633,10 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) {
},
}

cnf := createTestConfigurator()
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}

result := cnf.generateNginxCfgForMergeableIngresses(mergeableIngresses)

Expand All @@ -620,40 +652,61 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) {
}

func TestAddOrUpdateIngress(t *testing.T) {
cnf := createTestConfigurator()
validIngress := createCafeIngressEx()
err := cnf.AddOrUpdateIngress(&validIngress)
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}
ingress := createCafeIngressEx()
err = cnf.AddOrUpdateIngress(&ingress)
if err != nil {
t.Errorf("AddOrUpdateIngress returned: \n%v, but expected: \n%v", err, nil)
}

cnfHasIngress := cnf.HasIngress(ingress.Ingress)
if !cnfHasIngress {
t.Errorf("AddOrUpdateIngress didn't add ingress successfully. HasIngress returned %v, expected %v", cnfHasIngress, true)
}
}

func TestAddOrUpdateMergableIngress(t *testing.T) {
cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}
mergeableIngess := createMergeableCafeIngress()
err = cnf.AddOrUpdateMergableIngress(mergeableIngess)
if err != nil {
t.Errorf("Adding ingress failed: %v", err)
t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", err, nil)
}

expectedIng := cnf.ingresses["default-cafe-ingress"]
if !reflect.DeepEqual(&validIngress, expectedIng) {
t.Errorf("TestAddOrUpdateIngress returned: \n%v, but expected: \n%v", validIngress, expectedIng)
cnfHasMergableIngress := cnf.HasIngress(mergeableIngess.Master.Ingress)
if !cnfHasMergableIngress {
t.Errorf("AddOrUpdateMergableIngress didn't add mergable ingress successfully. HasIngress returned %v, expected %v", cnfHasMergableIngress, true)
}
}

func TestAddOrUpdateMIngress(t *testing.T) {
cnf := createTestConfigurator()
validMergeableIngress := createMergeableCafeIngress()
err := cnf.AddOrUpdateMergableIngress(validMergeableIngress)
func TestAddOrUpdateIngressFailsWithInvalidIngressTemplate(t *testing.T) {
cnf, err := createTestConfiguratorInvalidIngressTemplate()
if err != nil {
t.Errorf("Adding mergable ingress failed: %v", err)
t.Errorf("Failed to create a test configurator: %v", err)
}

expectedIng := cnf.ingresses["default-cafe-ingress-master"]
if !reflect.DeepEqual(validMergeableIngress.Master, expectedIng) {
t.Errorf("TestAddOrUpdateMergableIngress returned: \n%v, but expected: \n%v", validMergeableIngress.Master, expectedIng)
ingress := createCafeIngressEx()
err = cnf.AddOrUpdateIngress(&ingress)
if err == nil {
t.Errorf("AddOrUpdateIngressFailsWithInvalidTemplate returned \n%v, but expected \n%v", nil, "template execution error")
}
}

func TestAddOrUpdateIngressFailsWithInvalidTemplate(t *testing.T) {
t.Skip() // Skip test until TODO is completed
cnf := createTestConfigurator()
validIngress := createCafeIngressEx()
// TODO: Alter template to be invalid
err := cnf.AddOrUpdateIngress(&validIngress)
func TestAddOrUpdateMergableIngressFailsWithInvalidIngressTemplate(t *testing.T) {
cnf, err := createTestConfiguratorInvalidIngressTemplate()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}

mergeableIngess := createMergeableCafeIngress()
err = cnf.AddOrUpdateMergableIngress(mergeableIngess)
if err == nil {
t.Errorf("TestAddOrUpdateIngressFailsWithInvalidTemplate returned: \n%v, but expected \n template execution error", err)
t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", nil, "template execution error")
}
}

0 comments on commit fe6bf81

Please sign in to comment.