Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix xDS protocol mishandling #1013

Closed
wants to merge 1 commit into from
Closed

fix xDS protocol mishandling #1013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 3 additions & 43 deletions internal/e2e/cds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,10 @@ func TestClusterLongServiceName(t *testing.T) {

// check that it's been translated correctly.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kbujbkuh-c83ceb/8080/da39a3ee5e", "default/kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r", "default_kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r_8080")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -125,12 +123,10 @@ func TestClusterAddUpdateDelete(t *testing.T) {
rh.OnAdd(s1)

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// s2 is the same as s2, but the service port has a name
Expand All @@ -146,12 +142,10 @@ func TestClusterAddUpdateDelete(t *testing.T) {

// check that we get two CDS records because the port is now named.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// s3 is like s2, but has a second named port. The k8s spec
Expand All @@ -177,13 +171,11 @@ func TestClusterAddUpdateDelete(t *testing.T) {
// check that we get four CDS records. Order is important
// because the CDS cache is sorted.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443")),
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// s4 is s3 with the http port removed.
Expand All @@ -202,12 +194,10 @@ func TestClusterAddUpdateDelete(t *testing.T) {
// check that we get two CDS records only, and that the 80 and http
// records have been removed even though the service object remains.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -262,13 +252,11 @@ func TestClusterRenameUpdateDelete(t *testing.T) {

rh.OnAdd(s1)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443")),
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// s2 removes the name on port 80, moves it to port 443 and deletes the https port
Expand All @@ -282,33 +270,27 @@ func TestClusterRenameUpdateDelete(t *testing.T) {

rh.OnUpdate(s1, s2)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/443/da39a3ee5e", "default/kuard", "default_kuard_443")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// now replace s2 with s1 to check it works in the other direction.
rh.OnUpdate(s2, s1)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443")),
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// cleanup and check
rh.OnDelete(s1)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{},
TypeUrl: clusterType,
Nonce: "0",
Resources: []types.Any{},
TypeUrl: clusterType,
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -341,12 +323,10 @@ func TestIssue243(t *testing.T) {
)
rh.OnAdd(s1)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
})
}
Expand Down Expand Up @@ -384,12 +364,10 @@ func TestIssue247(t *testing.T) {
)
rh.OnAdd(s1)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}
func TestCDSResourceFiltering(t *testing.T) {
Expand Down Expand Up @@ -443,32 +421,26 @@ func TestCDSResourceFiltering(t *testing.T) {
)
rh.OnAdd(s2)
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
// note, resources are sorted by Cluster.Name
any(t, cluster("default/httpbin/8080/da39a3ee5e", "default/httpbin", "default_httpbin_8080")),
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// assert we can filter on one resource
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc, "default/kuard/80/da39a3ee5e"))

// assert a non matching filter returns no results
// note: streamCDS would stall at this point.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
TypeUrl: clusterType,
Nonce: "0",
TypeUrl: clusterType,
}, streamCDS(t, cc, "default/httpbin/9000"))
}

Expand Down Expand Up @@ -509,7 +481,6 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) {

// check that it's been translated correctly.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, &v2.Cluster{
Name: "default/kuard/8080/da39a3ee5e",
Expand All @@ -533,7 +504,6 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) {
}),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))

// update s1 with slightly weird values
Expand All @@ -555,7 +525,6 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) {

// check that it's been translated correctly.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, &v2.Cluster{
Name: "default/kuard/8080/da39a3ee5e",
Expand All @@ -576,7 +545,6 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) {
}),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -626,12 +594,10 @@ func TestClusterPerServiceParameters(t *testing.T) {
})

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80")),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -681,7 +647,6 @@ func TestClusterLoadBalancerStrategyPerRoute(t *testing.T) {
})

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, &v2.Cluster{
Name: "default/kuard/80/58d888c08a",
Expand Down Expand Up @@ -709,7 +674,6 @@ func TestClusterLoadBalancerStrategyPerRoute(t *testing.T) {
}),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -753,12 +717,10 @@ func TestClusterWithHealthChecks(t *testing.T) {
})

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, clusterWithHealthCheck("default/kuard/80/bc862a33ca", "default/kuard", "default_kuard_80", "/healthz", true)),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down Expand Up @@ -804,12 +766,10 @@ func TestClusterServiceTLSBackend(t *testing.T) {
want := tlscluster("default/kuard/443/da39a3ee5e", "default/kuard/securebackend", "default_kuard_443")

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, want),
},
TypeUrl: clusterType,
Nonce: "0",
}, streamCDS(t, cc))
}

Expand Down
24 changes: 5 additions & 19 deletions internal/e2e/eds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ func TestAddRemoveEndpoints(t *testing.T) {

// check that it's been translated correctly.
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, clusterloadassignment(
"super-long-namespace-name-oh-boy/what-a-descriptive-service-name-you-must-be-so-proud/http",
Expand All @@ -71,17 +70,14 @@ func TestAddRemoveEndpoints(t *testing.T) {
)),
},
TypeUrl: endpointType,
Nonce: "0",
}, streamEDS(t, cc))

// remove e1 and check that the EDS cache is now empty.
rh.OnDelete(e1)

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{},
TypeUrl: endpointType,
Nonce: "0",
Resources: []types.Any{},
TypeUrl: endpointType,
}, streamEDS(t, cc))
}

Expand Down Expand Up @@ -139,7 +135,6 @@ func TestAddEndpointComplicated(t *testing.T) {
rh.OnAdd(e1)

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, clusterloadassignment(
"default/kuard/admin",
Expand All @@ -153,7 +148,6 @@ func TestAddEndpointComplicated(t *testing.T) {
)),
},
TypeUrl: endpointType,
Nonce: "0",
}, streamEDS(t, cc))
}

Expand Down Expand Up @@ -199,7 +193,6 @@ func TestEndpointFilter(t *testing.T) {
rh.OnAdd(e1)

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, clusterloadassignment(
"default/kuard/foo",
Expand All @@ -208,13 +201,10 @@ func TestEndpointFilter(t *testing.T) {
)),
},
TypeUrl: endpointType,
Nonce: "0",
}, streamEDS(t, cc, "default/kuard/foo"))

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
TypeUrl: endpointType,
Nonce: "0",
TypeUrl: endpointType,
}, streamEDS(t, cc, "default/kuard/bar"))

}
Expand All @@ -235,23 +225,19 @@ func TestIssue602(t *testing.T) {

// Assert endpoint was added
assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{
any(t, clusterloadassignment("default/simple", envoy.LBEndpoint("192.168.183.24", 8080))),
},
TypeUrl: endpointType,
Nonce: "0",
}, streamEDS(t, cc))

// e2 is the same as e1, but without endpoint subsets
e2 := endpoints("default", "simple")
rh.OnUpdate(e1, e2)

assertEqual(t, &v2.DiscoveryResponse{
VersionInfo: "0",
Resources: []types.Any{},
TypeUrl: endpointType,
Nonce: "0",
Resources: []types.Any{},
TypeUrl: endpointType,
}, streamEDS(t, cc))
}

Expand Down
Loading