The virtual_domains not generated correctly with AUTO_REACHABLE_SERVICES and MeshTrafficPermission

[Icarus9913]

What happened?

version: 2.9.2

What happened?

I eanbled the AUTO_REACHABLE_SERVICES feature and create 2 MTPs and found the virtual_domains was not generate correctly from the XDS configuration

Steps to reproduce

1. create a single zone with 2.9.2
2. enable auto_reachable_services with ENV KUMA_EXPERIMENTAL_AUTO_REACHABLE_SERVICES
3. Apply the following yamls:

apiVersion: kuma.io/v1alpha1
kind: Mesh
metadata:
  name: default
spec:
  meshServices:
    mode: Exclusive
  mtls:
    enabledBackend: ca-1
    backends:
    - name: ca-1
      type: builtin
---
apiVersion: kuma.io/v1alpha1
kind: HostnameGenerator
metadata:
  name: default
  namespace: kuma-system
spec:
  selector:
    meshService:
      matchLabels:
        kuma.io/origin: zone
  template: "{{ .DisplayName }}.{{ .Namespace }}.default.gg.mesh"

4. Create 2 applications with sidecar injected:

apiVersion: v1
kind: Namespace
metadata:
  labels:
    kuma.io/sidecar-injection: enabled
  name: kuma-demo
---
apiVersion: v1
kind: Namespace
metadata:
  labels:
    kuma.io/sidecar-injection: enabled
  name: kuma-one
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: 2048-app
  namespace: kuma-demo
spec:
  replicas: 1
  selector:
    matchLabels:
      app: 2048-app
  template:
    metadata:
      labels:
        app: 2048-app
    spec:
      containers:
      - name: 2048-app
        image: ghcr.io/daocloud/dao-2048:v1.4.1
        ports:
        - containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
  name: service-2048-app
  namespace: kuma-demo
spec:
  type: ClusterIP
  selector:
    app: 2048-app
  ports:
    - name: http
      appProtocol: http
      protocol: TCP
      port: 80
      targetPort: 80
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
  namespace: kuma-one
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:stable
        ports:
          - containerPort: 80
            name: http-web-svc
---
apiVersion: v1
kind: Service
metadata:
  name: nginx-service
  namespace: kuma-one
spec:
  selector:
    app: nginx
  ports:
  - name: name-of-service-port
    appProtocol: http
    protocol: TCP
    port: 80
    targetPort: http-web-svc

5. Create the following MTPs one by one:

apiVersion: kuma.io/v1alpha1
kind: MeshTrafficPermission
metadata:
  name: mtp-allow-kuma-one
  namespace: kuma-system
  labels:
    kuma.io/mesh: default
spec:
  targetRef:
    kind: Mesh
  from:
  - targetRef:
      kind: MeshSubset
      tags:
        k8s.kuma.io/namespace: kuma-one
    default:
      action: Allow
---
apiVersion: kuma.io/v1alpha1
kind: MeshTrafficPermission
metadata:
  name: mtp-allow-kuma-other-ns-and-tag
  namespace: kuma-system
  labels:
    kuma.io/mesh: default
spec:
  targetRef:
    kind: Mesh
  from:
  - targetRef:
      kind: MeshSubset
      tags:
        asdfasdfasdf: asdfasdfasdf
        k8s.kuma.io/namespace: kuma-other
    default:
      action: Allow

Result

XDS configurations:

        {
          "name": "kuma:dns",
          "active_state": {
            "version_info": "64c1c7c1-8609-40a6-b7c3-ea3949b850e1",
            "listener": {
              "@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
              "name": "kuma:dns",
              "listener_filters": [
                {
                  "name": "envoy.filters.udp.dns_filter",
                  "typed_config": {
                    "@type": "type.googleapis.com/envoy.extensions.filters.udp.dns_filter.v3.DnsFilterConfig",
                    "stat_prefix": "kuma_dns",
                    "server_config": {
                      "inline_dns_table": {}
                    }
                  }
                }
              ],

[Icarus9913]

I created one PR to build the local test: #12274

The direct problem here is:

kuma/pkg/plugins/policies/meshtrafficpermission/graph/reachable_graph.go

Lines 48 to 51 in 8659306

	rule := r.backendRules[noPort].Compute(core_rules.SubsetFromTags(fromTags))
	if rule == nil {
	return false
	}

[Icarus9913]

I read the codes and found in rules.go, we'll assemble all of the MTPs tags then we'll have subsets as:

{
"asdfasdfasdf": "asdfasdfasdf",
"k8s.kuma.io/namespace": "kuma-other",
"k8s.kuma.io/namespace": "kuma-one"
}

But we'll return false due to this judgement:

kuma/pkg/plugins/policies/core/rules/rules.go

Lines 143 to 147 in 8659306

	for _, tag := range ss {
	oTags, ok := otherByKeys[tag.Key]
	if !ok {
	return false
	}

[Icarus9913]

Here's my humble workaround but not sure if it's correct:

// IsSubset returns true if 'other' is a subset of the current set.
// Empty set is a superset for all subsets.
func (ss Subset) IsSubset(other Subset) bool {
	if len(ss) == 0 {
		return true
	}
	if len(other) == 0 {
		return false
	}

	otherByKeys := map[string][]Tag{}
	for _, t := range other {
		otherByKeys[t.Key] = append(otherByKeys[t.Key], t)
	}

	noMatchedCount := 0
	for _, tag := range ss {
		oTags, ok := otherByKeys[tag.Key]
		if !ok {
			noMatchedCount++
			continue
			//return false
		}
		for _, otherTag := range oTags {
			if !isSubset(tag, otherTag) {
				return false
			}
		}
	}
	if noMatchedCount == len(ss) {
		return false
	}

	return true
}

[lobkovilya]

I think the method IsSubset should stay untouched. But you're right the problem is here:

kuma/pkg/plugins/policies/meshtrafficpermission/graph/reachable_graph.go

Lines 48 to 51 in 8659306

	rule := r.backendRules[noPort].Compute(core_rules.SubsetFromTags(fromTags))
	if rule == nil {
	return false
	}

We need to rewrite the method Compute so it doesn't use the IsSubset but uses some other method with the right logic.

The semantic of IsSubset is to check if one set "is subset" of another set. So it's relation between 2 sets. We need Compute to call a function that's called something like IsPointInSet(p Point). Though both Point and Subset are defined by key-value pairs, the output of IsSubset and IsPointInSet is different. For example:

set1 := map[string]string{
     "key1": "value1",
     "key2": "value2",
}
set2 := map[string]string{
     "key1": "value1",
}
fmt.Println(set1.IsSubset(set2)) // false

but

set1 := map[string]string{
     "key1": "value1",
     "key2": "value2",
}
point := map[string]string{
     "key1": "value1",
}
fmt.Println(set1.IsPointInSet(point)) // true

[lobkovilya]

fyi I think this issue is about the same thing #8155

[lukidzi]

Triage: We should implement the new method IsPointSelectedBy and use it to Compute for autoreachable services. We should also introduce new aliases because the current gives confusion. We should use Selector and Point

[lahabana]

So is this issue a dupe of #8155?

[Icarus9913]

So is this issue a dupe of #8155?

Yeah, I think so