Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
stevenctl committed Apr 29, 2024
1 parent 815149b commit 5151551
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 49 deletions.
12 changes: 10 additions & 2 deletions pkg/test/framework/components/echo/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ func (c Config) IsRegularPod() bool {
!c.DualStack
}

// WaypointClient means the client supports HBONE and does zTunnel redirection.
// Currently, only zTunnel captured sources do this. Eventually this might be enabled
// for ingress and/or sidecars.
func (c Config) WaypointClient() bool {
return c.ZTunnelCaptured() && !c.IsUncaptured()
}

// ZTunnelCaptured returns true in ambient enabled namespaces where there is no sidecar
func (c Config) ZTunnelCaptured() bool {
haveSubsets := len(c.Subsets) > 0
Expand Down Expand Up @@ -597,8 +604,9 @@ func (c Config) WorkloadClass() WorkloadClass {
return StatefulSet
} else if c.IsSotw() {
return Sotw
} else if c.ZTunnelCaptured() &&
!(c.HasServiceAddressedWaypointProxy() || c.HasWorkloadAddressedWaypointProxy()) {
} else if c.HasAnyWaypointProxy() {
return Waypoint
} else if c.ZTunnelCaptured() && !c.HasAnyWaypointProxy() {
return Captured
}
if c.IsHeadless() {
Expand Down
11 changes: 10 additions & 1 deletion pkg/test/framework/components/echo/echotest/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,19 @@ var ReachableDestinations CombinationFilter = func(from echo.Instance, to echo.I
reachableFromVM(from),
reachableFromProxylessGRPC(from),
reachableNakedDestinations(from),
reachableHeadlessDestinations(from)).
reachableHeadlessDestinations(from),
reachableWaypoints(from)).
GetMatches(to)
}

// reachableWaypoints removes waypointed targets when the client doesn't
func reachableWaypoints(from echo.Instance) match.Matcher {
if from.Config().WaypointClient() {
return match.Any
}
return match.NotWaypoint
}

// reachableHeadlessDestinations filters out headless services that aren't in the same cluster
// TODO(stevenctl): headless across-networks https://github.com/istio/istio/issues/38327
func reachableHeadlessDestinations(from echo.Instance) match.Matcher {
Expand Down
5 changes: 2 additions & 3 deletions pkg/test/framework/components/echo/match/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,9 @@ func WaypointService() Matcher {
}
}

