Repository: istio/istio (go)

Issue:
False IST0131 warning when using match / as catch all after 1.12.0 upgrade
### Bug Description

Getting this warning when applying the below manifest.  However accessing /blocked_stuff does result in the 400 error so it is working.

Warning: virtualService rule "20_blocked_urls" match #0 of prefix /blocked_stuff is not used (duplicate/overlapping match in rule #0 of prefix / on "90_main")

Istioctl analyze similarly reports 
Info [IST0131] (VirtualService default/productpage) VirtualService rule "20_blocked_urls" match #0 of prefix /blocked_stuff is not used (duplicate/overlapping match in rule #0 of prefix / on "90_main").

This didn't happen with 1.10.5, skipped 1.11.x and getting these warnings in 1.12.0
We 
Sample Manifest
`---
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: productpage
spec:
  hosts:
  - productpage
  http:
  - fault:
      abort:
        httpStatus: 400
        percentage:
          value: 100
    match:
    - uri:
        prefix: /blocked_stuff
    name: 20_blocked_urls
    route:
    - destination:
        host: productpage
        subset: v1
  - match:
    - uri:
        prefix: /
    name: 90_main
    route:
    - destination:
        host: productpage
        subset: v1`

It seems the warning goes away if instead of having a final match on '/' we just have a default route.  This is probably the correct definition looking at the current docs anyways.

Just wondering if the definition as we have it is actually valid? or could break at any time, or is this more a bug with the analyzer?

### Version

```prose
istioctl
client version: 1.12.0
control plane version: 1.12.0
data plane version: 1.12.0 (6 proxies)

kubectl
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.2", GitCommit:"f5743093fd1c663cb0cbc89748f730662345d44d", GitTreeState:"clean", BuildDate:"2020-09-16T13:41:02Z", GoVersion:"go1.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.13", GitCommit:"61be87ea533002fdf9251891b1a6e616c26a5cfd", GitTreeState:"clean", BuildDate:"2021-10-09T05:09:23Z", GoVersion:"go1.15.14", Compiler:"gc", Platform:"linux/amd64"}
```


### Additional Information

_No response_

### Affected product area

- [ ] Docs
- [ ] Installation
- [X] Networking
- [ ] Performance and Scalability
- [ ] Extensions and Telemetry
- [ ] Security
- [ ] Test and Release
- [X] User Experience
- [ ] Developer Infrastructure
- [X] Upgrade
- [ ] Multi Cluster
- [ ] Virtual Machine
- [ ] Control Plane Revisions

### Is this the right place to submit this?

- [X] This is not a security vulnerability
- [X] This is not a question about how to use Istio

Pull request:
Validation: do not warn if latter VS prefix match rule covers former one
fix #36245

Patch:
diff --git a/istio.deps b/istio.deps
index 13f1fc3bb5..e410a3245d 100644
--- a/istio.deps
+++ b/istio.deps
@@ -4,6 +4,6 @@
     "name": "PROXY_REPO_SHA",
     "repoName": "proxy",
     "file": "",
-    "lastStableSHA": "afcf498a60787a715a4cbe3c15f84b48d9b39050"
+    "lastStableSHA": "8043fff1683952834b511d7326c49e1251c305fa"
   }
 ]
