From bd5d90539a329db205ae06223a435bcacaa6918b Mon Sep 17 00:00:00 2001
From: Manuel de Brito Fontes <aledbf@gmail.com>
Date: Thu, 15 Dec 2016 11:18:06 -0300
Subject: [PATCH] Sort whitelist list to avoid random orders

---
 .../ingress/annotations/ipwhitelist/main.go   |  7 ++-
 .../annotations/ipwhitelist/main_test.go      | 43 +++++++++++++------
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/core/pkg/ingress/annotations/ipwhitelist/main.go b/core/pkg/ingress/annotations/ipwhitelist/main.go
index 14f3ce00ab..06a22f84da 100644
--- a/core/pkg/ingress/annotations/ipwhitelist/main.go
+++ b/core/pkg/ingress/annotations/ipwhitelist/main.go
@@ -18,6 +18,7 @@ package ipwhitelist
 
 import (
 	"errors"
+	"sort"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/apis/extensions"
@@ -47,8 +48,7 @@ type SourceRange struct {
 // Multiple ranges can specified using commas as separator
 // e.g. `18.0.0.0/8,56.0.0.0/8`
 func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) (*SourceRange, error) {
-	cidrs := []string{}
-
+	sort.Strings(cfg.WhitelistSourceRange)
 	if ing.GetAnnotations() == nil {
 		return &SourceRange{CIDR: cfg.WhitelistSourceRange}, parser.ErrMissingAnnotations
 	}
@@ -64,9 +64,12 @@ func ParseAnnotations(cfg defaults.Backend, ing *extensions.Ingress) (*SourceRan
 		return &SourceRange{CIDR: cfg.WhitelistSourceRange}, ErrInvalidCIDR
 	}
 
+	cidrs := []string{}
 	for k := range ipnets {
 		cidrs = append(cidrs, k)
 	}
 
+	sort.Strings(cidrs)
+
 	return &SourceRange{cidrs}, nil
 }
diff --git a/core/pkg/ingress/annotations/ipwhitelist/main_test.go b/core/pkg/ingress/annotations/ipwhitelist/main_test.go
index db623dd68f..257547ddfb 100644
--- a/core/pkg/ingress/annotations/ipwhitelist/main_test.go
+++ b/core/pkg/ingress/annotations/ipwhitelist/main_test.go
@@ -63,6 +63,7 @@ func buildIngress() *extensions.Ingress {
 }
 
 func TestParseAnnotations(t *testing.T) {
+	// TODO: convert test cases to tables
 	ing := buildIngress()
 
 	testNet := "10.0.0.0/24"
@@ -86,24 +87,42 @@ func TestParseAnnotations(t *testing.T) {
 	}
 
 	data[whitelist] = "www"
-	ing.SetAnnotations(data)
 	_, err = ParseAnnotations(defaults.Backend{}, ing)
 	if err == nil {
 		t.Errorf("Expected error parsing an invalid cidr")
 	}
 
-	delete(data, "whitelist")
+	delete(data, whitelist)
 	ing.SetAnnotations(data)
-	//sr, _ = ParseAnnotations(defaults.Backend{}, ing)
-	// TODO: fix test
-	/*
-		if !reflect.DeepEqual(sr.CIDR, []string{}) {
-			t.Errorf("Expected empty CIDR but %v returned", sr.CIDR)
-		}
+	sr, err = ParseAnnotations(defaults.Backend{}, ing)
+	if err == nil {
+		t.Errorf("Expected error parsing an invalid cidr")
+	}
+	if !strsEquals(sr.CIDR, []string{}) {
+		t.Errorf("Expected empty CIDR but %v returned", sr.CIDR)
+	}
 
-		sr, _ = ParseAnnotations(defaults.Upstream{}, &extensions.Ingress{})
-		if !reflect.DeepEqual(sr.CIDR, []string{}) {
-			t.Errorf("Expected empty CIDR but %v returned", sr.CIDR)
+	sr, _ = ParseAnnotations(defaults.Backend{}, &extensions.Ingress{})
+	if !strsEquals(sr.CIDR, []string{}) {
+		t.Errorf("Expected empty CIDR but %v returned", sr.CIDR)
+	}
+
+	data[whitelist] = "2.2.2.2/32,1.1.1.1/32,3.3.3.0/24"
+	sr, _ = ParseAnnotations(defaults.Backend{}, ing)
+	ecidr := []string{"1.1.1.1/32", "2.2.2.2/32", "3.3.3.0/24"}
+	if !strsEquals(sr.CIDR, ecidr) {
+		t.Errorf("Expected %v CIDR but %v returned", ecidr, sr.CIDR)
+	}
+}
+
+func strsEquals(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i, v := range a {
+		if v != b[i] {
+			return false
 		}
-	*/
+	}
+	return true
 }