Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: clarify the priority of embedded mode FilterPolicy #792

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions controller/internal/translation/merged_state_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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: {}
24 changes: 24 additions & 0 deletions controller/internal/translation/translation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package translation

import (
"context"
"encoding/json"
"maps"
"os"
"path/filepath"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion site/content/en/docs/concept/filterpolicy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion site/content/zh-hans/docs/concept/filterpolicy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 的对应关系

Expand Down
Loading