Fix istio#11818 fix workloadSelector for Sidecars (istio#12666)

* Fix test error in mixer/adapter/bypass

* Fixes istio#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 istio#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

@@ -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
 		}
 	}

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

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

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

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

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

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 = ""

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

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

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)
 	}