From fd7dd71e953b23a2af7b85f7888b01f1d5afc576 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 11 Nov 2021 11:22:49 +0800 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] *: 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/19] *: 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 90d09a66145232452e5512f7a27169b945e64432 Mon Sep 17 00:00:00 2001 From: Suhaha Date: Wed, 8 Dec 2021 12:37:38 +0800 Subject: [PATCH 17/19] refactor: authenticator register & feature flag --- pkg/apiserver/user/auth.go | 76 +++++-------------- pkg/apiserver/user/code/codeauth/auth.go | 19 ++--- pkg/apiserver/user/module.go | 8 +- pkg/apiserver/user/shared/authenticator.go | 41 ++++++++++ pkg/apiserver/user/shared/error.go | 14 ++++ pkg/apiserver/user/shared/feature_flag.go | 15 ++++ .../user/{ => shared}/verify_sql_user.go | 10 ++- .../user/{ => shared}/verify_sql_user_test.go | 2 +- pkg/apiserver/user/sqlauth/sqlauth.go | 29 ++++--- pkg/apiserver/user/sso/router.go | 3 +- pkg/apiserver/user/sso/service.go | 18 ++--- pkg/apiserver/user/sso/ssoauth/auth.go | 21 ++--- 12 files changed, 143 insertions(+), 113 deletions(-) create mode 100644 pkg/apiserver/user/shared/authenticator.go create mode 100644 pkg/apiserver/user/shared/error.go create mode 100644 pkg/apiserver/user/shared/feature_flag.go rename pkg/apiserver/user/{ => shared}/verify_sql_user.go (93%) rename pkg/apiserver/user/{ => shared}/verify_sql_user_test.go (99%) diff --git a/pkg/apiserver/user/auth.go b/pkg/apiserver/user/auth.go index 6755288b11..46cb15113e 100644 --- a/pkg/apiserver/user/auth.go +++ b/pkg/apiserver/user/auth.go @@ -18,30 +18,14 @@ import ( "github.com/pingcap/log" "go.uber.org/zap" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" - "github.com/pingcap/tidb-dashboard/util/featureflag" "github.com/pingcap/tidb-dashboard/util/rest" ) -var ( - ErrNS = errorx.NewNamespace("error.api.user") - ErrUnsupportedAuthType = ErrNS.NewType("unsupported_auth_type") - ErrNSSignIn = ErrNS.NewSubNamespace("signin") - ErrSignInOther = ErrNSSignIn.NewType("other") -) - type AuthService struct { - FeatureFlagNonRootLogin *featureflag.FeatureFlag - middleware *jwt.GinJWTMiddleware - authenticators map[utils.AuthType]Authenticator -} - -type AuthenticateForm struct { - Type utils.AuthType `json:"type" example:"0"` - Username string `json:"username" example:"root"` // Does not present for AuthTypeSharingCode - Password string `json:"password"` - Extra string `json:"extra"` // FIXME: Use strong type + authenticators map[utils.AuthType]shared.Authenticator } type TokenResponse struct { @@ -49,32 +33,7 @@ type TokenResponse struct { Expire time.Time `json:"expire"` } -type SignOutInfo struct { - EndSessionURL string `json:"end_session_url"` -} - -type Authenticator interface { - IsEnabled() (bool, error) - Authenticate(form AuthenticateForm) (*utils.SessionUser, error) - ProcessSession(u *utils.SessionUser) bool - SignOutInfo(u *utils.SessionUser, redirectURL string) (*SignOutInfo, error) -} - -type BaseAuthenticator struct{} - -func (a BaseAuthenticator) IsEnabled() (bool, error) { - return true, nil -} - -func (a BaseAuthenticator) ProcessSession(u *utils.SessionUser) bool { - return true -} - -func (a BaseAuthenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) (*SignOutInfo, error) { - return &SignOutInfo{}, nil -} - -func newAuthService(featureFlags *featureflag.Registry) *AuthService { +func newAuthService() *AuthService { var secret *[32]byte secretStr := os.Getenv("DASHBOARD_SESSION_SECRET") @@ -91,9 +50,8 @@ func newAuthService(featureFlags *featureflag.Registry) *AuthService { } service := &AuthService{ - FeatureFlagNonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"), - middleware: nil, - authenticators: map[utils.AuthType]Authenticator{}, + middleware: nil, + authenticators: map[utils.AuthType]shared.Authenticator{}, } middleware, err := jwt.New(&jwt.GinJWTMiddleware{ @@ -103,7 +61,7 @@ func newAuthService(featureFlags *featureflag.Registry) *AuthService { Timeout: time.Hour * 24, MaxRefresh: time.Hour * 24, Authenticator: func(c *gin.Context) (interface{}, error) { - var form AuthenticateForm + var form shared.AuthenticateForm if err := c.ShouldBindJSON(&form); err != nil { return nil, rest.ErrBadRequest.WrapWithNoMessage(err) } @@ -182,7 +140,7 @@ func newAuthService(featureFlags *featureflag.Registry) *AuthService { err = e } else if errors.Is(e, jwt.ErrFailedTokenCreation) { // Try to catch other sign in failure errors. - err = ErrSignInOther.WrapWithNoMessage(e) + err = shared.ErrSignInOther.WrapWithNoMessage(e) } else { // The remaining error comes from checking tokens for protected endpoints. err = rest.ErrUnauthenticated.NewWithNoMessage() @@ -210,10 +168,10 @@ func newAuthService(featureFlags *featureflag.Registry) *AuthService { return service } -func (s *AuthService) authForm(f AuthenticateForm) (*utils.SessionUser, error) { +func (s *AuthService) authForm(f shared.AuthenticateForm) (*utils.SessionUser, error) { a, ok := s.authenticators[f.Type] if !ok { - return nil, ErrUnsupportedAuthType.NewWithNoMessage() + return nil, shared.ErrUnsupportedAuthType.NewWithNoMessage() } u, err := a.Authenticate(f) if err != nil { @@ -272,11 +230,17 @@ func (s *AuthService) MWRequireWritePriv() gin.HandlerFunc { } } -// RegisterAuthenticator registers an authenticator in the authenticate pipeline. -func (s *AuthService) RegisterAuthenticator(typeID utils.AuthType, a Authenticator) { +var _ shared.AuthenticatorRegister = (*AuthService)(nil) + +// Register impl shared.AuthenticatorRegister, registers an authenticator in the authenticate pipeline. +func (s *AuthService) Register(typeID utils.AuthType, a shared.Authenticator) { s.authenticators[typeID] = a } +func provideAuthenticatorRegister(authService *AuthService) shared.AuthenticatorRegister { + return authService +} + type GetLoginInfoResponse struct { SupportedAuthTypes []int `json:"supported_auth_types"` } @@ -306,7 +270,7 @@ func (s *AuthService) getLoginInfoHandler(c *gin.Context) { // @ID userLogin // @Summary Log in -// @Param message body AuthenticateForm true "Credentials" +// @Param message body shared.AuthenticateForm true "Credentials" // @Success 200 {object} TokenResponse // @Failure 401 {object} rest.ErrorResponse // @Router /user/login [post] @@ -320,7 +284,7 @@ type GetSignOutInfoRequest struct { // @ID userGetSignOutInfo // @Summary Get sign out info -// @Success 200 {object} SignOutInfo +// @Success 200 {object} shared.SignOutInfo // @Param q query GetSignOutInfoRequest true "Query" // @Router /user/sign_out_info [get] // @Security JwtAuth @@ -336,7 +300,7 @@ func (s *AuthService) getSignOutInfoHandler(c *gin.Context) { u := utils.GetSession(c) a, ok := s.authenticators[u.AuthFrom] if !ok { - _ = c.Error(ErrUnsupportedAuthType.NewWithNoMessage()) + _ = c.Error(shared.ErrUnsupportedAuthType.NewWithNoMessage()) return } si, err := a.SignOutInfo(u, req.RedirectURL) diff --git a/pkg/apiserver/user/code/codeauth/auth.go b/pkg/apiserver/user/code/codeauth/auth.go index b9e2258f35..4e03940f38 100644 --- a/pkg/apiserver/user/code/codeauth/auth.go +++ b/pkg/apiserver/user/code/codeauth/auth.go @@ -7,36 +7,31 @@ import ( "go.uber.org/fx" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/code" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" ) const typeID utils.AuthType = 1 -var ErrSignInInvalidCode = user.ErrNSSignIn.NewType("invalid_code") // Invalid or expired +var ErrSignInInvalidCode = shared.ErrNSSignIn.NewType("invalid_code") // Invalid or expired type Authenticator struct { - user.BaseAuthenticator + shared.BaseAuthenticator sharingCodeService *code.Service } -func newAuthenticator(sharingCodeService *code.Service) *Authenticator { - return &Authenticator{ +func registerAuthenticator(r shared.AuthenticatorRegister, sharingCodeService *code.Service) { + r.Register(typeID, &Authenticator{ sharingCodeService: sharingCodeService, - } -} - -func registerAuthenticator(a *Authenticator, authService *user.AuthService) { - authService.RegisterAuthenticator(typeID, a) + }) } var Module = fx.Options( - fx.Provide(newAuthenticator), fx.Invoke(registerAuthenticator), ) -func (a *Authenticator) Authenticate(f user.AuthenticateForm) (*utils.SessionUser, error) { +func (a *Authenticator) Authenticate(f shared.AuthenticateForm) (*utils.SessionUser, error) { session := a.sharingCodeService.NewSessionFromSharingCode(f.Password) if session == nil { return nil, ErrSignInInvalidCode.NewWithNoMessage() diff --git a/pkg/apiserver/user/module.go b/pkg/apiserver/user/module.go index d3a19b264a..beb20186a6 100644 --- a/pkg/apiserver/user/module.go +++ b/pkg/apiserver/user/module.go @@ -4,9 +4,15 @@ package user import ( "go.uber.org/fx" + + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" ) var Module = fx.Options( - fx.Provide(newAuthService), + fx.Provide( + newAuthService, + provideAuthenticatorRegister, + shared.ProvideFeatureFlags, + ), fx.Invoke(registerRouter), ) diff --git a/pkg/apiserver/user/shared/authenticator.go b/pkg/apiserver/user/shared/authenticator.go new file mode 100644 index 0000000000..01338e1f36 --- /dev/null +++ b/pkg/apiserver/user/shared/authenticator.go @@ -0,0 +1,41 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package shared + +import "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" + +type AuthenticatorRegister interface { + Register(typeID utils.AuthType, a Authenticator) +} + +type AuthenticateForm struct { + Type utils.AuthType `json:"type" example:"0"` + Username string `json:"username" example:"root"` // Does not present for AuthTypeSharingCode + Password string `json:"password"` + Extra string `json:"extra"` // FIXME: Use strong type +} + +type SignOutInfo struct { + EndSessionURL string `json:"end_session_url"` +} + +type Authenticator interface { + IsEnabled() (bool, error) + Authenticate(form AuthenticateForm) (*utils.SessionUser, error) + ProcessSession(u *utils.SessionUser) bool + SignOutInfo(u *utils.SessionUser, redirectURL string) (*SignOutInfo, error) +} + +type BaseAuthenticator struct{} + +func (a BaseAuthenticator) IsEnabled() (bool, error) { + return true, nil +} + +func (a BaseAuthenticator) ProcessSession(u *utils.SessionUser) bool { + return true +} + +func (a BaseAuthenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) (*SignOutInfo, error) { + return &SignOutInfo{}, nil +} diff --git a/pkg/apiserver/user/shared/error.go b/pkg/apiserver/user/shared/error.go new file mode 100644 index 0000000000..39ad0c4342 --- /dev/null +++ b/pkg/apiserver/user/shared/error.go @@ -0,0 +1,14 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package shared + +import "github.com/joomcode/errorx" + +var ( + ErrNS = errorx.NewNamespace("error.api.user") + ErrUnsupportedAuthType = ErrNS.NewType("unsupported_auth_type") + ErrUnsupportedUser = ErrNS.NewType("unsupported_user") + ErrNSSignIn = ErrNS.NewSubNamespace("signin") + ErrSignInOther = ErrNSSignIn.NewType("other") + ErrInsufficientPrivs = ErrNSSignIn.NewType("insufficient_priv") +) diff --git a/pkg/apiserver/user/shared/feature_flag.go b/pkg/apiserver/user/shared/feature_flag.go new file mode 100644 index 0000000000..ca1f970bcd --- /dev/null +++ b/pkg/apiserver/user/shared/feature_flag.go @@ -0,0 +1,15 @@ +// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. + +package shared + +import "github.com/pingcap/tidb-dashboard/util/featureflag" + +type AuthFeatureFlags struct { + NonRootLogin *featureflag.FeatureFlag +} + +func ProvideFeatureFlags(featureFlags *featureflag.Registry) *AuthFeatureFlags { + return &AuthFeatureFlags{ + NonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"), + } +} diff --git a/pkg/apiserver/user/verify_sql_user.go b/pkg/apiserver/user/shared/verify_sql_user.go similarity index 93% rename from pkg/apiserver/user/verify_sql_user.go rename to pkg/apiserver/user/shared/verify_sql_user.go index 4794834504..611c2ca57e 100644 --- a/pkg/apiserver/user/verify_sql_user.go +++ b/pkg/apiserver/user/shared/verify_sql_user.go @@ -1,6 +1,6 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package user +package shared import ( "encoding/json" @@ -11,8 +11,6 @@ import ( "github.com/pingcap/tidb-dashboard/pkg/tidb" ) -var ErrInsufficientPrivs = ErrNSSignIn.NewType("insufficient_priv") - // TiDB config response // // "security": { @@ -29,7 +27,11 @@ type tidbSEMConfig struct { SkipGrantTable bool `json:"skip-grant-table"` } -func VerifySQLUser(tidbClient *tidb.Client, userName, password string) (writeable bool, err error) { +func VerifySQLUser(tidbClient *tidb.Client, featureFlags *AuthFeatureFlags, userName, password string) (writeable bool, err error) { + if !featureFlags.NonRootLogin.IsSupported() && userName != "root" { + return false, ErrUnsupportedUser.New("User must be root") + } + db, err := tidbClient.OpenSQLConn(userName, password) if err != nil { return false, err diff --git a/pkg/apiserver/user/verify_sql_user_test.go b/pkg/apiserver/user/shared/verify_sql_user_test.go similarity index 99% rename from pkg/apiserver/user/verify_sql_user_test.go rename to pkg/apiserver/user/shared/verify_sql_user_test.go index 6efe3f7a15..477ea2b5c8 100755 --- a/pkg/apiserver/user/verify_sql_user_test.go +++ b/pkg/apiserver/user/shared/verify_sql_user_test.go @@ -1,6 +1,6 @@ // Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0. -package user +package shared import ( "testing" diff --git a/pkg/apiserver/user/sqlauth/sqlauth.go b/pkg/apiserver/user/sqlauth/sqlauth.go index e066358f57..069a91f17f 100755 --- a/pkg/apiserver/user/sqlauth/sqlauth.go +++ b/pkg/apiserver/user/sqlauth/sqlauth.go @@ -6,7 +6,7 @@ import ( "github.com/joomcode/errorx" "go.uber.org/fx" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/tidb" ) @@ -14,37 +14,34 @@ import ( const typeID utils.AuthType = 0 type Authenticator struct { - user.BaseAuthenticator - tidbClient *tidb.Client + shared.BaseAuthenticator + tidbClient *tidb.Client + authFeatureFlags *shared.AuthFeatureFlags } -func newAuthenticator(tidbClient *tidb.Client) *Authenticator { - return &Authenticator{ - tidbClient: tidbClient, - } -} - -func registerAuthenticator(a *Authenticator, authService *user.AuthService) { - authService.RegisterAuthenticator(typeID, a) +func registerAuthenticator(r shared.AuthenticatorRegister, tidbClient *tidb.Client, ff *shared.AuthFeatureFlags) { + r.Register(typeID, &Authenticator{ + tidbClient: tidbClient, + authFeatureFlags: ff, + }) } var Module = fx.Options( - fx.Provide(newAuthenticator), fx.Invoke(registerAuthenticator), ) -func (a *Authenticator) Authenticate(f user.AuthenticateForm) (*utils.SessionUser, error) { - writeable, err := user.VerifySQLUser(a.tidbClient, f.Username, f.Password) +func (a *Authenticator) Authenticate(f shared.AuthenticateForm) (*utils.SessionUser, error) { + writeable, err := shared.VerifySQLUser(a.tidbClient, a.authFeatureFlags, f.Username, f.Password) if err != nil { if errorx.Cast(err) == nil { - return nil, user.ErrSignInOther.WrapWithNoMessage(err) + return nil, shared.ErrSignInOther.WrapWithNoMessage(err) } // Possible errors could be: // tidb.ErrNoAliveTiDB // tidb.ErrPDAccessFailed // tidb.ErrTiDBConnFailed // tidb.ErrTiDBAuthFailed - // user.ErrInsufficientPrivs + // shared.ErrInsufficientPrivs return nil, err } diff --git a/pkg/apiserver/user/sso/router.go b/pkg/apiserver/user/sso/router.go index 475c22a5f9..fe0a67433d 100644 --- a/pkg/apiserver/user/sso/router.go +++ b/pkg/apiserver/user/sso/router.go @@ -9,6 +9,7 @@ import ( "github.com/joomcode/errorx" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/util/rest" ) @@ -89,7 +90,7 @@ func (s *Service) createImpersonationHandler(c *gin.Context) { rec, err := s.createImpersonation(req.SQLUser, req.Password) if err != nil { _ = c.Error(err) - if errorx.IsOfType(err, ErrUnsupportedUser) || errorx.IsOfType(err, ErrInvalidImpersonateCredential) { + if errorx.IsOfType(err, shared.ErrUnsupportedUser) || errorx.IsOfType(err, ErrInvalidImpersonateCredential) { c.Status(http.StatusBadRequest) } return diff --git a/pkg/apiserver/user/sso/service.go b/pkg/apiserver/user/sso/service.go index 1dfccb49eb..b7cbe79f62 100644 --- a/pkg/apiserver/user/sso/service.go +++ b/pkg/apiserver/user/sso/service.go @@ -24,7 +24,7 @@ import ( "go.uber.org/zap" "golang.org/x/oauth2" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/pkg/config" "github.com/pingcap/tidb-dashboard/pkg/dbstore" @@ -33,7 +33,6 @@ import ( var ( ErrNS = errorx.NewNamespace("error.api.user.sso") - ErrUnsupportedUser = ErrNS.NewType("unsupported_user") ErrInvalidImpersonateCredential = ErrNS.NewType("invalid_impersonate_credential") ErrDiscoverFailed = ErrNS.NewType("discover_failed") ErrBadConfig = ErrNS.NewType("bad_config") @@ -48,9 +47,10 @@ const ( type ServiceParams struct { fx.In - LocalStore *dbstore.DB - TiDBClient *tidb.Client - ConfigManager *config.DynamicConfigManager + LocalStore *dbstore.DB + TiDBClient *tidb.Client + ConfigManager *config.DynamicConfigManager + AuthFeatureFlags *shared.AuthFeatureFlags } type Service struct { @@ -175,13 +175,13 @@ func (s *Service) newSessionFromImpersonation(userInfo *oAuthUserInfo, idToken s } // Check whether this user can access dashboard - writeable, err := user.VerifySQLUser(s.params.TiDBClient, userName, password) + writeable, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.AuthFeatureFlags, userName, password) if err != nil { if errorx.IsOfType(err, tidb.ErrTiDBAuthFailed) { _ = s.updateImpersonationStatus(userName, ImpersonateStatusAuthFail) return nil, ErrInvalidImpersonateCredential.Wrap(err, "Invalid SQL credential") } - if errorx.IsOfType(err, user.ErrInsufficientPrivs) { + if errorx.IsOfType(err, shared.ErrInsufficientPrivs) { _ = s.updateImpersonationStatus(userName, ImpersonateStatusInsufficientPrivs) return nil, ErrInvalidImpersonateCredential.Wrap(err, "Insufficient privileges") } @@ -204,12 +204,12 @@ func (s *Service) newSessionFromImpersonation(userInfo *oAuthUserInfo, idToken s func (s *Service) createImpersonation(userName string, password string) (*SSOImpersonationModel, error) { { // Check whether this user can access dashboard - _, err := user.VerifySQLUser(s.params.TiDBClient, userName, password) + _, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.AuthFeatureFlags, userName, password) if err != nil { if errorx.IsOfType(err, tidb.ErrTiDBAuthFailed) { return nil, ErrInvalidImpersonateCredential.Wrap(err, "Invalid SQL credential") } - if errorx.IsOfType(err, user.ErrInsufficientPrivs) { + if errorx.IsOfType(err, shared.ErrInsufficientPrivs) { return nil, ErrInvalidImpersonateCredential.Wrap(err, "Insufficient privileges") } return nil, err diff --git a/pkg/apiserver/user/sso/ssoauth/auth.go b/pkg/apiserver/user/sso/ssoauth/auth.go index cbaef95d81..ed68c230b7 100644 --- a/pkg/apiserver/user/sso/ssoauth/auth.go +++ b/pkg/apiserver/user/sso/ssoauth/auth.go @@ -7,7 +7,7 @@ import ( "go.uber.org/fx" - "github.com/pingcap/tidb-dashboard/pkg/apiserver/user" + "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/shared" "github.com/pingcap/tidb-dashboard/pkg/apiserver/user/sso" "github.com/pingcap/tidb-dashboard/pkg/apiserver/utils" "github.com/pingcap/tidb-dashboard/util/rest" @@ -16,22 +16,17 @@ import ( const typeID utils.AuthType = 2 type Authenticator struct { - user.BaseAuthenticator + shared.BaseAuthenticator ssoService *sso.Service } -func newAuthenticator(ssoService *sso.Service) *Authenticator { - return &Authenticator{ +func registerAuthenticator(r shared.AuthenticatorRegister, ssoService *sso.Service) { + r.Register(typeID, &Authenticator{ ssoService: ssoService, - } -} - -func registerAuthenticator(a *Authenticator, authService *user.AuthService) { - authService.RegisterAuthenticator(typeID, a) + }) } var Module = fx.Options( - fx.Provide(newAuthenticator), fx.Invoke(registerAuthenticator), ) @@ -41,7 +36,7 @@ type SSOExtra struct { RedirectURL string `json:"redirect_url"` } -func (a *Authenticator) Authenticate(f user.AuthenticateForm) (*utils.SessionUser, error) { +func (a *Authenticator) Authenticate(f shared.AuthenticateForm) (*utils.SessionUser, error) { var extra SSOExtra err := json.Unmarshal([]byte(f.Extra), &extra) if err != nil { @@ -58,12 +53,12 @@ func (a *Authenticator) IsEnabled() (bool, error) { return a.ssoService.IsEnabled() } -func (a *Authenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) (*user.SignOutInfo, error) { +func (a *Authenticator) SignOutInfo(u *utils.SessionUser, redirectURL string) (*shared.SignOutInfo, error) { esURL, err := a.ssoService.BuildEndSessionURL(u, redirectURL) if err != nil { return nil, err } - return &user.SignOutInfo{ + return &shared.SignOutInfo{ EndSessionURL: esURL, }, nil } From aa78a252da37dd4f209d7d7cb6dc04fbcf56e5ac Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 9 Dec 2021 10:41:45 +0800 Subject: [PATCH 18/19] test(util): update feature flag ut --- util/featureflag/featureflag.go | 2 +- util/featureflag/featureflag_test.go | 61 +++++++++++++--------------- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/util/featureflag/featureflag.go b/util/featureflag/featureflag.go index df4689177a..713b53d814 100644 --- a/util/featureflag/featureflag.go +++ b/util/featureflag/featureflag.go @@ -40,7 +40,7 @@ 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).WithProperty(rest.HTTPCodeProperty(http.StatusForbidden))) + _ = c.Error(ErrFeatureUnsupported.New(f.name).WithProperty(rest.HTTPCodeProperty(http.StatusBadRequest))) c.Abort() return } diff --git a/util/featureflag/featureflag_test.go b/util/featureflag/featureflag_test.go index 9e75d88188..61778d07e0 100644 --- a/util/featureflag/featureflag_test.go +++ b/util/featureflag/featureflag_test.go @@ -47,41 +47,38 @@ func Test_IsSupported(t *testing.T) { } func Test_VersionGuard(t *testing.T) { + gin.SetMode(gin.TestMode) 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(http.StatusOK, "pong") + t.Run("Success", func(t *testing.T) { + e := gin.Default() + e.Use(f1.VersionGuard()) + e.GET("/ping", func(c *gin.Context) { + c.String(http.StatusOK, "pong") + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/ping", nil) + e.ServeHTTP(w, req) + + r.Equal(http.StatusOK, w.Code) + r.Equal("pong", w.Body.String()) }) - w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/ping", nil) - e.ServeHTTP(w, req) - - r.Equal(http.StatusOK, w.Code) - r.Equal("pong", w.Body.String()) - // abort - handled := false - e2 := gin.Default() - e2.Use(func(c *gin.Context) { - c.Next() - - handled = true - 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(http.StatusOK, "pong") + t.Run("Abort", func(t *testing.T) { + w := httptest.NewRecorder() + ctx, e := gin.CreateTestContext(w) + e.Use(f1.VersionGuard(), f2.VersionGuard()) + e.GET("/ping", func(c *gin.Context) { + c.String(http.StatusOK, "pong") + }) + ctx.Request, _ = http.NewRequest(http.MethodGet, "/ping", nil) + e.HandleContext(ctx) + + r.Len(ctx.Errors, 1) + r.True(errorx.IsOfType(ctx.Errors.Last().Err, ErrFeatureUnsupported)) }) - w2 := httptest.NewRecorder() - req2, _ := http.NewRequest("GET", "/ping", nil) - e2.ServeHTTP(w2, req2) - - r.Equal(true, handled) } func Test_VersionGuardWith_ErrorHandlerFn(t *testing.T) { @@ -94,9 +91,9 @@ func Test_VersionGuardWith_ErrorHandlerFn(t *testing.T) { 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) + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/ping", nil) + e.ServeHTTP(w, req) - r.Equal(http.StatusForbidden, w2.Code) + r.Equal(http.StatusBadRequest, w.Code) } From 4c90ad647a23c83dcc1e07d689104d5c55d1e61b Mon Sep 17 00:00:00 2001 From: Suhaha Date: Thu, 9 Dec 2021 12:37:10 +0800 Subject: [PATCH 19/19] chore: update user feature flag name --- pkg/apiserver/user/shared/feature_flag.go | 6 +++--- pkg/apiserver/user/shared/verify_sql_user.go | 2 +- pkg/apiserver/user/sqlauth/sqlauth.go | 8 ++++---- pkg/apiserver/user/sso/service.go | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/apiserver/user/shared/feature_flag.go b/pkg/apiserver/user/shared/feature_flag.go index ca1f970bcd..36f51bcdd9 100644 --- a/pkg/apiserver/user/shared/feature_flag.go +++ b/pkg/apiserver/user/shared/feature_flag.go @@ -4,12 +4,12 @@ package shared import "github.com/pingcap/tidb-dashboard/util/featureflag" -type AuthFeatureFlags struct { +type UserFeatureFlags struct { NonRootLogin *featureflag.FeatureFlag } -func ProvideFeatureFlags(featureFlags *featureflag.Registry) *AuthFeatureFlags { - return &AuthFeatureFlags{ +func ProvideFeatureFlags(featureFlags *featureflag.Registry) *UserFeatureFlags { + return &UserFeatureFlags{ NonRootLogin: featureFlags.Register("nonRootLogin", ">= 5.3.0"), } } diff --git a/pkg/apiserver/user/shared/verify_sql_user.go b/pkg/apiserver/user/shared/verify_sql_user.go index 611c2ca57e..b77ac9270b 100644 --- a/pkg/apiserver/user/shared/verify_sql_user.go +++ b/pkg/apiserver/user/shared/verify_sql_user.go @@ -27,7 +27,7 @@ type tidbSEMConfig struct { SkipGrantTable bool `json:"skip-grant-table"` } -func VerifySQLUser(tidbClient *tidb.Client, featureFlags *AuthFeatureFlags, userName, password string) (writeable bool, err error) { +func VerifySQLUser(tidbClient *tidb.Client, featureFlags *UserFeatureFlags, userName, password string) (writeable bool, err error) { if !featureFlags.NonRootLogin.IsSupported() && userName != "root" { return false, ErrUnsupportedUser.New("User must be root") } diff --git a/pkg/apiserver/user/sqlauth/sqlauth.go b/pkg/apiserver/user/sqlauth/sqlauth.go index 069a91f17f..61b44f79df 100755 --- a/pkg/apiserver/user/sqlauth/sqlauth.go +++ b/pkg/apiserver/user/sqlauth/sqlauth.go @@ -16,13 +16,13 @@ const typeID utils.AuthType = 0 type Authenticator struct { shared.BaseAuthenticator tidbClient *tidb.Client - authFeatureFlags *shared.AuthFeatureFlags + userFeatureFlags *shared.UserFeatureFlags } -func registerAuthenticator(r shared.AuthenticatorRegister, tidbClient *tidb.Client, ff *shared.AuthFeatureFlags) { +func registerAuthenticator(r shared.AuthenticatorRegister, tidbClient *tidb.Client, ff *shared.UserFeatureFlags) { r.Register(typeID, &Authenticator{ tidbClient: tidbClient, - authFeatureFlags: ff, + userFeatureFlags: ff, }) } @@ -31,7 +31,7 @@ var Module = fx.Options( ) func (a *Authenticator) Authenticate(f shared.AuthenticateForm) (*utils.SessionUser, error) { - writeable, err := shared.VerifySQLUser(a.tidbClient, a.authFeatureFlags, f.Username, f.Password) + writeable, err := shared.VerifySQLUser(a.tidbClient, a.userFeatureFlags, f.Username, f.Password) if err != nil { if errorx.Cast(err) == nil { return nil, shared.ErrSignInOther.WrapWithNoMessage(err) diff --git a/pkg/apiserver/user/sso/service.go b/pkg/apiserver/user/sso/service.go index b7cbe79f62..1e81002306 100644 --- a/pkg/apiserver/user/sso/service.go +++ b/pkg/apiserver/user/sso/service.go @@ -50,7 +50,7 @@ type ServiceParams struct { LocalStore *dbstore.DB TiDBClient *tidb.Client ConfigManager *config.DynamicConfigManager - AuthFeatureFlags *shared.AuthFeatureFlags + UserFeatureFlags *shared.UserFeatureFlags } type Service struct { @@ -175,7 +175,7 @@ func (s *Service) newSessionFromImpersonation(userInfo *oAuthUserInfo, idToken s } // Check whether this user can access dashboard - writeable, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.AuthFeatureFlags, userName, password) + writeable, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.UserFeatureFlags, userName, password) if err != nil { if errorx.IsOfType(err, tidb.ErrTiDBAuthFailed) { _ = s.updateImpersonationStatus(userName, ImpersonateStatusAuthFail) @@ -204,7 +204,7 @@ func (s *Service) newSessionFromImpersonation(userInfo *oAuthUserInfo, idToken s func (s *Service) createImpersonation(userName string, password string) (*SSOImpersonationModel, error) { { // Check whether this user can access dashboard - _, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.AuthFeatureFlags, userName, password) + _, err := shared.VerifySQLUser(s.params.TiDBClient, s.params.UserFeatureFlags, userName, password) if err != nil { if errorx.IsOfType(err, tidb.ErrTiDBAuthFailed) { return nil, ErrInvalidImpersonateCredential.Wrap(err, "Invalid SQL credential")