From 3b3992b0ecf501fdc9724abcd30128b7823be77f Mon Sep 17 00:00:00 2001 From: Robert Panzer Date: Thu, 11 Apr 2019 18:04:40 +0200 Subject: [PATCH] Fix #11818 fix workloadSelector for Sidecars (#12666) * Fix test error in mixer/adapter/bypass * Fixes #11818. Extend ServiceRegistries to return workload labels independent of Services * Added test for getting workload labels from consul registry * Removed expected errors and results for now from MemoryRegistry.GetProxyWorkloadLabels() * Added LDS test for consumer without Service and workload specific Sidecar * Removed unnecessary fake for service_accounts * Fix #12957. Match EnvoyFilter.workloadSelector against Pod labels, instead of Service labels * Don't dump config in EnvoyFilter LDSTest * Added missing test data * Implemented review comments. * Added test for generation of inbound listeners for proxies without services. * Add ingress to Sidecar configuration for consumer-only Sidecar.workloadSelector test * Formatted imports based review comments * Only log at debug level if ServiceRegistries fail at determining workload labels --- mixer/adapter/bypass/bypass.go | 2 +- mixer/test/client/env/ports.go | 1 + mixer/test/client/gateway/gateway_test.go | 3 + .../client/pilotplugin/pilotplugin_test.go | 3 + .../pilotplugin_mtls/pilotplugin_mtls_test.go | 3 + .../pilotplugin_tcp/pilotplugin_tcp_test.go | 3 + pilot/pkg/model/context.go | 18 +- pilot/pkg/model/push_context.go | 7 +- pilot/pkg/model/service.go | 4 + .../networking/core/v1alpha3/envoyfilter.go | 9 +- .../v1alpha3/fakes/fake_service_accounts.go | 103 -------- .../v1alpha3/fakes/fake_service_discovery.go | 236 ++++++++++-------- .../networking/core/v1alpha3/listener_test.go | 39 +++ pilot/pkg/proxy/envoy/v2/ads.go | 7 + pilot/pkg/proxy/envoy/v2/lds_test.go | 208 +++++++++++++++ pilot/pkg/proxy/envoy/v2/mem.go | 21 ++ .../serviceregistry/aggregate/controller.go | 27 +- .../pkg/serviceregistry/consul/controller.go | 31 +++ .../serviceregistry/consul/controller_test.go | 59 +++++ .../external/servicediscovery.go | 18 ++ pilot/pkg/serviceregistry/kube/controller.go | 11 + pilot/pkg/serviceregistry/memory/discovery.go | 8 + .../envoyfilter-without-service/configs.yaml | 106 ++++++++ .../envoyfilter-without-service/mesh.yaml | 10 + .../sidecar-without-service/configs.yaml | 142 +++++++++++ .../sidecar-without-service/mesh.yaml | 10 + 26 files changed, 866 insertions(+), 223 deletions(-) delete mode 100644 pilot/pkg/networking/core/v1alpha3/fakes/fake_service_accounts.go create mode 100644 tests/testdata/networking/envoyfilter-without-service/configs.yaml create mode 100644 tests/testdata/networking/envoyfilter-without-service/mesh.yaml create mode 100644 tests/testdata/networking/sidecar-without-service/configs.yaml create mode 100644 tests/testdata/networking/sidecar-without-service/mesh.yaml diff --git a/mixer/adapter/bypass/bypass.go b/mixer/adapter/bypass/bypass.go index 748fe1259e87..f0af1d82ae6b 100644 --- a/mixer/adapter/bypass/bypass.go +++ b/mixer/adapter/bypass/bypass.go @@ -116,7 +116,7 @@ func (b *builder) Validate() (ce *adapter.ConfigErrors) { } if resp.Status.Code != int32(codes.OK) { - ce = ce.Appendf("params", "validation error: $d/%s", resp.Status.Code, resp.Status.Message) + ce = ce.Appendf("params", "validation error: %d/%s", resp.Status.Code, resp.Status.Message) return } } diff --git a/mixer/test/client/env/ports.go b/mixer/test/client/env/ports.go index 7a2b83952b4c..6add5aead716 100644 --- a/mixer/test/client/env/ports.go +++ b/mixer/test/client/env/ports.go @@ -62,6 +62,7 @@ const ( RbacPolicyPermissiveTest GatewayTest SidecarTest + SidecarConsumerOnlyTest // The number of total tests. has to be the last one. maxTestNum diff --git a/mixer/test/client/gateway/gateway_test.go b/mixer/test/client/gateway/gateway_test.go index 42782a27f88e..5e62bcfdf76b 100644 --- a/mixer/test/client/gateway/gateway_test.go +++ b/mixer/test/client/gateway/gateway_test.go @@ -231,6 +231,9 @@ func (mock) ID(*core.Node) string { func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) { return nil, nil } +func (mock) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + return nil, nil +} func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil } func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) { return nil, nil diff --git a/mixer/test/client/pilotplugin/pilotplugin_test.go b/mixer/test/client/pilotplugin/pilotplugin_test.go index aa9fa8ca9c08..446a613054ac 100644 --- a/mixer/test/client/pilotplugin/pilotplugin_test.go +++ b/mixer/test/client/pilotplugin/pilotplugin_test.go @@ -297,6 +297,9 @@ func (mock) ID(*core.Node) string { func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) { return nil, nil } +func (mock) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + return nil, nil +} func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil } func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) { return nil, nil diff --git a/mixer/test/client/pilotplugin_mtls/pilotplugin_mtls_test.go b/mixer/test/client/pilotplugin_mtls/pilotplugin_mtls_test.go index eb87753bc222..b13199534eeb 100644 --- a/mixer/test/client/pilotplugin_mtls/pilotplugin_mtls_test.go +++ b/mixer/test/client/pilotplugin_mtls/pilotplugin_mtls_test.go @@ -312,6 +312,9 @@ func (mock) ID(*core.Node) string { func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) { return nil, nil } +func (mock) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + return nil, nil +} func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil } func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) { return nil, nil diff --git a/mixer/test/client/pilotplugin_tcp/pilotplugin_tcp_test.go b/mixer/test/client/pilotplugin_tcp/pilotplugin_tcp_test.go index 9fd72b6aea13..53bf913caf71 100644 --- a/mixer/test/client/pilotplugin_tcp/pilotplugin_tcp_test.go +++ b/mixer/test/client/pilotplugin_tcp/pilotplugin_tcp_test.go @@ -174,6 +174,9 @@ func (mock) ID(*core.Node) string { func (mock) GetProxyServiceInstances(_ *model.Proxy) ([]*model.ServiceInstance, error) { return nil, nil } +func (mock) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + return nil, nil +} func (mock) GetService(_ model.Hostname) (*model.Service, error) { return nil, nil } func (mock) InstancesByPort(_ model.Hostname, _ int, _ model.LabelsCollection) ([]*model.ServiceInstance, error) { return nil, nil diff --git a/pilot/pkg/model/context.go b/pilot/pkg/model/context.go index 105bcfaeefcf..3b7b8649428f 100644 --- a/pilot/pkg/model/context.go +++ b/pilot/pkg/model/context.go @@ -106,6 +106,9 @@ type Proxy struct { // service instances associated with the proxy ServiceInstances []*ServiceInstance + + // labels associated with the workload + WorkloadLabels LabelsCollection } // NodeType decides the responsibility of the proxy serves in the mesh @@ -184,8 +187,8 @@ func (node *Proxy) GetRouterMode() RouterMode { // Listener generation code will still use the SidecarScope object directly // as it needs the set of services for each listener port. func (node *Proxy) SetSidecarScope(ps *PushContext) { - instances := node.ServiceInstances - node.SidecarScope = ps.getSidecarScope(node, instances) + labels := node.WorkloadLabels + node.SidecarScope = ps.getSidecarScope(node, labels) } func (node *Proxy) SetServiceInstances(env *Environment) error { @@ -199,6 +202,17 @@ func (node *Proxy) SetServiceInstances(env *Environment) error { return nil } +func (node *Proxy) SetWorkloadLabels(env *Environment) error { + labels, err := env.GetProxyWorkloadLabels(node) + if err != nil { + log.Warnf("failed to get service proxy workload labels: %v, defaulting to proxy metadata", err) + labels = LabelsCollection{node.Metadata} + } + + node.WorkloadLabels = labels + return nil +} + // UnnamedNetwork is the default network that proxies in the mesh // get when they don't request a specific network view. const UnnamedNetwork = "" diff --git a/pilot/pkg/model/push_context.go b/pilot/pkg/model/push_context.go index 0e22f5d2fbcb..2795a4f4b2aa 100644 --- a/pilot/pkg/model/push_context.go +++ b/pilot/pkg/model/push_context.go @@ -429,12 +429,7 @@ func (ps *PushContext) VirtualServices(proxy *Proxy, gateways map[string]bool) [ // // Callers can check if the sidecarScope is from user generated object or not // by checking the sidecarScope.Config field, that contains the user provided config -func (ps *PushContext) getSidecarScope(proxy *Proxy, proxyInstances []*ServiceInstance) *SidecarScope { - - var workloadLabels LabelsCollection - for _, w := range proxyInstances { - workloadLabels = append(workloadLabels, w.Labels) - } +func (ps *PushContext) getSidecarScope(proxy *Proxy, workloadLabels LabelsCollection) *SidecarScope { // Find the most specific matching sidecar config from the proxy's // config namespace If none found, construct a sidecarConfig on the fly diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index e3c603e2755f..5577423a2556 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -470,6 +470,8 @@ type ServiceAttributes struct { } // ServiceDiscovery enumerates Istio service instances. +// nolint: lll +//go:generate $GOPATH/src/istio.io/istio/bin/counterfeiter.sh -o $GOPATH/src/istio.io/istio/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_discovery.go --fake-name ServiceDiscovery . ServiceDiscovery type ServiceDiscovery interface { // Services list declarations of all services in the system Services() ([]*Service, error) @@ -521,6 +523,8 @@ type ServiceDiscovery interface { // determine the intended destination of a connection without a Host header on the request. GetProxyServiceInstances(*Proxy) ([]*ServiceInstance, error) + GetProxyWorkloadLabels(*Proxy) (LabelsCollection, error) + // ManagementPorts lists set of management ports associated with an IPv4 address. // These management ports are typically used by the platform for out of band management // tasks such as health checks, etc. In a scenario where the proxy functions in the diff --git a/pilot/pkg/networking/core/v1alpha3/envoyfilter.go b/pilot/pkg/networking/core/v1alpha3/envoyfilter.go index 30b22e3b047a..3b149045c755 100644 --- a/pilot/pkg/networking/core/v1alpha3/envoyfilter.go +++ b/pilot/pkg/networking/core/v1alpha3/envoyfilter.go @@ -115,15 +115,8 @@ func insertUserFilters(in *plugin.InputParams, listener *xdsapi.Listener, // is undefined. func getUserFiltersForWorkload(in *plugin.InputParams) *networking.EnvoyFilter { env := in.Env - // collect workload labels - workloadInstances := in.ProxyInstances - var workloadLabels model.LabelsCollection - for _, w := range workloadInstances { - workloadLabels = append(workloadLabels, w.Labels) - } - - f := env.EnvoyFilter(workloadLabels) + f := env.EnvoyFilter(in.Node.WorkloadLabels) if f != nil { return f.Spec.(*networking.EnvoyFilter) } diff --git a/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_accounts.go b/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_accounts.go deleted file mode 100644 index c414c810afa6..000000000000 --- a/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_accounts.go +++ /dev/null @@ -1,103 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package fakes - -import ( - "sync" - - "istio.io/istio/pilot/pkg/model" -) - -type ServiceAccounts struct { - GetIstioServiceAccountsStub func(hostname model.Hostname, ports []int) []string - getIstioServiceAccountsMutex sync.RWMutex - getIstioServiceAccountsArgsForCall []struct { - hostname model.Hostname - ports []int - } - getIstioServiceAccountsReturns struct { - result1 []string - } - getIstioServiceAccountsReturnsOnCall map[int]struct { - result1 []string - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *ServiceAccounts) GetIstioServiceAccounts(hostname model.Hostname, ports []int) []string { - var portsCopy []int - if ports != nil { - portsCopy = make([]int, len(ports)) - copy(portsCopy, ports) - } - fake.getIstioServiceAccountsMutex.Lock() - ret, specificReturn := fake.getIstioServiceAccountsReturnsOnCall[len(fake.getIstioServiceAccountsArgsForCall)] - fake.getIstioServiceAccountsArgsForCall = append(fake.getIstioServiceAccountsArgsForCall, struct { - hostname model.Hostname - ports []int - }{hostname, portsCopy}) - fake.recordInvocation("GetIstioServiceAccounts", []interface{}{hostname, portsCopy}) - fake.getIstioServiceAccountsMutex.Unlock() - if fake.GetIstioServiceAccountsStub != nil { - return fake.GetIstioServiceAccountsStub(hostname, ports) - } - if specificReturn { - return ret.result1 - } - return fake.getIstioServiceAccountsReturns.result1 -} - -func (fake *ServiceAccounts) GetIstioServiceAccountsCallCount() int { - fake.getIstioServiceAccountsMutex.RLock() - defer fake.getIstioServiceAccountsMutex.RUnlock() - return len(fake.getIstioServiceAccountsArgsForCall) -} - -func (fake *ServiceAccounts) GetIstioServiceAccountsArgsForCall(i int) (model.Hostname, []int) { - fake.getIstioServiceAccountsMutex.RLock() - defer fake.getIstioServiceAccountsMutex.RUnlock() - return fake.getIstioServiceAccountsArgsForCall[i].hostname, fake.getIstioServiceAccountsArgsForCall[i].ports -} - -func (fake *ServiceAccounts) GetIstioServiceAccountsReturns(result1 []string) { - fake.GetIstioServiceAccountsStub = nil - fake.getIstioServiceAccountsReturns = struct { - result1 []string - }{result1} -} - -func (fake *ServiceAccounts) GetIstioServiceAccountsReturnsOnCall(i int, result1 []string) { - fake.GetIstioServiceAccountsStub = nil - if fake.getIstioServiceAccountsReturnsOnCall == nil { - fake.getIstioServiceAccountsReturnsOnCall = make(map[int]struct { - result1 []string - }) - } - fake.getIstioServiceAccountsReturnsOnCall[i] = struct { - result1 []string - }{result1} -} - -func (fake *ServiceAccounts) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.getIstioServiceAccountsMutex.RLock() - defer fake.getIstioServiceAccountsMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *ServiceAccounts) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} diff --git a/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_discovery.go b/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_discovery.go index 83c0bf957b84..3873c076e98f 100644 --- a/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_discovery.go +++ b/pilot/pkg/networking/core/v1alpha3/fakes/fake_service_discovery.go @@ -32,34 +32,6 @@ type ServiceDiscovery struct { result1 *model.Service result2 error } - GetServiceAttributesStub func(hostname model.Hostname) (*model.ServiceAttributes, error) - getServiceAttributesMutex sync.RWMutex - getServiceAttributesArgsForCall []struct { - hostname model.Hostname - } - getServiceAttributesReturns struct { - result1 *model.ServiceAttributes - result2 error - } - getServiceAttributesReturnsOnCall map[int]struct { - result1 *model.ServiceAttributes - result2 error - } - InstancesStub func(hostname model.Hostname, ports []string, labels model.LabelsCollection) ([]*model.ServiceInstance, error) - instancesMutex sync.RWMutex - instancesArgsForCall []struct { - hostname model.Hostname - ports []string - labels model.LabelsCollection - } - instancesReturns struct { - result1 []*model.ServiceInstance - result2 error - } - instancesReturnsOnCall map[int]struct { - result1 []*model.ServiceInstance - result2 error - } InstancesByPortStub func(hostname model.Hostname, servicePort int, labels model.LabelsCollection) ([]*model.ServiceInstance, error) instancesByPortMutex sync.RWMutex instancesByPortArgsForCall []struct { @@ -88,6 +60,19 @@ type ServiceDiscovery struct { result1 []*model.ServiceInstance result2 error } + GetProxyWorkloadLabelsStub func(*model.Proxy) (model.LabelsCollection, error) + getProxyWorkloadLabelsMutex sync.RWMutex + getProxyWorkloadLabelsArgsForCall []struct { + arg1 *model.Proxy + } + getProxyWorkloadLabelsReturns struct { + result1 model.LabelsCollection + result2 error + } + getProxyWorkloadLabelsReturnsOnCall map[int]struct { + result1 model.LabelsCollection + result2 error + } ManagementPortsStub func(addr string) model.PortList managementPortsMutex sync.RWMutex managementPortsArgsForCall []struct { @@ -110,10 +95,20 @@ type ServiceDiscovery struct { workloadHealthCheckInfoReturnsOnCall map[int]struct { result1 model.ProbeList } + GetIstioServiceAccountsStub func(hostname model.Hostname, ports []int) []string + getIstioServiceAccountsMutex sync.RWMutex + getIstioServiceAccountsArgsForCall []struct { + hostname model.Hostname + ports []int + } + getIstioServiceAccountsReturns struct { + result1 []string + } + getIstioServiceAccountsReturnsOnCall map[int]struct { + result1 []string + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex - - ServiceAccounts } func (fake *ServiceDiscovery) Services() ([]*model.Service, error) { @@ -210,74 +205,6 @@ func (fake *ServiceDiscovery) GetServiceReturnsOnCall(i int, result1 *model.Serv }{result1, result2} } -func (fake *ServiceDiscovery) GetServiceAttributesCallCount() int { - fake.getServiceAttributesMutex.RLock() - defer fake.getServiceAttributesMutex.RUnlock() - return len(fake.getServiceAttributesArgsForCall) -} - -func (fake *ServiceDiscovery) GetServiceAttributesArgsForCall(i int) model.Hostname { - fake.getServiceAttributesMutex.RLock() - defer fake.getServiceAttributesMutex.RUnlock() - return fake.getServiceAttributesArgsForCall[i].hostname -} - -func (fake *ServiceDiscovery) GetServiceAttributesReturns(result1 *model.ServiceAttributes, result2 error) { - fake.GetServiceAttributesStub = nil - fake.getServiceAttributesReturns = struct { - result1 *model.ServiceAttributes - result2 error - }{result1, result2} -} - -func (fake *ServiceDiscovery) GetServiceAttributesReturnsOnCall(i int, result1 *model.ServiceAttributes, result2 error) { - fake.GetServiceAttributesStub = nil - if fake.getServiceAttributesReturnsOnCall == nil { - fake.getServiceAttributesReturnsOnCall = make(map[int]struct { - result1 *model.ServiceAttributes - result2 error - }) - } - fake.getServiceAttributesReturnsOnCall[i] = struct { - result1 *model.ServiceAttributes - result2 error - }{result1, result2} -} - -func (fake *ServiceDiscovery) InstancesCallCount() int { - fake.instancesMutex.RLock() - defer fake.instancesMutex.RUnlock() - return len(fake.instancesArgsForCall) -} - -func (fake *ServiceDiscovery) InstancesArgsForCall(i int) (model.Hostname, []string, model.LabelsCollection) { - fake.instancesMutex.RLock() - defer fake.instancesMutex.RUnlock() - return fake.instancesArgsForCall[i].hostname, fake.instancesArgsForCall[i].ports, fake.instancesArgsForCall[i].labels -} - -func (fake *ServiceDiscovery) InstancesReturns(result1 []*model.ServiceInstance, result2 error) { - fake.InstancesStub = nil - fake.instancesReturns = struct { - result1 []*model.ServiceInstance - result2 error - }{result1, result2} -} - -func (fake *ServiceDiscovery) InstancesReturnsOnCall(i int, result1 []*model.ServiceInstance, result2 error) { - fake.InstancesStub = nil - if fake.instancesReturnsOnCall == nil { - fake.instancesReturnsOnCall = make(map[int]struct { - result1 []*model.ServiceInstance - result2 error - }) - } - fake.instancesReturnsOnCall[i] = struct { - result1 []*model.ServiceInstance - result2 error - }{result1, result2} -} - func (fake *ServiceDiscovery) InstancesByPort(hostname model.Hostname, servicePort int, labels model.LabelsCollection) ([]*model.ServiceInstance, error) { fake.instancesByPortMutex.Lock() ret, specificReturn := fake.instancesByPortReturnsOnCall[len(fake.instancesByPortArgsForCall)] @@ -382,6 +309,57 @@ func (fake *ServiceDiscovery) GetProxyServiceInstancesReturnsOnCall(i int, resul }{result1, result2} } +func (fake *ServiceDiscovery) GetProxyWorkloadLabels(arg1 *model.Proxy) (model.LabelsCollection, error) { + fake.getProxyWorkloadLabelsMutex.Lock() + ret, specificReturn := fake.getProxyWorkloadLabelsReturnsOnCall[len(fake.getProxyWorkloadLabelsArgsForCall)] + fake.getProxyWorkloadLabelsArgsForCall = append(fake.getProxyWorkloadLabelsArgsForCall, struct { + arg1 *model.Proxy + }{arg1}) + fake.recordInvocation("GetProxyWorkloadLabels", []interface{}{arg1}) + fake.getProxyWorkloadLabelsMutex.Unlock() + if fake.GetProxyWorkloadLabelsStub != nil { + return fake.GetProxyWorkloadLabelsStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fake.getProxyWorkloadLabelsReturns.result1, fake.getProxyWorkloadLabelsReturns.result2 +} + +func (fake *ServiceDiscovery) GetProxyWorkloadLabelsCallCount() int { + fake.getProxyWorkloadLabelsMutex.RLock() + defer fake.getProxyWorkloadLabelsMutex.RUnlock() + return len(fake.getProxyWorkloadLabelsArgsForCall) +} + +func (fake *ServiceDiscovery) GetProxyWorkloadLabelsArgsForCall(i int) *model.Proxy { + fake.getProxyWorkloadLabelsMutex.RLock() + defer fake.getProxyWorkloadLabelsMutex.RUnlock() + return fake.getProxyWorkloadLabelsArgsForCall[i].arg1 +} + +func (fake *ServiceDiscovery) GetProxyWorkloadLabelsReturns(result1 model.LabelsCollection, result2 error) { + fake.GetProxyWorkloadLabelsStub = nil + fake.getProxyWorkloadLabelsReturns = struct { + result1 model.LabelsCollection + result2 error + }{result1, result2} +} + +func (fake *ServiceDiscovery) GetProxyWorkloadLabelsReturnsOnCall(i int, result1 model.LabelsCollection, result2 error) { + fake.GetProxyWorkloadLabelsStub = nil + if fake.getProxyWorkloadLabelsReturnsOnCall == nil { + fake.getProxyWorkloadLabelsReturnsOnCall = make(map[int]struct { + result1 model.LabelsCollection + result2 error + }) + } + fake.getProxyWorkloadLabelsReturnsOnCall[i] = struct { + result1 model.LabelsCollection + result2 error + }{result1, result2} +} + func (fake *ServiceDiscovery) ManagementPorts(addr string) model.PortList { fake.managementPortsMutex.Lock() ret, specificReturn := fake.managementPortsReturnsOnCall[len(fake.managementPortsArgsForCall)] @@ -478,6 +456,60 @@ func (fake *ServiceDiscovery) WorkloadHealthCheckInfoReturnsOnCall(i int, result }{result1} } +func (fake *ServiceDiscovery) GetIstioServiceAccounts(hostname model.Hostname, ports []int) []string { + var portsCopy []int + if ports != nil { + portsCopy = make([]int, len(ports)) + copy(portsCopy, ports) + } + fake.getIstioServiceAccountsMutex.Lock() + ret, specificReturn := fake.getIstioServiceAccountsReturnsOnCall[len(fake.getIstioServiceAccountsArgsForCall)] + fake.getIstioServiceAccountsArgsForCall = append(fake.getIstioServiceAccountsArgsForCall, struct { + hostname model.Hostname + ports []int + }{hostname, portsCopy}) + fake.recordInvocation("GetIstioServiceAccounts", []interface{}{hostname, portsCopy}) + fake.getIstioServiceAccountsMutex.Unlock() + if fake.GetIstioServiceAccountsStub != nil { + return fake.GetIstioServiceAccountsStub(hostname, ports) + } + if specificReturn { + return ret.result1 + } + return fake.getIstioServiceAccountsReturns.result1 +} + +func (fake *ServiceDiscovery) GetIstioServiceAccountsCallCount() int { + fake.getIstioServiceAccountsMutex.RLock() + defer fake.getIstioServiceAccountsMutex.RUnlock() + return len(fake.getIstioServiceAccountsArgsForCall) +} + +func (fake *ServiceDiscovery) GetIstioServiceAccountsArgsForCall(i int) (model.Hostname, []int) { + fake.getIstioServiceAccountsMutex.RLock() + defer fake.getIstioServiceAccountsMutex.RUnlock() + return fake.getIstioServiceAccountsArgsForCall[i].hostname, fake.getIstioServiceAccountsArgsForCall[i].ports +} + +func (fake *ServiceDiscovery) GetIstioServiceAccountsReturns(result1 []string) { + fake.GetIstioServiceAccountsStub = nil + fake.getIstioServiceAccountsReturns = struct { + result1 []string + }{result1} +} + +func (fake *ServiceDiscovery) GetIstioServiceAccountsReturnsOnCall(i int, result1 []string) { + fake.GetIstioServiceAccountsStub = nil + if fake.getIstioServiceAccountsReturnsOnCall == nil { + fake.getIstioServiceAccountsReturnsOnCall = make(map[int]struct { + result1 []string + }) + } + fake.getIstioServiceAccountsReturnsOnCall[i] = struct { + result1 []string + }{result1} +} + func (fake *ServiceDiscovery) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -485,18 +517,18 @@ func (fake *ServiceDiscovery) Invocations() map[string][][]interface{} { defer fake.servicesMutex.RUnlock() fake.getServiceMutex.RLock() defer fake.getServiceMutex.RUnlock() - fake.getServiceAttributesMutex.RLock() - defer fake.getServiceAttributesMutex.RUnlock() - fake.instancesMutex.RLock() - defer fake.instancesMutex.RUnlock() fake.instancesByPortMutex.RLock() defer fake.instancesByPortMutex.RUnlock() fake.getProxyServiceInstancesMutex.RLock() defer fake.getProxyServiceInstancesMutex.RUnlock() + fake.getProxyWorkloadLabelsMutex.RLock() + defer fake.getProxyWorkloadLabelsMutex.RUnlock() fake.managementPortsMutex.RLock() defer fake.managementPortsMutex.RUnlock() fake.workloadHealthCheckInfoMutex.RLock() defer fake.workloadHealthCheckInfoMutex.RUnlock() + fake.getIstioServiceAccountsMutex.RLock() + defer fake.getIstioServiceAccountsMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/pilot/pkg/networking/core/v1alpha3/listener_test.go b/pilot/pkg/networking/core/v1alpha3/listener_test.go index fada40250ab9..66db2824c9b8 100644 --- a/pilot/pkg/networking/core/v1alpha3/listener_test.go +++ b/pilot/pkg/networking/core/v1alpha3/listener_test.go @@ -121,8 +121,10 @@ func TestInboundListenerConfig_HTTP(t *testing.T) { // Add a service and verify it's config testInboundListenerConfig(t, buildService("test.com", wildcardIP, model.ProtocolHTTP, tnow)) + testInboundListenerConfigWithoutServices(t) testInboundListenerConfigWithSidecar(t, buildService("test.com", wildcardIP, model.ProtocolHTTP, tnow)) + testInboundListenerConfigWithSidecarWithoutServices(t) } func TestOutboundListenerConfig_WithSidecar(t *testing.T) { @@ -180,6 +182,15 @@ func testInboundListenerConfig(t *testing.T, services ...*model.Service) { } } +func testInboundListenerConfigWithoutServices(t *testing.T) { + t.Helper() + p := &fakePlugin{} + listeners := buildInboundListeners(p, nil) + if expected := 0; len(listeners) != expected { + t.Fatalf("expected %d listeners, found %d", expected, len(listeners)) + } +} + func testInboundListenerConfigWithSidecar(t *testing.T, services ...*model.Service) { t.Helper() p := &fakePlugin{} @@ -212,6 +223,34 @@ func testInboundListenerConfigWithSidecar(t *testing.T, services ...*model.Servi } } +func testInboundListenerConfigWithSidecarWithoutServices(t *testing.T) { + t.Helper() + p := &fakePlugin{} + sidecarConfig := &model.Config{ + ConfigMeta: model.ConfigMeta{ + Name: "foo-without-service", + Namespace: "not-default", + }, + Spec: &networking.Sidecar{ + Ingress: []*networking.IstioIngressListener{ + { + Port: &networking.Port{ + Number: 8080, + Protocol: "HTTP", + Name: "uds", + }, + Bind: "1.1.1.1", + DefaultEndpoint: "127.0.0.1:80", + }, + }, + }, + } + listeners := buildInboundListeners(p, sidecarConfig) + if expected := 0; len(listeners) != expected { + t.Fatalf("expected %d listeners, found %d", expected, len(listeners)) + } +} + func testOutboundListenerConfigWithSidecar(t *testing.T, services ...*model.Service) { t.Helper() p := &fakePlugin{} diff --git a/pilot/pkg/proxy/envoy/v2/ads.go b/pilot/pkg/proxy/envoy/v2/ads.go index 5e8a4a345093..ba8e789c1c0f 100644 --- a/pilot/pkg/proxy/envoy/v2/ads.go +++ b/pilot/pkg/proxy/envoy/v2/ads.go @@ -626,6 +626,9 @@ func (s *DiscoveryServer) initConnectionNode(discReq *xdsapi.DiscoveryRequest, c if err := nt.SetServiceInstances(s.Env); err != nil { return err } + if err := nt.SetWorkloadLabels(s.Env); err != nil { + return err + } // If the proxy has no service instances and its a gateway, kill the XDS connection as we cannot // serve any gateway config if we dont know the proxy's service instances if nt.Type == model.Router && (nt.ServiceInstances == nil || len(nt.ServiceInstances) == 0) { @@ -677,6 +680,10 @@ func (s *DiscoveryServer) pushConnection(con *XdsConnection, pushEv *XdsEvent) e return nil } + if err := con.modelNode.SetWorkloadLabels(s.Env); err != nil { + return err + } + if err := con.modelNode.SetServiceInstances(pushEv.push.Env); err != nil { return err } diff --git a/pilot/pkg/proxy/envoy/v2/lds_test.go b/pilot/pkg/proxy/envoy/v2/lds_test.go index 5232240d38e6..a91a789ad504 100644 --- a/pilot/pkg/proxy/envoy/v2/lds_test.go +++ b/pilot/pkg/proxy/envoy/v2/lds_test.go @@ -19,9 +19,14 @@ import ( "testing" "time" + xdsapi "github.com/envoyproxy/go-control-plane/envoy/api/v2" + xdsapi_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener" + xdsapi_http_connection_manager "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2" + testenv "istio.io/istio/mixer/test/client/env" "istio.io/istio/pilot/pkg/bootstrap" "istio.io/istio/pilot/pkg/model" + v2 "istio.io/istio/pilot/pkg/proxy/envoy/v2" "istio.io/istio/pkg/adsc" "istio.io/istio/pkg/test/env" "istio.io/istio/tests/util" @@ -398,6 +403,209 @@ func TestLDS(t *testing.T) { // TODO: dynamic checks ( see EDS ) } +// TestLDS using sidecar scoped on workload without Service +func TestLDSWithSidecarForWorkloadWithoutService(t *testing.T) { + server, tearDown := util.EnsureTestServer(func(args *bootstrap.PilotArgs) { + args.Plugins = bootstrap.DefaultPlugins + args.Config.FileDir = env.IstioSrc + "/tests/testdata/networking/sidecar-without-service" + args.Mesh.MixerAddress = "" + args.Mesh.RdsRefreshDelay = nil + args.Mesh.ConfigFile = env.IstioSrc + "/tests/testdata/networking/sidecar-without-service/mesh.yaml" + args.Service.Registries = []string{} + }) + registry := memServiceDiscovery(server, t) + registry.AddWorkload("98.1.1.1", model.Labels{"app": "consumeronly"}) // These labels must match the sidecars workload selector + + testEnv = testenv.NewTestSetup(testenv.SidecarConsumerOnlyTest, t) + testEnv.Ports().PilotGrpcPort = uint16(util.MockPilotGrpcPort) + testEnv.Ports().PilotHTTPPort = uint16(util.MockPilotHTTPPort) + testEnv.IstioSrc = env.IstioSrc + testEnv.IstioOut = env.IstioOut + + server.EnvoyXdsServer.ConfigUpdate(true) + defer tearDown() + + adsResponse, err := adsc.Dial(util.MockPilotGrpcAddr, "", &adsc.Config{ + Meta: map[string]string{ + model.NodeMetadataConfigNamespace: "consumerns", + model.NodeMetadataInstanceIPs: "98.1.1.1", // as service instance of ingress gateway + model.NodeMetadataIstioProxyVersion: "1.1.0", + }, + IP: "98.1.1.1", + Namespace: "consumerns", // namespace must match the namespace of the sidecar in the configs.yaml + NodeType: "sidecar", + }) + + if err != nil { + t.Fatal(err) + } + defer adsResponse.Close() + + adsResponse.DumpCfg = true + adsResponse.Watch() + + _, err = adsResponse.Wait("lds", 10*time.Second) + if err != nil { + t.Fatal("Failed to receive LDS response", err) + return + } + + // Expect 1 HTTP listeners for 8081 + if len(adsResponse.HTTPListeners) != 1 { + t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.HTTPListeners)) + } + + // TODO: This is flimsy. The ADSC code treats any listener with http connection manager as a HTTP listener + // instead of looking at it as a listener with multiple filter chains + if l := adsResponse.HTTPListeners["0.0.0.0_8081"]; l != nil { + if len(l.FilterChains) != 1 { + t.Fatalf("Expected 1 filter chains, got %d", len(l.FilterChains)) + } + } else { + t.Fatal("Expected listener for 0.0.0.0_8081") + } + + // Expect only one EDS cluster for http1.ns1.svc.cluster.local + if len(adsResponse.EDSClusters) != 1 { + t.Fatalf("Expected 1 eds cluster, got %d", len(adsResponse.EDSClusters)) + } + if cluster, ok := adsResponse.EDSClusters["outbound|8081||http1.ns1.svc.cluster.local"]; !ok { + t.Fatalf("Expected EDS cluster outbound|8081||http1.ns1.svc.cluster.local, got %v", cluster.Name) + } +} + +// TestLDS using default sidecar in root namespace +func TestLDSEnvoyFilterWithWorkloadSelector(t *testing.T) { + server, tearDown := util.EnsureTestServer(func(args *bootstrap.PilotArgs) { + args.Plugins = bootstrap.DefaultPlugins + args.Config.FileDir = env.IstioSrc + "/tests/testdata/networking/envoyfilter-without-service" + args.Mesh.MixerAddress = "" + args.Mesh.RdsRefreshDelay = nil + args.Mesh.ConfigFile = env.IstioSrc + "/tests/testdata/networking/envoyfilter-without-service/mesh.yaml" + args.Service.Registries = []string{} + }) + registry := memServiceDiscovery(server, t) + // The labels of 98.1.1.1 must match the envoyfilter workload selector + registry.AddWorkload("98.1.1.1", model.Labels{"app": "envoyfilter-test-app", "some": "otherlabel"}) + registry.AddWorkload("98.1.1.2", model.Labels{"app": "no-envoyfilter-test-app"}) + registry.AddWorkload("98.1.1.3", model.Labels{}) + + testEnv = testenv.NewTestSetup(testenv.SidecarConsumerOnlyTest, t) + testEnv.Ports().PilotGrpcPort = uint16(util.MockPilotGrpcPort) + testEnv.Ports().PilotHTTPPort = uint16(util.MockPilotHTTPPort) + testEnv.IstioSrc = env.IstioSrc + testEnv.IstioOut = env.IstioOut + + server.EnvoyXdsServer.ConfigUpdate(true) + defer tearDown() + + tests := []struct { + name string + ip string + expectLuaFilter bool + }{ + { + name: "Add filter with matching labels to sidecar", + ip: "98.1.1.1", + expectLuaFilter: true, + }, + { + name: "Ignore filter with not matching labels to sidecar", + ip: "98.1.1.2", + expectLuaFilter: false, + }, + { + name: "Ignore filter with empty labels to sidecar", + ip: "98.1.1.3", + expectLuaFilter: false, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + adsResponse, err := adsc.Dial(util.MockPilotGrpcAddr, "", &adsc.Config{ + Meta: map[string]string{ + model.NodeMetadataConfigNamespace: "consumerns", + model.NodeMetadataInstanceIPs: test.ip, // as service instance of ingress gateway + model.NodeMetadataIstioProxyVersion: "1.1.0", + }, + IP: test.ip, + Namespace: "consumerns", // namespace must match the namespace of the sidecar in the configs.yaml + NodeType: "sidecar", + }) + if err != nil { + t.Fatal(err) + } + defer adsResponse.Close() + + adsResponse.DumpCfg = false + adsResponse.Watch() + _, err = adsResponse.Wait("lds", 100*time.Second) + if err != nil { + t.Fatal("Failed to receive LDS response", err) + return + } + + // Expect 1 HTTP listeners for 8081 + if len(adsResponse.HTTPListeners) != 1 { + t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.HTTPListeners)) + } + // TODO: This is flimsy. The ADSC code treats any listener with http connection manager as a HTTP listener + // instead of looking at it as a listener with multiple filter chains + l := adsResponse.HTTPListeners["0.0.0.0_8081"] + + expectLuaFilter(t, l, test.expectLuaFilter) + }) + } +} + +func expectLuaFilter(t *testing.T, l *xdsapi.Listener, expected bool) { + + if l != nil { + if len(l.FilterChains) != 1 { + t.Fatalf("Expected 1 filter chains, got %d", len(l.FilterChains)) + } + if len(l.FilterChains[0].Filters) != 1 { + t.Fatalf("Expected 1 filter in first filter chain, got %d", len(l.FilterChains)) + } + filter := l.FilterChains[0].Filters[0] + if filter.Name != "envoy.http_connection_manager" { + t.Fatalf("Expected HTTP connection, found %v", l.FilterChains[0].Filters[0].Name) + } + httpCfg, ok := filter.ConfigType.(*xdsapi_listener.Filter_TypedConfig) + if !ok { + t.Fatalf("Expected Http Connection Manager Config Filter_TypedConfig, found %T", filter.ConfigType) + } + connectionManagerCfg := xdsapi_http_connection_manager.HttpConnectionManager{} + err := connectionManagerCfg.Unmarshal(httpCfg.TypedConfig.GetValue()) + if err != nil { + t.Fatalf("Could not deserialize http connection manager config: %v", err) + } + found := false + for _, filter := range connectionManagerCfg.HttpFilters { + if filter.Name == "envoy.lua" { + found = true + } + } + if expected != found { + t.Fatalf("Expected Lua filter: %v, found: %v", expected, found) + } + } +} + +func memServiceDiscovery(server *bootstrap.Server, t *testing.T) *v2.MemServiceDiscovery { + index, found := server.ServiceController.GetRegistryIndex("v2-debug") + if !found { + t.Fatal("Could not find Mock ServiceRegistry") + } + registry, ok := server.ServiceController.GetRegistries()[index].ServiceDiscovery.(*v2.MemServiceDiscovery) + if !ok { + t.Fatal("Unexpected type of Mock ServiceRegistry") + } + return registry +} + // TODO: helper to test the http listener content // - file access log // - generate request id diff --git a/pilot/pkg/proxy/envoy/v2/mem.go b/pilot/pkg/proxy/envoy/v2/mem.go index bd6fd61898c5..b7390fc0e8fb 100644 --- a/pilot/pkg/proxy/envoy/v2/mem.go +++ b/pilot/pkg/proxy/envoy/v2/mem.go @@ -73,6 +73,9 @@ type MemServiceDiscovery struct { controller model.Controller ClusterID string + // Used by GetProxyWorkloadLabels + ip2workloadLabels map[string]*model.Labels + // XDSUpdater will push EDS changes to the ADS model. EDSUpdater model.XDSUpdater @@ -89,6 +92,7 @@ func NewMemServiceDiscovery(services map[model.Hostname]*model.Service, versions instancesByPortNum: map[string][]*model.ServiceInstance{}, instancesByPortName: map[string][]*model.ServiceInstance{}, ip2instance: map[string][]*model.ServiceInstance{}, + ip2workloadLabels: map[string]*model.Labels{}, } } @@ -100,6 +104,10 @@ func (sd *MemServiceDiscovery) ClearErrors() { sd.GetProxyServiceInstancesError = nil } +func (sd *MemServiceDiscovery) AddWorkload(ip string, labels model.Labels) { + sd.ip2workloadLabels[ip] = &labels +} + // AddHTTPService is a helper to add a service of type http, named 'http-main', with the // specified vip and port. func (sd *MemServiceDiscovery) AddHTTPService(name, vip string, port int) { @@ -320,6 +328,19 @@ func (sd *MemServiceDiscovery) GetProxyServiceInstances(node *model.Proxy) ([]*m return out, sd.GetProxyServiceInstancesError } +func (sd *MemServiceDiscovery) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + sd.mutex.Lock() + defer sd.mutex.Unlock() + out := make(model.LabelsCollection, 0) + + for _, ip := range proxy.IPAddresses { + if labels, found := sd.ip2workloadLabels[ip]; found { + out = append(out, *labels) + } + } + return out, nil +} + // ManagementPorts implements discovery interface func (sd *MemServiceDiscovery) ManagementPorts(addr string) model.PortList { sd.mutex.Lock() diff --git a/pilot/pkg/serviceregistry/aggregate/controller.go b/pilot/pkg/serviceregistry/aggregate/controller.go index 2d669c364aeb..84a6d42dd673 100644 --- a/pilot/pkg/serviceregistry/aggregate/controller.go +++ b/pilot/pkg/serviceregistry/aggregate/controller.go @@ -234,7 +234,32 @@ func (c *Controller) GetProxyServiceInstances(node *model.Proxy) ([]*model.Servi if len(out) > 0 { if errs != nil { - log.Warnf("GetProxyServiceInstances() found match but encountered an error: %v", errs) + log.Debugf("GetProxyServiceInstances() found match but encountered an error: %v", errs) + } + return out, nil + } + + return out, errs +} + +func (c *Controller) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + out := make(model.LabelsCollection, 0) + var errs error + // It doesn't make sense for a single proxy to be found in more than one registry. + // TODO: if otherwise, warning or else what to do about it. + for _, r := range c.GetRegistries() { + labels, err := r.GetProxyWorkloadLabels(proxy) + if err != nil { + errs = multierror.Append(errs, err) + } else if len(labels) > 0 { + out = append(out, labels...) + break + } + } + + if len(out) > 0 { + if errs != nil { + log.Warnf("GetProxyWorkloadLabels() found match but encountered an error: %v", errs) } return out, nil } diff --git a/pilot/pkg/serviceregistry/consul/controller.go b/pilot/pkg/serviceregistry/consul/controller.go index c8f7443c1b9a..a66c34ee9dd2 100644 --- a/pilot/pkg/serviceregistry/consul/controller.go +++ b/pilot/pkg/serviceregistry/consul/controller.go @@ -179,6 +179,37 @@ func (c *Controller) GetProxyServiceInstances(node *model.Proxy) ([]*model.Servi return out, nil } +func (c *Controller) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + data, err := c.getServices() + if err != nil { + return nil, err + } + out := make(model.LabelsCollection, 0) + for svcName := range data { + endpoints, err := c.getCatalogService(svcName, nil) + if err != nil { + return nil, err + } + for _, endpoint := range endpoints { + addr := endpoint.ServiceAddress + if addr == "" { + addr = endpoint.Address + } + if len(proxy.IPAddresses) > 0 { + for _, ipAddress := range proxy.IPAddresses { + if ipAddress == addr { + labels := convertLabels(endpoint.ServiceTags) + out = append(out, labels) + break + } + } + } + } + } + + return out, nil +} + // Run all controllers until a signal is received func (c *Controller) Run(stop <-chan struct{}) { c.monitor.Start(stop) diff --git a/pilot/pkg/serviceregistry/consul/controller_test.go b/pilot/pkg/serviceregistry/consul/controller_test.go index 10b80ae43b08..daca90ac78d0 100644 --- a/pilot/pkg/serviceregistry/consul/controller_test.go +++ b/pilot/pkg/serviceregistry/consul/controller_test.go @@ -19,6 +19,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" "sync" "testing" "time" @@ -437,3 +438,61 @@ func TestGetProxyServiceInstancesWithMultiIPs(t *testing.T) { services[0].Service.Hostname, serviceHostname("productpage")) } } + +func TestGetProxyWorkloadLabels(t *testing.T) { + ts := newServer() + defer ts.Server.Close() + controller, err := NewController(ts.Server.URL, 3*time.Second) + if err != nil { + t.Errorf("could not create Consul Controller: %v", err) + } + + tests := []struct { + name string + ips []string + expected model.LabelsCollection + }{ + { + name: "Rating", + ips: []string{"10.78.11.18", "172.19.0.12"}, + expected: model.LabelsCollection{{"version": "v1"}}, + }, + { + name: "No proxy ip", + ips: nil, + expected: model.LabelsCollection{}, + }, + { + name: "No match", + ips: []string{"1.2.3.4", "2.3.4.5"}, + expected: model.LabelsCollection{}, + }, + { + name: "Only match on Service Address", + ips: []string{"172.19.0.5"}, + expected: model.LabelsCollection{}, + }, + { + name: "Match multiple services", + ips: []string{"172.19.0.7", "172.19.0.8"}, + expected: model.LabelsCollection{{"version": "v2"}, {"version": "v3"}}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + labels, err := controller.GetProxyWorkloadLabels(&model.Proxy{IPAddresses: test.ips}) + + if err != nil { + t.Errorf("client encountered error during GetProxyWorkloadLabels(): %v", err) + } + if labels == nil { + t.Error("labels should exist") + } + + if !reflect.DeepEqual(labels, test.expected) { + t.Errorf("GetProxyWorkloadLabels() wrong labels => returned %#v, want %#v", labels, test.expected) + } + }) + } +} diff --git a/pilot/pkg/serviceregistry/external/servicediscovery.go b/pilot/pkg/serviceregistry/external/servicediscovery.go index e5c9b805c054..ff9c814b69e8 100644 --- a/pilot/pkg/serviceregistry/external/servicediscovery.go +++ b/pilot/pkg/serviceregistry/external/servicediscovery.go @@ -238,6 +238,24 @@ func (d *ServiceEntryStore) GetProxyServiceInstances(node *model.Proxy) ([]*mode return out, nil } +func (d *ServiceEntryStore) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + d.update() + d.storeMutex.RLock() + defer d.storeMutex.RUnlock() + + out := make(model.LabelsCollection, 0) + + for _, ip := range proxy.IPAddresses { + instances, found := d.ip2instance[ip] + if found { + for _, instance := range instances { + out = append(out, instance.Labels) + } + } + } + return out, nil +} + // GetIstioServiceAccounts implements model.ServiceAccounts operation TODOg func (d *ServiceEntryStore) GetIstioServiceAccounts(hostname model.Hostname, ports []int) []string { //for service entries, there is no istio auth, no service accounts, etc. It is just a diff --git a/pilot/pkg/serviceregistry/kube/controller.go b/pilot/pkg/serviceregistry/kube/controller.go index 125f79126b6d..0b6605737235 100644 --- a/pilot/pkg/serviceregistry/kube/controller.go +++ b/pilot/pkg/serviceregistry/kube/controller.go @@ -619,6 +619,17 @@ func (c *Controller) getProxyServiceInstancesByPod(pod *v1.Pod, service *v1.Serv return out } +func (c *Controller) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + // There is only one IP for kube registry + proxyIP := proxy.IPAddresses[0] + + pod := c.pods.getPodByIP(proxyIP) + if pod != nil { + return model.LabelsCollection{pod.Labels}, nil + } + return nil, nil +} + func (c *Controller) getEndpoints(ip string, endpointPort int32, svcPort *model.Port, svc *model.Service) *model.ServiceInstance { labels, _ := c.pods.labelsByIP(ip) pod := c.pods.getPodByIP(ip) diff --git a/pilot/pkg/serviceregistry/memory/discovery.go b/pilot/pkg/serviceregistry/memory/discovery.go index 774fa262aaf6..96eee43730b0 100644 --- a/pilot/pkg/serviceregistry/memory/discovery.go +++ b/pilot/pkg/serviceregistry/memory/discovery.go @@ -245,6 +245,14 @@ func (sd *ServiceDiscovery) GetProxyServiceInstances(node *model.Proxy) ([]*mode return out, sd.GetProxyServiceInstancesError } +func (sd *ServiceDiscovery) GetProxyWorkloadLabels(proxy *model.Proxy) (model.LabelsCollection, error) { + if sd.GetProxyServiceInstancesError != nil { + return nil, sd.GetProxyServiceInstancesError + } + // no useful labels from the ServiceInstances created by MakeInstance() + return nil, nil +} + // ManagementPorts implements discovery interface func (sd *ServiceDiscovery) ManagementPorts(addr string) model.PortList { return model.PortList{{ diff --git a/tests/testdata/networking/envoyfilter-without-service/configs.yaml b/tests/testdata/networking/envoyfilter-without-service/configs.yaml new file mode 100644 index 000000000000..062fa85e8781 --- /dev/null +++ b/tests/testdata/networking/envoyfilter-without-service/configs.yaml @@ -0,0 +1,106 @@ +# Authentication policy to enable mutual TLS for all services (that have sidecar) in the mesh. +apiVersion: authentication.istio.io/v1alpha1 +kind: MeshPolicy +metadata: + name: default + namespace: istio-config +spec: + peers: + - mtls: {} +--- +# Corresponding destination rule to configure client side to use mutual TLS when talking to +# any service (host) in the mesh. +apiVersion: networking.istio.io/v1alpha3 +kind: DestinationRule +metadata: + name: default + namespace: istio-config +spec: + host: "*.local" + trafficPolicy: + tls: + mode: ISTIO_MUTUAL +--- +# Service entries for istio-policy and telemetry +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: istio-telemetry + namespace: istio-system +spec: + hosts: + - istio-telemetry.istio-system.svc.cluster.local + addresses: + - 1.1.1.1 + ports: + - number: 15004 + name: mtls + protocol: TCP + location: MESH_INTERNAL + resolution: DNS + exportTo: + - '*' +--- +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: istio-policy + namespace: istio-system +spec: + hosts: + - istio-policy.istio-system.svc.cluster.local + addresses: + - 1.1.1.2 + ports: + - number: 15004 + name: mtls + protocol: TCP + location: MESH_INTERNAL + resolution: DNS + exportTo: + - '*' +--- +# Services and configs in ns1 namespace +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: http1 + namespace: ns1 +spec: + hosts: + - http1.ns1.svc.cluster.local + addresses: + - 2.1.1.1 + ports: + - number: 8081 + name: http + protocol: HTTP2 + location: MESH_INTERNAL + resolution: STATIC + endpoints: + - address: 100.1.1.1 + labels: + version: v1 + ports: + http: 8080 +--- +# The sidecar for the consumer only application +apiVersion: networking.istio.io/v1alpha3 +kind: EnvoyFilter +metadata: + name: test-lua + #namespace: sr-test +spec: + workloadLabels: + app: envoyfilter-test-app + filters: + - listenerMatch: + listenerType: SIDECAR_OUTBOUND + listenerProtocol: HTTP + filterName: envoy.lua + filterType: HTTP + filterConfig: + inlineCode: | + function envoy_on_request(request_handle) + request_handle:logWarn("Hello World") + end \ No newline at end of file diff --git a/tests/testdata/networking/envoyfilter-without-service/mesh.yaml b/tests/testdata/networking/envoyfilter-without-service/mesh.yaml new file mode 100644 index 000000000000..ee316432d23a --- /dev/null +++ b/tests/testdata/networking/envoyfilter-without-service/mesh.yaml @@ -0,0 +1,10 @@ +mixerCheckServer: istio-policy.istio-system.svc.cluster.local:15004 +mixerReportServer: istio-telemetry.istio-system.svc.cluster.local:15004 + +rootNamespace: istio-config +defaultServiceExportTo: + - "*" +defaultVirtualServiceExportTo: + - "*" +defaultDestinationRuleExportTo: + - "*" diff --git a/tests/testdata/networking/sidecar-without-service/configs.yaml b/tests/testdata/networking/sidecar-without-service/configs.yaml new file mode 100644 index 000000000000..3be87a8f5f59 --- /dev/null +++ b/tests/testdata/networking/sidecar-without-service/configs.yaml @@ -0,0 +1,142 @@ +apiVersion: networking.istio.io/v1alpha3 +kind: Sidecar +metadata: + name: default-sidecar-scope + namespace: istio-config +spec: + egress: + - hosts: + - "./*" + - "istio-system/istio-telemetry.istio-system.svc.cluster.local" + - "istio-system/istio-policy.istio-system.svc.cluster.local" +--- +# Authentication policy to enable mutual TLS for all services (that have sidecar) in the mesh. +apiVersion: authentication.istio.io/v1alpha1 +kind: MeshPolicy +metadata: + name: default + namespace: istio-config +spec: + peers: + - mtls: {} +--- +# Corresponding destination rule to configure client side to use mutual TLS when talking to +# any service (host) in the mesh. +apiVersion: networking.istio.io/v1alpha3 +kind: DestinationRule +metadata: + name: default + namespace: istio-config +spec: + host: "*.local" + trafficPolicy: + tls: + mode: ISTIO_MUTUAL +--- +# Service entries for istio-policy and telemetry +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: istio-telemetry + namespace: istio-system +spec: + hosts: + - istio-telemetry.istio-system.svc.cluster.local + addresses: + - 1.1.1.1 + ports: + - number: 15004 + name: mtls + protocol: TCP + location: MESH_INTERNAL + resolution: DNS + exportTo: + - '*' +--- +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: istio-policy + namespace: istio-system +spec: + hosts: + - istio-policy.istio-system.svc.cluster.local + addresses: + - 1.1.1.2 + ports: + - number: 15004 + name: mtls + protocol: TCP + location: MESH_INTERNAL + resolution: DNS + exportTo: + - '*' +--- +# Services and configs in ns1 namespace +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: http1 + namespace: ns1 +spec: + hosts: + - http1.ns1.svc.cluster.local + addresses: + - 2.1.1.1 + ports: + - number: 8081 + name: http + protocol: HTTP2 + location: MESH_INTERNAL + resolution: STATIC + endpoints: + - address: 100.1.1.1 + labels: + version: v1 + ports: + http: 8080 +--- +# Services and configs in ns1 namespace +apiVersion: networking.istio.io/v1alpha3 +kind: ServiceEntry +metadata: + name: http2 + namespace: ns2 +spec: + hosts: + - http2.ns2.svc.cluster.local + addresses: + - 2.1.1.2 + ports: + - number: 8082 + name: http + protocol: HTTP2 + location: MESH_INTERNAL + resolution: STATIC + endpoints: + - address: 100.2.1.1 + ports: + http: 8080 +--- +# The sidecar for the consumer only application +apiVersion: networking.istio.io/v1alpha3 +kind: Sidecar +metadata: + name: consumerapp + namespace: consumerns +spec: + egress: + - hosts: + - "ns1/*" + - "istio-system/istio-telemetry.istio-system.svc.cluster.local" + - "istio-system/istio-policy.istio-system.svc.cluster.local" + ingress: + - port: + number: 9080 + protocol: HTTP + name: http-admin + defaultEndpoint: 127.0.0.1:8080 + captureMode: IPTABLES + workloadSelector: + labels: + app: consumeronly \ No newline at end of file diff --git a/tests/testdata/networking/sidecar-without-service/mesh.yaml b/tests/testdata/networking/sidecar-without-service/mesh.yaml new file mode 100644 index 000000000000..ee316432d23a --- /dev/null +++ b/tests/testdata/networking/sidecar-without-service/mesh.yaml @@ -0,0 +1,10 @@ +mixerCheckServer: istio-policy.istio-system.svc.cluster.local:15004 +mixerReportServer: istio-telemetry.istio-system.svc.cluster.local:15004 + +rootNamespace: istio-config +defaultServiceExportTo: + - "*" +defaultVirtualServiceExportTo: + - "*" +defaultDestinationRuleExportTo: + - "*"