Skip to content

Commit 51be685

Browse files
haywoodshhaywoodsh
committed
restore removed tests
Signed-off-by: Haywood Shannon <[email protected]> Signed-off-by: Haywood Shannon <[email protected]>
1 parent 5c1d721 commit 51be685

File tree

2 files changed

+293
-2
lines changed

2 files changed

+293
-2
lines changed

‎internal/k8s/configuration.go‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,34 @@ func (vsc *VirtualServerConfiguration) IsEqual(resource Resource) bool {
273273
}
274274
}
275275

276+
// Check VirtualServerRouteSelectors maps for equality
277+
if len(vsc.VirtualServerRouteSelectors) != len(vsConfig.VirtualServerRouteSelectors) {
278+
return false
279+
}
280+
281+
for selector, routes := range vsc.VirtualServerRouteSelectors {
282+
otherRoutes, exists := vsConfig.VirtualServerRouteSelectors[selector]
283+
if !exists {
284+
return false
285+
}
286+
287+
if len(routes) != len(otherRoutes) {
288+
return false
289+
}
290+
291+
// Create maps for O(1) lookup to compare route slices
292+
routeSet := make(map[string]bool)
293+
for _, route := range routes {
294+
routeSet[route] = true
295+
}
296+
297+
for _, otherRoute := range otherRoutes {
298+
if !routeSet[otherRoute] {
299+
return false
300+
}
301+
}
302+
}
303+
276304
return true
277305
}
278306

