Skip to content

Commit

Permalink
pilot: fix invalid destination clusters with ENABLE_EXTERNAL_NAME_ALI…
Browse files Browse the repository at this point in the history
…AS=false (#51241)

Co-authored-by: John Howard <[email protected]>
  • Loading branch information
istio-testing and howardjohn committed May 24, 2024
1 parent 202bb31 commit 3fc7666
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pilot/pkg/networking/core/v1alpha3/route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (

meshconfig "istio.io/api/mesh/v1alpha1"
networking "istio.io/api/networking/v1alpha3"
"istio.io/istio/pilot/pkg/features"
"istio.io/istio/pilot/pkg/model"
"istio.io/istio/pilot/pkg/networking/core/v1alpha3/route/retry"
"istio.io/istio/pilot/pkg/networking/telemetry"
Expand Down Expand Up @@ -331,7 +332,7 @@ func GetDestinationCluster(destination *networking.Destination, service *model.S
h := host.Name(destination.Host)
// If this is an Alias, point to the concrete service
// TODO: this will not work if we have Alias -> Alias -> Concrete service.
if service != nil && service.Attributes.K8sAttributes.ExternalName != "" {
if features.EnableExternalNameAlias && service != nil && service.Attributes.K8sAttributes.ExternalName != "" {
h = host.Name(service.Attributes.K8sAttributes.ExternalName)
}
port := listenerPort
Expand Down
203 changes: 203 additions & 0 deletions pilot/pkg/networking/core/v1alpha3/sidecar_simulation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,209 @@ spec:
})
}

func TestExternalNameServicesWithoutAliases(t *testing.T) {
test.SetForTest(t, &features.EnableExternalNameAlias, false)
ports := `
- name: http
port: 80
- name: auto
port: 81
- name: tcp
port: 82
- name: tls
port: 83
- name: https
port: 84`

type tc struct {
call simulation.Call
expected string
}
calls := []simulation.Expect{}
for _, call := range []tc{
{call: simulation.Call{Address: "1.2.3.4", Port: 80, Protocol: simulation.HTTP, HostHeader: "alias.default.svc.cluster.local"}, expected: "alias"},

// Auto port should support any protocol
{call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.HTTP, HostHeader: "alias.default.svc.cluster.local"}, expected: "concrete"},
{call: simulation.Call{Address: "1.1.1.1", Port: 81, Protocol: simulation.HTTP, HostHeader: "alias.default.svc.cluster.local"}, expected: "alias"},
{
call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.HTTP, TLS: simulation.TLS, HostHeader: "alias.default.svc.cluster.local"},
expected: "concrete",
},
{call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.TCP}, expected: "concrete"},

{call: simulation.Call{Address: "1.2.3.4", Port: 82, Protocol: simulation.TCP}, expected: "concrete"},

// Use short host name
{call: simulation.Call{Address: "1.2.3.4", Port: 83, Protocol: simulation.TCP, TLS: simulation.TLS, HostHeader: "alias.default"}, expected: "concrete"},
{call: simulation.Call{Address: "1.2.3.4", Port: 84, Protocol: simulation.HTTP, TLS: simulation.TLS, HostHeader: "alias.default"}, expected: "concrete"},
} {
calls = append(calls, simulation.Expect{
Name: fmt.Sprintf("%s-%d", call.call.Protocol, call.call.Port),
Call: call.call,
Result: simulation.Result{
ClusterMatched: fmt.Sprintf("outbound|%d||%s.default.svc.cluster.local", call.call.Port, call.expected),
},
})
}
service := `apiVersion: v1
kind: Service
metadata:
name: alias
namespace: default
spec:
type: ExternalName
externalName: concrete.default.svc.cluster.local
ports:` + ports + `
---
apiVersion: v1
kind: Service
metadata:
name: concrete
namespace: default
spec:
clusterIP: 1.2.3.4
ports:` + ports
runSimulationTest(t, nil, xds.FakeOptions{}, simulationTest{
kubeConfig: service,
calls: calls,
})

