diff --git a/pkg/teams/postgres_team.go b/pkg/teams/postgres_team.go index 0b577a1af..abb82ce31 100644 --- a/pkg/teams/postgres_team.go +++ b/pkg/teams/postgres_team.go @@ -2,6 +2,7 @@ package teams import ( acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + "github.com/zalando/postgres-operator/pkg/util" ) // PostgresTeamMap is the operator's internal representation of all PostgresTeam CRDs @@ -55,7 +56,7 @@ func fetchTeams(teamset *map[string]struct{}, set teamHashSet) { } } -func (ptm *PostgresTeamMap) fetchAdditionalTeams(team string, superuserTeams bool, transitive bool, exclude *[]string) []string { +func (ptm *PostgresTeamMap) fetchAdditionalTeams(team string, superuserTeams bool, transitive bool, exclude []string) []string { var teams []string @@ -65,19 +66,12 @@ func (ptm *PostgresTeamMap) fetchAdditionalTeams(team string, superuserTeams boo teams = (*ptm)[team].AdditionalTeams } if transitive { - *exclude = append(*exclude, team) + exclude = append(exclude, team) for _, additionalTeam := range teams { - getTransitiveTeams := true - for _, excludedTeam := range *exclude { - if additionalTeam == excludedTeam { - getTransitiveTeams = false - } - } - if getTransitiveTeams { + if !(util.SliceContains(exclude, additionalTeam)) { transitiveTeams := (*ptm).fetchAdditionalTeams(additionalTeam, superuserTeams, transitive, exclude) - - if len(transitiveTeams) > 0 { - for _, transitiveTeam := range transitiveTeams { + for _, transitiveTeam := range transitiveTeams { + if !(util.SliceContains(exclude, transitiveTeam)) && !(util.SliceContains(teams, transitiveTeam)) { teams = append(teams, transitiveTeam) } } @@ -90,12 +84,12 @@ func (ptm *PostgresTeamMap) fetchAdditionalTeams(team string, superuserTeams boo // GetAdditionalTeams function to retrieve list of additional teams func (ptm *PostgresTeamMap) GetAdditionalTeams(team string, transitive bool) []string { - return ptm.fetchAdditionalTeams(team, false, transitive, &[]string{}) + return ptm.fetchAdditionalTeams(team, false, transitive, []string{}) } // GetAdditionalTeams function to retrieve list of additional teams func (ptm *PostgresTeamMap) GetAdditionalSuperuserTeams(team string, transitive bool) []string { - return ptm.fetchAdditionalTeams(team, true, transitive, &[]string{}) + return ptm.fetchAdditionalTeams(team, true, transitive, []string{}) } // Load function to import data from PostgresTeam CRD diff --git a/pkg/teams/postgres_team_test.go b/pkg/teams/postgres_team_test.go index 5516affb0..38cd36976 100644 --- a/pkg/teams/postgres_team_test.go +++ b/pkg/teams/postgres_team_test.go @@ -1,16 +1,51 @@ package teams import ( - "reflect" "testing" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + "github.com/zalando/postgres-operator/pkg/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var ( - True = true - False = false + True = true + False = false + pgTeamList = acidv1.PostgresTeamList{ + TypeMeta: metav1.TypeMeta{ + Kind: "List", + APIVersion: "v1", + }, + Items: []acidv1.PostgresTeam{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "PostgresTeam", + APIVersion: "acid.zalan.do/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "teamAB", + }, + Spec: acidv1.PostgresTeamSpec{ + AdditionalSuperuserTeams: map[string][]string{"teamA": []string{"teamB", "team24/7"}, "teamB": []string{"teamA", "team24/7"}}, + AdditionalTeams: map[string][]string{"teamA": []string{"teamC"}, "teamB": []string{}}, + AdditionalMembers: map[string][]string{"team24/7": []string{"optimusprime"}, "teamB": []string{"drno"}}, + }, + }, { + TypeMeta: metav1.TypeMeta{ + Kind: "PostgresTeam", + APIVersion: "acid.zalan.do/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "teamC", + }, + Spec: acidv1.PostgresTeamSpec{ + AdditionalSuperuserTeams: map[string][]string{"teamC": []string{"team24/7"}}, + AdditionalTeams: map[string][]string{"teamA": []string{"teamC"}, "teamC": []string{"teamA", "teamB", "acid"}}, + AdditionalMembers: map[string][]string{"acid": []string{"batman"}}, + }, + }, + }, + } ) // PostgresTeamMap is the operator's internal representation of all PostgresTeam CRDs @@ -18,71 +53,37 @@ func TestLoadingPostgresTeamCRD(t *testing.T) { tests := []struct { name string crd acidv1.PostgresTeamList - pgt PostgresTeamMap + ptm PostgresTeamMap error string }{ { "Check that CRD is imported correctly into the internal format", - acidv1.PostgresTeamList{ - TypeMeta: metav1.TypeMeta{ - Kind: "List", - APIVersion: "v1", - }, - Items: []acidv1.PostgresTeam{ - { - TypeMeta: metav1.TypeMeta{ - Kind: "PostgresTeam", - APIVersion: "acid.zalan.do/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "teamAB", - }, - Spec: acidv1.PostgresTeamSpec{ - AdditionalSuperuserTeams: map[string][]string{"teamA": []string{"teamB", "team24/7"}, "teamB": []string{"teamA", "team24/7"}}, - AdditionalTeams: map[string][]string{"teamA": []string{"teamC"}, "teamB": []string{}}, - AdditionalMembers: map[string][]string{"team24/7": []string{"optimusprime"}, "teamB": []string{"drno"}}, - }, - }, { - TypeMeta: metav1.TypeMeta{ - Kind: "PostgresTeam", - APIVersion: "acid.zalan.do/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "teamC", - }, - Spec: acidv1.PostgresTeamSpec{ - AdditionalSuperuserTeams: map[string][]string{"teamC": []string{"team24/7"}}, - AdditionalTeams: map[string][]string{"teamA": []string{"teamC"}, "teamC": []string{"teamA", "teamB"}}, - AdditionalMembers: map[string][]string{"acid": []string{"batman"}}, - }, - }, - }, - }, + pgTeamList, PostgresTeamMap{ "teamA": { AdditionalSuperuserTeams: []string{"teamB", "team24/7"}, - AdditionalTeams: []string{"teamC"}, - AdditionalMembers: nil, + AdditionalTeams: []string{"teamC"}, + AdditionalMembers: nil, }, "teamB": { AdditionalSuperuserTeams: []string{"teamA", "team24/7"}, - AdditionalTeams: []string{}, - AdditionalMembers: []string{"drno"}, + AdditionalTeams: []string{}, + AdditionalMembers: []string{"drno"}, }, "teamC": { AdditionalSuperuserTeams: []string{"team24/7"}, - AdditionalTeams: []string{"teamA", "teamB"}, - AdditionalMembers: nil, + AdditionalTeams: []string{"teamA", "teamB", "acid"}, + AdditionalMembers: nil, }, "team24/7": { AdditionalSuperuserTeams: nil, - AdditionalTeams: nil, - AdditionalMembers: []string{"optimusprime"}, + AdditionalTeams: nil, + AdditionalMembers: []string{"optimusprime"}, }, "acid": { AdditionalSuperuserTeams: nil, - AdditionalTeams: nil, - AdditionalMembers: []string{"batman"}, + AdditionalTeams: nil, + AdditionalMembers: []string{"batman"}, }, }, "Mismatch between PostgresTeam CRD and internal map", @@ -92,9 +93,88 @@ func TestLoadingPostgresTeamCRD(t *testing.T) { for _, tt := range tests { postgresTeamMap := PostgresTeamMap{} postgresTeamMap.Load(&tt.crd) - // TODO order in slice is not deterministic so choose other compare method - if !reflect.DeepEqual(postgresTeamMap, tt.pgt) { - t.Errorf("%s: %v: expected %#v, got %#v", tt.name, tt.error, tt.pgt, postgresTeamMap) + for team, ptmeamMembership := range postgresTeamMap { + if !util.IsEqualIgnoreOrder(ptmeamMembership.AdditionalSuperuserTeams, tt.ptm[team].AdditionalSuperuserTeams) { + t.Errorf("%s: %v: expected additional members %#v, got %#v", tt.name, tt.error, tt.ptm, postgresTeamMap) + } + if !util.IsEqualIgnoreOrder(ptmeamMembership.AdditionalTeams, tt.ptm[team].AdditionalTeams) { + t.Errorf("%s: %v: expected additional teams %#v, got %#v", tt.name, tt.error, tt.ptm, postgresTeamMap) + } + if !util.IsEqualIgnoreOrder(ptmeamMembership.AdditionalMembers, tt.ptm[team].AdditionalMembers) { + t.Errorf("%s: %v: expected additional superuser teams %#v, got %#v", tt.name, tt.error, tt.ptm, postgresTeamMap) + } + } + } +} + +// TestGetAdditionalTeams if returns teams with and without transitive dependencies +func TestGetAdditionalTeams(t *testing.T) { + tests := []struct { + name string + team string + transitive bool + teams []string + error string + }{ + { + "Check that additional teams are returned", + "teamA", + false, + []string{"teamC"}, + "GetAdditionalTeams returns wrong list", + }, + { + "Check that additional teams are returned incl. transitive teams", + "teamA", + true, + []string{"teamC", "teamB", "acid"}, + "GetAdditionalTeams returns wrong list", + }, + } + + postgresTeamMap := PostgresTeamMap{} + postgresTeamMap.Load(&pgTeamList) + + for _, tt := range tests { + additionalTeams := postgresTeamMap.GetAdditionalTeams(tt.team, tt.transitive) + if !util.IsEqualIgnoreOrder(additionalTeams, tt.teams) { + t.Errorf("%s: %v: expected additional teams %#v, got %#v", tt.name, tt.error, tt.teams, additionalTeams) + } + } +} + +// TestGetAdditionalSuperuserTeams if returns teams with and without transitive dependencies +func TestGetAdditionalSuperuserTeams(t *testing.T) { + tests := []struct { + name string + team string + transitive bool + teams []string + error string + }{ + { + "Check that additional teams are returned", + "teamA", + false, + []string{"teamB", "team24/7"}, + "GetAdditionalTeams returns wrong list", + }, + { + "Check that additional teams are returned incl. transitive teams", + "teamA", + true, + []string{"teamB", "team24/7"}, + "GetAdditionalTeams returns wrong list", + }, + } + + postgresTeamMap := PostgresTeamMap{} + postgresTeamMap.Load(&pgTeamList) + + for _, tt := range tests { + additionalTeams := postgresTeamMap.GetAdditionalSuperuserTeams(tt.team, tt.transitive) + if !util.IsEqualIgnoreOrder(additionalTeams, tt.teams) { + t.Errorf("%s: %v: expected additional teams %#v, got %#v", tt.name, tt.error, tt.teams, additionalTeams) } } } diff --git a/pkg/util/util.go b/pkg/util/util.go index abb9be01f..20e2915ba 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -10,7 +10,9 @@ import ( "fmt" "math/big" "math/rand" + "reflect" "regexp" + "sort" "strings" "time" @@ -134,6 +136,21 @@ func PrettyDiff(a, b interface{}) string { return strings.Join(Diff(a, b), "\n") } +// Compare two string slices while ignoring the order of elements +func IsEqualIgnoreOrder(a, b []string) bool { + if len(a) != len(b) { + return false + } + a_copy := make([]string, len(a)) + b_copy := make([]string, len(b)) + copy(a_copy, a) + copy(b_copy, b) + sort.Strings(a_copy) + sort.Strings(b_copy) + + return reflect.DeepEqual(a_copy, b_copy) +} + // SubstractStringSlices finds elements in a that are not in b and return them as a result slice. func SubstractStringSlices(a []string, b []string) (result []string, equal bool) { // Slices are assumed to contain unique elements only @@ -176,6 +193,20 @@ func FindNamedStringSubmatch(r *regexp.Regexp, s string) map[string]string { return res } +// SliceContains +func SliceContains(slice interface{}, item interface{}) bool { + s := reflect.ValueOf(slice) + if s.Kind() != reflect.Slice { + panic("Invalid data-type") + } + for i := 0; i < s.Len(); i++ { + if s.Index(i).Interface() == item { + return true + } + } + return false +} + // MapContains returns true if and only if haystack contains all the keys from the needle with matching corresponding values func MapContains(haystack, needle map[string]string) bool { if len(haystack) < len(needle) { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index a9d25112b..6cf179033 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -43,6 +43,16 @@ var prettyDiffTest = []struct { {[]int{1, 2, 3, 4}, []int{1, 2, 3, 4}, ""}, } +var isEqualIgnoreOrderTest = []struct { + inA []string + inB []string + outEqual bool +}{ + {[]string{"a", "b", "c"}, []string{"a", "b", "c"}, true}, + {[]string{"a", "b", "c"}, []string{"a", "c", "b"}, true}, + {[]string{"a", "b"}, []string{"a", "c", "b"}, false}, +} + var substractTest = []struct { inA []string inB []string @@ -53,6 +63,16 @@ var substractTest = []struct { {[]string{"a", "b", "c", "d"}, []string{"a", "bb", "c", "d"}, []string{"b"}, false}, } +var sliceContaintsTest = []struct { + slice []string + item string + out bool +}{ + {[]string{"a", "b", "c"}, "a", true}, + {[]string{"a", "b", "c"}, "d", false}, + {[]string{}, "d", false}, +} + var mapContaintsTest = []struct { inA map[string]string inB map[string]string @@ -136,6 +156,15 @@ func TestPrettyDiff(t *testing.T) { } } +func TestIsEqualIgnoreOrder(t *testing.T) { + for _, tt := range isEqualIgnoreOrderTest { + actualEqual := IsEqualIgnoreOrder(tt.inA, tt.inB) + if actualEqual != tt.outEqual { + t.Errorf("IsEqualIgnoreOrder expected: %t, got: %t", tt.outEqual, actualEqual) + } + } +} + func TestSubstractSlices(t *testing.T) { for _, tt := range substractTest { actualRes, actualEqual := SubstractStringSlices(tt.inA, tt.inB) @@ -160,6 +189,15 @@ func TestFindNamedStringSubmatch(t *testing.T) { } } +func TestSliceContains(t *testing.T) { + for _, tt := range sliceContaintsTest { + res := SliceContains(tt.slice, tt.item) + if res != tt.out { + t.Errorf("SliceContains expected: %#v, got: %#v", tt.out, res) + } + } +} + func TestMapContains(t *testing.T) { for _, tt := range mapContaintsTest { res := MapContains(tt.inA, tt.inB)