diff --git a/pilot/pkg/networking/core/v1alpha3/listener.go b/pilot/pkg/networking/core/v1alpha3/listener.go
index 1f36cd75f4..cd28ea17f3 100644
--- a/pilot/pkg/networking/core/v1alpha3/listener.go
+++ b/pilot/pkg/networking/core/v1alpha3/listener.go
@@ -1302,8 +1302,10 @@ func buildHTTPConnectionManager(listenerOpts buildListenerOpts, httpOpts *httpLi
 
 	routerFilterCtx := configureTracing(listenerOpts, connectionManager)
 
-	filters := make([]*hcm.HttpFilter, len(httpFilters))
-	copy(filters, httpFilters)
+	// Add fault filter at the top.
+	filters := make([]*hcm.HttpFilter, len(httpFilters)+1)
+	filters[0] = xdsfilters.Fault
+	copy(filters[1:], httpFilters)
 
 	if features.MetadataExchange {
 		filters = append(filters, xdsfilters.HTTPMx)
@@ -1322,7 +1324,7 @@ func buildHTTPConnectionManager(listenerOpts buildListenerOpts, httpOpts *httpLi
 		filters = append(filters, xdsfilters.Alpn)
 	}
 
-	filters = append(filters, xdsfilters.Cors, xdsfilters.Fault)
+	filters = append(filters, xdsfilters.Cors)
 	filters = append(filters, listenerOpts.push.Telemetry.HTTPFilters(listenerOpts.proxy, listenerOpts.class)...)
 	filters = append(filters, xdsfilters.BuildRouterFilter(routerFilterCtx))
 
diff --git a/pilot/pkg/networking/core/v1alpha3/tracing.go b/pilot/pkg/networking/core/v1alpha3/tracing.go
index d365f0b138..69f095437c 100644
--- a/pilot/pkg/networking/core/v1alpha3/tracing.go
+++ b/pilot/pkg/networking/core/v1alpha3/tracing.go
@@ -235,10 +235,12 @@ func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMet
 type typedConfigGenFromClusterFn func(clusterName string) (*anypb.Any, error)
 
 func zipkinConfigGen(cluster string) (*anypb.Any, error) {
+	_, _, hostname, _ := model.ParseSubsetKey(cluster)
 	zc := &tracingcfg.ZipkinConfig{
 		CollectorCluster:         cluster,
 		CollectorEndpoint:        "/api/v2/spans",                   // envoy deprecated v1 support
 		CollectorEndpointVersion: tracingcfg.ZipkinConfig_HTTP_JSON, // use v2 JSON for now
+		CollectorHostname:        string(hostname),                  // http host header
 		TraceId_128Bit:           true,
 		SharedSpanContext:        wrapperspb.Bool(false),
 	}
diff --git a/pilot/pkg/xds/ads.go b/pilot/pkg/xds/ads.go
index 13a04003fa..ba9f975c00 100644
--- a/pilot/pkg/xds/ads.go
+++ b/pilot/pkg/xds/ads.go
@@ -15,7 +15,6 @@
 package xds
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 	"sync/atomic"
@@ -37,7 +36,6 @@ import (
 	v3 "istio.io/istio/pilot/pkg/xds/v3"
 	"istio.io/istio/pkg/cluster"
 	"istio.io/istio/pkg/config/schema/gvk"
-	"istio.io/istio/pkg/spiffe"
 	"istio.io/pkg/env"
 	istiolog "istio.io/pkg/log"
 )
@@ -72,9 +70,6 @@ type Connection struct {
 	// PeerAddr is the address of the client, from network layer.
 	PeerAddr string
 
-	// Defines associated identities for the connection
-	Identities []string
-
 	// Time of connection, for debugging
 	Connect time.Time
 
@@ -177,7 +172,6 @@ func (s *DiscoveryServer) receive(con *Connection) {
 				con.errorChan <- status.New(codes.InvalidArgument, "missing node information").Err()
 				return
 			}
-			// TODO: We should validate that the namespace in the cert matches the claimed namespace in metadata.
 			if err := s.initConnection(req.Node, con); err != nil {
 				con.errorChan <- err
 				return
@@ -304,7 +298,6 @@ func (s *DiscoveryServer) Stream(stream DiscoveryStream) error {
 		return status.Error(codes.Unavailable, "error reading config")
 	}
 	con := newConnection(peerAddr, stream)
-	con.Identities = ids
 
 	// Do not call: defer close(con.pushChannel). The push channel will be garbage collected
 	// when the connection is no longer used. Closing the channel can cause subtle race conditions
@@ -321,6 +314,10 @@ func (s *DiscoveryServer) Stream(stream DiscoveryStream) error {
 	// reqChannel and the connection not being enqueued for pushes to pushChannel until the
 	// initialization is complete.
 	<-con.initialized
+	// authorize client
+	if err = s.authorize(con, ids); err != nil {
+		return err
+	}
 
 	for {
 		select {
@@ -376,25 +373,18 @@ func (s *DiscoveryServer) shouldRespond(con *Connection, request *discovery.Disc
 		return false
 	}
 
-	// This is first request - initialize typeUrl watches.
-	if request.ResponseNonce == "" {
-		log.Debugf("ADS:%s: INIT %s %s %s", stype, con.ConID, request.VersionInfo, request.ResponseNonce)
-		con.proxy.Lock()
-		con.proxy.WatchedResources[request.TypeUrl] = &model.WatchedResource{TypeUrl: request.TypeUrl, ResourceNames: request.ResourceNames, LastRequest: request}
-		con.proxy.Unlock()
-		return true
-	}
-
 	con.proxy.RLock()
 	previousInfo := con.proxy.WatchedResources[request.TypeUrl]
 	con.proxy.RUnlock()
 
-	// This is a case of Envoy reconnecting Istiod i.e. Istiod does not have
+	// This can happen in two cases:
+	// 1. Envoy initially send request to Istiod
+	// 2. Envoy reconnect to Istiod i.e. Istiod does not have
 	// information about this typeUrl, but Envoy sends response nonce - either
 	// because Istiod is restarted or Envoy disconnects and reconnects.
 	// We should always respond with the current resource names.
-	if previousInfo == nil {
-		log.Debugf("ADS:%s: RECONNECT %s %s %s", stype, con.ConID, request.VersionInfo, request.ResponseNonce)
+	if request.ResponseNonce == "" || previousInfo == nil {
+		log.Debugf("ADS:%s: INIT/RECONNECT %s %s %s", stype, con.ConID, request.VersionInfo, request.ResponseNonce)
 		con.proxy.Lock()
 		con.proxy.WatchedResources[request.TypeUrl] = &model.WatchedResource{TypeUrl: request.TypeUrl, ResourceNames: request.ResourceNames, LastRequest: request}
 		con.proxy.Unlock()
@@ -501,15 +491,6 @@ func (s *DiscoveryServer) initConnection(node *core.Node, con *Connection) error
 	con.ConID = connectionID(proxy.ID)
 	con.node = node
 	con.proxy = proxy
-	if features.EnableXDSIdentityCheck && con.Identities != nil {
-		// TODO: allow locking down, rejecting unauthenticated requests.
-		id, err := checkConnectionIdentity(con)
-		if err != nil {
-			log.Warnf("Unauthorized XDS: %v with identity %v: %v", con.PeerAddr, con.Identities, err)
-			return status.Newf(codes.PermissionDenied, "authorization failed: %v", err).Err()
-		}
-		con.proxy.VerifiedIdentity = id
-	}
 
 	// Register the connection. this allows pushes to be triggered for the proxy. Note: the timing of
 	// this and initializeProxy important. While registering for pushes *after* initialization is complete seems like
@@ -547,23 +528,6 @@ func (s *DiscoveryServer) closeConnection(con *Connection) {
 	s.WorkloadEntryController.QueueUnregisterWorkload(con.proxy, con.Connect)
 }
 
-func checkConnectionIdentity(con *Connection) (*spiffe.Identity, error) {
-	for _, rawID := range con.Identities {
-		spiffeID, err := spiffe.ParseIdentity(rawID)
-		if err != nil {
-			continue
-		}
-		if con.proxy.ConfigNamespace != "" && spiffeID.Namespace != con.proxy.ConfigNamespace {
-			continue
-		}
-		if con.proxy.Metadata.ServiceAccount != "" && spiffeID.ServiceAccount != con.proxy.Metadata.ServiceAccount {
-			continue
-		}
-		return &spiffeID, nil
-	}
-	return nil, fmt.Errorf("no identities (%v) matched %v/%v", con.Identities, con.proxy.ConfigNamespace, con.proxy.Metadata.ServiceAccount)
-}
-
 func connectionID(node string) string {
 	id := atomic.AddInt64(&connectionNumber, 1)
 	return node + "-" + strconv.FormatInt(id, 10)
@@ -878,7 +842,7 @@ func (s *DiscoveryServer) AdsPushAll(version string, req *model.PushRequest) {
 			version, s.adsClientCount(), req.Push.PushVersion)
 	} else {
 		totalService := len(req.Push.Services(nil))
-		log.Infof("XDS: Pushing:%s Services:%d ConnectedEndpoints:%d  Version:%s",
+		log.Infof("XDS: Pushing:%s Services:%d ConnectedEndpoints:%d Version:%s",
 			version, totalService, s.adsClientCount(), req.Push.PushVersion)
 		monServices.Record(float64(totalService))
 
@@ -1028,5 +992,5 @@ func (conn *Connection) Watched(typeUrl string) *model.WatchedResource {
 }
 
 func (conn *Connection) Stop() {
-	conn.stop <- struct{}{}
+	close(conn.stop)
 }
diff --git a/pilot/pkg/xds/authentication.go b/pilot/pkg/xds/auth.go
similarity index 65%
rename from pilot/pkg/xds/authentication.go
rename to pilot/pkg/xds/auth.go
index 03948cc223..3fbe98e17d 100644
--- a/pilot/pkg/xds/authentication.go
+++ b/pilot/pkg/xds/auth.go
@@ -20,10 +20,14 @@ import (
 	"fmt"
 	"strings"
 
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/peer"
+	"google.golang.org/grpc/status"
 
 	"istio.io/istio/pilot/pkg/features"
+	"istio.io/istio/pilot/pkg/model"
+	"istio.io/istio/pkg/spiffe"
 	"istio.io/pkg/env"
 )
 
@@ -65,3 +69,37 @@ func (s *DiscoveryServer) authenticate(ctx context.Context) ([]string, error) {
 	log.Errorf("Failed to authenticate client from %s: %s", peerInfo.Addr.String(), strings.Join(authFailMsgs, "; "))
 	return nil, errors.New("authentication failure")
 }
+
+func (s *DiscoveryServer) authorize(con *Connection, identities []string) error {
+	if con == nil || con.proxy == nil {
+		return nil
+	}
+
+	if features.EnableXDSIdentityCheck && identities != nil {
+		// TODO: allow locking down, rejecting unauthenticated requests.
+		id, err := checkConnectionIdentity(con.proxy, identities)
+		if err != nil {
+			log.Warnf("Unauthorized XDS: %v with identity %v: %v", con.PeerAddr, identities, err)
+			return status.Newf(codes.PermissionDenied, "authorization failed: %v", err).Err()
+		}
+		con.proxy.VerifiedIdentity = id
+	}
+	return nil
+}
+
+func checkConnectionIdentity(proxy *model.Proxy, identities []string) (*spiffe.Identity, error) {
+	for _, rawID := range identities {
+		spiffeID, err := spiffe.ParseIdentity(rawID)
+		if err != nil {
+			continue
+		}
+		if proxy.ConfigNamespace != "" && spiffeID.Namespace != proxy.ConfigNamespace {
+			continue
+		}
+		if proxy.Metadata.ServiceAccount != "" && spiffeID.ServiceAccount != proxy.Metadata.ServiceAccount {
+			continue
+		}
+		return &spiffeID, nil
+	}
+	return nil, fmt.Errorf("no identities (%v) matched %v/%v", identities, proxy.ConfigNamespace, proxy.Metadata.ServiceAccount)
+}
diff --git a/pilot/pkg/xds/delta.go b/pilot/pkg/xds/delta.go
index 08cbb690ce..13a916ba1c 100644
--- a/pilot/pkg/xds/delta.go
+++ b/pilot/pkg/xds/delta.go
@@ -16,7 +16,6 @@ package xds
 
 import (
 	"errors"
-	"fmt"
 	"strings"
 	"time"
 
@@ -67,7 +66,7 @@ func (s *DiscoveryServer) StreamDeltas(stream DeltaDiscoveryStream) error {
 
 	ids, err := s.authenticate(ctx)
 	if err != nil {
-		return err
+		return status.Error(codes.Unauthenticated, err.Error())
 	}
 	if ids != nil {
 		log.Debugf("Authenticated XDS: %v with identity %v", peerAddr, ids)
@@ -80,10 +79,9 @@ func (s *DiscoveryServer) StreamDeltas(stream DeltaDiscoveryStream) error {
 		// Error accessing the data - log and close, maybe a different pilot replica
 		// has more luck
 		log.Warnf("Error reading config %v", err)
-		return err
+		return status.Error(codes.Unavailable, "error reading config")
 	}
 	con := newDeltaConnection(peerAddr, stream)
-	con.Identities = ids
 
 	// Do not call: defer close(con.pushChannel). The push channel will be garbage collected
 	// when the connection is no longer used. Closing the channel can cause subtle race conditions
@@ -100,14 +98,16 @@ func (s *DiscoveryServer) StreamDeltas(stream DeltaDiscoveryStream) error {
 	// reqChannel and the connection not being enqueued for pushes to pushChannel until the
 	// initialization is complete.
 	<-con.initialized
+	// authorize client
+	if err = s.authorize(con, ids); err != nil {
+		return err
+	}
 
 	for {
 		select {
 		case req, ok := <-con.deltaReqChan:
 			if ok {
-				// processRequest is calling pushXXX, accessing common structs with pushConnection.
-				// Adding sync is the second issue to be resolved if we want to save 1/2 of the threads.
-				log.Debugf("Got Delta Request: %+v", req.TypeUrl)
+				log.Debugf("Got Delta Request: %s", req.TypeUrl)
 				if err := s.processDeltaRequest(req, con); err != nil {
 					return err
 				}
@@ -222,7 +222,6 @@ func (s *DiscoveryServer) receiveDelta(con *Connection) {
 				con.errorChan <- status.New(codes.InvalidArgument, "missing node information").Err()
 				return
 			}
-			// TODO: We should validate that the namespace in the cert matches the claimed namespace in metadata.
 			if err := s.initConnection(req.Node, con); err != nil {
 				con.errorChan <- err
 				return
@@ -346,11 +345,13 @@ func (s *DiscoveryServer) shouldRespondDelta(con *Connection, request *discovery
 	previousInfo := con.proxy.WatchedResources[request.TypeUrl]
 	con.proxy.RUnlock()
 
-	// This is a case of Envoy reconnecting Istiod i.e. Istiod does not have
+	// This can happen in two cases:
+	// 1. Envoy initially send request to Istiod
+	// 2. Envoy reconnect to Istiod i.e. Istiod does not have
 	// information about this typeUrl, but Envoy sends response nonce - either
 	// because Istiod is restarted or Envoy disconnects and reconnects.
 	// We should always respond with the current resource names.
-	if previousInfo == nil {
+	if request.ResponseNonce == "" || previousInfo == nil {
 		// TODO: can we distinguish init and reconnect? Do we care?
 		log.Debugf("dADS:%s: INIT/RECONNECT %s %s", stype, con.ConID, request.ResponseNonce)
 		con.proxy.Lock()
@@ -380,27 +381,18 @@ func (s *DiscoveryServer) shouldRespondDelta(con *Connection, request *discovery
 	// If it comes here, that means nonce match. This an ACK. We should record
 	// the ack details and respond if there is a change in resource names.
 	con.proxy.Lock()
+	defer con.proxy.Unlock()
 	previousResources := con.proxy.WatchedResources[request.TypeUrl].ResourceNames
 	con.proxy.WatchedResources[request.TypeUrl].VersionAcked = ""
 	con.proxy.WatchedResources[request.TypeUrl].NonceAcked = request.ResponseNonce
 	con.proxy.WatchedResources[request.TypeUrl].NonceNacked = ""
 	con.proxy.WatchedResources[request.TypeUrl].ResourceNames = deltaWatchedResources(previousResources, request)
 	con.proxy.WatchedResources[request.TypeUrl].LastRequest = deltaToSotwRequest(request)
-	con.proxy.Unlock()
-
-	oldAck := listEqualUnordered(previousResources, con.proxy.WatchedResources[request.TypeUrl].ResourceNames)
-	newAck := request.ResponseNonce != ""
-	if newAck != oldAck {
-		// Not sure which is better, lets just log if they don't match for now and compare.
-		log.Errorf("dADS:%s: New ACK and old ACK check mismatch: %v vs %v", stype, oldAck, newAck)
-		if features.EnableUnsafeAssertions {
-			panic(fmt.Sprintf("dADS:%s: New ACK and old ACK check mismatch: %v vs %v", stype, oldAck, newAck))
-		}
-	}
+
 	// Envoy can send two DiscoveryRequests with same version and nonce
 	// when it detects a new resource. We should respond if they change.
-	if oldAck {
-		log.Debugf("dADS:%s: ACK %s %s", stype, con.ConID, request.ResponseNonce)
+	if listEqualUnordered(previousResources, con.proxy.WatchedResources[request.TypeUrl].ResourceNames) {
+		log.Debugf("dADS:%s: ACK  %s %s", stype, con.ConID, request.ResponseNonce)
 		return false
 	}
 	log.Debugf("dADS:%s: RESOURCE CHANGE previous resources: %v, new resources: %v %s %s", stype,
@@ -448,9 +440,10 @@ func (s *DiscoveryServer) pushDeltaXds(con *Connection, push *model.PushContext,
 		// If subscribe is set, client is requesting specific resources. We should just give it the
 		// new resources it needs, rather than the entire set of known resources.
 		subres := sets.NewSet(subscribe...)
-		filteredResponse := []*discovery.Resource{}
+		filteredResponse := make([]*discovery.Resource, 0, len(subscribe))
 		for _, r := range res {
 			if subres.Contains(r.Name) {
+				log.Debugf("----- %s", r.Name)
 				filteredResponse = append(filteredResponse, r)
 			} else {
 				log.Debugf("ADS:%v SKIP %v", v3.GetShortType(w.TypeUrl), r.Name)
@@ -552,7 +545,6 @@ func deltaWatchedResources(existing []string, request *discovery.DeltaDiscoveryR
 	res := sets.NewSet(existing...)
 	res.Insert(request.ResourceNamesSubscribe...)
 	res.Delete(request.ResourceNamesUnsubscribe...)
-	// TODO initial request?
 	return res.SortedList()
 }
 
diff --git a/pilot/pkg/xds/discovery.go b/pilot/pkg/xds/discovery.go
index 6ca7d4ab95..14678389d2 100644
--- a/pilot/pkg/xds/discovery.go
+++ b/pilot/pkg/xds/discovery.go
@@ -653,7 +653,7 @@ func (s *DiscoveryServer) SendResponse(connections []*Connection, res *discovery
 		go func() {
 			err := con.stream.Send(res)
 			if err != nil {
-				log.Info("Failed to send internal event ", con.ConID, " ", err)
+				log.Errorf("Failed to send internal event %s: %v", con.ConID, err)
 			}
 		}()
 	}
diff --git a/pilot/pkg/xds/sds.go b/pilot/pkg/xds/sds.go
index 9c8a4c891b..7c3da3f739 100644
--- a/pilot/pkg/xds/sds.go
+++ b/pilot/pkg/xds/sds.go
@@ -90,7 +90,7 @@ func (s *SecretGen) parseResources(names []string, proxy *model.Proxy) []SecretR
 func (s *SecretGen) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource,
 	req *model.PushRequest) (model.Resources, model.XdsLogDetails, error) {
 	if proxy.VerifiedIdentity == nil {
-		log.Warnf("proxy %v is not authorized to receive credscontroller. Ensure you are connecting over TLS port and are authenticated.", proxy.ID)
+		log.Warnf("proxy %s is not authorized to receive credscontroller. Ensure you are connecting over TLS port and are authenticated.", proxy.ID)
 		return nil, model.DefaultXdsLogDetails, nil
 	}
 	if req == nil || !needsUpdate(proxy, req.ConfigsUpdated) {
@@ -104,13 +104,13 @@ func (s *SecretGen) Generate(proxy *model.Proxy, push *model.PushContext, w *mod
 	// TODO: For the new gateway-api, we should always search the config namespace and stop reading across all clusters
 	proxyClusterSecrets, err := s.secrets.ForCluster(proxy.Metadata.ClusterID)
 	if err != nil {
-		log.Warnf("proxy %v is from an unknown cluster, cannot retrieve certificates: %v", proxy.ID, err)
+		log.Warnf("proxy %s is from an unknown cluster, cannot retrieve certificates: %v", proxy.ID, err)
 		pilotSDSCertificateErrors.Increment()
 		return nil, model.DefaultXdsLogDetails, nil
 	}
 	configClusterSecrets, err := s.secrets.ForCluster(s.configCluster)
 	if err != nil {
-		log.Warnf("proxy %v is from an unknown cluster, cannot retrieve certificates: %v", proxy.ID, err)
+		log.Warnf("proxy %s is from an unknown cluster, cannot retrieve certificates: %v", proxy.ID, err)
 		pilotSDSCertificateErrors.Increment()
 		return nil, model.DefaultXdsLogDetails, nil
 	}
@@ -154,7 +154,7 @@ func (s *SecretGen) Generate(proxy *model.Proxy, push *model.PushContext, w *mod
 			secret, err := secretController.GetCaCert(sr.Name, sr.Namespace)
 			if err != nil {
 				pilotSDSCertificateErrors.Increment()
-				log.Warnf("failed to fetch ca certificate for %v: %v", sr.ResourceName, err)
+				log.Warnf("failed to fetch ca certificate for %s: %v", sr.ResourceName, err)
 			} else {
 				res := toEnvoyCaSecret(sr.ResourceName, secret)
 				results = append(results, res)
@@ -164,7 +164,7 @@ func (s *SecretGen) Generate(proxy *model.Proxy, push *model.PushContext, w *mod
 			key, cert, err := secretController.GetKeyAndCert(sr.Name, sr.Namespace)
 			if err != nil {
 				pilotSDSCertificateErrors.Increment()
-				log.Warnf("failed to fetch key and certificate for %v: %v", sr.ResourceName, err)
+				log.Warnf("failed to fetch key and certificate for %s: %v", sr.ResourceName, err)
 			} else {
 				res := toEnvoyKeyCertSecret(sr.ResourceName, key, cert)
 				results = append(results, res)
@@ -236,7 +236,7 @@ func filterAuthorizedResources(resources []SecretResource, proxy *model.Proxy, s
 		if errMessage == nil {
 			errMessage = fmt.Errorf("cross namespace secret reference requires ReferencePolicy")
 		}
-		log.Warnf("proxy %v attempted to access unauthorized certificates %v: %v", proxy.ID, atMostNJoin(deniedResources, 3), errMessage)
+		log.Warnf("proxy %s attempted to access unauthorized certificates %s: %v", proxy.ID, atMostNJoin(deniedResources, 3), errMessage)
 		pilotSDSCertificateErrors.Increment()
 	}
 
diff --git a/pilot/pkg/xds/simple.go b/pilot/pkg/xds/simple.go
index 4eb9c5a822..28507127e4 100644
--- a/pilot/pkg/xds/simple.go
+++ b/pilot/pkg/xds/simple.go
@@ -123,7 +123,7 @@ func NewXDS(stop chan struct{}) *SimpleServer {
 		configController,
 	})
 	if err != nil {
-		log.Fatala("Creating aggregate config ", err)
+		log.Fatalf("Creating aggregate config: %v", err)
 	}
 
 	// TODO: fix the mess of store interfaces - most are too generic for their own good.
@@ -145,7 +145,7 @@ func (s *SimpleServer) StartGRPC(addr string) error {
 	go func() {
 		err = gs.Serve(lis)
 		if err != nil {
-			log.Info("Serve done ", err)
+			log.Infof("Serve done with %v", err)
 		}
 	}()
 	return nil
diff --git a/pkg/config/validation/validation.go b/pkg/config/validation/validation.go
index cbf2e79521..180351baa9 100644
--- a/pkg/config/validation/validation.go
+++ b/pkg/config/validation/validation.go
@@ -2273,21 +2273,12 @@ func analyzeUnreachableHTTPRules(routes []*networking.HTTPRoute,
 				if strings.Compare(matchHTTPRoutes[rIndex].Prefix, routePrefix.Prefix) == 0 {
 					continue
 				}
-				// Valid from A to B for matchHTTPRoutes
-				isAtoBCover := coveredValidation(routePrefix, matchHTTPRoutes[rIndex])
-				if isAtoBCover {
+				// Validate former prefix match does not cover the latter one.
+				if coveredValidation(routePrefix, matchHTTPRoutes[rIndex]) {
 					prefixMatchA := matchHTTPRoutes[rIndex].MatchStr + " of prefix " + matchHTTPRoutes[rIndex].Prefix
 					prefixMatchB := routePrefix.MatchStr + " of prefix " + routePrefix.Prefix + " on " + routePrefix.RouteStr
 					reportIneffective(matchHTTPRoutes[rIndex].RouteStr, prefixMatchA, prefixMatchB)
 				}
-
-				// Valid from B to A for matchHTTPRoutes
-				isBtoACover := coveredValidation(matchHTTPRoutes[rIndex], routePrefix)
-				if isBtoACover {
-					prefixMatchA := routePrefix.MatchStr + " of prefix " + routePrefix.Prefix
-					prefixMatchB := matchHTTPRoutes[rIndex].MatchStr + " of prefix " + matchHTTPRoutes[rIndex].Prefix + " on " + matchHTTPRoutes[rIndex].RouteStr
-					reportIneffective(routePrefix.RouteStr, prefixMatchA, prefixMatchB)
-				}
 			}
 		}
 	}


Test patch:
diff --git a/pilot/pkg/model/push_context_test.go b/pilot/pkg/model/push_context_test.go
index ad1dd2aea5..d06a226b60 100644
--- a/pilot/pkg/model/push_context_test.go
+++ b/pilot/pkg/model/push_context_test.go
@@ -390,10 +390,24 @@ func TestEnvoyFilterOrder(t *testing.T) {
 				},
 			},
 		},
+		{
+			Meta: config.Meta{Name: "super-high-priority", Namespace: "testns", GroupVersionKind: gvk.EnvoyFilter},
+			Spec: &networking.EnvoyFilter{
+				Priority: -10,
+				ConfigPatches: []*networking.EnvoyFilter_EnvoyConfigObjectPatch{
+					{
+						Patch: &networking.EnvoyFilter_Patch{},
+						Match: &networking.EnvoyFilter_EnvoyConfigObjectMatch{
+							Proxy: &networking.EnvoyFilter_ProxyMatch{ProxyVersion: `foobar`},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	expectedns := []string{
-		"testns/high-priority", "testns/default-priority", "testns/a-medium-priority",
+		"testns/super-high-priority", "testns/high-priority", "testns/default-priority", "testns/a-medium-priority",
 		"testns/b-medium-priority", "testns/b-low-priority", "testns/a-low-priority",
 	}
 
diff --git a/pilot/pkg/networking/core/v1alpha3/listener_test.go b/pilot/pkg/networking/core/v1alpha3/listener_test.go
index e479afe38f..e1d41bd8ee 100644
--- a/pilot/pkg/networking/core/v1alpha3/listener_test.go
+++ b/pilot/pkg/networking/core/v1alpha3/listener_test.go
@@ -2261,7 +2261,7 @@ func verifyInboundEnvoyListenerNumber(t *testing.T, l *listener.Listener) {
 			t.Fatalf("expected HTTP filter, found none")
 		}
 
-		expect := []string{xdsfilters.MxFilterName, xdsfilters.Cors.Name, xdsfilters.Fault.Name, xdsfilters.Router.Name}
+		expect := []string{xdsfilters.Fault.Name, xdsfilters.MxFilterName, xdsfilters.Cors.Name, xdsfilters.Router.Name}
 		got := getHCMFilters(t, f)
 		if !reflect.DeepEqual(expect, got) {
 			t.Fatalf("expected http filters %v, found %v", expect, got)
diff --git a/pilot/pkg/networking/core/v1alpha3/tracing_test.go b/pilot/pkg/networking/core/v1alpha3/tracing_test.go
index ed3a9e528e..90e6ce31a0 100644
--- a/pilot/pkg/networking/core/v1alpha3/tracing_test.go
+++ b/pilot/pkg/networking/core/v1alpha3/tracing_test.go
@@ -367,10 +367,12 @@ var fakeEnvTag = &tracing.CustomTag{
 }
 
 func fakeZipkinProvider(expectClusterName, expectProviderName string) *tracingcfg.Tracing_Http {
+	_, _, hostname, _ := model.ParseSubsetKey(expectClusterName)
 	fakeZipkinProviderConfig := &tracingcfg.ZipkinConfig{
 		CollectorCluster:         expectClusterName,
 		CollectorEndpoint:        "/api/v2/spans",
 		CollectorEndpointVersion: tracingcfg.ZipkinConfig_HTTP_JSON,
+		CollectorHostname:        string(hostname),
 		TraceId_128Bit:           true,
 		SharedSpanContext:        wrapperspb.Bool(false),
 	}
diff --git a/pilot/pkg/xds/delta_test.go b/pilot/pkg/xds/delta_test.go
index 3c7b3a893a..80e9c134f1 100644
--- a/pilot/pkg/xds/delta_test.go
+++ b/pilot/pkg/xds/delta_test.go
@@ -34,13 +34,15 @@ func TestDeltaAds(t *testing.T) {
 func TestDeltaAdsClusterUpdate(t *testing.T) {
 	s := NewFakeDiscoveryServer(t, FakeOptions{})
 	ads := s.ConnectDeltaADS().WithType(v3.EndpointType)
-
+	nonce := ""
 	sendEDSReqAndVerify := func(add, remove, expect []string) {
 		t.Helper()
 		res := ads.RequestResponseAck(&discovery.DeltaDiscoveryRequest{
+			ResponseNonce:            nonce,
 			ResourceNamesSubscribe:   add,
 			ResourceNamesUnsubscribe: remove,
 		})
+		nonce = res.Nonce
 		got := xdstest.MapKeys(xdstest.ExtractLoadAssignments(xdstest.UnmarshalClusterLoadAssignment(t, model.ResourcesToAny(res.Resources))))
 		if !reflect.DeepEqual(expect, got) {
 			t.Fatalf("expected clusters %v got %v", expect, got)
diff --git a/pilot/pkg/xds/proxy_dependencies_test.go b/pilot/pkg/xds/proxy_dependencies_test.go
index b764064cd3..fe9f222507 100644
--- a/pilot/pkg/xds/proxy_dependencies_test.go
+++ b/pilot/pkg/xds/proxy_dependencies_test.go
@@ -250,11 +250,8 @@ func TestCheckConnectionIdentity(t *testing.T) {
 	}
 	for _, tt := range cases {
 		t.Run(tt.name, func(t *testing.T) {
-			con := &Connection{
-				proxy:      &model.Proxy{ConfigNamespace: tt.namespace, Metadata: &model.NodeMetadata{ServiceAccount: tt.sa}},
-				Identities: tt.identity,
-			}
-			if _, err := checkConnectionIdentity(con); (err == nil) != tt.success {
+			proxy := &model.Proxy{ConfigNamespace: tt.namespace, Metadata: &model.NodeMetadata{ServiceAccount: tt.sa}}
+			if _, err := checkConnectionIdentity(proxy, tt.identity); (err == nil) != tt.success {
 				t.Fatalf("expected success=%v, got err=%v", tt.success, err)
 			}
 		})