// HTTP Routes
runSimulationTest(t, nil, xds.FakeOptions{}, simulationTest{
config: `apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: alias
spec:
hosts:
- alias.default.svc.cluster.local
http:
- name: "route1"
match:
- uri:
prefix: "/one"
route:
- destination:
host: concrete.default.svc.cluster.local`,
kubeConfig: service,
calls: []simulation.Expect{
{
// This work, Host is just an opaque hostname match
Name: "HTTP virtual service applies to alias fqdn",
Call: simulation.Call{Address: "1.2.3.4", Port: 80, Protocol: simulation.HTTP, HostHeader: "alias.default.svc.cluster.local", Path: "/one"},
Result: simulation.Result{
RouteMatched: "route1",
ClusterMatched: "outbound|80||concrete.default.svc.cluster.local",
},
},
{
// Host is expanded
Name: "HTTP virtual service does apply to alias without exact match",
Call: simulation.Call{Address: "1.2.3.4", Port: 80, Protocol: simulation.HTTP, HostHeader: "alias.default", Path: "/one"},
Result: simulation.Result{
RouteMatched: "route1",
ClusterMatched: "outbound|80||concrete.default.svc.cluster.local",
},
},
{
Name: "HTTP virtual service of alias does not apply to concrete",
Call: simulation.Call{Address: "1.2.3.4", Port: 80, Protocol: simulation.HTTP, HostHeader: "concrete.default.svc.cluster.local", Path: "/one"},
Result: simulation.Result{
RouteMatched: "default",
ClusterMatched: "outbound|80||concrete.default.svc.cluster.local",
},
},
// Auto
{
// No opaque host match for auto
Name: "Auto virtual service applies to alias fqdn",
Call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.HTTP, HostHeader: "alias.default.svc.cluster.local", Path: "/one"},
Result: simulation.Result{
RouteMatched: "default",
ClusterMatched: "outbound|81||concrete.default.svc.cluster.local",
},
},
{
// Host is opaque, so no expansion
Name: "Auto virtual service does not apply to alias without exact match",
Call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.HTTP, HostHeader: "alias.default", Path: "/one"},
Result: simulation.Result{
RouteMatched: "default",
ClusterMatched: "outbound|81||concrete.default.svc.cluster.local",
},
},
{
Name: "Auto virtual service of alias does not apply to concrete",
Call: simulation.Call{Address: "1.2.3.4", Port: 81, Protocol: simulation.HTTP, HostHeader: "concrete.default.svc.cluster.local", Path: "/one"},
Result: simulation.Result{
RouteMatched: "default",
ClusterMatched: "outbound|81||concrete.default.svc.cluster.local",
},
},
},
})

// TCP Routes
runSimulationTest(t, nil, xds.FakeOptions{}, simulationTest{
config: `apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: alias
spec:
hosts:
- alias.default.svc.cluster.local
tcp:
- name: "route1"
route:
- destination:
host: concrete.default.svc.cluster.local
port:
number: 80`,
kubeConfig: service,
calls: []simulation.Expect{
{
Name: "TCP virtual services do not apply",
Call: simulation.Call{Address: "1.2.3.4", Port: 82, Protocol: simulation.TCP, Path: "/one"},
Result: simulation.Result{
ClusterMatched: "outbound|82||concrete.default.svc.cluster.local",
},
},
},
})

// HTTP Routes to alias
runSimulationTest(t, nil, xds.FakeOptions{}, simulationTest{
config: `apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: alias
spec:
hosts:
- example.com
http:
- name: "route1"
match:
- uri:
prefix: "/one"
route:
- destination:
host: alias.default.svc.cluster.local`,
kubeConfig: service,
calls: []simulation.Expect{
{
// This work, Host is just an opaque hostname match
Name: "HTTP route to alias",
Call: simulation.Call{Port: 80, Protocol: simulation.HTTP, HostHeader: "example.com", Path: "/one"},
Result: simulation.Result{
RouteMatched: "route1",
ClusterMatched: "outbound|80||alias.default.svc.cluster.local",
},
},
},
})
}

func TestPassthroughTraffic(t *testing.T) {
calls := map[string]simulation.Call{}
for port := 80; port < 87; port++ {
Expand Down
7 changes: 7 additions & 0 deletions releasenotes/notes/fix-external-name.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: release-notes/v2
kind: bug-fix
area: traffic-management
releaseNotes:
- |
**Fixed** a regression in Istio 1.21.0 causing `VirtualService`s routing to `ExternalName` services to not work when
`ENABLE_EXTERNAL_NAME_ALIAS=false` is configured.

0 comments on commit 3fc7666

Please sign in to comment.