From fd7dd71e953b23a2af7b85f7888b01f1d5afc576 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 11 Nov 2021 11:22:49 +0800 Subject: [PATCH 01/18] refactor(*): feature flag --- pkg/apiserver/conprof/module.go | 8 ++- pkg/apiserver/conprof/service.go | 2 +- pkg/apiserver/info/info.go | 26 ++++---- pkg/apiserver/nonrootlogin/feature_support.go | 34 ---------- .../feature_support.go => user/module.go} | 20 +----- .../semver_check.go => utils/feature_flag.go} | 26 +++++++- pkg/utils/feature_flag_test.go | 64 +++++++++++++++++++ 7 files changed, 114 insertions(+), 66 deletions(-) delete mode 100644 pkg/apiserver/nonrootlogin/feature_support.go rename pkg/apiserver/{conprof/feature_support.go => user/module.go} (53%) rename pkg/{apiserver/utils/semver_check.go => utils/feature_flag.go} (67%) create mode 100644 pkg/utils/feature_flag_test.go diff --git a/pkg/apiserver/conprof/module.go b/pkg/apiserver/conprof/module.go index a88e4263df..9c72969771 100644 --- a/pkg/apiserver/conprof/module.go +++ b/pkg/apiserver/conprof/module.go @@ -13,7 +13,13 @@ package conprof -import "go.uber.org/fx" +import ( + "go.uber.org/fx" + + "github.com/pingcap/tidb-dashboard/pkg/utils" +) + +var FeatureFlagConprof = utils.NewFeatureFlag("conprof", []string{">= 5.3.0"}) var Module = fx.Options( fx.Provide(newService), diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 740fc1320d..ca290bf9b1 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -81,7 +81,7 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(utils.MWForbidByFeatureSupport(IsFeatureSupport(s.params.Config))) + endpoint.Use(utils.MWForbidByFeatureSupport(FeatureFlagConprof.IsSupport(s.params.Config.FeatureVersion))) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index 05a4e423f0..38de24b3d8 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -23,12 +23,12 @@ import ( "go.uber.org/fx" "github.com/pingcap/tidb-dashboard/pkg/apiserver/conprof" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/nonrootlogin" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + apiutils "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/dbstore" "github.com/pingcap/tidb-dashboard/pkg/tidb" + "github.com/pingcap/tidb-dashboard/pkg/utils" "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) @@ -53,7 +53,7 @@ func RegisterRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint.Use(auth.MWAuthRequired()) endpoint.GET("/whoami", s.whoamiHandler) - endpoint.Use(utils.MWConnectTiDB(s.params.TiDBClient)) + endpoint.Use(apiutils.MWConnectTiDB(s.params.TiDBClient)) endpoint.GET("/databases", s.databasesHandler) endpoint.GET("/tables", s.tablesHandler) } @@ -72,12 +72,16 @@ type InfoResponse struct { //nolint // @Security JwtAuth // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) infoHandler(c *gin.Context) { - supportedFeatures := []string{} - if conprof.IsFeatureSupport(s.params.Config) { - supportedFeatures = append(supportedFeatures, "conprof") + featureFlags := []*utils.FeatureFlag{ + conprof.FeatureFlagConprof, + user.FeatureFlagNonRootLogin, } - if nonrootlogin.IsFeatureSupport(s.params.Config) { - supportedFeatures = append(supportedFeatures, "nonRootLogin") + supportedFeatures := []string{} + for _, ff := range featureFlags { + if !ff.IsSupport(s.params.Config.FeatureVersion) { + continue + } + supportedFeatures = append(supportedFeatures, ff.Name) } resp := InfoResponse{ @@ -102,7 +106,7 @@ type WhoAmIResponse struct { // @Security JwtAuth // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) whoamiHandler(c *gin.Context) { - sessionUser := utils.GetSession(c) + sessionUser := apiutils.GetSession(c) resp := WhoAmIResponse{ DisplayName: sessionUser.DisplayName, IsShareable: sessionUser.IsShareable, @@ -122,7 +126,7 @@ func (s *Service) databasesHandler(c *gin.Context) { Databases string `gorm:"column:Database"` } var result []databaseSchemas - db := utils.GetTiDBConnection(c) + db := apiutils.GetTiDBConnection(c) err := db.Raw("SHOW DATABASES").Scan(&result).Error if err != nil { _ = c.Error(err) @@ -150,7 +154,7 @@ type tableSchema struct { // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) tablesHandler(c *gin.Context) { var result []tableSchema - db := utils.GetTiDBConnection(c) + db := apiutils.GetTiDBConnection(c) tx := db.Select([]string{"TABLE_NAME", "TIDB_TABLE_ID"}).Table("INFORMATION_SCHEMA.TABLES") databaseName := c.Query("database_name") diff --git a/pkg/apiserver/nonrootlogin/feature_support.go b/pkg/apiserver/nonrootlogin/feature_support.go deleted file mode 100644 index cd958db7c2..0000000000 --- a/pkg/apiserver/nonrootlogin/feature_support.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2020 PingCAP, Inc. -// -// 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, -// See the License for the specific language governing permissions and -// limitations under the License. - -package nonrootlogin - -import ( - "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" - "github.com/pingcap/tidb-dashboard/pkg/config" -) - -var ( - supportedTiDBVersions = []string{">= 5.3.0"} - featureSupported *bool -) - -func IsFeatureSupport(config *config.Config) (supported bool) { - if featureSupported != nil { - return *featureSupported - } - - supported = utils.IsVersionSupport(config.FeatureVersion, supportedTiDBVersions) - featureSupported = &supported - return -} diff --git a/pkg/apiserver/conprof/feature_support.go b/pkg/apiserver/user/module.go similarity index 53% rename from pkg/apiserver/conprof/feature_support.go rename to pkg/apiserver/user/module.go index 51f015dddd..6818410b11 100644 --- a/pkg/apiserver/conprof/feature_support.go +++ b/pkg/apiserver/user/module.go @@ -11,24 +11,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package conprof +package user import ( - "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" - "github.com/pingcap/tidb-dashboard/pkg/config" + "github.com/pingcap/tidb-dashboard/pkg/utils" ) -var ( - supportedTiDBVersions = []string{">= 5.3.0"} - featureSupported *bool -) - -func IsFeatureSupport(config *config.Config) (supported bool) { - if featureSupported != nil { - return *featureSupported - } - - supported = utils.IsVersionSupport(config.FeatureVersion, supportedTiDBVersions) - featureSupported = &supported - return -} +var FeatureFlagNonRootLogin = utils.NewFeatureFlag("nonRootLogin", []string{">= 5.3.0"}) diff --git a/pkg/apiserver/utils/semver_check.go b/pkg/utils/feature_flag.go similarity index 67% rename from pkg/apiserver/utils/semver_check.go rename to pkg/utils/feature_flag.go index 31626e49c2..92f773e4ec 100644 --- a/pkg/apiserver/utils/semver_check.go +++ b/pkg/utils/feature_flag.go @@ -1,4 +1,4 @@ -// Copyright 2020 PingCAP, Inc. +// Copyright 2021 PingCAP, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -8,6 +8,7 @@ // // 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. @@ -21,10 +22,31 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) +type FeatureFlag struct { + Name string + + supportedVersions []string + supported *bool +} + +func NewFeatureFlag(name string, supportedVersions []string) *FeatureFlag { + return &FeatureFlag{Name: name, supportedVersions: supportedVersions} +} + +func (ff *FeatureFlag) IsSupport(targetVersion string) bool { + if ff.supported != nil { + return *ff.supported + } + + supported := isVersionSupport(targetVersion, ff.supportedVersions) + ff.supported = &supported + return supported +} + // IsVersionSupport checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information -func IsVersionSupport(standaloneVersion string, constraints []string) bool { +func isVersionSupport(standaloneVersion string, constraints []string) bool { curVersion := standaloneVersion if version.Standalone == "No" { curVersion = version.PDVersion diff --git a/pkg/utils/feature_flag_test.go b/pkg/utils/feature_flag_test.go new file mode 100644 index 0000000000..27b8828d0e --- /dev/null +++ b/pkg/utils/feature_flag_test.go @@ -0,0 +1,64 @@ +// Copyright 2021 Suhaha +// +// 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 utils + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pingcap/tidb-dashboard/pkg/utils/version" +) + +func Test_IsSupport(t *testing.T) { + ff := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) + + require.Equal(t, false, ff.IsSupport("v5.2.2")) + // The results do not change because of caching. This is expected. + require.Equal(t, false, ff.IsSupport("v5.3.0")) + + ff2 := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) + require.Equal(t, true, ff2.IsSupport("v5.3.0")) + + ff3 := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) + require.Equal(t, true, ff3.IsSupport("v5.3.2")) +} + +func Test_isVersionSupport(t *testing.T) { + type Args struct { + target string + supported []string + } + tests := []struct { + want bool + args Args + }{ + {want: false, args: Args{target: "v4.2.0", supported: []string{">= 5.3.0"}}}, + {want: false, args: Args{target: "v5.2.0", supported: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0", supported: []string{">= 5.3.0"}}}, + {want: false, args: Args{target: "v5.2.0-alpha-xxx", supported: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0-alpha-xxx", supported: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0", supported: []string{"= 5.3.0"}}}, + {want: false, args: Args{target: "v5.3.1", supported: []string{"= 5.3.0"}}}, + } + + for _, tt := range tests { + isVersionSupport(tt.args.target, tt.args.supported) + } + + version.Standalone = "No" + version.PDVersion = "v5.3.0" + require.Equal(t, true, isVersionSupport("v100.0.0", []string{"= 5.3.0"})) +} From 8bd2e447221ae0d7e491f883bf8873964fba37f9 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 11 Nov 2021 12:35:48 +0800 Subject: [PATCH 02/18] chore: refine feature flag --- pkg/apiserver/conprof/service.go | 2 +- pkg/apiserver/info/info.go | 2 +- pkg/apiserver/utils/error.go | 2 -- pkg/apiserver/utils/mw_feature_support.go | 33 ------------------ pkg/utils/feature_flag.go | 42 +++++++++++++---------- pkg/utils/feature_flag_test.go | 40 ++++++++------------- 6 files changed, 40 insertions(+), 81 deletions(-) delete mode 100644 pkg/apiserver/utils/mw_feature_support.go diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index ca290bf9b1..80edca9f84 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -81,7 +81,7 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(utils.MWForbidByFeatureSupport(FeatureFlagConprof.IsSupport(s.params.Config.FeatureVersion))) + endpoint.Use(FeatureFlagConprof.Middleware(s.params.Config.FeatureVersion)) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index 38de24b3d8..b5221fb486 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -78,7 +78,7 @@ func (s *Service) infoHandler(c *gin.Context) { } supportedFeatures := []string{} for _, ff := range featureFlags { - if !ff.IsSupport(s.params.Config.FeatureVersion) { + if !ff.IsSupported(s.params.Config.FeatureVersion) { continue } supportedFeatures = append(supportedFeatures, ff.Name) diff --git a/pkg/apiserver/utils/error.go b/pkg/apiserver/utils/error.go index 6586906338..dd3756d6f3 100644 --- a/pkg/apiserver/utils/error.go +++ b/pkg/apiserver/utils/error.go @@ -54,8 +54,6 @@ func MakeInvalidRequestErrorFromError(c *gin.Context, err error) { var ErrExpNotEnabled = ErrNS.NewType("experimental_feature_not_enabled") -var ErrFeatureNotSupported = ErrNS.NewType("feature_not_supported") - type APIError struct { Error bool `json:"error"` Message string `json:"message"` diff --git a/pkg/apiserver/utils/mw_feature_support.go b/pkg/apiserver/utils/mw_feature_support.go deleted file mode 100644 index 76b63d668e..0000000000 --- a/pkg/apiserver/utils/mw_feature_support.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2020 PingCAP, Inc. -// -// 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, -// See the License for the specific language governing permissions and -// limitations under the License. - -package utils - -import ( - "net/http" - - "github.com/gin-gonic/gin" -) - -func MWForbidByFeatureSupport(enabled bool) gin.HandlerFunc { - return func(c *gin.Context) { - if !enabled { - _ = c.Error(ErrFeatureNotSupported.New("The feature is not supported")) - c.Status(http.StatusForbidden) - c.Abort() - return - } - - c.Next() - } -} diff --git a/pkg/utils/feature_flag.go b/pkg/utils/feature_flag.go index 92f773e4ec..0226060463 100644 --- a/pkg/utils/feature_flag.go +++ b/pkg/utils/feature_flag.go @@ -15,9 +15,12 @@ package utils import ( + "fmt" + "net/http" "strings" "github.com/Masterminds/semver" + "github.com/gin-gonic/gin" "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) @@ -25,29 +28,18 @@ import ( type FeatureFlag struct { Name string - supportedVersions []string - supported *bool + constraints []string } -func NewFeatureFlag(name string, supportedVersions []string) *FeatureFlag { - return &FeatureFlag{Name: name, supportedVersions: supportedVersions} +func NewFeatureFlag(name string, constraints []string) *FeatureFlag { + return &FeatureFlag{Name: name, constraints: constraints} } -func (ff *FeatureFlag) IsSupport(targetVersion string) bool { - if ff.supported != nil { - return *ff.supported - } - - supported := isVersionSupport(targetVersion, ff.supportedVersions) - ff.supported = &supported - return supported -} - -// IsVersionSupport checks if a semantic version fits within a set of constraints +// IsSupported checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information -func isVersionSupport(standaloneVersion string, constraints []string) bool { - curVersion := standaloneVersion +func (ff *FeatureFlag) IsSupported(targetVersion string) bool { + curVersion := targetVersion if version.Standalone == "No" { curVersion = version.PDVersion } @@ -57,7 +49,7 @@ func isVersionSupport(standaloneVersion string, constraints []string) bool { if err != nil { return false } - for _, ver := range constraints { + for _, ver := range ff.constraints { c, err := semver.NewConstraint(ver) if err != nil { continue @@ -68,3 +60,17 @@ func isVersionSupport(standaloneVersion string, constraints []string) bool { } return false } + +func (ff *FeatureFlag) Middleware(targetVersion string) gin.HandlerFunc { + isSupported := !ff.IsSupported(targetVersion) + return func(c *gin.Context) { + if isSupported { + _ = c.Error(fmt.Errorf("the feature is not supported")) + c.Status(http.StatusForbidden) + c.Abort() + return + } + + c.Next() + } +} diff --git a/pkg/utils/feature_flag_test.go b/pkg/utils/feature_flag_test.go index 27b8828d0e..76bdedabe4 100644 --- a/pkg/utils/feature_flag_test.go +++ b/pkg/utils/feature_flag_test.go @@ -22,43 +22,31 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) -func Test_IsSupport(t *testing.T) { - ff := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) - - require.Equal(t, false, ff.IsSupport("v5.2.2")) - // The results do not change because of caching. This is expected. - require.Equal(t, false, ff.IsSupport("v5.3.0")) - - ff2 := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) - require.Equal(t, true, ff2.IsSupport("v5.3.0")) - - ff3 := NewFeatureFlag("testFeature", []string{">= 5.3.0"}) - require.Equal(t, true, ff3.IsSupport("v5.3.2")) -} - -func Test_isVersionSupport(t *testing.T) { +func Test_IsSupported(t *testing.T) { type Args struct { - target string - supported []string + target string + constraints []string } tests := []struct { want bool args Args }{ - {want: false, args: Args{target: "v4.2.0", supported: []string{">= 5.3.0"}}}, - {want: false, args: Args{target: "v5.2.0", supported: []string{">= 5.3.0"}}}, - {want: true, args: Args{target: "v5.3.0", supported: []string{">= 5.3.0"}}}, - {want: false, args: Args{target: "v5.2.0-alpha-xxx", supported: []string{">= 5.3.0"}}}, - {want: true, args: Args{target: "v5.3.0-alpha-xxx", supported: []string{">= 5.3.0"}}}, - {want: true, args: Args{target: "v5.3.0", supported: []string{"= 5.3.0"}}}, - {want: false, args: Args{target: "v5.3.1", supported: []string{"= 5.3.0"}}}, + {want: false, args: Args{target: "v4.2.0", constraints: []string{">= 5.3.0"}}}, + {want: false, args: Args{target: "v5.2.0", constraints: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0", constraints: []string{">= 5.3.0"}}}, + {want: false, args: Args{target: "v5.2.0-alpha-xxx", constraints: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0-alpha-xxx", constraints: []string{">= 5.3.0"}}}, + {want: true, args: Args{target: "v5.3.0", constraints: []string{"= 5.3.0"}}}, + {want: false, args: Args{target: "v5.3.1", constraints: []string{"= 5.3.0"}}}, } for _, tt := range tests { - isVersionSupport(tt.args.target, tt.args.supported) + ff := NewFeatureFlag("testFeature", tt.args.constraints) + require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) } version.Standalone = "No" version.PDVersion = "v5.3.0" - require.Equal(t, true, isVersionSupport("v100.0.0", []string{"= 5.3.0"})) + ff := NewFeatureFlag("testFeature", []string{"= 5.3.0"}) + require.Equal(t, true, ff.IsSupported("v100.0.0")) } From 74a0937b8c7948e694bb9afcd28ec14f8227ba0b Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 11 Nov 2021 13:02:42 +0800 Subject: [PATCH 03/18] refine: feature flag middleware --- pkg/apiserver/conprof/service.go | 13 ++++--- pkg/apiserver/utils/mw_feature_flag.go | 48 ++++++++++++++++++++++++++ pkg/utils/feature_flag.go | 17 --------- 3 files changed, 56 insertions(+), 22 deletions(-) create mode 100644 pkg/apiserver/utils/mw_feature_flag.go diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 80edca9f84..45e3b7b517 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -30,8 +30,9 @@ import ( "golang.org/x/sync/singleflight" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + apiutils "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" + "github.com/pingcap/tidb-dashboard/pkg/utils" "github.com/pingcap/tidb-dashboard/pkg/utils/topology" ) @@ -81,7 +82,9 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(FeatureFlagConprof.Middleware(s.params.Config.FeatureVersion)) + endpoint.Use(apiutils.MWForbidByFeatureFlag([]*utils.FeatureFlag{ + FeatureFlagConprof, + }, s.params.Config.FeatureVersion)) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) @@ -107,9 +110,9 @@ func (s *Service) reverseProxy(targetPath string) gin.HandlerFunc { c.Request.URL.Path = targetPath token := c.Query("token") if token != "" { - queryStr, err := utils.ParseJWTString("conprof", token) + queryStr, err := apiutils.ParseJWTString("conprof", token) if err != nil { - utils.MakeInvalidRequestErrorFromError(c, err) + apiutils.MakeInvalidRequestErrorFromError(c, err) return } c.Request.URL.RawQuery = queryStr @@ -295,7 +298,7 @@ func (s *Service) conprofGroupProfileDetail(c *gin.Context) { // @Failure 500 {object} utils.APIError func (s *Service) genConprofActionToken(c *gin.Context) { q := c.Query("q") - token, err := utils.NewJWTString("conprof", q) + token, err := apiutils.NewJWTString("conprof", q) if err != nil { _ = c.Error(err) return diff --git a/pkg/apiserver/utils/mw_feature_flag.go b/pkg/apiserver/utils/mw_feature_flag.go new file mode 100644 index 0000000000..331d73788a --- /dev/null +++ b/pkg/apiserver/utils/mw_feature_flag.go @@ -0,0 +1,48 @@ +// Copyright 2021 Suhaha +// +// 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 utils + +import ( + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/pingcap/tidb-dashboard/pkg/utils" +) + +var ErrFeatureNotSupported = ErrNS.NewType("feature_not_supported") + +func MWForbidByFeatureFlag(featureFlags []*utils.FeatureFlag, targetVersion string) gin.HandlerFunc { + supported := true + unsupportedFeatures := make([]string, len(featureFlags)) + for _, ff := range featureFlags { + if !ff.IsSupported(targetVersion) { + supported = false + unsupportedFeatures = append(unsupportedFeatures, ff.Name) + continue + } + } + + return func(c *gin.Context) { + if !supported { + _ = c.Error(ErrFeatureNotSupported.New("unsupported features: %v", unsupportedFeatures)) + c.Status(http.StatusForbidden) + c.Abort() + return + } + + c.Next() + } +} diff --git a/pkg/utils/feature_flag.go b/pkg/utils/feature_flag.go index 0226060463..7e3b395b80 100644 --- a/pkg/utils/feature_flag.go +++ b/pkg/utils/feature_flag.go @@ -15,12 +15,9 @@ package utils import ( - "fmt" - "net/http" "strings" "github.com/Masterminds/semver" - "github.com/gin-gonic/gin" "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) @@ -60,17 +57,3 @@ func (ff *FeatureFlag) IsSupported(targetVersion string) bool { } return false } - -func (ff *FeatureFlag) Middleware(targetVersion string) gin.HandlerFunc { - isSupported := !ff.IsSupported(targetVersion) - return func(c *gin.Context) { - if isSupported { - _ = c.Error(fmt.Errorf("the feature is not supported")) - c.Status(http.StatusForbidden) - c.Abort() - return - } - - c.Next() - } -} From f6ec213b89c27ac411c3872206cea23887ed1988 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Fri, 12 Nov 2021 10:17:06 +0800 Subject: [PATCH 04/18] refactor: move version utils to util package --- Makefile | 10 +++++----- cmd/tidb-dashboard/main.go | 4 ++-- pkg/apiserver/apiserver.go | 4 ++-- pkg/apiserver/conprof/module.go | 5 ++--- pkg/apiserver/conprof/service.go | 4 ++-- pkg/apiserver/info/info.go | 15 +++++++-------- pkg/apiserver/user/module.go | 6 ++---- pkg/apiserver/utils/mw_feature_flag.go | 5 ++--- {pkg/utils => util/versionutil}/feature_flag.go | 8 +++----- .../versionutil}/feature_flag_test.go | 8 +++----- .../utils/version => util/versionutil}/version.go | 8 ++++---- 11 files changed, 34 insertions(+), 43 deletions(-) rename {pkg/utils => util/versionutil}/feature_flag.go (92%) rename {pkg/utils => util/versionutil}/feature_flag_test.go (92%) rename {pkg/utils/version => util/versionutil}/version.go (88%) diff --git a/Makefile b/Makefile index 44a12a8ef5..fe780f8f6e 100755 --- a/Makefile +++ b/Makefile @@ -14,11 +14,11 @@ ifeq ($(DISTRO_BUILD_TAG),1) BUILD_TAGS += distro endif -LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.InternalVersion=$(shell grep -v '^\#' ./release-version)" -LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.Standalone=Yes" -LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.PDVersion=N/A" -LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildTime=$(shell date -u '+%Y-%m-%d %I:%M:%S')" -LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildGitHash=$(shell git rev-parse HEAD)" +LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.InternalVersion=$(shell grep -v '^\#' ./release-version)" +LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.Standalone=Yes" +LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.PDVersion=N/A" +LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.BuildTime=$(shell date -u '+%Y-%m-%d %I:%M:%S')" +LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.BuildGitHash=$(shell git rev-parse HEAD)" default: server diff --git a/cmd/tidb-dashboard/main.go b/cmd/tidb-dashboard/main.go index 014e2dbffa..bfaddbd307 100755 --- a/cmd/tidb-dashboard/main.go +++ b/cmd/tidb-dashboard/main.go @@ -47,8 +47,8 @@ import ( keyvisualregion "github.com/pingcap/tidb-dashboard/pkg/keyvisual/region" "github.com/pingcap/tidb-dashboard/pkg/swaggerserver" "github.com/pingcap/tidb-dashboard/pkg/uiserver" - "github.com/pingcap/tidb-dashboard/pkg/utils/version" _ "github.com/pingcap/tidb-dashboard/populate/distro" + "github.com/pingcap/tidb-dashboard/util/versionutil" ) type DashboardCLIConfig struct { @@ -95,7 +95,7 @@ func NewCLIConfig() *DashboardCLIConfig { flag.Parse() if *showVersion { - version.PrintStandaloneModeInfo() + versionutil.PrintStandaloneModeInfo() _ = log.Sync() os.Exit(0) } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 44940bae76..bbf095b9ff 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -41,6 +41,7 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso/ssoauth" "github.com/pingcap/tidb-dashboard/pkg/tiflash" + "github.com/pingcap/tidb-dashboard/util/versionutil" // "github.com/pingcap/tidb-dashboard/pkg/apiserver/__APP_NAME__" // NOTE: Don't remove above comment line, it is a placeholder for code generator. @@ -58,7 +59,6 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/tidb" "github.com/pingcap/tidb-dashboard/pkg/tikv" "github.com/pingcap/tidb-dashboard/pkg/utils" - "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) func Handler(s *Service) http.Handler { @@ -171,7 +171,7 @@ func (s *Service) Start(ctx context.Context) error { return err } - version.Print() + versionutil.Print() return nil } diff --git a/pkg/apiserver/conprof/module.go b/pkg/apiserver/conprof/module.go index 9c72969771..3a53e3a7e7 100644 --- a/pkg/apiserver/conprof/module.go +++ b/pkg/apiserver/conprof/module.go @@ -14,12 +14,11 @@ package conprof import ( + "github.com/pingcap/tidb-dashboard/util/versionutil" "go.uber.org/fx" - - "github.com/pingcap/tidb-dashboard/pkg/utils" ) -var FeatureFlagConprof = utils.NewFeatureFlag("conprof", []string{">= 5.3.0"}) +var FeatureFlagConprof = versionutil.NewFeatureFlag("conprof", []string{">= 5.3.0"}) var Module = fx.Options( fx.Provide(newService), diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 45e3b7b517..74cc4eeec7 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -32,8 +32,8 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" apiutils "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" - "github.com/pingcap/tidb-dashboard/pkg/utils" "github.com/pingcap/tidb-dashboard/pkg/utils/topology" + "github.com/pingcap/tidb-dashboard/util/versionutil" ) var ( @@ -82,7 +82,7 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(apiutils.MWForbidByFeatureFlag([]*utils.FeatureFlag{ + endpoint.Use(apiutils.MWForbidByFeatureFlag([]*versionutil.FeatureFlag{ FeatureFlagConprof, }, s.params.Config.FeatureVersion)) { diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index b5221fb486..f5b74d00b1 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -28,8 +28,7 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/dbstore" "github.com/pingcap/tidb-dashboard/pkg/tidb" - "github.com/pingcap/tidb-dashboard/pkg/utils" - "github.com/pingcap/tidb-dashboard/pkg/utils/version" + "github.com/pingcap/tidb-dashboard/util/versionutil" ) type ServiceParams struct { @@ -59,10 +58,10 @@ func RegisterRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { } type InfoResponse struct { //nolint - Version *version.Info `json:"version"` - EnableTelemetry bool `json:"enable_telemetry"` - EnableExperimental bool `json:"enable_experimental"` - SupportedFeatures []string `json:"supported_features"` + Version *versionutil.Info `json:"version"` + EnableTelemetry bool `json:"enable_telemetry"` + EnableExperimental bool `json:"enable_experimental"` + SupportedFeatures []string `json:"supported_features"` } // @ID infoGet @@ -72,7 +71,7 @@ type InfoResponse struct { //nolint // @Security JwtAuth // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) infoHandler(c *gin.Context) { - featureFlags := []*utils.FeatureFlag{ + featureFlags := []*versionutil.FeatureFlag{ conprof.FeatureFlagConprof, user.FeatureFlagNonRootLogin, } @@ -85,7 +84,7 @@ func (s *Service) infoHandler(c *gin.Context) { } resp := InfoResponse{ - Version: version.GetInfo(), + Version: versionutil.GetInfo(), EnableTelemetry: s.params.Config.EnableTelemetry, EnableExperimental: s.params.Config.EnableExperimental, SupportedFeatures: supportedFeatures, diff --git a/pkg/apiserver/user/module.go b/pkg/apiserver/user/module.go index 6818410b11..d9cf1429a8 100644 --- a/pkg/apiserver/user/module.go +++ b/pkg/apiserver/user/module.go @@ -13,8 +13,6 @@ package user -import ( - "github.com/pingcap/tidb-dashboard/pkg/utils" -) +import "github.com/pingcap/tidb-dashboard/util/versionutil" -var FeatureFlagNonRootLogin = utils.NewFeatureFlag("nonRootLogin", []string{">= 5.3.0"}) +var FeatureFlagNonRootLogin = versionutil.NewFeatureFlag("nonRootLogin", []string{">= 5.3.0"}) diff --git a/pkg/apiserver/utils/mw_feature_flag.go b/pkg/apiserver/utils/mw_feature_flag.go index 331d73788a..a3ff23f55d 100644 --- a/pkg/apiserver/utils/mw_feature_flag.go +++ b/pkg/apiserver/utils/mw_feature_flag.go @@ -18,13 +18,12 @@ import ( "net/http" "github.com/gin-gonic/gin" - - "github.com/pingcap/tidb-dashboard/pkg/utils" + "github.com/pingcap/tidb-dashboard/util/versionutil" ) var ErrFeatureNotSupported = ErrNS.NewType("feature_not_supported") -func MWForbidByFeatureFlag(featureFlags []*utils.FeatureFlag, targetVersion string) gin.HandlerFunc { +func MWForbidByFeatureFlag(featureFlags []*versionutil.FeatureFlag, targetVersion string) gin.HandlerFunc { supported := true unsupportedFeatures := make([]string, len(featureFlags)) for _, ff := range featureFlags { diff --git a/pkg/utils/feature_flag.go b/util/versionutil/feature_flag.go similarity index 92% rename from pkg/utils/feature_flag.go rename to util/versionutil/feature_flag.go index 7e3b395b80..f5146df3f4 100644 --- a/pkg/utils/feature_flag.go +++ b/util/versionutil/feature_flag.go @@ -12,14 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -package utils +package versionutil import ( "strings" "github.com/Masterminds/semver" - - "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) type FeatureFlag struct { @@ -37,8 +35,8 @@ func NewFeatureFlag(name string, constraints []string) *FeatureFlag { // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information func (ff *FeatureFlag) IsSupported(targetVersion string) bool { curVersion := targetVersion - if version.Standalone == "No" { - curVersion = version.PDVersion + if Standalone == "No" { + curVersion = PDVersion } // drop "-alpha-xxx" suffix versionWithoutSuffix := strings.Split(curVersion, "-")[0] diff --git a/pkg/utils/feature_flag_test.go b/util/versionutil/feature_flag_test.go similarity index 92% rename from pkg/utils/feature_flag_test.go rename to util/versionutil/feature_flag_test.go index 76bdedabe4..28b7eb06ec 100644 --- a/pkg/utils/feature_flag_test.go +++ b/util/versionutil/feature_flag_test.go @@ -12,14 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -package utils +package versionutil import ( "testing" "github.com/stretchr/testify/require" - - "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) func Test_IsSupported(t *testing.T) { @@ -45,8 +43,8 @@ func Test_IsSupported(t *testing.T) { require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) } - version.Standalone = "No" - version.PDVersion = "v5.3.0" + Standalone = "No" + PDVersion = "v5.3.0" ff := NewFeatureFlag("testFeature", []string{"= 5.3.0"}) require.Equal(t, true, ff.IsSupported("v100.0.0")) } diff --git a/pkg/utils/version/version.go b/util/versionutil/version.go similarity index 88% rename from pkg/utils/version/version.go rename to util/versionutil/version.go index 71a70888b4..3d9de9afd9 100644 --- a/pkg/utils/version/version.go +++ b/util/versionutil/version.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package version +package versionutil import ( "fmt" @@ -20,7 +20,7 @@ import ( "github.com/pingcap/log" "go.uber.org/zap" - "github.com/pingcap/tidb-dashboard/pkg/utils/distro" + "github.com/pingcap/tidb-dashboard/util/distro" ) type Info struct { @@ -41,10 +41,10 @@ var ( ) func Print() { - log.Info(fmt.Sprintf("%s Dashboard started", distro.Data("tidb")), + log.Info(fmt.Sprintf("%s Dashboard started", distro.R().TiDB), zap.String("internal-version", InternalVersion), zap.String("standalone", Standalone), - zap.String(fmt.Sprintf("%s-version", strings.ToLower(distro.Data("pd"))), PDVersion), + zap.String(fmt.Sprintf("%s-version", strings.ToLower(distro.R().PD)), PDVersion), zap.String("build-time", BuildTime), zap.String("build-git-hash", BuildGitHash)) } From beafba3c27538fd4fa5bd473ccf6064117d8884e Mon Sep 17 00:00:00 2001 From: Suhaha Date: Fri, 12 Nov 2021 10:30:13 +0800 Subject: [PATCH 05/18] chore: make lint happy --- pkg/apiserver/conprof/module.go | 3 ++- pkg/apiserver/utils/mw_feature_flag.go | 1 + util/versionutil/feature_flag.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/apiserver/conprof/module.go b/pkg/apiserver/conprof/module.go index 3a53e3a7e7..06eb81fac2 100644 --- a/pkg/apiserver/conprof/module.go +++ b/pkg/apiserver/conprof/module.go @@ -14,8 +14,9 @@ package conprof import ( - "github.com/pingcap/tidb-dashboard/util/versionutil" "go.uber.org/fx" + + "github.com/pingcap/tidb-dashboard/util/versionutil" ) var FeatureFlagConprof = versionutil.NewFeatureFlag("conprof", []string{">= 5.3.0"}) diff --git a/pkg/apiserver/utils/mw_feature_flag.go b/pkg/apiserver/utils/mw_feature_flag.go index a3ff23f55d..e7b998a715 100644 --- a/pkg/apiserver/utils/mw_feature_flag.go +++ b/pkg/apiserver/utils/mw_feature_flag.go @@ -18,6 +18,7 @@ import ( "net/http" "github.com/gin-gonic/gin" + "github.com/pingcap/tidb-dashboard/util/versionutil" ) diff --git a/util/versionutil/feature_flag.go b/util/versionutil/feature_flag.go index f5146df3f4..fd7517ecbb 100644 --- a/util/versionutil/feature_flag.go +++ b/util/versionutil/feature_flag.go @@ -32,7 +32,7 @@ func NewFeatureFlag(name string, constraints []string) *FeatureFlag { // IsSupported checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) -// constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information +// constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information. func (ff *FeatureFlag) IsSupported(targetVersion string) bool { curVersion := targetVersion if Standalone == "No" { From 485796043ff6c5fe215bc6ef9706f1de96a7aaa0 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Mon, 15 Nov 2021 00:59:44 +0800 Subject: [PATCH 06/18] refine(feature): flag & manager --- Makefile | 10 +-- cmd/tidb-dashboard/main.go | 4 +- pkg/apiserver/apiserver.go | 9 ++- pkg/apiserver/conprof/module.go | 6 +- pkg/apiserver/conprof/service.go | 17 ++--- pkg/apiserver/info/info.go | 45 +++++------- pkg/apiserver/user/auth.go | 4 +- pkg/apiserver/user/module.go | 13 +++- pkg/apiserver/utils/mw_feature_flag.go | 48 ------------ pkg/config/config.go | 4 +- .../utils/version}/version.go | 8 +- .../feature_flag.go => feature/flag.go} | 34 ++++++--- .../flag_test.go} | 23 +----- util/feature/manager.go | 73 +++++++++++++++++++ 14 files changed, 159 insertions(+), 139 deletions(-) delete mode 100644 pkg/apiserver/utils/mw_feature_flag.go rename {util/versionutil => pkg/utils/version}/version.go (84%) rename util/{versionutil/feature_flag.go => feature/flag.go} (52%) rename util/{versionutil/feature_flag_test.go => feature/flag_test.go} (52%) create mode 100644 util/feature/manager.go diff --git a/Makefile b/Makefile index fe780f8f6e..44a12a8ef5 100755 --- a/Makefile +++ b/Makefile @@ -14,11 +14,11 @@ ifeq ($(DISTRO_BUILD_TAG),1) BUILD_TAGS += distro endif -LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.InternalVersion=$(shell grep -v '^\#' ./release-version)" -LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.Standalone=Yes" -LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.PDVersion=N/A" -LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.BuildTime=$(shell date -u '+%Y-%m-%d %I:%M:%S')" -LDFLAGS += -X "$(DASHBOARD_PKG)/util/versionutil.BuildGitHash=$(shell git rev-parse HEAD)" +LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.InternalVersion=$(shell grep -v '^\#' ./release-version)" +LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.Standalone=Yes" +LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.PDVersion=N/A" +LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildTime=$(shell date -u '+%Y-%m-%d %I:%M:%S')" +LDFLAGS += -X "$(DASHBOARD_PKG)/pkg/utils/version.BuildGitHash=$(shell git rev-parse HEAD)" default: server diff --git a/cmd/tidb-dashboard/main.go b/cmd/tidb-dashboard/main.go index 7de023d2ae..26022828d1 100755 --- a/cmd/tidb-dashboard/main.go +++ b/cmd/tidb-dashboard/main.go @@ -36,8 +36,8 @@ import ( keyvisualregion "github.com/pingcap/tidb-dashboard/pkg/keyvisual/region" "github.com/pingcap/tidb-dashboard/pkg/swaggerserver" "github.com/pingcap/tidb-dashboard/pkg/uiserver" + "github.com/pingcap/tidb-dashboard/pkg/utils/version" _ "github.com/pingcap/tidb-dashboard/populate/distro" - "github.com/pingcap/tidb-dashboard/util/versionutil" ) type DashboardCLIConfig struct { @@ -84,7 +84,7 @@ func NewCLIConfig() *DashboardCLIConfig { flag.Parse() if *showVersion { - versionutil.PrintStandaloneModeInfo() + version.PrintStandaloneModeInfo() _ = log.Sync() os.Exit(0) } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index c8efbf2a95..5d07514ced 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -30,7 +30,8 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso/ssoauth" "github.com/pingcap/tidb-dashboard/pkg/tiflash" - "github.com/pingcap/tidb-dashboard/util/versionutil" + "github.com/pingcap/tidb-dashboard/pkg/utils/version" + "github.com/pingcap/tidb-dashboard/util/feature" // "github.com/pingcap/tidb-dashboard/pkg/apiserver/__APP_NAME__" // NOTE: Don't remove above comment line, it is a placeholder for code generator. @@ -103,6 +104,7 @@ func (s *Service) Start(ctx context.Context) error { s.app = fx.New( fx.Logger(utils.NewFxPrinter()), fx.Provide( + feature.NewManagerProvider(s.config.FeatureVersion), newAPIHandlerEngine, s.provideLocals, dbstore.NewDBStore, @@ -114,7 +116,6 @@ func (s *Service) Start(ctx context.Context) error { tikv.NewTiKVClient, tiflash.NewTiFlashClient, utils.NewSysSchema, - user.NewAuthService, info.NewService, clusterinfo.NewService, logsearch.NewService, @@ -126,6 +127,7 @@ func (s *Service) Start(ctx context.Context) error { // __APP_NAME__.NewService, // NOTE: Don't remove above comment line, it is a placeholder for code generator ), + user.Module, codeauth.Module, sqlauth.Module, ssoauth.Module, @@ -138,7 +140,6 @@ func (s *Service) Start(ctx context.Context) error { debugapi.Module, fx.Populate(&s.apiHandlerEngine), fx.Invoke( - user.RegisterRouter, info.RegisterRouter, clusterinfo.RegisterRouter, profiling.RegisterRouter, @@ -160,7 +161,7 @@ func (s *Service) Start(ctx context.Context) error { return err } - versionutil.Print() + version.Print() return nil } diff --git a/pkg/apiserver/conprof/module.go b/pkg/apiserver/conprof/module.go index 369bf00acb..85de61312c 100644 --- a/pkg/apiserver/conprof/module.go +++ b/pkg/apiserver/conprof/module.go @@ -5,12 +5,12 @@ package conprof import ( "go.uber.org/fx" - "github.com/pingcap/tidb-dashboard/util/versionutil" + "github.com/pingcap/tidb-dashboard/util/feature" ) -var FeatureFlagConprof = versionutil.NewFeatureFlag("conprof", []string{">= 5.3.0"}) +var FeatureFlagConprof = feature.NewFlag("conprof", []string{">= 5.3.0"}) var Module = fx.Options( fx.Provide(newService), - fx.Invoke(registerRouter), + fx.Invoke(registerRouter, FeatureFlagConprof.Register()), ) diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 74b44c9d91..a5eede1427 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -19,10 +19,10 @@ import ( "golang.org/x/sync/singleflight" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" - apiutils "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/utils/topology" - "github.com/pingcap/tidb-dashboard/util/versionutil" + "github.com/pingcap/tidb-dashboard/util/feature" ) var ( @@ -68,12 +68,11 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { } // Register register the handlers to the service. -func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { +func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service, fm *feature.Manager) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(apiutils.MWForbidByFeatureFlag([]*versionutil.FeatureFlag{ - FeatureFlagConprof, - }, s.params.Config.FeatureVersion)) + featureFlags := []*feature.Flag{FeatureFlagConprof} + endpoint.Use(fm.Guard(featureFlags)) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) @@ -99,9 +98,9 @@ func (s *Service) reverseProxy(targetPath string) gin.HandlerFunc { c.Request.URL.Path = targetPath token := c.Query("token") if token != "" { - queryStr, err := apiutils.ParseJWTString("conprof", token) + queryStr, err := utils.ParseJWTString("conprof", token) if err != nil { - apiutils.MakeInvalidRequestErrorFromError(c, err) + utils.MakeInvalidRequestErrorFromError(c, err) return } c.Request.URL.RawQuery = queryStr @@ -287,7 +286,7 @@ func (s *Service) conprofGroupProfileDetail(c *gin.Context) { // @Failure 500 {object} utils.APIError func (s *Service) genConprofActionToken(c *gin.Context) { q := c.Query("q") - token, err := apiutils.NewJWTString("conprof", q) + token, err := utils.NewJWTString("conprof", q) if err != nil { _ = c.Error(err) return diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index c09e01786d..c2679d4334 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -11,20 +11,21 @@ import ( "github.com/thoas/go-funk" "go.uber.org/fx" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/conprof" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" - apiutils "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/dbstore" "github.com/pingcap/tidb-dashboard/pkg/tidb" - "github.com/pingcap/tidb-dashboard/util/versionutil" + "github.com/pingcap/tidb-dashboard/pkg/utils/version" + "github.com/pingcap/tidb-dashboard/util/feature" ) type ServiceParams struct { fx.In - Config *config.Config - LocalStore *dbstore.DB - TiDBClient *tidb.Client + Config *config.Config + LocalStore *dbstore.DB + TiDBClient *tidb.Client + FeatureManager *feature.Manager } type Service struct { @@ -41,16 +42,16 @@ func RegisterRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint.Use(auth.MWAuthRequired()) endpoint.GET("/whoami", s.whoamiHandler) - endpoint.Use(apiutils.MWConnectTiDB(s.params.TiDBClient)) + endpoint.Use(utils.MWConnectTiDB(s.params.TiDBClient)) endpoint.GET("/databases", s.databasesHandler) endpoint.GET("/tables", s.tablesHandler) } type InfoResponse struct { //nolint - Version *versionutil.Info `json:"version"` - EnableTelemetry bool `json:"enable_telemetry"` - EnableExperimental bool `json:"enable_experimental"` - SupportedFeatures []string `json:"supported_features"` + Version *version.Info `json:"version"` + EnableTelemetry bool `json:"enable_telemetry"` + EnableExperimental bool `json:"enable_experimental"` + SupportedFeatures []string `json:"supported_features"` } // @ID infoGet @@ -60,23 +61,11 @@ type InfoResponse struct { //nolint // @Security JwtAuth // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) infoHandler(c *gin.Context) { - featureFlags := []*versionutil.FeatureFlag{ - conprof.FeatureFlagConprof, - user.FeatureFlagNonRootLogin, - } - supportedFeatures := []string{} - for _, ff := range featureFlags { - if !ff.IsSupported(s.params.Config.FeatureVersion) { - continue - } - supportedFeatures = append(supportedFeatures, ff.Name) - } - resp := InfoResponse{ - Version: versionutil.GetInfo(), + Version: version.GetInfo(), EnableTelemetry: s.params.Config.EnableTelemetry, EnableExperimental: s.params.Config.EnableExperimental, - SupportedFeatures: supportedFeatures, + SupportedFeatures: s.params.FeatureManager.SupportedFeatures(), } c.JSON(http.StatusOK, resp) } @@ -94,7 +83,7 @@ type WhoAmIResponse struct { // @Security JwtAuth // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) whoamiHandler(c *gin.Context) { - sessionUser := apiutils.GetSession(c) + sessionUser := utils.GetSession(c) resp := WhoAmIResponse{ DisplayName: sessionUser.DisplayName, IsShareable: sessionUser.IsShareable, @@ -114,7 +103,7 @@ func (s *Service) databasesHandler(c *gin.Context) { Databases string `gorm:"column:Database"` } var result []databaseSchemas - db := apiutils.GetTiDBConnection(c) + db := utils.GetTiDBConnection(c) err := db.Raw("SHOW DATABASES").Scan(&result).Error if err != nil { _ = c.Error(err) @@ -142,7 +131,7 @@ type tableSchema struct { // @Failure 401 {object} utils.APIError "Unauthorized failure" func (s *Service) tablesHandler(c *gin.Context) { var result []tableSchema - db := apiutils.GetTiDBConnection(c) + db := utils.GetTiDBConnection(c) tx := db.Select([]string{"TABLE_NAME", "TIDB_TABLE_ID"}).Table("INFORMATION_SCHEMA.TABLES") databaseName := c.Query("database_name") diff --git a/pkg/apiserver/user/auth.go b/pkg/apiserver/user/auth.go index 1f47794315..c45a347e01 100644 --- a/pkg/apiserver/user/auth.go +++ b/pkg/apiserver/user/auth.go @@ -70,7 +70,7 @@ func (a BaseAuthenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) return &SignOutInfo{}, nil } -func NewAuthService() *AuthService { +func newAuthService() *AuthService { var secret *[32]byte secretStr := os.Getenv("DASHBOARD_SESSION_SECRET") @@ -219,7 +219,7 @@ func (s *AuthService) authForm(f AuthenticateForm) (*utils.SessionUser, error) { return u, nil } -func RegisterRouter(r *gin.RouterGroup, s *AuthService) { +func registerRouter(r *gin.RouterGroup, s *AuthService) { endpoint := r.Group("/user") endpoint.GET("/login_info", s.getLoginInfoHandler) endpoint.POST("/login", s.loginHandler) diff --git a/pkg/apiserver/user/module.go b/pkg/apiserver/user/module.go index 331a02cdde..a3d4560a55 100644 --- a/pkg/apiserver/user/module.go +++ b/pkg/apiserver/user/module.go @@ -2,6 +2,15 @@ package user -import "github.com/pingcap/tidb-dashboard/util/versionutil" +import ( + "go.uber.org/fx" -var FeatureFlagNonRootLogin = versionutil.NewFeatureFlag("nonRootLogin", []string{">= 5.3.0"}) + "github.com/pingcap/tidb-dashboard/util/feature" +) + +var FeatureFlagNonRootLogin = feature.NewFlag("nonRootLogin", []string{">= 5.3.0"}) + +var Module = fx.Options( + fx.Provide(newAuthService), + fx.Invoke(registerRouter, FeatureFlagNonRootLogin.Register()), +) diff --git a/pkg/apiserver/utils/mw_feature_flag.go b/pkg/apiserver/utils/mw_feature_flag.go deleted file mode 100644 index e7b998a715..0000000000 --- a/pkg/apiserver/utils/mw_feature_flag.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2021 Suhaha -// -// 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 utils - -import ( - "net/http" - - "github.com/gin-gonic/gin" - - "github.com/pingcap/tidb-dashboard/util/versionutil" -) - -var ErrFeatureNotSupported = ErrNS.NewType("feature_not_supported") - -func MWForbidByFeatureFlag(featureFlags []*versionutil.FeatureFlag, targetVersion string) gin.HandlerFunc { - supported := true - unsupportedFeatures := make([]string, len(featureFlags)) - for _, ff := range featureFlags { - if !ff.IsSupported(targetVersion) { - supported = false - unsupportedFeatures = append(unsupportedFeatures, ff.Name) - continue - } - } - - return func(c *gin.Context) { - if !supported { - _ = c.Error(ErrFeatureNotSupported.New("unsupported features: %v", unsupportedFeatures)) - c.Status(http.StatusForbidden) - c.Abort() - return - } - - c.Next() - } -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 824d942858..1e6069b30c 100755 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -6,6 +6,8 @@ import ( "crypto/tls" "net/url" "strings" + + "github.com/pingcap/tidb-dashboard/pkg/utils/version" ) const ( @@ -40,7 +42,7 @@ func Default() *Config { TiDBTLSConfig: nil, EnableTelemetry: true, EnableExperimental: false, - FeatureVersion: "", + FeatureVersion: version.PDVersion, } } diff --git a/util/versionutil/version.go b/pkg/utils/version/version.go similarity index 84% rename from util/versionutil/version.go rename to pkg/utils/version/version.go index ce442b2c86..dc620025fb 100644 --- a/util/versionutil/version.go +++ b/pkg/utils/version/version.go @@ -1,6 +1,6 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package versionutil +package version import ( "fmt" @@ -9,7 +9,7 @@ import ( "github.com/pingcap/log" "go.uber.org/zap" - "github.com/pingcap/tidb-dashboard/util/distro" + "github.com/pingcap/tidb-dashboard/pkg/utils/distro" ) type Info struct { @@ -30,10 +30,10 @@ var ( ) func Print() { - log.Info(fmt.Sprintf("%s Dashboard started", distro.R().TiDB), + log.Info(fmt.Sprintf("%s Dashboard started", distro.Data("tidb")), zap.String("internal-version", InternalVersion), zap.String("standalone", Standalone), - zap.String(fmt.Sprintf("%s-version", strings.ToLower(distro.R().PD)), PDVersion), + zap.String(fmt.Sprintf("%s-version", strings.ToLower(distro.Data("pd"))), PDVersion), zap.String("build-time", BuildTime), zap.String("build-git-hash", BuildGitHash)) } diff --git a/util/versionutil/feature_flag.go b/util/feature/flag.go similarity index 52% rename from util/versionutil/feature_flag.go rename to util/feature/flag.go index 4bf5047c33..ebd25614e9 100644 --- a/util/versionutil/feature_flag.go +++ b/util/feature/flag.go @@ -1,38 +1,40 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package versionutil +package feature import ( "strings" + "sync" "github.com/Masterminds/semver" + + "github.com/pingcap/tidb-dashboard/util/nocopy" ) -type FeatureFlag struct { +type Flag struct { + nocopy.NoCopy + Name string + once sync.Once constraints []string } -func NewFeatureFlag(name string, constraints []string) *FeatureFlag { - return &FeatureFlag{Name: name, constraints: constraints} +func NewFlag(name string, constraints []string) *Flag { + return &Flag{Name: name, constraints: constraints} } // IsSupported checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information. -func (ff *FeatureFlag) IsSupported(targetVersion string) bool { - curVersion := targetVersion - if Standalone == "No" { - curVersion = PDVersion - } +func (f *Flag) IsSupported(targetVersion string) bool { // drop "-alpha-xxx" suffix - versionWithoutSuffix := strings.Split(curVersion, "-")[0] + versionWithoutSuffix := strings.Split(targetVersion, "-")[0] v, err := semver.NewVersion(versionWithoutSuffix) if err != nil { return false } - for _, ver := range ff.constraints { + for _, ver := range f.constraints { c, err := semver.NewConstraint(ver) if err != nil { continue @@ -43,3 +45,13 @@ func (ff *FeatureFlag) IsSupported(targetVersion string) bool { } return false } + +// Register feature.Flag to feature.Manager, this can be easily used with fx. +// e.g. `fx.Invoke(featureFlag.Register())`. +func (f *Flag) Register() func(m *Manager) { + return func(m *Manager) { + f.once.Do(func() { + m.Register(f) + }) + } +} diff --git a/util/versionutil/feature_flag_test.go b/util/feature/flag_test.go similarity index 52% rename from util/versionutil/feature_flag_test.go rename to util/feature/flag_test.go index 28b7eb06ec..a1b4f5667f 100644 --- a/util/versionutil/feature_flag_test.go +++ b/util/feature/flag_test.go @@ -1,18 +1,6 @@ -// Copyright 2021 Suhaha -// -// 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. +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package versionutil +package feature import ( "testing" @@ -39,12 +27,7 @@ func Test_IsSupported(t *testing.T) { } for _, tt := range tests { - ff := NewFeatureFlag("testFeature", tt.args.constraints) + ff := NewFlag("testFeature", tt.args.constraints) require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) } - - Standalone = "No" - PDVersion = "v5.3.0" - ff := NewFeatureFlag("testFeature", []string{"= 5.3.0"}) - require.Equal(t, true, ff.IsSupported("v100.0.0")) } diff --git a/util/feature/manager.go b/util/feature/manager.go new file mode 100644 index 0000000000..ae230ce631 --- /dev/null +++ b/util/feature/manager.go @@ -0,0 +1,73 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package feature + +import ( + "fmt" + "net/http" + + "github.com/gin-gonic/gin" +) + +type Manager struct { + version string + flags []*Flag + supportedMap map[string]struct{} +} + +func NewManager(version string) *Manager { + return &Manager{ + version: version, + flags: []*Flag{}, + supportedMap: map[string]struct{}{}, + } +} + +// NewManagerProvider returns a fx.Provider. +func NewManagerProvider(version string) func() *Manager { + return func() *Manager { + return NewManager(version) + } +} + +// SupportedFeatures returns supported feature's names. +func (m *Manager) SupportedFeatures() []string { + sf := make([]string, 0, len(m.supportedMap)) + for k := range m.supportedMap { + sf = append(sf, k) + } + return sf +} + +// Register feature flag. +func (m *Manager) Register(f *Flag) { + m.flags = append(m.flags, f) + if f.IsSupported(m.version) { + m.supportedMap[f.Name] = struct{}{} + } +} + +// Guard returns gin.HandlerFunc as guard middleware. +// It will determine if features are available in the current version. +func (m *Manager) Guard(featureFlags []*Flag) gin.HandlerFunc { + supported := true + unsupportedFeatures := make([]string, len(featureFlags)) + for _, ff := range featureFlags { + if _, ok := m.supportedMap[ff.Name]; !ok { + supported = false + unsupportedFeatures = append(unsupportedFeatures, ff.Name) + continue + } + } + + return func(c *gin.Context) { + if !supported { + _ = c.Error(fmt.Errorf("unsupported features: %v", unsupportedFeatures)) + c.Status(http.StatusForbidden) + c.Abort() + return + } + + c.Next() + } +} From 8c9c89cc03aa5a1850335db1244063f3c188092c Mon Sep 17 00:00:00 2001 From: Suhaha Date: Mon, 15 Nov 2021 09:48:15 +0800 Subject: [PATCH 07/18] test(feature_flag): add manager tests --- util/feature/flag_test.go | 12 ++++++ util/feature/manager.go | 18 ++++---- util/feature/manager_test.go | 79 ++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 util/feature/manager_test.go diff --git a/util/feature/flag_test.go b/util/feature/flag_test.go index a1b4f5667f..75661a9f04 100644 --- a/util/feature/flag_test.go +++ b/util/feature/flag_test.go @@ -31,3 +31,15 @@ func Test_IsSupported(t *testing.T) { require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) } } + +func Test_Register_toManager(t *testing.T) { + f1 := NewFlag("testFeature", []string{">= 5.3.0"}) + f2 := injectManager(f1.Register()) + require.Equal(t, f1, f2) +} + +func injectManager(rf func(m *Manager)) *Flag { + m := NewManager("v5.3.0") + rf(m) + return m.flags[0] +} diff --git a/util/feature/manager.go b/util/feature/manager.go index ae230ce631..57d571b509 100644 --- a/util/feature/manager.go +++ b/util/feature/manager.go @@ -5,6 +5,7 @@ package feature import ( "fmt" "net/http" + "sort" "github.com/gin-gonic/gin" ) @@ -30,23 +31,24 @@ func NewManagerProvider(version string) func() *Manager { } } +// Register feature flag. +func (m *Manager) Register(f *Flag) { + m.flags = append(m.flags, f) + if f.IsSupported(m.version) { + m.supportedMap[f.Name] = struct{}{} + } +} + // SupportedFeatures returns supported feature's names. func (m *Manager) SupportedFeatures() []string { sf := make([]string, 0, len(m.supportedMap)) for k := range m.supportedMap { sf = append(sf, k) } + sort.Strings(sf) return sf } -// Register feature flag. -func (m *Manager) Register(f *Flag) { - m.flags = append(m.flags, f) - if f.IsSupported(m.version) { - m.supportedMap[f.Name] = struct{}{} - } -} - // Guard returns gin.HandlerFunc as guard middleware. // It will determine if features are available in the current version. func (m *Manager) Guard(featureFlags []*Flag) gin.HandlerFunc { diff --git a/util/feature/manager_test.go b/util/feature/manager_test.go new file mode 100644 index 0000000000..500aa6242d --- /dev/null +++ b/util/feature/manager_test.go @@ -0,0 +1,79 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package feature + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" +) + +func Test_Register(t *testing.T) { + m := NewManager("v5.3.0") + tests := []struct { + supported bool + flag *Flag + }{ + {supported: true, flag: NewFlag("testFeature1", []string{">= 5.3.0"})}, + {supported: true, flag: NewFlag("testFeature2", []string{">= 4.0.0"})}, + {supported: false, flag: NewFlag("testFeature3", []string{">= 5.3.1"})}, + } + + for _, tt := range tests { + m.Register(tt.flag) + } + + for i, tt := range tests { + require.Equal(t, m.flags[i], tt.flag) + _, ok := m.supportedMap[tt.flag.Name] + require.Equal(t, tt.supported, ok) + } +} + +func Test_SupportedFeatures(t *testing.T) { + m := NewManager("v5.3.0") + f1 := NewFlag("testFeature1", []string{">= 5.3.0"}) + f2 := NewFlag("testFeature2", []string{">= 4.0.0"}) + f3 := NewFlag("testFeature3", []string{">= 5.3.1"}) + m.Register(f1) + m.Register(f2) + m.Register(f3) + + require.Equal(t, []string{"testFeature1", "testFeature2"}, m.SupportedFeatures()) +} + +func Test_Guard(t *testing.T) { + m := NewManager("v5.3.0") + f1 := NewFlag("testFeature1", []string{">= 5.3.0"}) + f2 := NewFlag("testFeature2", []string{">= 5.3.1"}) + m.Register(f1) + m.Register(f2) + + // success + r := gin.Default() + r.Use(m.Guard([]*Flag{f1})) + r.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/ping", nil) + r.ServeHTTP(w, req) + + require.Equal(t, 200, w.Code) + require.Equal(t, "pong", w.Body.String()) + + // StatusForbidden + r2 := gin.Default() + r2.Use(m.Guard([]*Flag{f1, f2})) + r2.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest("GET", "/ping", nil) + r2.ServeHTTP(w2, req2) + + require.Equal(t, 403, w2.Code) +} From 28a97f31674b26a3a5d3b99b1e06525d214d6a91 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 14:53:44 +0800 Subject: [PATCH 08/18] refine(util): feature flag --- util/feature/manager.go | 75 ------------------ util/feature/manager_test.go | 79 ------------------- .../flag.go => featureflag/featureflag.go} | 36 ++++----- .../featureflag_test.go} | 24 +++--- util/featureflag/registry.go | 48 +++++++++++ util/featureflag/registry_test.go | 53 +++++++++++++ util/featureflag/version_guard.go | 38 +++++++++ util/featureflag/version_guard_test.go | 54 +++++++++++++ 8 files changed, 217 insertions(+), 190 deletions(-) delete mode 100644 util/feature/manager.go delete mode 100644 util/feature/manager_test.go rename util/{feature/flag.go => featureflag/featureflag.go} (58%) rename util/{feature/flag_test.go => featureflag/featureflag_test.go} (74%) create mode 100644 util/featureflag/registry.go create mode 100644 util/featureflag/registry_test.go create mode 100644 util/featureflag/version_guard.go create mode 100644 util/featureflag/version_guard_test.go diff --git a/util/feature/manager.go b/util/feature/manager.go deleted file mode 100644 index 57d571b509..0000000000 --- a/util/feature/manager.go +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. - -package feature - -import ( - "fmt" - "net/http" - "sort" - - "github.com/gin-gonic/gin" -) - -type Manager struct { - version string - flags []*Flag - supportedMap map[string]struct{} -} - -func NewManager(version string) *Manager { - return &Manager{ - version: version, - flags: []*Flag{}, - supportedMap: map[string]struct{}{}, - } -} - -// NewManagerProvider returns a fx.Provider. -func NewManagerProvider(version string) func() *Manager { - return func() *Manager { - return NewManager(version) - } -} - -// Register feature flag. -func (m *Manager) Register(f *Flag) { - m.flags = append(m.flags, f) - if f.IsSupported(m.version) { - m.supportedMap[f.Name] = struct{}{} - } -} - -// SupportedFeatures returns supported feature's names. -func (m *Manager) SupportedFeatures() []string { - sf := make([]string, 0, len(m.supportedMap)) - for k := range m.supportedMap { - sf = append(sf, k) - } - sort.Strings(sf) - return sf -} - -// Guard returns gin.HandlerFunc as guard middleware. -// It will determine if features are available in the current version. -func (m *Manager) Guard(featureFlags []*Flag) gin.HandlerFunc { - supported := true - unsupportedFeatures := make([]string, len(featureFlags)) - for _, ff := range featureFlags { - if _, ok := m.supportedMap[ff.Name]; !ok { - supported = false - unsupportedFeatures = append(unsupportedFeatures, ff.Name) - continue - } - } - - return func(c *gin.Context) { - if !supported { - _ = c.Error(fmt.Errorf("unsupported features: %v", unsupportedFeatures)) - c.Status(http.StatusForbidden) - c.Abort() - return - } - - c.Next() - } -} diff --git a/util/feature/manager_test.go b/util/feature/manager_test.go deleted file mode 100644 index 500aa6242d..0000000000 --- a/util/feature/manager_test.go +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. - -package feature - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/gin-gonic/gin" - "github.com/stretchr/testify/require" -) - -func Test_Register(t *testing.T) { - m := NewManager("v5.3.0") - tests := []struct { - supported bool - flag *Flag - }{ - {supported: true, flag: NewFlag("testFeature1", []string{">= 5.3.0"})}, - {supported: true, flag: NewFlag("testFeature2", []string{">= 4.0.0"})}, - {supported: false, flag: NewFlag("testFeature3", []string{">= 5.3.1"})}, - } - - for _, tt := range tests { - m.Register(tt.flag) - } - - for i, tt := range tests { - require.Equal(t, m.flags[i], tt.flag) - _, ok := m.supportedMap[tt.flag.Name] - require.Equal(t, tt.supported, ok) - } -} - -func Test_SupportedFeatures(t *testing.T) { - m := NewManager("v5.3.0") - f1 := NewFlag("testFeature1", []string{">= 5.3.0"}) - f2 := NewFlag("testFeature2", []string{">= 4.0.0"}) - f3 := NewFlag("testFeature3", []string{">= 5.3.1"}) - m.Register(f1) - m.Register(f2) - m.Register(f3) - - require.Equal(t, []string{"testFeature1", "testFeature2"}, m.SupportedFeatures()) -} - -func Test_Guard(t *testing.T) { - m := NewManager("v5.3.0") - f1 := NewFlag("testFeature1", []string{">= 5.3.0"}) - f2 := NewFlag("testFeature2", []string{">= 5.3.1"}) - m.Register(f1) - m.Register(f2) - - // success - r := gin.Default() - r.Use(m.Guard([]*Flag{f1})) - r.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") - }) - w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/ping", nil) - r.ServeHTTP(w, req) - - require.Equal(t, 200, w.Code) - require.Equal(t, "pong", w.Body.String()) - - // StatusForbidden - r2 := gin.Default() - r2.Use(m.Guard([]*Flag{f1, f2})) - r2.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") - }) - w2 := httptest.NewRecorder() - req2, _ := http.NewRequest("GET", "/ping", nil) - r2.ServeHTTP(w2, req2) - - require.Equal(t, 403, w2.Code) -} diff --git a/util/feature/flag.go b/util/featureflag/featureflag.go similarity index 58% rename from util/feature/flag.go rename to util/featureflag/featureflag.go index ebd25614e9..b2d1ea09b4 100644 --- a/util/feature/flag.go +++ b/util/featureflag/featureflag.go @@ -1,33 +1,35 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package feature +package featureflag import ( "strings" - "sync" "github.com/Masterminds/semver" - - "github.com/pingcap/tidb-dashboard/util/nocopy" + "github.com/joomcode/errorx" ) -type Flag struct { - nocopy.NoCopy - - Name string +var ( + ErrNS = errorx.NewNamespace("feature_flag") +) - once sync.Once +type FeatureFlag struct { + name string constraints []string } -func NewFlag(name string, constraints []string) *Flag { - return &Flag{Name: name, constraints: constraints} +func NewFeatureFlag(name string, constraints ...string) *FeatureFlag { + return &FeatureFlag{name: name, constraints: constraints} +} + +func (f *FeatureFlag) Name() string { + return f.name } // IsSupported checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information. -func (f *Flag) IsSupported(targetVersion string) bool { +func (f *FeatureFlag) IsSupported(targetVersion string) bool { // drop "-alpha-xxx" suffix versionWithoutSuffix := strings.Split(targetVersion, "-")[0] v, err := semver.NewVersion(versionWithoutSuffix) @@ -45,13 +47,3 @@ func (f *Flag) IsSupported(targetVersion string) bool { } return false } - -// Register feature.Flag to feature.Manager, this can be easily used with fx. -// e.g. `fx.Invoke(featureFlag.Register())`. -func (f *Flag) Register() func(m *Manager) { - return func(m *Manager) { - f.once.Do(func() { - m.Register(f) - }) - } -} diff --git a/util/feature/flag_test.go b/util/featureflag/featureflag_test.go similarity index 74% rename from util/feature/flag_test.go rename to util/featureflag/featureflag_test.go index 75661a9f04..78cc39f3e7 100644 --- a/util/feature/flag_test.go +++ b/util/featureflag/featureflag_test.go @@ -1,6 +1,6 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package feature +package featureflag import ( "testing" @@ -8,6 +8,14 @@ import ( "github.com/stretchr/testify/require" ) +func Test_Name(t *testing.T) { + f1 := &FeatureFlag{} + require.Equal(t, f1.Name(), "") + + f2 := NewFeatureFlag("testFeature") + require.Equal(t, f2.Name(), "testFeature") +} + func Test_IsSupported(t *testing.T) { type Args struct { target string @@ -27,19 +35,7 @@ func Test_IsSupported(t *testing.T) { } for _, tt := range tests { - ff := NewFlag("testFeature", tt.args.constraints) + ff := NewFeatureFlag("testFeature", tt.args.constraints...) require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) } } - -func Test_Register_toManager(t *testing.T) { - f1 := NewFlag("testFeature", []string{">= 5.3.0"}) - f2 := injectManager(f1.Register()) - require.Equal(t, f1, f2) -} - -func injectManager(rf func(m *Manager)) *Flag { - m := NewManager("v5.3.0") - rf(m) - return m.flags[0] -} diff --git a/util/featureflag/registry.go b/util/featureflag/registry.go new file mode 100644 index 0000000000..59d788e9fc --- /dev/null +++ b/util/featureflag/registry.go @@ -0,0 +1,48 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package featureflag + +import ( + "sort" +) + +type Registry struct { + version string + flags []*FeatureFlag + supportedMap map[string]struct{} +} + +func NewRegistry(version string) *Registry { + return &Registry{ + version: version, + flags: []*FeatureFlag{}, + supportedMap: map[string]struct{}{}, + } +} + +// NewRegistryProvider returns a fx.Provider. +func NewRegistryProvider(version string) func() *Registry { + return func() *Registry { + return NewRegistry(version) + } +} + +// Register feature flag. +func (m *Registry) Register(name string, constraints ...string) *FeatureFlag { + f := NewFeatureFlag(name, constraints...) + m.flags = append(m.flags, f) + if f.IsSupported(m.version) { + m.supportedMap[f.Name()] = struct{}{} + } + return f +} + +// SupportedFeatures returns supported feature's names. +func (m *Registry) SupportedFeatures() []string { + sf := make([]string, 0, len(m.supportedMap)) + for k := range m.supportedMap { + sf = append(sf, k) + } + sort.Strings(sf) + return sf +} diff --git a/util/featureflag/registry_test.go b/util/featureflag/registry_test.go new file mode 100644 index 0000000000..7d17957d90 --- /dev/null +++ b/util/featureflag/registry_test.go @@ -0,0 +1,53 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package featureflag + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_Register(t *testing.T) { + m := NewRegistry("v5.3.0") + tests := []*struct { + supported bool + name string + constraints []string + flag *FeatureFlag + }{ + {supported: true, name: "testFeature1", constraints: []string{">= 5.3.0"}}, + {supported: true, name: "testFeature2", constraints: []string{">= 4.0.0"}}, + {supported: false, name: "testFeature3", constraints: []string{">= 5.3.1"}}, + } + + for _, tt := range tests { + tt.flag = m.Register(tt.name, tt.constraints...) + } + + for i, tt := range tests { + // check whether flag is in flags & supportedMap + require.Equal(t, m.flags[i], tt.flag) + _, ok := m.supportedMap[tt.flag.name] + require.Equal(t, tt.supported, ok) + } +} + +func Test_SupportedFeatures(t *testing.T) { + m := NewRegistry("v5.3.0") + tests := []*struct { + supported bool + name string + constraints []string + }{ + {supported: true, name: "testFeature1", constraints: []string{">= 5.3.0"}}, + {supported: true, name: "testFeature2", constraints: []string{">= 4.0.0"}}, + {supported: false, name: "testFeature3", constraints: []string{">= 5.3.1"}}, + } + + for _, tt := range tests { + m.Register(tt.name, tt.constraints...) + } + + require.Equal(t, []string{"testFeature1", "testFeature2"}, m.SupportedFeatures()) +} diff --git a/util/featureflag/version_guard.go b/util/featureflag/version_guard.go new file mode 100644 index 0000000000..4afb51f864 --- /dev/null +++ b/util/featureflag/version_guard.go @@ -0,0 +1,38 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package featureflag + +import ( + "net/http" + + "github.com/gin-gonic/gin" +) + +var ( + ErrFeatureUnsupported = ErrNS.NewType("feature_unsupported") +) + +// VersionGuard returns gin.HandlerFunc as guard middleware. +// It will determine if features are available in the target version. +func VersionGuard(version string, featureFlags ...*FeatureFlag) gin.HandlerFunc { + supported := true + unsupportedFeatures := make([]string, len(featureFlags)) + for _, ff := range featureFlags { + if !ff.IsSupported(version) { + supported = false + unsupportedFeatures = append(unsupportedFeatures, ff.Name()) + continue + } + } + + return func(c *gin.Context) { + if !supported { + _ = c.Error(ErrFeatureUnsupported.New("list: %v", unsupportedFeatures)) + c.Status(http.StatusForbidden) + c.Abort() + return + } + + c.Next() + } +} diff --git a/util/featureflag/version_guard_test.go b/util/featureflag/version_guard_test.go new file mode 100644 index 0000000000..bd19eb899a --- /dev/null +++ b/util/featureflag/version_guard_test.go @@ -0,0 +1,54 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package featureflag + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/joomcode/errorx" + "github.com/stretchr/testify/require" +) + +func Test_VersionGuard(t *testing.T) { + r := require.New(t) + m := NewRegistry("v5.3.0") + f1 := m.Register("testFeature1", ">= 5.3.0") + f2 := m.Register("testFeature2", ">= 5.3.1") + + // success + e := gin.Default() + e.Use(VersionGuard("v5.3.0", f1)) + e.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/ping", nil) + e.ServeHTTP(w, req) + + r.Equal(200, w.Code) + r.Equal("pong", w.Body.String()) + + // StatusForbidden + e2 := gin.Default() + Handled := false + e2.Use(func(c *gin.Context) { + c.Next() + + // test error type + Handled = true + r.Equal(true, errorx.IsOfType(c.Errors[0].Err, ErrFeatureUnsupported)) + }) + e2.Use(VersionGuard("v5.3.0", f1, f2)) + e2.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest("GET", "/ping", nil) + e2.ServeHTTP(w2, req2) + + r.Equal(http.StatusForbidden, w2.Code) + r.Equal(true, Handled) +} From a9382e9cad7e1b77d8fd4aed0e9773e1fbe9ccd1 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 15:06:14 +0800 Subject: [PATCH 09/18] *: update feature flag --- pkg/apiserver/apiserver.go | 4 ++-- pkg/apiserver/conprof/module.go | 6 +----- pkg/apiserver/conprof/service.go | 22 +++++++++++++++------- pkg/apiserver/user/auth.go | 10 +++++++--- pkg/apiserver/user/module.go | 6 +----- util/featureflag/registry.go | 4 ++-- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 5d07514ced..cc977b4b41 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -31,7 +31,7 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso/ssoauth" "github.com/pingcap/tidb-dashboard/pkg/tiflash" "github.com/pingcap/tidb-dashboard/pkg/utils/version" - "github.com/pingcap/tidb-dashboard/util/feature" + "github.com/pingcap/tidb-dashboard/util/featureflag" // "github.com/pingcap/tidb-dashboard/pkg/apiserver/__APP_NAME__" // NOTE: Don't remove above comment line, it is a placeholder for code generator. @@ -104,7 +104,7 @@ func (s *Service) Start(ctx context.Context) error { s.app = fx.New( fx.Logger(utils.NewFxPrinter()), fx.Provide( - feature.NewManagerProvider(s.config.FeatureVersion), + featureflag.ProvideRegistry(s.config.FeatureVersion), newAPIHandlerEngine, s.provideLocals, dbstore.NewDBStore, diff --git a/pkg/apiserver/conprof/module.go b/pkg/apiserver/conprof/module.go index 85de61312c..0e943dadce 100644 --- a/pkg/apiserver/conprof/module.go +++ b/pkg/apiserver/conprof/module.go @@ -4,13 +4,9 @@ package conprof import ( "go.uber.org/fx" - - "github.com/pingcap/tidb-dashboard/util/feature" ) -var FeatureFlagConprof = feature.NewFlag("conprof", []string{">= 5.3.0"}) - var Module = fx.Options( fx.Provide(newService), - fx.Invoke(registerRouter, FeatureFlagConprof.Register()), + fx.Invoke(registerRouter), ) diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index a5eede1427..e74a90ea8f 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -22,7 +22,7 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/utils/topology" - "github.com/pingcap/tidb-dashboard/util/feature" + "github.com/pingcap/tidb-dashboard/util/featureflag" ) var ( @@ -44,11 +44,14 @@ type ngMonitoringAddrCacheEntity struct { type ServiceParams struct { fx.In - EtcdClient *clientv3.Client - Config *config.Config + EtcdClient *clientv3.Client + Config *config.Config + FeatureFlagRegistry *featureflag.Registry } type Service struct { + FeatureFlagConprof *featureflag.FeatureFlag + params ServiceParams lifecycleCtx context.Context @@ -57,22 +60,27 @@ type Service struct { } func newService(lc fx.Lifecycle, p ServiceParams) *Service { - s := &Service{params: p} + + s := &Service{ + FeatureFlagConprof: p.FeatureFlagRegistry.Register("conprof", ">= 5.3.0"), + params: p, + } + lc.Append(fx.Hook{ OnStart: func(ctx context.Context) error { s.lifecycleCtx = ctx return nil }, }) + return s } // Register register the handlers to the service. -func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service, fm *feature.Manager) { +func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - featureFlags := []*feature.Flag{FeatureFlagConprof} - endpoint.Use(fm.Guard(featureFlags)) + endpoint.Use(featureflag.VersionGuard(s.params.Config.FeatureVersion, s.FeatureFlagConprof)) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) diff --git a/pkg/apiserver/user/auth.go b/pkg/apiserver/user/auth.go index c45a347e01..9d6b85404f 100644 --- a/pkg/apiserver/user/auth.go +++ b/pkg/apiserver/user/auth.go @@ -19,6 +19,7 @@ import ( "go.uber.org/zap" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + "github.com/pingcap/tidb-dashboard/util/featureflag" ) var ( @@ -29,6 +30,8 @@ var ( ) type AuthService struct { + FeatureFlagNonRootLogin *featureflag.FeatureFlag + middleware *jwt.GinJWTMiddleware authenticators map[utils.AuthType]Authenticator } @@ -70,7 +73,7 @@ func (a BaseAuthenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) return &SignOutInfo{}, nil } -func newAuthService() *AuthService { +func newAuthService(featureFlagRegistry *featureflag.Registry) *AuthService { var secret *[32]byte secretStr := os.Getenv("DASHBOARD_SESSION_SECRET") @@ -87,8 +90,9 @@ func newAuthService() *AuthService { } service := &AuthService{ - middleware: nil, - authenticators: map[utils.AuthType]Authenticator{}, + FeatureFlagNonRootLogin: featureFlagRegistry.Register("nonRootLogin", ">= 5.3.0"), + middleware: nil, + authenticators: map[utils.AuthType]Authenticator{}, } middleware, err := jwt.New(&jwt.GinJWTMiddleware{ diff --git a/pkg/apiserver/user/module.go b/pkg/apiserver/user/module.go index a3d4560a55..d3a19b264a 100644 --- a/pkg/apiserver/user/module.go +++ b/pkg/apiserver/user/module.go @@ -4,13 +4,9 @@ package user import ( "go.uber.org/fx" - - "github.com/pingcap/tidb-dashboard/util/feature" ) -var FeatureFlagNonRootLogin = feature.NewFlag("nonRootLogin", []string{">= 5.3.0"}) - var Module = fx.Options( fx.Provide(newAuthService), - fx.Invoke(registerRouter, FeatureFlagNonRootLogin.Register()), + fx.Invoke(registerRouter), ) diff --git a/util/featureflag/registry.go b/util/featureflag/registry.go index 59d788e9fc..cb1aee5e06 100644 --- a/util/featureflag/registry.go +++ b/util/featureflag/registry.go @@ -20,8 +20,8 @@ func NewRegistry(version string) *Registry { } } -// NewRegistryProvider returns a fx.Provider. -func NewRegistryProvider(version string) func() *Registry { +// ProvideRegistry returns a constructor for fx.Provide. +func ProvideRegistry(version string) func() *Registry { return func() *Registry { return NewRegistry(version) } From 43b297de258ad48bdaefbed6dafaa82dd99fa841 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 15:10:53 +0800 Subject: [PATCH 10/18] *: update feature flag --- pkg/apiserver/conprof/service.go | 1 - pkg/apiserver/info/info.go | 12 ++++++------ util/featureflag/featureflag.go | 4 +--- util/featureflag/version_guard.go | 4 +--- util/featureflag/version_guard_test.go | 6 +++--- 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index ef1a28a26c..8c56f5341f 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -61,7 +61,6 @@ type Service struct { } func newService(lc fx.Lifecycle, p ServiceParams) *Service { - s := &Service{ FeatureFlagConprof: p.FeatureFlagRegistry.Register("conprof", ">= 5.3.0"), params: p, diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index a89bb6a2fe..7bb14daf60 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -17,15 +17,15 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/dbstore" "github.com/pingcap/tidb-dashboard/pkg/tidb" "github.com/pingcap/tidb-dashboard/pkg/utils/version" - "github.com/pingcap/tidb-dashboard/util/feature" + "github.com/pingcap/tidb-dashboard/util/featureflag" ) type ServiceParams struct { fx.In - Config *config.Config - LocalStore *dbstore.DB - TiDBClient *tidb.Client - FeatureManager *feature.Manager + Config *config.Config + LocalStore *dbstore.DB + TiDBClient *tidb.Client + FeatureFlagRegistry *featureflag.Registry } type Service struct { @@ -65,7 +65,7 @@ func (s *Service) infoHandler(c *gin.Context) { Version: version.GetInfo(), EnableTelemetry: s.params.Config.EnableTelemetry, EnableExperimental: s.params.Config.EnableExperimental, - SupportedFeatures: s.params.FeatureManager.SupportedFeatures(), + SupportedFeatures: s.params.FeatureFlagRegistry.SupportedFeatures(), } c.JSON(http.StatusOK, resp) } diff --git a/util/featureflag/featureflag.go b/util/featureflag/featureflag.go index b2d1ea09b4..c6167df52a 100644 --- a/util/featureflag/featureflag.go +++ b/util/featureflag/featureflag.go @@ -9,9 +9,7 @@ import ( "github.com/joomcode/errorx" ) -var ( - ErrNS = errorx.NewNamespace("feature_flag") -) +var ErrNS = errorx.NewNamespace("feature_flag") type FeatureFlag struct { name string diff --git a/util/featureflag/version_guard.go b/util/featureflag/version_guard.go index 4afb51f864..c03c04e5cf 100644 --- a/util/featureflag/version_guard.go +++ b/util/featureflag/version_guard.go @@ -8,9 +8,7 @@ import ( "github.com/gin-gonic/gin" ) -var ( - ErrFeatureUnsupported = ErrNS.NewType("feature_unsupported") -) +var ErrFeatureUnsupported = ErrNS.NewType("feature_unsupported") // VersionGuard returns gin.HandlerFunc as guard middleware. // It will determine if features are available in the target version. diff --git a/util/featureflag/version_guard_test.go b/util/featureflag/version_guard_test.go index bd19eb899a..8912db5853 100644 --- a/util/featureflag/version_guard_test.go +++ b/util/featureflag/version_guard_test.go @@ -33,12 +33,12 @@ func Test_VersionGuard(t *testing.T) { // StatusForbidden e2 := gin.Default() - Handled := false + handled := false e2.Use(func(c *gin.Context) { c.Next() // test error type - Handled = true + handled = true r.Equal(true, errorx.IsOfType(c.Errors[0].Err, ErrFeatureUnsupported)) }) e2.Use(VersionGuard("v5.3.0", f1, f2)) @@ -50,5 +50,5 @@ func Test_VersionGuard(t *testing.T) { e2.ServeHTTP(w2, req2) r.Equal(http.StatusForbidden, w2.Code) - r.Equal(true, Handled) + r.Equal(true, handled) } From cc55adc8bdf99c2187769bbde0861b238ccd1b58 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 16:06:41 +0800 Subject: [PATCH 11/18] chore: make lint happy --- ui/lib/apps/InstanceProfiling/pages/Detail.tsx | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx index 18cef97a2c..e35ee8d1a9 100644 --- a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx +++ b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx @@ -121,18 +121,10 @@ export default function Page() { if (!token) { return } - - // make both generated graph(svg) file and protobuf file viewable online - let profileURL = `${client.getBasePath()}/profiling/single/view?token=${token}` - - if (rec.profile_output_type === 'protobuf') { - const titleOnTab = rec.target.kind + '_' + rec.target.display_name - profileURL = `/dashboard/speedscope#profileURL=${encodeURIComponent( - profileURL - )}&title=${titleOnTab}` - } - - window.open(`${profileURL}`, '_blank') + window.open( + `${client.getBasePath()}/profiling/single/view?token=${token}`, + '_blank' + ) } ) From 15490d58bc8188bae8124afdfccb904a47c02235 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 16:13:31 +0800 Subject: [PATCH 12/18] chore: revert lint --- ui/lib/apps/InstanceProfiling/pages/Detail.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx index e35ee8d1a9..18cef97a2c 100644 --- a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx +++ b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx @@ -121,10 +121,18 @@ export default function Page() { if (!token) { return } - window.open( - `${client.getBasePath()}/profiling/single/view?token=${token}`, - '_blank' - ) + + // make both generated graph(svg) file and protobuf file viewable online + let profileURL = `${client.getBasePath()}/profiling/single/view?token=${token}` + + if (rec.profile_output_type === 'protobuf') { + const titleOnTab = rec.target.kind + '_' + rec.target.display_name + profileURL = `/dashboard/speedscope#profileURL=${encodeURIComponent( + profileURL + )}&title=${titleOnTab}` + } + + window.open(`${profileURL}`, '_blank') } ) From 3401a7f4a5c4b67719a0f41575b26af911c23a7a Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 16:14:35 +0800 Subject: [PATCH 13/18] Update index.tsx --- ui/dashboardApp/layout/signin/index.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/dashboardApp/layout/signin/index.tsx b/ui/dashboardApp/layout/signin/index.tsx index e9bf6f84cc..913153c457 100755 --- a/ui/dashboardApp/layout/signin/index.tsx +++ b/ui/dashboardApp/layout/signin/index.tsx @@ -423,9 +423,10 @@ function SSOSignInForm({ successRoute, onClickAlternative }) { } function App({ registry }) { - const successRoute = useMemo(() => `#${registry.getDefaultRouter()}`, [ - registry, - ]) + const successRoute = useMemo( + () => `#${registry.getDefaultRouter()}`, + [registry] + ) const [alternativeVisible, setAlternativeVisible] = useState(false) const [formType, setFormType] = useState(DisplayFormType.uninitialized) const [supportedAuthTypes, setSupportedAuthTypes] = useState>([ From c7e50e0b41b83bf98f0f11976ff969c26710573d Mon Sep 17 00:00:00 2001 From: Suhaha Date: Tue, 7 Dec 2021 16:15:09 +0800 Subject: [PATCH 14/18] Update Detail.tsx --- ui/lib/apps/InstanceProfiling/pages/Detail.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx index 18cef97a2c..c940cb18be 100644 --- a/ui/lib/apps/InstanceProfiling/pages/Detail.tsx +++ b/ui/lib/apps/InstanceProfiling/pages/Detail.tsx @@ -41,7 +41,11 @@ export default function Page() { const { t } = useTranslation() const { id } = useQueryParams() - const { data: respData, isLoading, error } = useClientRequestWithPolling( + const { + data: respData, + isLoading, + error, + } = useClientRequestWithPolling( (reqConfig) => client.getInstance().getProfilingGroupDetail(id, reqConfig), { shouldPoll: (data) => !isFinished(data), From c4e1a03b1087bc8ba494f87f5d9291a76cd939e8 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Wed, 8 Dec 2021 01:14:48 +0800 Subject: [PATCH 15/18] refine: feature flag --- go.mod | 4 +- go.sum | 27 +++++++------ pkg/apiserver/apiserver.go | 2 +- pkg/apiserver/conprof/service.go | 10 ++--- pkg/apiserver/info/info.go | 10 ++--- pkg/apiserver/user/auth.go | 4 +- util/featureflag/featureflag.go | 36 ++++++++++++++--- util/featureflag/featureflag_test.go | 48 +++++++++++++++++++++-- util/featureflag/registry.go | 37 ++++++++---------- util/featureflag/registry_test.go | 13 +++++-- util/featureflag/version_guard.go | 36 ----------------- util/featureflag/version_guard_test.go | 54 -------------------------- 12 files changed, 132 insertions(+), 149 deletions(-) delete mode 100644 util/featureflag/version_guard.go delete mode 100644 util/featureflag/version_guard_test.go diff --git a/go.mod b/go.mod index 55e1a7a772..ef53ecaf24 100644 --- a/go.mod +++ b/go.mod @@ -40,8 +40,8 @@ require ( github.com/vmihailenco/msgpack/v5 v5.3.5 go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 go.uber.org/atomic v1.9.0 - go.uber.org/fx v1.10.0 - go.uber.org/goleak v1.1.10 + go.uber.org/fx v1.16.0 + go.uber.org/goleak v1.1.11 go.uber.org/zap v1.19.0 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be golang.org/x/sync v0.0.0-20210220032951-036812b2e83c diff --git a/go.sum b/go.sum index aa99f7719c..1c2090a396 100644 --- a/go.sum +++ b/go.sum @@ -31,8 +31,9 @@ github.com/antonmedv/expr v1.9.0 h1:j4HI3NHEdgDnN9p6oI6Ndr0G5QryMY0FNxT4ONrFDGU= github.com/antonmedv/expr v1.9.0/go.mod h1:5qsM3oLGDND7sDmQGDXHkYfkjYMUX14qsgqmHhwGEk8= github.com/appleboy/gofight/v2 v2.1.2 h1:VOy3jow4vIK8BRQJoC/I9muxyYlJ2yb9ht2hZoS3rf4= github.com/appleboy/gofight/v2 v2.1.2/go.mod h1:frW+U1QZEdDgixycTj4CygQ48yLTUhplt43+Wczp3rw= -github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= +github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= +github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0 h1:HWo1m869IqiPhD389kmkxeTalrjNbbJTC8LXupb+sl0= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= @@ -329,6 +330,7 @@ github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 h1:VcrIfasaLFkyjk6KNlXQSzO+B0fZcnECiDrKJsfxka0= @@ -339,13 +341,13 @@ go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/dig v1.8.0 h1:1rR6hnL/bu1EVcjnRDN5kx1vbIjEJDTGhSQ2B3ddpcI= -go.uber.org/dig v1.8.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= -go.uber.org/fx v1.10.0 h1:S2K/H8oNied0Je/mLKdWzEWKZfv9jtxSDm8CnwK+5Fg= -go.uber.org/fx v1.10.0/go.mod h1:vLRicqpG/qQEzno4SYU86iCwfT95EZza+Eba0ItuxqY= -go.uber.org/goleak v0.10.0/go.mod h1:VCZuO8V8mFPlL0F5J5GK1rtHV3DrFcQ1R8ryq7FK0aI= -go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0= +go.uber.org/dig v1.12.0 h1:l1GQeZpEbss0/M4l/ZotuBndCrkMdjnygzgcuOjAdaY= +go.uber.org/dig v1.12.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= +go.uber.org/fx v1.16.0 h1:N8i80+X1DCX+qMRiKzM+jPPZiIiyK/bVCysga3+B+1w= +go.uber.org/fx v1.16.0/go.mod h1:OMoT5BnXcOaiexlpjtpE4vcAmzyDKyRs9TRYXCzamx8= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= +go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= +go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.4.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= @@ -378,8 +380,9 @@ golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5 h1:2M3HP5CCK1Si9FQhwnzYhXdG golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= -golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181005035420-146acd28ed58/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -424,8 +427,10 @@ golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210217105451-b926d437f341/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44 h1:Bli41pIlzTzf3KEY06n+xnzK/BESIg2ze4Pgfh/aI8c= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210903071746-97244b99971b h1:3Dq0eVHn0uaQJmPO+/aYPI/fRMqdrVDbu7MQcku54gg= +golang.org/x/sys v0.0.0-20210903071746-97244b99971b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -450,11 +455,11 @@ golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191030062658-86caa796c7ab/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191107010934-f79515f33823/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20191114200427-caa0b0f7d508/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.0.0-20210112230658-8b4aab62c064 h1:BmCFkEH4nJrYcAc2L08yX5RhYGD4j58PTMkEUDkpz2I= golang.org/x/tools v0.0.0-20210112230658-8b4aab62c064/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= +golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 039d84591e..2907ca0069 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -101,7 +101,7 @@ func (s *Service) Start(ctx context.Context) error { s.app = fx.New( fx.Logger(utils.NewFxPrinter()), fx.Provide( - featureflag.ProvideRegistry(s.config.FeatureVersion), + fx.Supply(featureflag.NewRegistry(s.config.FeatureVersion)), newAPIHandlerEngine, s.provideLocals, dbstore.NewDBStore, diff --git a/pkg/apiserver/conprof/service.go b/pkg/apiserver/conprof/service.go index 8c56f5341f..8c45d4d1d7 100644 --- a/pkg/apiserver/conprof/service.go +++ b/pkg/apiserver/conprof/service.go @@ -45,9 +45,9 @@ type ngMonitoringAddrCacheEntity struct { type ServiceParams struct { fx.In - EtcdClient *clientv3.Client - Config *config.Config - FeatureFlagRegistry *featureflag.Registry + EtcdClient *clientv3.Client + Config *config.Config + FeatureFlags *featureflag.Registry } type Service struct { @@ -62,7 +62,7 @@ type Service struct { func newService(lc fx.Lifecycle, p ServiceParams) *Service { s := &Service{ - FeatureFlagConprof: p.FeatureFlagRegistry.Register("conprof", ">= 5.3.0"), + FeatureFlagConprof: p.FeatureFlags.Register("conprof", ">= 5.3.0"), params: p, } @@ -80,7 +80,7 @@ func newService(lc fx.Lifecycle, p ServiceParams) *Service { func registerRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) { endpoint := r.Group("/continuous_profiling") - endpoint.Use(featureflag.VersionGuard(s.params.Config.FeatureVersion, s.FeatureFlagConprof)) + endpoint.Use(s.FeatureFlagConprof.VersionGuard()) { endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig) endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig) diff --git a/pkg/apiserver/info/info.go b/pkg/apiserver/info/info.go index 7bb14daf60..6e610ac4d8 100644 --- a/pkg/apiserver/info/info.go +++ b/pkg/apiserver/info/info.go @@ -22,10 +22,10 @@ import ( type ServiceParams struct { fx.In - Config *config.Config - LocalStore *dbstore.DB - TiDBClient *tidb.Client - FeatureFlagRegistry *featureflag.Registry + Config *config.Config + LocalStore *dbstore.DB + TiDBClient *tidb.Client + FeatureFlags *featureflag.Registry } type Service struct { @@ -65,7 +65,7 @@ func (s *Service) infoHandler(c *gin.Context) { Version: version.GetInfo(), EnableTelemetry: s.params.Config.EnableTelemetry, EnableExperimental: s.params.Config.EnableExperimental, - SupportedFeatures: s.params.FeatureFlagRegistry.SupportedFeatures(), + SupportedFeatures: s.params.FeatureFlags.SupportedFeatures(), } c.JSON(http.StatusOK, resp) } diff --git a/pkg/apiserver/user/auth.go b/pkg/apiserver/user/auth.go index 2c507bf495..6755288b11 100644 --- a/pkg/apiserver/user/auth.go +++ b/pkg/apiserver/user/auth.go @@ -74,7 +74,7 @@ func (a BaseAuthenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) return &SignOutInfo{}, nil } -func newAuthService(featureFlagRegistry *featureflag.Registry) *AuthService { +func newAuthService(featureFlags *featureflag.Registry) *AuthService { var secret *[32]byte secretStr := os.Getenv("DASHBOARD_SESSION_SECRET") @@ -91,7 +91,7 @@ func newAuthService(featureFlagRegistry *featureflag.Registry) *AuthService { } service := &AuthService{ - FeatureFlagNonRootLogin: featureFlagRegistry.Register("nonRootLogin", ">= 5.3.0"), + FeatureFlagNonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"), middleware: nil, authenticators: map[utils.AuthType]Authenticator{}, } diff --git a/util/featureflag/featureflag.go b/util/featureflag/featureflag.go index c6167df52a..85ff237918 100644 --- a/util/featureflag/featureflag.go +++ b/util/featureflag/featureflag.go @@ -3,31 +3,55 @@ package featureflag import ( + "net/http" "strings" "github.com/Masterminds/semver" - "github.com/joomcode/errorx" + "github.com/gin-gonic/gin" + + "github.com/pingcap/tidb-dashboard/util/rest" ) -var ErrNS = errorx.NewNamespace("feature_flag") +var ErrFeatureUnsupported = rest.ErrForbidden.NewSubtype("feature_unsupported") type FeatureFlag struct { name string constraints []string + isSupported bool } -func NewFeatureFlag(name string, constraints ...string) *FeatureFlag { - return &FeatureFlag{name: name, constraints: constraints} +func newFeatureFlag(name, targetVersion string, constraints ...string) *FeatureFlag { + f := &FeatureFlag{name: name, constraints: constraints} + f.isSupported = f.isSupportedIn(targetVersion) + return f } func (f *FeatureFlag) Name() string { return f.name } -// IsSupported checks if a semantic version fits within a set of constraints +func (f *FeatureFlag) IsSupported() bool { + return f.isSupported +} + +// VersionGuard returns gin.HandlerFunc as guard middleware. +// It will determine if features are available in the target version. +func (f *FeatureFlag) VersionGuard() gin.HandlerFunc { + return func(c *gin.Context) { + if !f.isSupported { + _ = c.Error(ErrFeatureUnsupported.New(f.name)) + c.AbortWithStatus(http.StatusForbidden) + return + } + + c.Next() + } +} + +// IsSupportedIn checks if a semantic version fits within a set of constraints // pdVersion, standaloneVersion examples: "v5.2.2", "v5.3.0", "v5.4.0-alpha-xxx", "5.3.0" (semver can handle `v` prefix by itself) // constraints examples: "~5.2.2", ">= 5.3.0", see semver docs to get more information. -func (f *FeatureFlag) IsSupported(targetVersion string) bool { +func (f *FeatureFlag) isSupportedIn(targetVersion string) bool { // drop "-alpha-xxx" suffix versionWithoutSuffix := strings.Split(targetVersion, "-")[0] v, err := semver.NewVersion(versionWithoutSuffix) diff --git a/util/featureflag/featureflag_test.go b/util/featureflag/featureflag_test.go index 78cc39f3e7..2f6a52323f 100644 --- a/util/featureflag/featureflag_test.go +++ b/util/featureflag/featureflag_test.go @@ -3,8 +3,12 @@ package featureflag import ( + "net/http" + "net/http/httptest" "testing" + "github.com/gin-gonic/gin" + "github.com/joomcode/errorx" "github.com/stretchr/testify/require" ) @@ -12,7 +16,7 @@ func Test_Name(t *testing.T) { f1 := &FeatureFlag{} require.Equal(t, f1.Name(), "") - f2 := NewFeatureFlag("testFeature") + f2 := newFeatureFlag("testFeature", "v5.3.0", ">= 5.3.0") require.Equal(t, f2.Name(), "testFeature") } @@ -35,7 +39,45 @@ func Test_IsSupported(t *testing.T) { } for _, tt := range tests { - ff := NewFeatureFlag("testFeature", tt.args.constraints...) - require.Equal(t, tt.want, ff.IsSupported(tt.args.target)) + ff := newFeatureFlag("testFeature", tt.args.target, tt.args.constraints...) + require.Equal(t, tt.want, ff.IsSupported()) } } + +func Test_VersionGuard(t *testing.T) { + r := require.New(t) + f1 := newFeatureFlag("testFeature1", "v5.3.0", ">= 5.3.0") + f2 := newFeatureFlag("testFeature2", "v5.3.0", ">= 5.3.1") + + // success + e := gin.Default() + e.Use(f1.VersionGuard()) + e.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/ping", nil) + e.ServeHTTP(w, req) + + r.Equal(200, w.Code) + r.Equal("pong", w.Body.String()) + + // StatusForbidden + e2 := gin.Default() + e2.Use(func(c *gin.Context) { + c.Next() + + // test error type + c.Status(http.StatusForbidden) + r.Equal(true, errorx.IsOfType(c.Errors.Last().Err, ErrFeatureUnsupported)) + }) + e2.Use(f1.VersionGuard(), f2.VersionGuard()) + e2.GET("/ping", func(c *gin.Context) { + c.String(200, "pong") + }) + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest("GET", "/ping", nil) + e2.ServeHTTP(w2, req2) + + r.Equal(http.StatusForbidden, w2.Code) +} diff --git a/util/featureflag/registry.go b/util/featureflag/registry.go index cb1aee5e06..b234a18a3f 100644 --- a/util/featureflag/registry.go +++ b/util/featureflag/registry.go @@ -7,40 +7,37 @@ import ( ) type Registry struct { - version string - flags []*FeatureFlag - supportedMap map[string]struct{} + version string + flags map[string]*FeatureFlag + supportedFeatures map[string]struct{} } func NewRegistry(version string) *Registry { return &Registry{ - version: version, - flags: []*FeatureFlag{}, - supportedMap: map[string]struct{}{}, + version: version, + flags: map[string]*FeatureFlag{}, + supportedFeatures: map[string]struct{}{}, } } -// ProvideRegistry returns a constructor for fx.Provide. -func ProvideRegistry(version string) func() *Registry { - return func() *Registry { - return NewRegistry(version) +// Register create and register feature flag to registry. +func (m *Registry) Register(name string, constraints ...string) *FeatureFlag { + if f, ok := m.flags[name]; ok { + return f } -} -// Register feature flag. -func (m *Registry) Register(name string, constraints ...string) *FeatureFlag { - f := NewFeatureFlag(name, constraints...) - m.flags = append(m.flags, f) - if f.IsSupported(m.version) { - m.supportedMap[f.Name()] = struct{}{} + nf := newFeatureFlag(name, m.version, constraints...) + m.flags[name] = nf + if nf.IsSupported() { + m.supportedFeatures[nf.Name()] = struct{}{} } - return f + return nf } // SupportedFeatures returns supported feature's names. func (m *Registry) SupportedFeatures() []string { - sf := make([]string, 0, len(m.supportedMap)) - for k := range m.supportedMap { + sf := make([]string, 0, len(m.supportedFeatures)) + for k := range m.supportedFeatures { sf = append(sf, k) } sort.Strings(sf) diff --git a/util/featureflag/registry_test.go b/util/featureflag/registry_test.go index 7d17957d90..3a7e1b33b9 100644 --- a/util/featureflag/registry_test.go +++ b/util/featureflag/registry_test.go @@ -25,12 +25,17 @@ func Test_Register(t *testing.T) { tt.flag = m.Register(tt.name, tt.constraints...) } - for i, tt := range tests { - // check whether flag is in flags & supportedMap - require.Equal(t, m.flags[i], tt.flag) - _, ok := m.supportedMap[tt.flag.name] + for _, tt := range tests { + // check whether flag is in flags & supportedFeatures + require.Equal(t, m.flags[tt.flag.name], tt.flag) + _, ok := m.supportedFeatures[tt.flag.name] require.Equal(t, tt.supported, ok) } + + // duplicated register + f := m.Register("testFeature3", ">= 5.3.2") + require.Equal(t, f.name, "testFeature3") + require.Equal(t, f.constraints[0], ">= 5.3.1") } func Test_SupportedFeatures(t *testing.T) { diff --git a/util/featureflag/version_guard.go b/util/featureflag/version_guard.go deleted file mode 100644 index c03c04e5cf..0000000000 --- a/util/featureflag/version_guard.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. - -package featureflag - -import ( - "net/http" - - "github.com/gin-gonic/gin" -) - -var ErrFeatureUnsupported = ErrNS.NewType("feature_unsupported") - -// VersionGuard returns gin.HandlerFunc as guard middleware. -// It will determine if features are available in the target version. -func VersionGuard(version string, featureFlags ...*FeatureFlag) gin.HandlerFunc { - supported := true - unsupportedFeatures := make([]string, len(featureFlags)) - for _, ff := range featureFlags { - if !ff.IsSupported(version) { - supported = false - unsupportedFeatures = append(unsupportedFeatures, ff.Name()) - continue - } - } - - return func(c *gin.Context) { - if !supported { - _ = c.Error(ErrFeatureUnsupported.New("list: %v", unsupportedFeatures)) - c.Status(http.StatusForbidden) - c.Abort() - return - } - - c.Next() - } -} diff --git a/util/featureflag/version_guard_test.go b/util/featureflag/version_guard_test.go deleted file mode 100644 index 8912db5853..0000000000 --- a/util/featureflag/version_guard_test.go +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. - -package featureflag - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/gin-gonic/gin" - "github.com/joomcode/errorx" - "github.com/stretchr/testify/require" -) - -func Test_VersionGuard(t *testing.T) { - r := require.New(t) - m := NewRegistry("v5.3.0") - f1 := m.Register("testFeature1", ">= 5.3.0") - f2 := m.Register("testFeature2", ">= 5.3.1") - - // success - e := gin.Default() - e.Use(VersionGuard("v5.3.0", f1)) - e.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") - }) - w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/ping", nil) - e.ServeHTTP(w, req) - - r.Equal(200, w.Code) - r.Equal("pong", w.Body.String()) - - // StatusForbidden - e2 := gin.Default() - handled := false - e2.Use(func(c *gin.Context) { - c.Next() - - // test error type - handled = true - r.Equal(true, errorx.IsOfType(c.Errors[0].Err, ErrFeatureUnsupported)) - }) - e2.Use(VersionGuard("v5.3.0", f1, f2)) - e2.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") - }) - w2 := httptest.NewRecorder() - req2, _ := http.NewRequest("GET", "/ping", nil) - e2.ServeHTTP(w2, req2) - - r.Equal(http.StatusForbidden, w2.Code) - r.Equal(true, handled) -} From 94e5e955439cd78a76e4a4c02652aa30e7d0e7b7 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Wed, 8 Dec 2021 10:47:30 +0800 Subject: [PATCH 16/18] fix: fx & indirect x/tools version --- go.mod | 4 ++-- go.sum | 27 +++++++++++---------------- pkg/apiserver/apiserver.go | 2 +- scripts/go.sum | 4 ++-- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index ef53ecaf24..98721bbab2 100644 --- a/go.mod +++ b/go.mod @@ -40,8 +40,8 @@ require ( github.com/vmihailenco/msgpack/v5 v5.3.5 go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 go.uber.org/atomic v1.9.0 - go.uber.org/fx v1.16.0 - go.uber.org/goleak v1.1.11 + go.uber.org/fx v1.12.0 + go.uber.org/goleak v1.1.10 go.uber.org/zap v1.19.0 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be golang.org/x/sync v0.0.0-20210220032951-036812b2e83c diff --git a/go.sum b/go.sum index 1c2090a396..03c2f6c43b 100644 --- a/go.sum +++ b/go.sum @@ -31,9 +31,8 @@ github.com/antonmedv/expr v1.9.0 h1:j4HI3NHEdgDnN9p6oI6Ndr0G5QryMY0FNxT4ONrFDGU= github.com/antonmedv/expr v1.9.0/go.mod h1:5qsM3oLGDND7sDmQGDXHkYfkjYMUX14qsgqmHhwGEk8= github.com/appleboy/gofight/v2 v2.1.2 h1:VOy3jow4vIK8BRQJoC/I9muxyYlJ2yb9ht2hZoS3rf4= github.com/appleboy/gofight/v2 v2.1.2/go.mod h1:frW+U1QZEdDgixycTj4CygQ48yLTUhplt43+Wczp3rw= +github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= -github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= -github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0 h1:HWo1m869IqiPhD389kmkxeTalrjNbbJTC8LXupb+sl0= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= @@ -330,7 +329,6 @@ github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 h1:VcrIfasaLFkyjk6KNlXQSzO+B0fZcnECiDrKJsfxka0= @@ -341,13 +339,13 @@ go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/dig v1.12.0 h1:l1GQeZpEbss0/M4l/ZotuBndCrkMdjnygzgcuOjAdaY= -go.uber.org/dig v1.12.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= -go.uber.org/fx v1.16.0 h1:N8i80+X1DCX+qMRiKzM+jPPZiIiyK/bVCysga3+B+1w= -go.uber.org/fx v1.16.0/go.mod h1:OMoT5BnXcOaiexlpjtpE4vcAmzyDKyRs9TRYXCzamx8= +go.uber.org/dig v1.9.0 h1:pJTDXKEhRqBI8W7rU7kwT5EgyRZuSMVSFcZolOvKK9U= +go.uber.org/dig v1.9.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= +go.uber.org/fx v1.12.0 h1:+1+3Cz9M0dFMPy9SW9XUIUHye8bnPUm7q7DroNGWYG4= +go.uber.org/fx v1.12.0/go.mod h1:egT3Kyg1JFYQkvKLZ3EsykxkNrZxgXS+gKoKo7abERY= +go.uber.org/goleak v0.10.0/go.mod h1:VCZuO8V8mFPlL0F5J5GK1rtHV3DrFcQ1R8ryq7FK0aI= +go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= -go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.4.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= @@ -380,9 +378,8 @@ golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5 h1:2M3HP5CCK1Si9FQhwnzYhXdG golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= -golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181005035420-146acd28ed58/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -427,10 +424,8 @@ golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210217105451-b926d437f341/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44 h1:Bli41pIlzTzf3KEY06n+xnzK/BESIg2ze4Pgfh/aI8c= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210903071746-97244b99971b h1:3Dq0eVHn0uaQJmPO+/aYPI/fRMqdrVDbu7MQcku54gg= -golang.org/x/sys v0.0.0-20210903071746-97244b99971b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= @@ -455,11 +450,11 @@ golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191030062658-86caa796c7ab/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191107010934-f79515f33823/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20191114200427-caa0b0f7d508/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20210112230658-8b4aab62c064 h1:BmCFkEH4nJrYcAc2L08yX5RhYGD4j58PTMkEUDkpz2I= golang.org/x/tools v0.0.0-20210112230658-8b4aab62c064/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= -golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 2907ca0069..c91b408480 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -100,8 +100,8 @@ func (s *Service) Start(ctx context.Context) error { s.app = fx.New( fx.Logger(utils.NewFxPrinter()), + fx.Supply(featureflag.NewRegistry(s.config.FeatureVersion)), fx.Provide( - fx.Supply(featureflag.NewRegistry(s.config.FeatureVersion)), newAPIHandlerEngine, s.provideLocals, dbstore.NewDBStore, diff --git a/scripts/go.sum b/scripts/go.sum index 302fbcdb21..31a983de77 100644 --- a/scripts/go.sum +++ b/scripts/go.sum @@ -281,8 +281,8 @@ go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/dig v1.8.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= -go.uber.org/fx v1.10.0/go.mod h1:vLRicqpG/qQEzno4SYU86iCwfT95EZza+Eba0ItuxqY= +go.uber.org/dig v1.9.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= +go.uber.org/fx v1.12.0/go.mod h1:egT3Kyg1JFYQkvKLZ3EsykxkNrZxgXS+gKoKo7abERY= go.uber.org/goleak v0.10.0/go.mod h1:VCZuO8V8mFPlL0F5J5GK1rtHV3DrFcQ1R8ryq7FK0aI= go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= From 8087baf18686cbd52fd885481a50cbd2091bf773 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Wed, 8 Dec 2021 15:41:26 +0800 Subject: [PATCH 17/18] refine: version guard error --- util/featureflag/featureflag.go | 7 ++++--- util/featureflag/featureflag_test.go | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/util/featureflag/featureflag.go b/util/featureflag/featureflag.go index 85ff237918..df4689177a 100644 --- a/util/featureflag/featureflag.go +++ b/util/featureflag/featureflag.go @@ -8,11 +8,12 @@ import ( "github.com/Masterminds/semver" "github.com/gin-gonic/gin" + "github.com/joomcode/errorx" "github.com/pingcap/tidb-dashboard/util/rest" ) -var ErrFeatureUnsupported = rest.ErrForbidden.NewSubtype("feature_unsupported") +var ErrFeatureUnsupported = errorx.CommonErrors.NewType("feature_unsupported") type FeatureFlag struct { name string @@ -39,8 +40,8 @@ func (f *FeatureFlag) IsSupported() bool { func (f *FeatureFlag) VersionGuard() gin.HandlerFunc { return func(c *gin.Context) { if !f.isSupported { - _ = c.Error(ErrFeatureUnsupported.New(f.name)) - c.AbortWithStatus(http.StatusForbidden) + _ = c.Error(ErrFeatureUnsupported.New(f.name).WithProperty(rest.HTTPCodeProperty(http.StatusForbidden))) + c.Abort() return } diff --git a/util/featureflag/featureflag_test.go b/util/featureflag/featureflag_test.go index 2f6a52323f..6d91043ba5 100644 --- a/util/featureflag/featureflag_test.go +++ b/util/featureflag/featureflag_test.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "github.com/joomcode/errorx" + "github.com/pingcap/tidb-dashboard/util/rest" "github.com/stretchr/testify/require" ) @@ -62,14 +63,22 @@ func Test_VersionGuard(t *testing.T) { r.Equal(200, w.Code) r.Equal("pong", w.Body.String()) - // StatusForbidden + // abort with other middlewares + handled := false e2 := gin.Default() e2.Use(func(c *gin.Context) { c.Next() - // test error type - c.Status(http.StatusForbidden) - r.Equal(true, errorx.IsOfType(c.Errors.Last().Err, ErrFeatureUnsupported)) + handled = true + + // check error type + currentErr := c.Errors.Last().Err + codeProperty, _ := rest.HTTPCodeProperty(http.StatusForbidden) + code, ok := c.Errors.Last().Err.(*errorx.Error).Property(codeProperty) + r.True(ok) + + r.Equal(true, errorx.IsOfType(currentErr, ErrFeatureUnsupported)) + r.Equal(http.StatusForbidden, code) }) e2.Use(f1.VersionGuard(), f2.VersionGuard()) e2.GET("/ping", func(c *gin.Context) { @@ -79,5 +88,5 @@ func Test_VersionGuard(t *testing.T) { req2, _ := http.NewRequest("GET", "/ping", nil) e2.ServeHTTP(w2, req2) - r.Equal(http.StatusForbidden, w2.Code) + r.Equal(true, handled) } From 0623ecc5a498025e56ff878981cfc447187bcc3d Mon Sep 17 00:00:00 2001 From: Suhaha Date: Wed, 8 Dec 2021 16:13:35 +0800 Subject: [PATCH 18/18] test: adjust feature flag version guard test --- util/featureflag/featureflag_test.go | 38 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/util/featureflag/featureflag_test.go b/util/featureflag/featureflag_test.go index 140196bc2f..9e75d88188 100644 --- a/util/featureflag/featureflag_test.go +++ b/util/featureflag/featureflag_test.go @@ -9,8 +9,9 @@ import ( "github.com/gin-gonic/gin" "github.com/joomcode/errorx" - "github.com/pingcap/tidb-dashboard/util/rest" "github.com/stretchr/testify/require" + + "github.com/pingcap/tidb-dashboard/util/rest" ) func Test_Name(t *testing.T) { @@ -54,35 +55,27 @@ func Test_VersionGuard(t *testing.T) { e := gin.Default() e.Use(f1.VersionGuard()) e.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") + c.String(http.StatusOK, "pong") }) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/ping", nil) e.ServeHTTP(w, req) - r.Equal(200, w.Code) + r.Equal(http.StatusOK, w.Code) r.Equal("pong", w.Body.String()) - // abort with other middlewares + // abort handled := false e2 := gin.Default() e2.Use(func(c *gin.Context) { c.Next() handled = true - - // check error type - currentErr := c.Errors.Last().Err - codeProperty, _ := rest.HTTPCodeProperty(http.StatusForbidden) - code, ok := currentErr.(*errorx.Error).Property(codeProperty) - r.True(ok) - - r.Equal(true, errorx.IsOfType(currentErr, ErrFeatureUnsupported)) - r.Equal(http.StatusForbidden, code) + r.Equal(true, errorx.IsOfType(c.Errors.Last().Err, ErrFeatureUnsupported)) }) e2.Use(f1.VersionGuard(), f2.VersionGuard()) e2.GET("/ping", func(c *gin.Context) { - c.String(200, "pong") + c.String(http.StatusOK, "pong") }) w2 := httptest.NewRecorder() req2, _ := http.NewRequest("GET", "/ping", nil) @@ -90,3 +83,20 @@ func Test_VersionGuard(t *testing.T) { r.Equal(true, handled) } + +func Test_VersionGuardWith_ErrorHandlerFn(t *testing.T) { + r := require.New(t) + f := newFeatureFlag("testFeature", "v5.3.0", ">= 5.3.1") + + e := gin.Default() + e.Use(rest.ErrorHandlerFn()) + e.Use(f.VersionGuard()) + e.GET("/ping", func(c *gin.Context) { + c.String(http.StatusOK, "pong") + }) + w2 := httptest.NewRecorder() + req2, _ := http.NewRequest("GET", "/ping", nil) + e.ServeHTTP(w2, req2) + + r.Equal(http.StatusForbidden, w2.Code) +}