From d00f780516da75c1f52e244e6746c45e589145b2 Mon Sep 17 00:00:00 2001 From: spacewander Date: Tue, 5 Nov 2024 12:22:53 +0800 Subject: [PATCH 1/2] chore: clarify the priority of embedded mode FilterPolicy Signed-off-by: spacewander --- .../internal/translation/merged_state_test.go | 70 +++++++++++++++++++ site/content/en/docs/concept/filterpolicy.md | 4 +- .../zh-hans/docs/concept/filterpolicy.md | 3 +- 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 controller/internal/translation/merged_state_test.go diff --git a/controller/internal/translation/merged_state_test.go b/controller/internal/translation/merged_state_test.go new file mode 100644 index 00000000..52923f53 --- /dev/null +++ b/controller/internal/translation/merged_state_test.go @@ -0,0 +1,70 @@ +// Copyright The HTNN Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package translation + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + mosniov1 "mosn.io/htnn/types/apis/v1" +) + +func TestSortFilterPolicy(t *testing.T) { + ps := []*FilterPolicyWrapper{ + {FilterPolicy: &mosniov1.FilterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-policy", + }, + }, scope: PolicyScopeGateway}, + {FilterPolicy: &mosniov1.FilterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-policy", + CreationTimestamp: metav1.Time{ + Time: metav1.Now().Add(-1000), + }, + }, + }, scope: PolicyScopeRoute}, + {FilterPolicy: &mosniov1.FilterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-policy-embeded", + }, + }, scope: PolicyScopeRoute}, + {FilterPolicy: &mosniov1.FilterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-policy-latest", + CreationTimestamp: metav1.Time{ + Time: metav1.Now().Add(-1), + }, + }, + }, scope: PolicyScopeRoute}, + {FilterPolicy: &mosniov1.FilterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "route-policy-different-name", + CreationTimestamp: metav1.Time{ + Time: metav1.Now().Add(-1000), + }, + }, + }, scope: PolicyScopeRoute}, + } + sortFilterPolicy(ps) + + assert.Equal(t, "route-policy-embeded", ps[0].GetName()) + assert.Equal(t, "route-policy", ps[1].GetName()) + assert.Equal(t, "route-policy-different-name", ps[2].GetName()) + assert.Equal(t, "route-policy-latest", ps[3].GetName()) + assert.Equal(t, "gateway-policy", ps[4].GetName()) +} diff --git a/site/content/en/docs/concept/filterpolicy.md b/site/content/en/docs/concept/filterpolicy.md index 88966143..7f52676d 100644 --- a/site/content/en/docs/concept/filterpolicy.md +++ b/site/content/en/docs/concept/filterpolicy.md @@ -375,7 +375,9 @@ For example, to issue a policy for a particular Listener of a Gateway API's Gate average: 1 ``` -Plugins configured by different FilterPolicies with overlapping scopes will merge and then execute in the order specified at the time the plugins were registered. If different levels of FilterPolicy configure the same plugin, the configuration on the smaller scoped FilterPolicy will override the broader scoped configuration, namely `SectionName` > `VirtualService/HTTPRoute` > `Gateway`. If the same plugin is configured by the same level of FilterPolicy, the FilterPolicy created earliest takes precedence; if the timings are the same, they are ordered by the namespace and name of the FilterPolicy. +Plugins configured by different FilterPolicies with overlapping scopes will merge and then execute in the order specified at the time the plugins were registered. If different levels of FilterPolicy configure the same plugin, the configuration on the smaller scoped FilterPolicy will override the broader scoped configuration, namely `SectionName` > `VirtualService/HTTPRoute` > `Gateway`. + +If the same plugin is configured by the same level of FilterPolicy, then the FilterPolicy with the earlier creation time takes precedence (the creation time depends on the k8s auto-popopulated creationTimestamp field); if the times are the same, then the FilterPolicy is sorted by its namespace and name. Since FilterPolicy in embedded mode doesn't have auto-populated creationTimestamp field, FilterPolicy in embedded mode will always have the highest priority. ## The Relationship between FilterPolicy and Plugins diff --git a/site/content/zh-hans/docs/concept/filterpolicy.md b/site/content/zh-hans/docs/concept/filterpolicy.md index 622aa9ab..ebd33f56 100644 --- a/site/content/zh-hans/docs/concept/filterpolicy.md +++ b/site/content/zh-hans/docs/concept/filterpolicy.md @@ -377,7 +377,8 @@ status: 生效范围重叠的不同的 FilterPolicy 配置的插件会合并，然后按注册插件时指定的顺序执行插件。 如果不同级别的 FilterPolicy 配置了同一个插件，那么范围更小的 FilterPolicy 上的配置会覆盖掉范围更大的配置，即 `SectionName` > `VirtualService/HTTPRoute` > `Gateway`。 -如果同一级别的 FilterPolicy 配置了同一个插件，那么创建时间更早的 FilterPolicy 优先；如果时间都一样，则按 FilterPolicy 的 namespace 和 name 排序。 + +如果同一级别的 FilterPolicy 配置了同一个插件，那么创建时间更早的 FilterPolicy 优先（创建时间取决于 k8s 自动填充的 creationTimestamp 字段）；如果时间都一样，则按 FilterPolicy 的 namespace 和 name 排序。因为 embedded mode 下的 FilterPolicy 不存在自动填充的 creationTimestamp 字段，所以 embedded mode 下的 FilterPolicy 总是最优先。 ## 插件和 FilterPolicy 的对应关系 From 49383cf7b4628430c2b3845b87ed34bf4dab6de6 Mon Sep 17 00:00:00 2001 From: spacewander Date: Wed, 6 Nov 2024 11:45:59 +0800 Subject: [PATCH 2/2] more tests Signed-off-by: spacewander --- .../virtualservice_embedded_mode.in.yml | 60 +++++++++++++++++++ .../virtualservice_embedded_mode.out.yml | 39 ++++++++++++ .../internal/translation/translation_test.go | 24 ++++++++ 3 files changed, 123 insertions(+) create mode 100644 controller/internal/translation/testdata/translation/virtualservice_embedded_mode.in.yml create mode 100644 controller/internal/translation/testdata/translation/virtualservice_embedded_mode.out.yml diff --git a/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.in.yml b/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.in.yml new file mode 100644 index 00000000..596f19c9 --- /dev/null +++ b/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.in.yml @@ -0,0 +1,60 @@ +istioGateway: +- apiVersion: networking.istio.io/v1beta1 + kind: Gateway + metadata: + name: httpbin-gateway + namespace: default + spec: + selector: + istio: ingressgateway + servers: + - hosts: + - httpbin.example.com + port: + name: http + number: 80 + protocol: HTTP +virtualService: + httpbin-gateway: + - apiVersion: networking.istio.io/v1beta1 + kind: VirtualService + metadata: + name: httpbin + namespace: default + annotations: + htnn.mosn.io/filterpolicy: | + {"apiVersion":"htnn.mosn.io/v1","kind":"FilterPolicy","metadata":{"name":"policy","namespace":"default"},"spec":{"filters":{"animal":{"config":{"hostName":"fish"}},"localReply":{"config":{"code":404}}}}} + spec: + gateways: + - httpbin-gateway + hosts: + - httpbin.example.com + http: + - match: + - uri: + prefix: /status + name: policy + route: + - destination: + host: httpbin + port: + number: 8000 +filterPolicy: + httpbin: + - apiVersion: htnn.mosn.io/v1 + kind: FilterPolicy + metadata: + name: policy + namespace: default + spec: + targetRef: + group: networking.istio.io + kind: VirtualService + name: httpbin + filters: + animal: + config: + hostName: goldfish + demo: + config: + hostName: micky diff --git a/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.out.yml b/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.out.yml new file mode 100644 index 00000000..31a5aa77 --- /dev/null +++ b/controller/internal/translation/testdata/translation/virtualservice_embedded_mode.out.yml @@ -0,0 +1,39 @@ +- metadata: + annotations: + htnn.mosn.io/info: '{"filterpolicies":["default/embedded-virtualservice-httpbin","default/policy"]}' + creationTimestamp: null + labels: + htnn.mosn.io/created-by: FilterPolicy + name: htnn-h-httpbin.example.com + namespace: default + spec: + configPatches: + - applyTo: HTTP_ROUTE + match: + routeConfiguration: + vhost: + name: httpbin.example.com:80 + route: + name: policy + patch: + operation: MERGE + value: + typed_per_filter_config: + htnn.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: + hostName: fish + name: animal + - config: + hostName: micky + name: demo + - config: + code: 404 + name: localReply + status: {} diff --git a/controller/internal/translation/translation_test.go b/controller/internal/translation/translation_test.go index 1df60a22..844ce6d1 100644 --- a/controller/internal/translation/translation_test.go +++ b/controller/internal/translation/translation_test.go @@ -16,6 +16,7 @@ package translation import ( "context" + "encoding/json" "maps" "os" "path/filepath" @@ -32,6 +33,7 @@ import ( "mosn.io/htnn/controller/internal/config" "mosn.io/htnn/controller/internal/istio" "mosn.io/htnn/controller/internal/log" + "mosn.io/htnn/controller/pkg/constant" _ "mosn.io/htnn/controller/plugins" // register plugins _ "mosn.io/htnn/controller/registries" // register registries mosniov1 "mosn.io/htnn/types/apis/v1" @@ -213,6 +215,17 @@ func TestTranslate(t *testing.T) { } s.AddPolicyForVirtualService(fp, wrapper.vs, wrapper.gws) } + + ann := wrapper.vs.Annotations + if ann[constant.AnnotationFilterPolicy] != "" { + var policy mosniov1.FilterPolicy + err := json.Unmarshal([]byte(ann[constant.AnnotationFilterPolicy]), &policy) + require.NoError(t, err) + policy.Namespace = wrapper.vs.Namespace + // Name convention is "embedded-$kind-$name" + policy.Name = "embedded-virtualservice-" + wrapper.vs.Name + s.AddPolicyForVirtualService(&policy, wrapper.vs, wrapper.gws) + } } // For gateway-only cases @@ -225,6 +238,17 @@ func TestTranslate(t *testing.T) { } s.AddPolicyForIstioGateway(fp, gw) } + + ann := gw.Annotations + if ann[constant.AnnotationFilterPolicy] != "" { + var policy mosniov1.FilterPolicy + err := json.Unmarshal([]byte(ann[constant.AnnotationFilterPolicy]), &policy) + require.NoError(t, err) + policy.Namespace = gw.Namespace + // Name convention is "embedded-$kind-$name" + policy.Name = "embedded-gateway-" + gw.Name + s.AddPolicyForIstioGateway(&policy, gw) + } } if config.EnableLDSPluginViaECDS() { for _, gw := range input.IstioGateway {