Skip to content

Commit

Permalink
ci: add forcetypeassert linter (#596)
Browse files Browse the repository at this point in the history
This linter is required by Ant Group QA Team.
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
  • Loading branch information
spacewander authored Jun 25, 2024
1 parent d66817c commit e95743b
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 16 deletions.
9 changes: 9 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ linters:
- contextcheck
- errcheck
- exportloopref
- forcetypeassert
- gocheckcompilerdirectives
- gocritic
- gosec
Expand Down Expand Up @@ -38,14 +39,22 @@ issues:
- path: _test\.go # unit tests
linters:
- errcheck
- forcetypeassert
- gosec
- unparam
- path: tests/ # integration tests
linters:
- bodyclose
- errcheck
- forcetypeassert
- gosec
- unparam
- path: plugins/ # too much plugin config type assert in the plugins
linters:
- forcetypeassert
- path: registries/ # ditto
linters:
- forcetypeassert
# Show the complete output
max-issues-per-linter: 0
max-same-issues: 0
7 changes: 6 additions & 1 deletion api/pkg/consumer/consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package consumer

import (
"errors"
"fmt"
"reflect"

xds "github.com/cncf/xds/go/xds/type/v3"
capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api"
Expand All @@ -35,7 +37,10 @@ type consumerManager struct {
}

func ConsumerManagerFactory(c interface{}) capi.StreamFilterFactory {
conf := c.(*consumerManagerConfig)
conf, ok := c.(*consumerManagerConfig)
if !ok {
panic(fmt.Sprintf("wrong config type: %s", reflect.TypeOf(c)))
}
return func(callbacks capi.FilterCallbackHandler) capi.StreamFilter {
return &consumerManager{
callbacks: callbacks,
Expand Down
6 changes: 6 additions & 0 deletions api/pkg/consumer/consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,9 @@ func TestParse(t *testing.T) {
})
}
}

func TestConsumerManagerFactory(t *testing.T) {
assert.PanicsWithValuef(t, "wrong config type: *struct {}", func() {
ConsumerManagerFactory(&struct{}{})
}, "check if the panic message contains the wrong type")
}
13 changes: 11 additions & 2 deletions api/pkg/filtermanager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package filtermanager
import (
"encoding/json"
"errors"
"fmt"
"reflect"
"sort"
"sync"

Expand Down Expand Up @@ -266,8 +268,15 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC
}

func (p *FilterManagerConfigParser) Merge(parent interface{}, child interface{}) interface{} {
httpFilterCfg := parent.(*filterManagerConfig)
routeCfg := child.(*filterManagerConfig)
httpFilterCfg, ok := parent.(*filterManagerConfig)
if !ok {
panic(fmt.Sprintf("wrong config type: %s", reflect.TypeOf(httpFilterCfg)))
}
routeCfg, ok := child.(*filterManagerConfig)
if !ok {
panic(fmt.Sprintf("wrong config type: %s", reflect.TypeOf(routeCfg)))
}

if httpFilterCfg == nil || len(httpFilterCfg.parsed) == 0 {
return routeCfg
}
Expand Down
15 changes: 13 additions & 2 deletions api/pkg/filtermanager/filtermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package filtermanager

import (
"encoding/json"
"fmt"
"reflect"
"runtime/debug"
"sort"
"sync"
Expand Down Expand Up @@ -120,7 +122,11 @@ func needLogExecution() bool {
}

func FilterManagerFactory(c interface{}) capi.StreamFilterFactory {
conf := c.(*filterManagerConfig)
conf, ok := c.(*filterManagerConfig)
if !ok {
panic(fmt.Sprintf("wrong config type: %s", reflect.TypeOf(c)))
}

parsedConfig := conf.parsed

return func(cb capi.FilterCallbackHandler) (streamFilter capi.StreamFilter) {
Expand All @@ -132,7 +138,12 @@ func FilterManagerFactory(c interface{}) capi.StreamFilterFactory {
}
}()

fm := conf.pool.Get().(*filterManager)
data := conf.pool.Get()
fm, ok := data.(*filterManager)
if !ok {
panic(fmt.Sprintf("unexpected type: %s", reflect.TypeOf(data)))
}

fm.callbacks.FilterCallbackHandler = cb

canSkipMethod := fm.canSkipMethod
Expand Down
7 changes: 6 additions & 1 deletion api/pkg/filtermanager/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package filtermanager

import (
"fmt"
"reflect"
"time"

"mosn.io/htnn/api/pkg/filtermanager/api"
Expand Down Expand Up @@ -135,7 +137,10 @@ func (f *debugFilter) recordExecution(start time.Time, method string) {
f.callbacks.PluginState().Set("debugMode", "executionRecords", executionRecords)
}

records := executionRecords.([]model.ExecutionRecord)
records, ok := executionRecords.([]model.ExecutionRecord)
if !ok {
panic(fmt.Sprintf("unexpected type: %s", reflect.TypeOf(executionRecords)))
}
for _, record := range records {
if record.PluginName == f.name {
record.Record[method] += duration
Expand Down
4 changes: 3 additions & 1 deletion controller/internal/translation/final_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ func toFinalState(_ *Ctx, state *mergedState) (*FinalState, error) {
bName := b.Patch.Value.AsMap()["name"]
if aName != nil && bName != nil {
// EnvoyFilter for ECDS
return aName.(string) < bName.(string)
as, _ := aName.(string)
bs, _ := bName.(string)
return as < bs
} else if aName != nil {
return true
} else if bName != nil {
Expand Down
20 changes: 16 additions & 4 deletions controller/internal/translation/merged_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package translation
import (
"encoding/json"
"fmt"
"reflect"
"slices"
"sort"

Expand Down Expand Up @@ -137,7 +138,11 @@ func translateFilterManagerConfigToPolicyInRDS(fmc *filtermanager.FilterManagerC

var cfg interface{}
// we validated the filter at the beginning, so theorily err should not happen
_ = json.Unmarshal(plugin.Config.([]byte), &cfg)
b, ok := plugin.Config.([]byte)
if !ok {
panic(fmt.Sprintf("unexpected type: %s", reflect.TypeOf(plugin.Config)))
}
_ = json.Unmarshal(b, &cfg)

nativePlugin, ok := p.(plugins.NativePlugin)
if !ok {
Expand All @@ -148,7 +153,10 @@ func translateFilterManagerConfigToPolicyInRDS(fmc *filtermanager.FilterManagerC
conf := p.Config()
desc := conf.ProtoReflect().Descriptor()
fieldDescs := desc.Fields()
m := cfg.(map[string]interface{})
m, ok := cfg.(map[string]interface{})
if !ok {
panic(fmt.Sprintf("unexpected type: %s", reflect.TypeOf(cfg)))
}
// TODO: unify the name style of fields. Currently, the field names can be in snake_case or camelCase.
// Each style is fine because the protobuf support both of them. However, if people want to
// rewrite the configuration, we should take care of this.
Expand Down Expand Up @@ -236,11 +244,15 @@ func translateFilterManagerConfigToPolicyInECDS(fmc *filtermanager.FilterManager

var cfg interface{}
// we validated the filter at the beginning, so theorily err should not happen
_ = json.Unmarshal(plugin.Config.([]byte), &cfg)
b, ok := plugin.Config.([]byte)
if !ok {
panic(fmt.Sprintf("unexpected type: %s", reflect.TypeOf(plugin.Config)))
}
_ = json.Unmarshal(b, &cfg)

plugin.Config = cfg
goFilterManager.Plugins = append(goFilterManager.Plugins, plugin)
_, ok := p.(plugins.ConsumerPlugin)
_, ok = p.(plugins.ConsumerPlugin)
if ok {
consumerNeeded = true
}
Expand Down
27 changes: 22 additions & 5 deletions types/pkg/expr/cel.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,21 @@ var varsPool = sync.Pool{
}

func (s *CelScript) EvalWithRequest(cb api.FilterCallbackHandler, headers api.RequestHeaderMap) (any, error) {
vars := varsPool.Get().(map[string]any)
r := vars["request"].(*request)
data := varsPool.Get()
vars, ok := data.(map[string]any)
if !ok {
return nil, fmt.Errorf("unexpected type: %s", reflect.TypeOf(data))
}
r, ok := vars["request"].(*request)
if !ok {
return nil, fmt.Errorf("unexpected request type: %s", reflect.TypeOf(vars["request"]))
}
r.headers = headers
r.callback = cb
so := vars["source"].(*source)
so, ok := vars["source"].(*source)
if !ok {
return nil, fmt.Errorf("unexpected source type: %s", reflect.TypeOf(vars["source"]))
}
so.callback = cb

res, _, err := s.program.Eval(vars)
Expand Down Expand Up @@ -212,12 +222,19 @@ func (r *request) Receive(function string, overload string, args []ref.Val) ref.
case "method":
return types.String(r.headers.Method())
case "header":
name := args[0].Value().(string)
name, ok := args[0].Value().(string)
if !ok {
// We have type check in the cel definition. This branch is only to satisfy the lint.
return types.NewErr("unexpected type: %s", reflect.TypeOf(args[0].Value()))
}
return types.String(r.Header(name))
case "query_path":
return types.String(r.headers.Url().RawQuery)
case "query":
name := args[0].Value().(string)
name, ok := args[0].Value().(string)
if !ok {
return types.NewErr("unexpected type: %s", reflect.TypeOf(args[0].Value()))
}
return types.String(r.Query(name))
case "id":
return fromProperty(r.callback, "request.id")
Expand Down
4 changes: 4 additions & 0 deletions types/pkg/expr/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ func TestCompileCel(t *testing.T) {
name: "bad arguments",
expr: `request.header()`,
},
{
name: "bad argument type",
expr: `request.header(1)`,
},
{
name: "bad return type",
expr: `1 + 2`,
Expand Down

0 comments on commit e95743b

Please sign in to comment.