Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Refactor feature flag to support more modules #1057

Merged
merged 23 commits into from
Dec 8, 2021
Merged

Refactor feature flag to support more modules #1057

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fd7dd71
refactor(*): feature flag
shhdgit Nov 11, 2021
8bd2e44
chore: refine feature flag
shhdgit Nov 11, 2021
74a0937
refine: feature flag middleware
shhdgit Nov 11, 2021
178a377
Merge branch 'master' into refactor/feature-flag
shhdgit Nov 12, 2021
f6ec213
refactor: move version utils to util package
shhdgit Nov 12, 2021
beafba3
chore: make lint happy
shhdgit Nov 12, 2021
0b4dd21
Merge branch 'master' into refactor/feature-flag
shhdgit Nov 14, 2021
4857960
refine(feature): flag & manager
shhdgit Nov 14, 2021
8c9c89c
test(feature_flag): add manager tests
shhdgit Nov 15, 2021
28a97f3
refine(util): feature flag
shhdgit Dec 7, 2021
a9382e9
*: update feature flag
shhdgit Dec 7, 2021
1a39419
Merge branch 'master' into refactor/feature-flag
shhdgit Dec 7, 2021
43b297d
*: update feature flag
shhdgit Dec 7, 2021
cc55adc
chore: make lint happy
shhdgit Dec 7, 2021
15490d5
chore: revert lint
shhdgit Dec 7, 2021
3401a7f
Update index.tsx
shhdgit Dec 7, 2021
c7e50e0
Update Detail.tsx
shhdgit Dec 7, 2021
c4e1a03
refine: feature flag
shhdgit Dec 7, 2021
94e5e95
fix: fx & indirect x/tools version
shhdgit Dec 8, 2021
194a4ed
Merge branch 'master' into refactor/feature-flag
shhdgit Dec 8, 2021
8087baf
refine: version guard error
shhdgit Dec 8, 2021
61a7d73
Merge branch 'refactor/feature-flag' of github.com:shhdgit/tidb-dashb…
shhdgit Dec 8, 2021
0623ecc
test: adjust feature flag version guard test
shhdgit Dec 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/apiserver/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +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/pkg/utils/version"
"github.com/pingcap/tidb-dashboard/util/featureflag"
"github.com/pingcap/tidb-dashboard/util/rest"

