controller: merge policies (#81)

Signed-off-by: spacewander <spacewanderlzx@gmail.com>

controller/internal/controller/httpfilterpolicy_controller.go

@@ -288,9 +288,9 @@ func (r *HTTPFilterPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return controller.Complete(r)
 }
 
-func findAffectedObjects(ctx context.Context, reader client.Reader, obj client.Object, name string, idx string) []reconcile.Request {
+func findAffectedObjects(ctx context.Context, reader client.Reader, obj client.Object, kind string, idx string) []reconcile.Request {
 	logger := log.FromContext(ctx)
-	logger.Info("Target changed", "name", name, "namespace", obj.GetNamespace(), "name", obj.GetName())
+	logger.Info("Target changed", "kind", kind, "namespace", obj.GetNamespace(), "name", obj.GetName())
 
 	policies := &mosniov1.HTTPFilterPolicyList{}
 	listOps := &client.ListOptions{
@@ -313,6 +313,10 @@ func findAffectedObjects(ctx context.Context, reader client.Reader, obj client.O
 		}
 	}
 
-	logger.Info("Target changed, trigger reconciliation", "name", name, "requests", requests)
+	if len(requests) > 0 {
+		logger.Info("Target changed, trigger reconciliation", "kind", kind, "requests", requests)
+		// As we do full regeneration, we only need to reconcile one HTTPFilterPolicy
+		return []reconcile.Request{requests[0]}
+	}
 	return requests
 }

controller/internal/translation/final_state.go

@@ -10,6 +10,10 @@ import (
 	"mosn.io/moe/controller/internal/model"
 )
 
+const (
+	AnnotationInfo = "htnn.mosn.io/info"
+)
+
 func nameFromHost(host *model.VirtualHost) string {
 	// We use the NsName as the EnvoyFilter name because the host name may contain invalid characters.
 	// This design also make it easier to reference the original CR with the EnvoyFilter.
@@ -39,6 +43,9 @@ func toFinalState(_ *Ctx, state *mergedState) (*FinalState, error) {
 		ef := istio.GenerateHostFilter(host.VirtualHost, host.Policy)
 		name := nameFromHost(host.VirtualHost)
 		ef.SetName(name)
+		ef.SetAnnotations(map[string]string{
+			AnnotationInfo: host.Info.String(),
+		})
 
 		if curr, ok := efs[name]; ok {
 			curr.Spec.ConfigPatches = append(curr.Spec.ConfigPatches, ef.Spec.ConfigPatches...)

controller/internal/translation/merged_state.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"sort"
 
+	"k8s.io/apimachinery/pkg/runtime"
+
 	mosniov1 "mosn.io/moe/controller/api/v1"
 	"mosn.io/moe/controller/internal/model"
 	"mosn.io/moe/pkg/filtermanager"
@@ -22,27 +24,65 @@ type mergedHostPolicy struct {
 	VirtualHost *model.VirtualHost
 	Routes      map[string]*mergedRoutePolicy
 	Policy      *filtermanager.FilterManagerConfig
+	Info        *Info
 }
 
 type mergedRoutePolicy struct {
 	Policy *filtermanager.FilterManagerConfig
 }
 
+func toNsName(policy *mosniov1.HTTPFilterPolicy) string {
+	return policy.Namespace + "/" + policy.Name
+}
+
+// highest priority policy will be first
+func sortHttpFilterPolicy(policies []*mosniov1.HTTPFilterPolicy) {
+	// use Slice instead of SliceStable because each policy has unique namespace/name
+	sort.Slice(policies, func(i, j int) bool {
+		if policies[i].CreationTimestamp == policies[j].CreationTimestamp {
+			return toNsName(policies[i]) < toNsName(policies[j])
+		}
+		return policies[i].CreationTimestamp.Before(&policies[j].CreationTimestamp)
+	})
+}
+
 func toMergedState(ctx *Ctx, state *dataPlaneState) (*FinalState, error) {
 	s := &mergedState{
 		Hosts: make(map[string]*mergedHostPolicy),
 	}
+	// According to the https://gateway-api.sigs.k8s.io/geps/gep-713/,
+	// 1. A Policy targeting a more specific scope wins over a policy targeting a lesser specific scope.
+	// 2. If multiple polices configure the same plugin, the oldest one (based on creation timestamp) wins.
+	// 3. If there are multiple oldest polices, the one appearing first in alphabetical order by {namespace}/{name} wins.
 	for name, host := range state.Hosts {
 		mh := &mergedHostPolicy{
 			VirtualHost: host.VirtualHost,
 			Routes:      make(map[string]*mergedRoutePolicy),
+			Info: &Info{
+				HTTPFilterPolicies: []string{},
+			},
+		}
+
+		sortHttpFilterPolicy(host.Policies)
+		mergedPolicy := &mosniov1.HTTPFilterPolicy{
+			Spec: mosniov1.HTTPFilterPolicySpec{
+				Filters: make(map[string]runtime.RawExtension),
+			},
 		}
-		// FIXME: implement merge policy
-		// According to the https://gateway-api.sigs.k8s.io/geps/gep-713/,
-		// 1. A Policy targeting a more specific scope wins over a policy targeting a lesser specific scope.
-		// 2. If multiple polices configure the same plugin, the oldest one (based on creation timestamp) wins.
-		// 3. If there are multiple oldest polices, the one appearing first in alphabetical order by {namespace}/{name} wins.
-		mergedPolicy := host.Policies[0]
+		for _, policy := range host.Policies {
+			used := false
+			for name, filter := range policy.Spec.Filters {
+				if _, ok := mergedPolicy.Spec.Filters[name]; !ok {
+					mergedPolicy.Spec.Filters[name] = filter
+					used = true
+				}
+			}
+
+			if used {
+				mh.Info.HTTPFilterPolicies = append(mh.Info.HTTPFilterPolicies, toNsName(policy))
+			}
+		}
+
 		fmc := translateHTTPFilterPolicyToFilterManagerConfig(mergedPolicy)
 		mh.Policy = fmc
 		s.Hosts[name] = mh

controller/internal/translation/testdata/translation/merge-same-level-httpfilterpolicy.in.yml

@@ -0,0 +1,126 @@
+httpFilterPolicy:
+  - apiVersion: mosn.io/v1
+    kind: HTTPFilterPolicy
+    metadata:
+      name: policy
+      namespace: test
+    spec:
+      targetRef:
+        group: networking.istio.io
+        kind: VirtualService
+        name: httpbin
+      filters:
+        localReply:
+          config:
+            need: true
+            decode: true
+        animal:
+          config:
+            pet: dog
+  - apiVersion: mosn.io/v1
+    kind: HTTPFilterPolicy
+    metadata:
+      name: opolicy
+      namespace: test
+    spec:
+      targetRef:
+        group: networking.istio.io
+        kind: VirtualService
+        name: httpbin
+      filters:
+        animal:
+          config:
+            pet: cat
+  - apiVersion: mosn.io/v1
+    kind: HTTPFilterPolicy
+    metadata:
+      creationTimestamp: "2023-12-01T15:04:05Z"
+      name: a
+      namespace: test
+    spec:
+      targetRef:
+        group: networking.istio.io
+        kind: VirtualService
+        name: httpbin
+      filters:
+        animal:
+          config:
+            pet: bird
+virtualService:
+  a:
+    - apiVersion: networking.istio.io/v1beta1
+      kind: VirtualService
+      metadata:
+        name: httpbin
+        namespace: test
+      spec:
+        gateways:
+        - httpbin-gateway
+        hosts:
+        - "*.httpbin.example.com"
+        http:
+        - match:
+          - uri:
+              prefix: /status
+          route:
+          - destination:
+              host: httpbin
+              port:
+                number: 8000
+  opolicy:
+    - apiVersion: networking.istio.io/v1beta1
+      kind: VirtualService
+      metadata:
+        name: httpbin
+        namespace: test
+      spec:
+        gateways:
+        - httpbin-gateway
+        hosts:
+        - "*.httpbin.example.com"
+        http:
+        - match:
+          - uri:
+              prefix: /status
+          route:
+          - destination:
+              host: httpbin
+              port:
+                number: 8000
+  policy:
+    - apiVersion: networking.istio.io/v1beta1
+      kind: VirtualService
+      metadata:
+        name: httpbin
+        namespace: test
+      spec:
+        gateways:
+        - httpbin-gateway
+        hosts:
+        - "*.httpbin.example.com"
+        http:
+        - match:
+          - uri:
+              prefix: /status
+          route:
+          - destination:
+              host: httpbin
+              port:
+                number: 8000
+gateway:
+  httpbin:
+    - apiVersion: networking.istio.io/v1beta1
+      kind: Gateway
+      metadata:
+        name: httpbin-gateway
+        namespace: test
+      spec:
+        selector:
+          istio: ingressgateway
+        servers:
+        - hosts:
+          - "*.httpbin.example.com"
+          port:
+            name: http
+            number: 80
+            protocol: HTTP

controller/internal/translation/testdata/translation/merge-same-level-httpfilterpolicy.out.yml

@@ -0,0 +1,32 @@
+- metadata:
+    annotations:
+      htnn.mosn.io/info: '{"httpfilterpolicies":["test/opolicy","test/policy"]}'
+    creationTimestamp: null
+    name: htnn-h-test--httpbin
+  spec:
+    configPatches:
+    - applyTo: VIRTUAL_HOST
+      match:
+        routeConfiguration:
+          vhost:
+            name: '*.httpbin.example.com:80'
+      patch:
+        operation: MERGE
+        value:
+          typed_per_filter_config:
+            envoy.filters.http.golang:
+              '@type': type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.ConfigsPerRoute
+              plugins_config:
+                fm:
+                  config:
+                    '@type': type.googleapis.com/xds.type.v3.TypedStruct
+                    value:
+                      plugins:
+                      - config:
+                          pet: cat
+                        name: animal
+                      - config:
+                          decode: true
+                          need: true
+                        name: localReply
+  status: {}

controller/internal/translation/testdata/translation/virtualservice-wildcard-gateway.out.yml

@@ -1,4 +1,6 @@
 - metadata:
+    annotations:
+      htnn.mosn.io/info: '{"httpfilterpolicies":["test/policy"]}'
     creationTimestamp: null
     name: htnn-h-test--httpbin
   spec:

controller/internal/translation/testdata/translation/virtualservice-wildcard-host.out.yml

@@ -1,4 +1,6 @@
 - metadata:
+    annotations:
+      htnn.mosn.io/info: '{"httpfilterpolicies":["test/policy"]}'
     creationTimestamp: null
     name: htnn-h-test--httpbin
   spec:

controller/internal/translation/testdata/translation/virtualservice.out.yml

@@ -1,4 +1,6 @@
 - metadata:
+    annotations:
+      htnn.mosn.io/info: '{"httpfilterpolicies":["default/policy"]}'
     creationTimestamp: null
     name: htnn-h-default--httpbin
   spec:

controller/internal/translation/translation.go

@@ -2,6 +2,7 @@ package translation
 
 import (
 	"context"
+	"encoding/json"
 
 	"github.com/go-logr/logr"
 )
@@ -15,3 +16,13 @@ type Ctx struct {
 func (c *Ctx) Logger() *logr.Logger {
 	return c.logger
 }
+
+type Info struct {
+	// HTTPFilterPolicies indicates what HTTPFilterPolicies are used to generated the EnvoyFilter.
+	HTTPFilterPolicies []string `json:"httpfilterpolicies"`
+}
+
+func (info *Info) String() string {
+	b, _ := json.Marshal(info)
+	return string(b)
+}

controller/tests/integration/httpfilterpolicy_controller_test.go

@@ -59,6 +59,10 @@ var _ = Describe("HTTPFilterPolicy controller", func() {
 		interval = time.Millisecond * 250
 	)
 
+	var (
+		DefaultVirtualService *istiov1b1.VirtualService
+	)
+
 	Context("When reconciling HTTPFilterPolicy", func() {
 		BeforeEach(func() {
 			var policies mosniov1.HTTPFilterPolicyList
@@ -94,6 +98,9 @@ var _ = Describe("HTTPFilterPolicy controller", func() {
 
 			for _, in := range input {
 				obj := mapToObj(in)
+				if obj.GetObjectKind().GroupVersionKind().Kind == "VirtualService" {
+					DefaultVirtualService = obj.(*istiov1b1.VirtualService)
+				}
 				Expect(k8sClient.Create(ctx, obj)).Should(Succeed())
 			}
 
@@ -145,6 +152,40 @@ var _ = Describe("HTTPFilterPolicy controller", func() {
 			Expect(envoyfilters.Items[0].Name).To(Equal("htnn-http-filter"))
 		})
 
+		It("deal with multi policies to one virtualservice", func() {
+			ctx := context.Background()
+			input := []map[string]interface{}{}
+			mustReadInput("multi-policies", &input)
+
+			for _, in := range input {
+				obj := mapToObj(in)
+				Expect(k8sClient.Create(ctx, obj)).Should(Succeed())
+			}
+
+			var envoyfilters istiov1a3.EnvoyFilterList
+			Eventually(func() bool {
+				if err := k8sClient.List(ctx, &envoyfilters); err != nil {
+					return false
+				}
+				return len(envoyfilters.Items) == 2
+			}, timeout, interval).Should(BeTrue())
+
+			names := []string{}
+			for _, ef := range envoyfilters.Items {
+				names = append(names, ef.Name)
+			}
+			Expect(names).To(ConsistOf([]string{"htnn-http-filter", "htnn-h-default--default"}))
+
+			Expect(k8sClient.Delete(ctx, DefaultVirtualService)).Should(Succeed())
+			Eventually(func() bool {
+				if err := k8sClient.List(ctx, &envoyfilters); err != nil {
+					return false
+				}
+				return len(envoyfilters.Items) == 1
+			}, timeout, interval).Should(BeTrue())
+			Expect(envoyfilters.Items[0].Name).To(Equal("htnn-http-filter"))
+		})
+
 		It("diff envoyfilters", func() {
 			ctx := context.Background()
 			input := []map[string]interface{}{}