// add a new matcher for "captured service -> service"
func CapturedService() Matcher {
func AmbientCaptured() Matcher {
return func(i echo.Instance) bool {
return i.Config().ZTunnelCaptured()
return i.Config().ZTunnelCaptured() && !i.Config().IsUncaptured()
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/test/framework/components/echo/workloadclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ const (
StatefulSet WorkloadClass = "statefulset"
Headless WorkloadClass = "headless"
Captured WorkloadClass = "captured"
Waypoint WorkloadClass = "waypoint"
Standard WorkloadClass = "standard"
)
57 changes: 33 additions & 24 deletions tests/integration/ambient/baseline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ func supportsL7(opt echo.CallOptions, src, dst echo.Instance) bool {
// Assumption is ambient test suite sidecars will support HBONE
// If the assumption is incorrect hboneClient may return invalid result
func hboneClient(instance echo.Instance) bool {
return instance.Config().ZTunnelCaptured() ||
instance.Config().HasSidecar()
return instance.Config().ZTunnelCaptured()
}

func TestServices(t *testing.T) {
Expand All @@ -159,13 +158,6 @@ func TestServices(t *testing.T) {
opt.Check = tcpValidator
}

if !hboneClient(src) && dst.Config().HasAnyWaypointProxy() {
// For this case, it is broken if the src and dst are on the same node.
// Because client request is not captured to perform the hairpin
// TODO(https://github.com/istio/istio/issues/43238): fix this and remove this skip
t.Skip("https://github.com/istio/istio/issues/44530")
}

if !dst.Config().HasServiceAddressedWaypointProxy() &&
!src.Config().HasServiceAddressedWaypointProxy() &&
(src.Config().Service != dst.Config().Service) &&
Expand All @@ -176,12 +168,15 @@ func TestServices(t *testing.T) {
opt.Check = check.And(opt.Check, OriginalSourceCheck(t, src))
}

if src.Config().ZTunnelCaptured() && dst.Config().HasWorkloadAddressedWaypointProxy() {
// this is to svc traffic on a wl with only a workload addressed waypoint, it is going to bypass the waypoint by design
// we can't check http because we bypass the waypoint
// I don't think it makes sense to change the supportsL7 function for this case since it requires contect
// about how the traffic will be addressed
opt.Check = tcpValidator
// Non-HBONE clients will attempt to bypass the waypoint
if !src.Config().WaypointClient() && dst.Config().HasAnyWaypointProxy() {
opt.Check = check.NotOK()
}

// Any client will attempt to bypass a workload waypoint (not both service and workload waypoint)
// because this test always addresses by service.
if dst.Config().HasWorkloadAddressedWaypointProxy() && !dst.Config().HasServiceAddressedWaypointProxy() {
opt.Check = check.Error()
}

if src.Config().HasSidecar() && dst.Config().HasWorkloadAddressedWaypointProxy() {
Expand Down Expand Up @@ -217,22 +212,29 @@ func TestPodIP(t *testing.T) {
opt.Check = tcpValidator
}

if src.Config().IsUncaptured() && dst.Config().HasAnyWaypointProxy() {
// hairpinning isn't going to be implemented AND
// waypoint requirements are expressed via L4 policy which is not in place for this test:
// expected result is a plaintext passthrough by ztunnel
opt.Check = tcpValidator
opt.Address = dstWl.Address()
opt.Check = check.And(opt.Check, check.Hostname(dstWl.PodName()))

opt.Port = echo.Port{ServicePort: ports.All().MustForName(opt.Port.Name).WorkloadPort}
opt.ToWorkload = dst.WithWorkloads(dstWl)

// Uncaptured means we won't traverse the waypoint
// We cannot bypass the waypoint, so this fails.
if !src.Config().WaypointClient() && dst.Config().HasAnyWaypointProxy() {
opt.Check = check.NotOK()
}

// Only marked to use service waypoint. We'll deny since it's not traversed.
// Not traversed, since traffic is to-workload IP.
if dst.Config().HasServiceAddressedWaypointProxy() && !dst.Config().HasWorkloadAddressedWaypointProxy() {
opt.Check = check.NotOK()
}

if selfSend {
// Calls to ourself (by pod IP) are not captured
opt.Check = tcpValidator
}
opt.Address = dstWl.Address()
opt.Check = check.And(opt.Check, check.Hostname(dstWl.PodName()))

opt.Port = echo.Port{ServicePort: ports.All().MustForName(opt.Port.Name).WorkloadPort}
opt.ToWorkload = dst.WithWorkloads(dstWl)
t.NewSubTestf("%v", opt.Scheme).RunParallel(func(t framework.TestContext) {
src.WithWorkloads(srcWl).CallOrFail(t, opt)
})
Expand Down Expand Up @@ -2209,6 +2211,13 @@ func TestIngress(t *testing.T) {
if opt.Scheme != scheme.HTTP {
return
}

// Ingress currently never sends to Waypoints
// We cannot bypass the waypoint, so this fails.
if dst.Config().HasAnyWaypointProxy() {
opt.Check = check.Error()
}

t.ConfigIstio().Eval(apps.Namespace.Name(), map[string]string{
"Destination": dst.Config().Service,
}, `apiVersion: networking.istio.io/v1alpha3
Expand Down
10 changes: 4 additions & 6 deletions tests/integration/ambient/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,16 @@ func SetupApps(t resource.Context, i istio.Instance, apps *EchoDeployments) erro
Replicas: 1,
Version: "v1",
Labels: map[string]string{
"app": ServiceAddressedWaypoint,
"version": "v1",
constants.AmbientUseWaypoint: "waypoint",
"app": ServiceAddressedWaypoint,
"version": "v1",
},
},
{
Replicas: 1,
Version: "v2",
Labels: map[string]string{
"app": ServiceAddressedWaypoint,
"version": "v2",
constants.AmbientUseWaypoint: "waypoint",
"app": ServiceAddressedWaypoint,
"version": "v2",
},
},
},
Expand Down
28 changes: 15 additions & 13 deletions tests/integration/pilot/common/routing.go
Original file line number Diff line number Diff line change
Expand Up @@ -1666,14 +1666,15 @@ func gatewayCases(t TrafficContext) {
}

// SingleRegualrPod is already applied leaving one regular pod, to only regular pods should leave a single workload.
singleTarget := []match.Matcher{match.RegularPod}
// the following cases don't actually target workloads, we use the singleTarget filter to avoid duplicate cases
// Gateways don't support talking directly to waypoint, so we skip that here as well.
matchers := []match.Matcher{match.RegularPod, match.NotWaypoint}

gatewayListenPort := 80
gatewayListenPortName := "http"
// the following cases don't actually target workloads, we use the singleTarget filter to avoid duplicate cases
t.RunTraffic(TrafficTestCase{
name: "404",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: httpGateway("*", gatewayListenPort, gatewayListenPortName, "HTTP", t.Istio.Settings().IngressGatewayIstioLabel),
Expand All @@ -1691,7 +1692,7 @@ func gatewayCases(t TrafficContext) {
})
t.RunTraffic(TrafficTestCase{
name: "https redirect",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -1729,7 +1730,7 @@ spec:
t.RunTraffic(TrafficTestCase{
// See https://github.com/istio/istio/issues/27315
name: "https with x-forwarded-proto",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -1847,7 +1848,7 @@ spec:
t.RunTraffic(TrafficTestCase{
// See https://github.com/istio/istio/issues/34609
name: "http redirect when vs port specify https",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -1890,7 +1891,7 @@ spec:
// See https://github.com/istio/istio/issues/27315
// See https://github.com/istio/istio/issues/34609
name: "http return 400 with with x-forwarded-proto https when vs port specify https",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -1966,7 +1967,7 @@ spec:
t.RunTraffic(TrafficTestCase{
// https://github.com/istio/istio/issues/37196
name: "client protocol - http1",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -2009,7 +2010,7 @@ spec:
// https://github.com/istio/istio/issues/37196
name: "client protocol - http2",
skip: skipEnvoyPeerMeta,
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -2061,7 +2062,7 @@ spec:
})
t.RunTraffic(TrafficTestCase{
name: "wildcard hostname",
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -2150,7 +2151,7 @@ spec:
// https://github.com/istio/istio/issues/37196
name: fmt.Sprintf("client protocol - %v use client with %v", protoName, port),
skip: skipEnvoyPeerMeta,
targetMatchers: singleTarget,
targetMatchers: matchers,
workloadAgnostic: true,
viaIngress: true,
config: `apiVersion: networking.istio.io/v1alpha3
Expand Down Expand Up @@ -2241,7 +2242,7 @@ spec:
check.RequestHeader("Istio-Custom-Header", "user-defined-value")),
},
// to keep tests fast, we only run the basic protocol test per-workload and scheme match once (per cluster)
targetMatchers: singleTarget,
targetMatchers: matchers,
viaIngress: true,
workloadAgnostic: true,
})
Expand Down Expand Up @@ -4062,7 +4063,8 @@ spec:
claim: "wrong_claim"
---
`
matchers := []match.Matcher{match.Or(match.ServiceName(t.Apps.B.NamespacedName()), match.WaypointService(), match.CapturedService())}
// No waypoint here, these are all via ingress which doesn't forward to waypoint
matchers := []match.Matcher{match.Or(match.ServiceName(t.Apps.B.NamespacedName()), match.AmbientCaptured())}
headersWithToken := map[string][]string{
"Host": {"foo.bar"},
"Authorization": {"Bearer " + jwt.TokenIssuer1WithNestedClaims1},
Expand Down

0 comments on commit 5151551

Please sign in to comment.