// "github.com/pingcap/tidb-dashboard/pkg/apiserver/__APP_NAME__"
Expand All @@ -46,7 +48,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 {
Expand Down Expand Up @@ -100,6 +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),
breezewish marked this conversation as resolved.
Show resolved Hide resolved
newAPIHandlerEngine,
s.provideLocals,
dbstore.NewDBStore,
Expand All @@ -111,7 +113,6 @@ func (s *Service) Start(ctx context.Context) error {
tikv.NewTiKVClient,
tiflash.NewTiFlashClient,
utils.NewSysSchema,
user.NewAuthService,
info.NewService,
clusterinfo.NewService,
logsearch.NewService,
Expand All @@ -123,6 +124,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,
Expand All @@ -135,7 +137,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,
Expand Down
23 changes: 0 additions & 23 deletions pkg/apiserver/conprof/feature_support.go
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion pkg/apiserver/conprof/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

package conprof

import "go.uber.org/fx"
import (
"go.uber.org/fx"
)

var Module = fx.Options(
fx.Provide(newService),
Expand Down
17 changes: 13 additions & 4 deletions pkg/apiserver/conprof/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +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/featureflag"
"github.com/pingcap/tidb-dashboard/util/rest"
)

Expand All @@ -44,11 +45,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

Expand All @@ -57,21 +61,26 @@ 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) {
endpoint := r.Group("/continuous_profiling")

endpoint.Use(utils.MWForbidByFeatureSupport(IsFeatureSupport(s.params.Config)))
endpoint.Use(featureflag.VersionGuard(s.params.Config.FeatureVersion, s.FeatureFlagConprof))
breezewish marked this conversation as resolved.
Show resolved Hide resolved
{
endpoint.GET("/config", auth.MWAuthRequired(), s.reverseProxy("/config"), s.conprofConfig)
endpoint.POST("/config", auth.MWAuthRequired(), auth.MWRequireWritePriv(), s.reverseProxy("/config"), s.updateConprofConfig)
Expand Down
20 changes: 6 additions & 14 deletions pkg/apiserver/info/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +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/nonrootlogin"
"github.com/pingcap/tidb-dashboard/pkg/apiserver/user"
"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/version"
"github.com/pingcap/tidb-dashboard/util/featureflag"
)

type ServiceParams struct {
fx.In
Config *config.Config
LocalStore *dbstore.DB
TiDBClient *tidb.Client
Config *config.Config
LocalStore *dbstore.DB
TiDBClient *tidb.Client
FeatureFlagRegistry *featureflag.Registry
}

type Service struct {
Expand Down Expand Up @@ -61,19 +61,11 @@ type InfoResponse struct { //nolint
// @Security JwtAuth
// @Failure 401 {object} rest.ErrorResponse
func (s *Service) infoHandler(c *gin.Context) {
supportedFeatures := []string{}
if conprof.IsFeatureSupport(s.params.Config) {
supportedFeatures = append(supportedFeatures, "conprof")
}
if nonrootlogin.IsFeatureSupport(s.params.Config) {
supportedFeatures = append(supportedFeatures, "nonRootLogin")
}

resp := InfoResponse{
Version: version.GetInfo(),
EnableTelemetry: s.params.Config.EnableTelemetry,
EnableExperimental: s.params.Config.EnableExperimental,
SupportedFeatures: supportedFeatures,
SupportedFeatures: s.params.FeatureFlagRegistry.SupportedFeatures(),
breezewish marked this conversation as resolved.
Show resolved Hide resolved
}
c.JSON(http.StatusOK, resp)
}
Expand Down
23 changes: 0 additions & 23 deletions pkg/apiserver/nonrootlogin/feature_support.go
View file
Open in desktop

This file was deleted.

12 changes: 8 additions & 4 deletions pkg/apiserver/user/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"go.uber.org/zap"

"github.com/pingcap/tidb-dashboard/pkg/apiserver/utils"
"github.com/pingcap/tidb-dashboard/util/featureflag"
"github.com/pingcap/tidb-dashboard/util/rest"
)

Expand All @@ -30,6 +31,8 @@ var (
)

type AuthService struct {
FeatureFlagNonRootLogin *featureflag.FeatureFlag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature should be placed inside SQL auth, as only SQL auth needs it and other kind of auth does not care about the "non root login".


middleware *jwt.GinJWTMiddleware
authenticators map[utils.AuthType]Authenticator
}
Expand Down Expand Up @@ -71,7 +74,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")
Expand All @@ -88,8 +91,9 @@ func NewAuthService() *AuthService {
}

service := &AuthService{
middleware: nil,
authenticators: map[utils.AuthType]Authenticator{},
FeatureFlagNonRootLogin: featureFlagRegistry.Register("nonRootLogin", ">= 5.3.0"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that this FeatureFlagNonRootLogin is not used. It doesn't look good as this may imply that this protection is purely happening at frontend and there is no backend protection for corresponding features.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a new PR #1089 to refactor the "non root login" for better review experience.

middleware: nil,
authenticators: map[utils.AuthType]Authenticator{},
}

middleware, err := jwt.New(&jwt.GinJWTMiddleware{
Expand Down Expand Up @@ -219,7 +223,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)
Expand Down
12 changes: 12 additions & 0 deletions pkg/apiserver/user/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2021 PingCAP, Inc. Licensed under Apache-2.0.

package user

import (
"go.uber.org/fx"
)

var Module = fx.Options(
fx.Provide(newAuthService),
fx.Invoke(registerRouter),
)
2 changes: 0 additions & 2 deletions pkg/apiserver/utils/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ import (
var ErrNS = errorx.NewNamespace("error.api")

var ErrExpNotEnabled = ErrNS.NewType("experimental_feature_not_enabled")

var ErrFeatureNotSupported = ErrNS.NewType("feature_not_supported")
22 changes: 0 additions & 22 deletions pkg/apiserver/utils/mw_feature_support.go
View file
Open in desktop

This file was deleted.

37 changes: 0 additions & 37 deletions pkg/apiserver/utils/semver_check.go
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"crypto/tls"
"net/url"
"strings"

"github.com/pingcap/tidb-dashboard/pkg/utils/version"
)

const (
Expand Down Expand Up @@ -40,7 +42,7 @@ func Default() *Config {
TiDBTLSConfig: nil,
EnableTelemetry: true,
EnableExperimental: false,
FeatureVersion: "",
FeatureVersion: version.PDVersion,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a nice improvement.

}
}

Expand Down
Loading