Skip to content

Commit

Permalink
Allow specifying multiple egress host entries with same namespace (is…
Browse files Browse the repository at this point in the history 
…tio#11258)

* allow multiple hosts in same namespace in sidecar egress host

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* merge

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* undo

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* nit

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* lint

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
  • Loading branch information
@rshriram
rshriram authored and louiscryan committed Feb 14, 2019
1 parent b91fe15 commit 3dce196
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 117 deletions.
10 changes: 5 additions & 5 deletions pilot/pkg/model/context.go
View file
Expand Up @@ -272,7 +272,7 @@ func GetProxyConfigNamespace(proxy *Proxy) string {

// First look for ISTIO_META_CONFIG_NAMESPACE
// All newer proxies (from Istio 1.1 onwards) are supposed to supply this
if configNamespace, found := proxy.Metadata[NodeConfigNamespace]; found {
if configNamespace, found := proxy.Metadata[NodeMetadataConfigNamespace]; found {
return configNamespace
}

Expand Down Expand Up @@ -485,13 +485,13 @@ const (
// traffic interception mode at the proxy
NodeMetadataInterceptionMode = "INTERCEPTION_MODE"

// NodeConfigNamespace is the name of the metadata variable that carries info about
// NodeMetadataConfigNamespace is the name of the metadata variable that carries info about
// the config namespace associated with the proxy
NodeConfigNamespace = "CONFIG_NAMESPACE"
NodeMetadataConfigNamespace = "CONFIG_NAMESPACE"

// NodeMetadataUID is the user ID running envoy. Pilot can check if envoy runs as root, and may generate
// NodeMetadataSidecarUID is the user ID running envoy. Pilot can check if envoy runs as root, and may generate
// different configuration. If not set, the default istio-proxy UID (1337) is assumed.
NodeMetadataUID = "UID"
NodeMetadataSidecarUID = "SIDECAR_UID"
)

// TrafficInterceptionMode indicates how traffic to/from the workload is captured and
Expand Down
120 changes: 77 additions & 43 deletions pilot/pkg/model/sidecar.go
View file
Expand Up @@ -95,7 +95,10 @@ type IstioEgressListenerWrapper struct {
// The hosts field has entries of the form namespace/dnsName. For
// example ns1/*, */*, */foo.tcp.com, etc. This map preprocesses all
// these string fields into a map of namespace and services.
listenerHosts map[string]Hostname
// We cannot use a map of Hostnames because Hostname match allows
// wildcard matching semantics (i.e. foo.bar.com will match import like *.com).
// Go's map/hash data structure doesn't do such semantic matches
listenerHosts map[string][]Hostname

// List of services imported by this egress listener extracted from the
// listenerHosts above. This will be used by LDS and RDS code when
Expand Down Expand Up @@ -129,7 +132,7 @@ func DefaultSidecarScopeForNamespace(ps *PushContext, configNamespace string) *S
}

defaultEgressListener := &IstioEgressListenerWrapper{
listenerHosts: map[string]Hostname{wildcardNamespace: wildcardService},
listenerHosts: map[string][]Hostname{wildcardNamespace: {wildcardService}},
}
defaultEgressListener.services = ps.Services(&dummyNode)

Expand Down Expand Up @@ -210,18 +213,20 @@ func convertIstioListenerToWrapper(ps *PushContext, sidecarConfig *Config,

out := &IstioEgressListenerWrapper{
IstioListener: istioListener,
listenerHosts: make(map[string]Hostname),
listenerHosts: make(map[string][]Hostname),
}

if istioListener.Hosts != nil {
// TODO: BUG, this means you can't have 2 explicit imports for same namespace ( override each other )
for _, h := range istioListener.Hosts {
parts := strings.SplitN(h, "/", 2)
if parts[0] == currentNamespace {
out.listenerHosts[sidecarConfig.Namespace] = Hostname(parts[1])
} else {
out.listenerHosts[parts[0]] = Hostname(parts[1])
parts[0] = sidecarConfig.Namespace
}
if _, exists := out.listenerHosts[parts[0]]; !exists {
out.listenerHosts[parts[0]] = make([]Hostname, 0)
}

out.listenerHosts[parts[0]] = append(out.listenerHosts[parts[0]], Hostname(parts[1]))
}
}

Expand Down Expand Up @@ -322,34 +327,54 @@ func (ilw *IstioEgressListenerWrapper) selectVirtualServices(virtualServices []C
configNamespace := c.Namespace
rule := c.Spec.(*networking.VirtualService)

// Selection algorithm:
// virtualservices have a list of hosts in the API spec
// Sidecars have a list of hosts in the api spec (namespace/host format)
// if any host in the virtualService.hosts matches the sidecar's egress'
// entry <virtualServiceNamespace>/virtualServiceHost, select the virtual service
// and break out of the loop.
// OR if any host in the virtualService.hosts matches the sidecar's egress'
// entry */virtualServiceHost, select the virtual service and break out of the loop.

// Check if there is an explicit import of form ns/* or ns/host
if hostMatch, nsFound := ilw.listenerHosts[configNamespace]; nsFound {
// Check if the hostnames match per usual hostname matching rules
hostFound := false
for _, h := range rule.Hosts {
// TODO: This is a bug. VirtualServices can have many hosts
// while the user might be importing only a single host
// We need to generate a new VirtualService with just the matched host
if hostMatch.Matches(Hostname(h)) {
importedVirtualServices = append(importedVirtualServices, c)
hostFound = true
if importedHosts, nsFound := ilw.listenerHosts[configNamespace]; nsFound {
for _, importedHost := range importedHosts {
// Check if the hostnames match per usual hostname matching rules
hostFound := false
for _, h := range rule.Hosts {
// TODO: This is a bug. VirtualServices can have many hosts
// while the user might be importing only a single host
// We need to generate a new VirtualService with just the matched host
if importedHost.Matches(Hostname(h)) {
importedVirtualServices = append(importedVirtualServices, c)
hostFound = true
break
}
}

if hostFound {
break
}
}
if hostFound {
break
}
}

// Check if there is an import of form */host or */*
if hostMatch, wnsFound := ilw.listenerHosts[wildcardNamespace]; wnsFound {
// Check if the hostnames match per usual hostname matching rules
for _, h := range rule.Hosts {
// TODO: This is a bug. VirtualServices can have many hosts
// while the user might be importing only a single host
// We need to generate a new VirtualService with just the matched host
if hostMatch.Matches(Hostname(h)) {
importedVirtualServices = append(importedVirtualServices, c)
if importedHosts, wnsFound := ilw.listenerHosts[wildcardNamespace]; wnsFound {
for _, importedHost := range importedHosts {
// Check if the hostnames match per usual hostname matching rules
hostFound := false
for _, h := range rule.Hosts {
// TODO: This is a bug. VirtualServices can have many hosts
// while the user might be importing only a single host
// We need to generate a new VirtualService with just the matched host
if importedHost.Matches(Hostname(h)) {
importedVirtualServices = append(importedVirtualServices, c)
hostFound = true
break
}
}

if hostFound {
break
}
}
Expand All @@ -367,26 +392,35 @@ func (ilw *IstioEgressListenerWrapper) selectServices(services []*Service) []*Se
for _, s := range services {
configNamespace := s.Attributes.Namespace
// Check if there is an explicit import of form ns/* or ns/host
if hostMatch, nsFound := ilw.listenerHosts[configNamespace]; nsFound {
// Check if the hostnames match per usual hostname matching rules
if hostMatch.Matches(s.Hostname) {
// TODO: See if the service's ports match.
// If there is a listener port for this Listener, then
// check if the service has a port of same value.
// If not, check if the service has a single port - and choose that port
// if service has multiple ports none of which match the listener port, check if there is
// a virtualService with match Port
importedServices = append(importedServices, s)
if importedHosts, nsFound := ilw.listenerHosts[configNamespace]; nsFound {
hostFound := false
for _, importedHost := range importedHosts {
// Check if the hostnames match per usual hostname matching rules
if importedHost.Matches(s.Hostname) {
// TODO: See if the service's ports match.
// If there is a listener port for this Listener, then
// check if the service has a port of same value.
// If not, check if the service has a single port - and choose that port
// if service has multiple ports none of which match the listener port, check if there is
// a virtualService with match Port
importedServices = append(importedServices, s)
hostFound = true
break
}
}
if hostFound {
continue
}
// hostname didn't match. Check if its imported as */host
}

// Check if there is an import of form */host or */*
if hostMatch, wnsFound := ilw.listenerHosts[wildcardNamespace]; wnsFound {
// Check if the hostnames match per usual hostname matching rules
if hostMatch.Matches(s.Hostname) {
importedServices = append(importedServices, s)
if importedHosts, wnsFound := ilw.listenerHosts[wildcardNamespace]; wnsFound {
for _, importedHost := range importedHosts {
// Check if the hostnames match per usual hostname matching rules
if importedHost.Matches(s.Hostname) {
importedServices = append(importedServices, s)
break
}
}
}
}
Expand Down
60 changes: 24 additions & 36 deletions pilot/pkg/networking/core/v1alpha3/cluster.go
View file
Expand Up @@ -468,8 +468,6 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen
return clusters
}
rule := sidecarScope.Config.Spec.(*networking.Sidecar)
// Will generate inbound listeners based on ingress listed ports.
// TODO: verify that Ingress contains the ports defined by app ( no need to duplicate )
for _, ingressListener := range rule.Ingress {
// LDS would have setup the inbound clusters
// as inbound|portNumber|portName|Hostname
Expand All @@ -482,12 +480,12 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen
// When building an inbound cluster for the ingress listener, we take the defaultEndpoint specified
// by the user and parse it into host:port or a unix domain socket
// The default endpoint can be 127.0.0.1:port or :port or unix domain socket
bind := LocalhostAddress
endpointAddress := LocalhostAddress
port := 0
var err error
if strings.HasPrefix(ingressListener.DefaultEndpoint, model.UnixAddressPrefix) {
// this is a UDS endpoint. assign it as is
bind = ingressListener.DefaultEndpoint
endpointAddress = ingressListener.DefaultEndpoint
} else {
// parse the ip, port. Validation guarantees presence of :
parts := strings.Split(ingressListener.DefaultEndpoint, ":")
Expand All @@ -496,31 +494,21 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen
}
}

// TODO: Sidecar config has port number and name - but the port number is the targetPort.
// Right now we don't track the targetPort in the ServiceInstance - only the service port.
// For now we'll match on name ( and require the name of the port to be unique inside a namespace )
// Proper fix is to track targetPort, or use service port number in Sidecar
instance := configgen.findInstance(instances, ingressListener)
// Find the service instance that corresponds to this ingress listener by looking
// for a service instance that either matches this ingress port or one that has
// a port with same name as this ingress port
instance := configgen.findServiceInstanceForIngressListener(instances, ingressListener)

if instance == nil {
// We didn't find a matching port
// We didn't find a matching instance
continue
}
//// TODO: this can't be correct, what happens with the other instances ? Likely works for test with single svc only
//// First create a copy of a service instance
//instance := &model.ServiceInstance{
// Endpoint: instances[0].Endpoint,
// Service: instances[0].Service,
// Labels: instances[0].Labels,
// ServiceAccount: instances[0].ServiceAccount,
//}

// Update the values here so that the plugins use the right ports
// uds values
// TODO: all plugins need to be updated to account for the fact that
// the port may be 0 but bind may have a UDS value
// Inboundroute will be different for
instance.Endpoint.Address = bind
instance.Endpoint.Address = endpointAddress
instance.Endpoint.ServicePort = listenPort
instance.Endpoint.Port = port

Expand All @@ -530,7 +518,7 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen
ServiceInstance: instance,
Port: listenPort,
Push: push,
Bind: bind,
Bind: endpointAddress,
}
localCluster := configgen.buildInboundClusterForPortOrUDS(pluginParams)
clusters = append(clusters, localCluster)
Expand All @@ -540,32 +528,32 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(env *model.Environmen
return clusters
}

func (configgen *ConfigGeneratorImpl) findInstance(instances []*model.ServiceInstance,
func (configgen *ConfigGeneratorImpl) findServiceInstanceForIngressListener(instances []*model.ServiceInstance,
ingressListener *networking.IstioIngressListener) *model.ServiceInstance {
var instance *model.ServiceInstance
// Search by port
for _, realinstance := range instances {
if realinstance.Endpoint.Port == int(ingressListener.Port.Number) {
for _, realInstance := range instances {
if realInstance.Endpoint.Port == int(ingressListener.Port.Number) {
instance = &model.ServiceInstance{
Endpoint: realinstance.Endpoint,
Service: realinstance.Service,
Labels: realinstance.Labels,
ServiceAccount: realinstance.ServiceAccount,
Endpoint: realInstance.Endpoint,
Service: realInstance.Service,
Labels: realInstance.Labels,
ServiceAccount: realInstance.ServiceAccount,
}
return instance
}
}

// search by name
for _, realinstance := range instances {
// TODO: UDS may not work, we could use the port name
for _, iport := range realinstance.Service.Ports {
// If the port number does not match, the user might have specified a
// UDS socket with port number 0. So search by name
for _, realInstance := range instances {
for _, iport := range realInstance.Service.Ports {
if iport.Name == ingressListener.Port.Name {
instance = &model.ServiceInstance{
Endpoint: realinstance.Endpoint,
Service: realinstance.Service,
Labels: realinstance.Labels,
ServiceAccount: realinstance.ServiceAccount,
Endpoint: realInstance.Endpoint,
Service: realInstance.Service,
Labels: realInstance.Labels,
ServiceAccount: realInstance.ServiceAccount,
}
return instance
}
Expand Down

0 comments on commit 3dce196

Please sign in to comment.