@@ -1691,7 +1719,6 @@ func (c *Configuration) buildVirtualServerRoutes(vs *conf_v1.VirtualServer) ([]*
16911719
MatchLabels: r.RouteSelector.MatchLabels,
16921720
}
16931721
sel, err := metav1.LabelSelectorAsSelector(selector)
1694-
16951722
if err != nil {
16961723
warning := fmt.Sprintf("VirtualServerRoute LabelSelector %s is invalid: %v", selector, err)
16971724
warnings = append(warnings, warning)
@@ -1705,7 +1732,7 @@ func (c *Configuration) buildVirtualServerRoutes(vs *conf_v1.VirtualServer) ([]*
17051732
}
17061733

17071734
for vsrKey, vsr := range c.virtualServerRoutes {
1708-
if sel.Matches(labels.Set(vsr.ObjectMeta.Labels)) {
1735+
if sel.Matches(labels.Set(vsr.Labels)) {
17091736
err := c.virtualServerValidator.ValidateVirtualServerRouteForVirtualServer(vsr, vs.Spec.Host, r.Path)
17101737
if err != nil {
17111738
warning := fmt.Sprintf("VirtualServerRoute %s is invalid: %v", vsrKey, err)

‎internal/k8s/configuration_test.go‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/google/go-cmp/cmp"
9+
"github.com/google/go-cmp/cmp/cmpopts"
910
nl "github.com/nginx/kubernetes-ingress/internal/logger"
1011
conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1"
1112
"github.com/nginx/kubernetes-ingress/pkg/apis/configuration/validation"
@@ -3858,6 +3859,30 @@ func createTestVirtualServerRoute(name string, host string, path string) *conf_v
38583859
}
38593860
}
38603861

3862+
func createTestVirtualServerRouteWithLabels(name string, path string, labels map[string]string) *conf_v1.VirtualServerRoute {
3863+
return &conf_v1.VirtualServerRoute{
3864+
ObjectMeta: metav1.ObjectMeta{
3865+
Namespace: "default",
3866+
Name: name,
3867+
Labels: labels,
3868+
},
3869+
Spec: conf_v1.VirtualServerRouteSpec{
3870+
IngressClass: "nginx",
3871+
Host: "foo.example.com",
3872+
Subroutes: []conf_v1.Route{
3873+
{
3874+
Path: path,
3875+
Action: &conf_v1.Action{
3876+
Return: &conf_v1.ActionReturn{
3877+
Body: "vsr",
3878+
},
3879+
},
3880+
},
3881+
},
3882+
},
3883+
}
3884+
}
3885+
38613886
func createTestChallengeVirtualServerRoute(name string, host string, path string) *conf_v1.VirtualServerRoute {
38623887
return &conf_v1.VirtualServerRoute{
38633888
ObjectMeta: metav1.ObjectMeta{
@@ -4566,6 +4591,7 @@ func TestIsEqualForIngressConfigurations(t *testing.T) {
45664591
}
45674592
}
45684593

4594+
// TODO: vsr route selector test
45694595
func TestIsEqualForVirtualServers(t *testing.T) {
45704596
t.Parallel()
45714597
vs := createTestVirtualServerWithRoutes(
@@ -4964,3 +4990,241 @@ func TestTransportServerListenerHostCollisions(t *testing.T) {
49644990
t.Errorf("AddOrUpdateTransportServer(ts6) returned problems %v", problems)
49654991
}
49664992
}
4993+
4994+
func TestMatchVSwithVSRusingSelector(t *testing.T) {
4995+
t.Parallel()
4996+
4997+
configuration := createTestConfiguration()
4998+
4999+
// Add VirtualServerRoute
5000+
5001+
labels := make(map[string]string)
5002+
vsr := createTestVirtualServerRouteWithLabels("virtualserverroute", "/first", labels)
5003+
5004+
var expectedChanges []ResourceChange
5005+
expectedProblems := []ConfigurationProblem{
5006+
{
5007+
Object: vsr,
5008+
Reason: "NoVirtualServerFound",
5009+
Message: "VirtualServer is invalid or doesn't exist",
5010+
},
5011+
}
5012+
5013+
// adding VSR to the configuration; no VS exist at this stage, chance we get problems
5014+
//
5015+
// if we don't get it right now we call t.Fatal as there is no
5016+
// point to continue the test - preconditions are not setup correctly.
5017+
changes, problems := configuration.AddOrUpdateVirtualServerRoute(vsr)
5018+
if !cmp.Equal(expectedChanges, changes, cmpopts.IgnoreFields(ConfigurationProblem{}, "Message")) {
5019+
t.Fatal(cmp.Diff(expectedChanges, changes))
5020+
}
5021+
if !cmp.Equal(expectedProblems, problems, cmpopts.IgnoreFields(ConfigurationProblem{}, "Message")) {
5022+
t.Fatal(cmp.Diff(expectedProblems, problems))
5023+
}
5024+
5025+
// Add VS with VRS with the RouteSelector (LabelSelector)
5026+
routes := []conf_v1.Route{
5027+
{
5028+
Path: "/first",
5029+
Route: "virtualserverroute",
5030+
},
5031+
{
5032+
Path: "/",
5033+
RouteSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "route"}},
5034+
},
5035+
}
5036+
5037+
vs := createTestVirtualServerWithRoutes("virtualserver", "foo.example.com", routes)
5038+
5039+
expectedChanges = []ResourceChange{
5040+
{
5041+
Op: AddOrUpdate,
5042+
Resource: &VirtualServerConfiguration{
5043+
VirtualServer: vs,
5044+
VirtualServerRoutes: []*conf_v1.VirtualServerRoute{vsr},
5045+
VirtualServerRouteSelectors: map[string][]string{"app=route": {}},
5046+
},
5047+
},
5048+
}
5049+
expectedProblems = nil
5050+
5051+
changes, problems = configuration.AddOrUpdateVirtualServer(vs)
5052+
if !cmp.Equal(expectedChanges, changes) {
5053+
t.Error(cmp.Diff(expectedChanges, changes))
5054+
}
5055+
if !cmp.Equal(expectedProblems, problems) {
5056+
t.Error(cmp.Diff(expectedProblems, problems))
5057+
}
5058+
}
5059+
5060+
func TestAddVirtualServerWithVirtualServerRoutesVSR(t *testing.T) {
5061+
configuration := createTestConfiguration()
5062+
5063+
// Add VirtualServerRoute-1
5064+
5065+
labels := make(map[string]string)
5066+
vsr1 := createTestVirtualServerRouteWithLabels("virtualserverroute-1", "/first", labels)
5067+
5068+
var expectedChanges []ResourceChange
5069+
expectedProblems := []ConfigurationProblem{
5070+
{
5071+
Object: vsr1,
5072+
Reason: "NoVirtualServerFound",
5073+
Message: "VirtualServer is invalid or doesn't exist",
5074+
},
5075+
}
5076+
5077+
changes, problems := configuration.AddOrUpdateVirtualServerRoute(vsr1)
5078+
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
5079+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5080+
}
5081+
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
5082+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5083+
}
5084+
5085+
// Add VirtualServer
5086+
5087+
vs := createTestVirtualServerWithRoutes(
5088+
"virtualserver",
5089+
"foo.example.com",
5090+
[]conf_v1.Route{
5091+
{
5092+
Path: "/first",
5093+
Route: "virtualserverroute-1",
5094+
},
5095+
{
5096+
Path: "/second",
5097+
Route: "virtualserverroute-2",
5098+
},
5099+
{
5100+
Path: "/",
5101+
RouteSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "route"}},
5102+
},
5103+
})
5104+
expectedChanges = []ResourceChange{
5105+
{
5106+
Op: AddOrUpdate,
5107+
Resource: &VirtualServerConfiguration{
5108+
VirtualServer: vs,
5109+
VirtualServerRoutes: []*conf_v1.VirtualServerRoute{vsr1},
5110+
VirtualServerRouteSelectors: map[string][]string{"app=route": {}},
5111+
Warnings: []string{"VirtualServerRoute default/virtualserverroute-2 doesn't exist or invalid"},
5112+
},
5113+
},
5114+
}
5115+
expectedProblems = nil
5116+
5117+
changes, problems = configuration.AddOrUpdateVirtualServer(vs)
5118+
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
5119+
t.Errorf("AddOrUpdateVirtualServer() returned unexpected result (-want +got):\n%s", diff)
5120+
}
5121+
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
5122+
t.Errorf("AddOrUpdateVirtualServer() returned unexpected result (-want +got):\n%s", diff)
5123+
}
5124+
5125+
// Add VirtualServerRoute-2
5126+
5127+
vsr2 := createTestVirtualServerRouteWithLabels("virtualserverroute-2", "/second", nil)
5128+
5129+
expectedChanges = []ResourceChange{
5130+
{
5131+
Op: AddOrUpdate,
5132+
Resource: &VirtualServerConfiguration{
5133+
VirtualServer: vs,
5134+
VirtualServerRoutes: []*conf_v1.VirtualServerRoute{vsr1, vsr2},
5135+
VirtualServerRouteSelectors: map[string][]string{"app=route": {}},
5136+
},
5137+
},
5138+
}
5139+
expectedProblems = nil
5140+
5141+
changes, problems = configuration.AddOrUpdateVirtualServerRoute(vsr2)
5142+
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
5143+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5144+
}
5145+
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
5146+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5147+
}
5148+
5149+
// Add VirtualServerRoute-3 with RouteSelector labels
5150+
5151+
vsr3 := createTestVirtualServerRouteWithLabels("virtualserverroute-3", "/third", map[string]string{"app": "route"})
5152+
expectedChanges = []ResourceChange{
5153+
{
5154+
Op: AddOrUpdate,
5155+
Resource: &VirtualServerConfiguration{
5156+
VirtualServer: vs,
5157+
VirtualServerRoutes: []*conf_v1.VirtualServerRoute{vsr1, vsr2, vsr3},
5158+
VirtualServerRouteSelectors: map[string][]string{"app=route": {"default/virtualserverroute-3"}},
5159+
},
5160+
},
5161+
}
5162+
expectedProblems = nil
5163+
5164+
changes, problems = configuration.AddOrUpdateVirtualServerRoute(vsr3)
5165+
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
5166+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5167+
}
5168+
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
5169+
t.Errorf("AddOrUpdateVirtualServerRoute() returned unexpected result (-want +got):\n%s", diff)
5170+
}
5171+
}
5172+
5173+
func TestIsEqualForVirtualServersVSR(t *testing.T) {
5174+
t.Parallel()
5175+
vs := createTestVirtualServerWithRoutes(
5176+
"virtualserver",
5177+
"foo.example.com",
5178+
[]conf_v1.Route{
5179+
{
5180+
Path: "/",
5181+
Route: "virtualserverroute",
5182+
},
5183+
})
5184+
vsr := createTestVirtualServerRouteWithLabels("virtualserverroute", "/", nil)
5185+
5186+
vsWithUpdatedGen := vs.DeepCopy()
5187+
vsWithUpdatedGen.Generation++
5188+
5189+
vsrWithUpdatedGen := vsr.DeepCopy()
5190+
vsrWithUpdatedGen.Generation++
5191+
5192+
tests := []struct {
5193+
vsConfig1 *VirtualServerConfiguration
5194+
vsConfig2 *VirtualServerConfiguration
5195+
expected bool
5196+
msg string
5197+
}{
5198+
{
5199+
vsConfig1: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5200+
vsConfig2: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5201+
expected: true,
5202+
msg: "equal virtual servers",
5203+
},
5204+
{
5205+
vsConfig1: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5206+
vsConfig2: NewVirtualServerConfiguration(vsWithUpdatedGen, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5207+
expected: false,
5208+
msg: "virtual servers with different generation",
5209+
},
5210+
{
5211+
vsConfig1: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5212+
vsConfig2: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{}, nil, []string{}),
5213+
expected: false,
5214+
msg: "virtual servers with different number of virtual server routes",
5215+
},
5216+
{
5217+
vsConfig1: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsr}, nil, []string{}),
5218+
vsConfig2: NewVirtualServerConfiguration(vs, []*conf_v1.VirtualServerRoute{vsrWithUpdatedGen}, nil, []string{}),
5219+
expected: false,
5220+
msg: "virtual servers with virtual server routes with different generation",
5221+
},
5222+
}
5223+
5224+
for _, test := range tests {
5225+
result := test.vsConfig1.IsEqual(test.vsConfig2)
5226+
if result != test.expected {
5227+
t.Errorf("IsEqual() returned %v but expected %v for the case of %s", result, test.expected, test.msg)
5228+
}
5229+
}
5230+
}

0 commit comments

Comments
 (0)