From 876238d68892b4a8988c1a709f63b6994f9e587f Mon Sep 17 00:00:00 2001
From: Alirie Gray <alirie.gray@gmail.com>
Date: Thu, 28 May 2020 08:26:08 -0700
Subject: [PATCH] feat: add feature flag for new labels service (#18215)
---
cmd/influxd/launcher/launcher.go | 29 +++++++++-
flags.yml | 8 +++
http/api_handler.go | 3 -
http/label_service.go | 4 ++
kit/feature/list.go | 16 ++++++
label/controller.go | 94 ++++++++++++++++++++++++++++++++
label/controller_test.go | 55 +++++++++++++++++++
label/middleware_auth.go | 27 +++++----
label/middleware_auth_test.go | 26 +++++----
label/service.go | 7 +++
10 files changed, 243 insertions(+), 26 deletions(-)
create mode 100644 label/controller.go
create mode 100644 label/controller_test.go
diff --git a/cmd/influxd/launcher/launcher.go b/cmd/influxd/launcher/launcher.go
index afb81815a0..e90ead99d4 100644
--- a/cmd/influxd/launcher/launcher.go
+++ b/cmd/influxd/launcher/launcher.go
@@ -37,6 +37,7 @@ import (
"github.com/influxdata/influxdb/v2/kit/tracing"
kithttp "github.com/influxdata/influxdb/v2/kit/transport/http"
"github.com/influxdata/influxdb/v2/kv"
+ "github.com/influxdata/influxdb/v2/label"
influxlogger "github.com/influxdata/influxdb/v2/logger"
"github.com/influxdata/influxdb/v2/nats"
"github.com/influxdata/influxdb/v2/pkger"
@@ -592,7 +593,6 @@ func (m *Launcher) run(ctx context.Context) (err error) {
orgLogSvc platform.OrganizationOperationLogService = m.kvService
scraperTargetSvc platform.ScraperTargetStoreService = m.kvService
telegrafSvc platform.TelegrafConfigStore = m.kvService
- labelSvc platform.LabelService = m.kvService
secretSvc platform.SecretService = m.kvService
lookupSvc platform.LookupService = m.kvService
notificationEndpointStore platform.NotificationEndpointService = m.kvService
@@ -869,6 +869,17 @@ func (m *Launcher) run(ctx context.Context) (err error) {
sessionSvc = session.NewServiceController(flagger, m.kvService, sessionSvc)
}
+ var labelSvc platform.LabelService
+ {
+ labelsStore, err := label.NewStore(m.kvStore)
+ if err != nil {
+ m.log.Error("Failed creating new labels store", zap.Error(err))
+ return err
+ }
+ ls := label.NewService(labelsStore, m.kvService)
+ labelSvc = label.NewLabelController(flagger, m.kvService, ls)
+ }
+
m.apibackend = &http.APIBackend{
AssetsPath: m.assetsPath,
HTTPErrorHandler: kithttp.ErrorHandler(0),
@@ -970,6 +981,21 @@ func (m *Launcher) run(ctx context.Context) (err error) {
onboardHTTPServer = tenant.NewHTTPOnboardHandler(m.log, onboardSvc)
}
+ // feature flagging for new labels service
+ var labelsHTTPServer *kithttp.FeatureHandler
+ {
+ b := m.apibackend
+ labelSvcWithOrg := authorizer.NewLabelServiceWithOrg(labelSvc, b.OrgLookupService)
+ oldHandler := http.NewLabelHandler(m.log.With(zap.String("handler", "labels")), labelSvcWithOrg, kithttp.ErrorHandler(0))
+
+ labelSvc = label.NewAuthedLabelService(labelSvc, b.OrgLookupService)
+ labelSvc = label.NewLabelLogger(m.log.With(zap.String("handler", "labels")), labelSvc)
+ labelSvc = label.NewLabelMetrics(m.reg, labelSvc)
+ newHandler := label.NewHTTPLabelHandler(m.log, labelSvc)
+
+ labelsHTTPServer = kithttp.NewFeatureHandler(feature.NewLabelPackage(), flagger, oldHandler, newHandler, newHandler.Prefix())
+ }
+
// feature flagging for new authorization service
var authHTTPServer *kithttp.FeatureHandler
{
@@ -1006,6 +1032,7 @@ func (m *Launcher) run(ctx context.Context) (err error) {
http.WithResourceHandler(pkgHTTPServer),
http.WithResourceHandler(onboardHTTPServer),
http.WithResourceHandler(authHTTPServer),
+ http.WithResourceHandler(labelsHTTPServer),
http.WithResourceHandler(kithttp.NewFeatureHandler(feature.SessionService(), flagger, oldSessionHandler, sessionHTTPServer.SignInResourceHandler(), sessionHTTPServer.SignInResourceHandler().Prefix())),
http.WithResourceHandler(kithttp.NewFeatureHandler(feature.SessionService(), flagger, oldSessionHandler, sessionHTTPServer.SignOutResourceHandler(), sessionHTTPServer.SignOutResourceHandler().Prefix())),
http.WithResourceHandler(userHTTPServer.MeResourceHandler()),
diff --git a/flags.yml b/flags.yml
index d721460d59..9f2f8a8649 100644
--- a/flags.yml
+++ b/flags.yml
@@ -58,3 +58,11 @@
key: pushDownGroupAggregateCount
default: false
contact: Query Team
+
+- name: New Label Package
+ description: Enables the refactored labels api
+ key: newLabels
+ default: false
+ contact: Alirie Gray
+ lifetime: temporary
+
diff --git a/http/api_handler.go b/http/api_handler.go
index 726bcb20d4..3eb5de10d7 100644
--- a/http/api_handler.go
+++ b/http/api_handler.go
@@ -126,7 +126,6 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
noAuthUserResourceMappingService := b.UserResourceMappingService
b.UserResourceMappingService = authorizer.NewURMService(b.OrgLookupService, b.UserResourceMappingService)
- b.LabelService = authorizer.NewLabelServiceWithOrg(b.LabelService, b.OrgLookupService)
h.Mount("/api/v2", serveLinksHandler(b.HTTPErrorHandler))
@@ -155,8 +154,6 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
fluxBackend := NewFluxBackend(b.Logger.With(zap.String("handler", "query")), b)
h.Mount(prefixQuery, NewFluxHandler(b.Logger, fluxBackend))
- h.Mount(prefixLabels, NewLabelHandler(b.Logger, b.LabelService, b.HTTPErrorHandler))
-
notificationEndpointBackend := NewNotificationEndpointBackend(b.Logger.With(zap.String("handler", "notificationEndpoint")), b)
notificationEndpointBackend.NotificationEndpointService = authorizer.NewNotificationEndpointService(b.NotificationEndpointService,
b.UserResourceMappingService, b.OrganizationService)
diff --git a/http/label_service.go b/http/label_service.go
index 1f17c4457b..015823d971 100644
--- a/http/label_service.go
+++ b/http/label_service.go
@@ -47,6 +47,10 @@ func NewLabelHandler(log *zap.Logger, s influxdb.LabelService, he influxdb.HTTPE
return h
}
+func (h *LabelHandler) Prefix() string {
+ return prefixLabels
+}
+
// handlePostLabel is the HTTP handler for the POST /api/v2/labels route.
func (h *LabelHandler) handlePostLabel(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
diff --git a/kit/feature/list.go b/kit/feature/list.go
index a2bce7d183..8790a06aeb 100644
--- a/kit/feature/list.go
+++ b/kit/feature/list.go
@@ -100,6 +100,20 @@ func PushDownGroupAggregateCount() BoolFlag {
return pushDownGroupAggregateCount
}
+var newLabels = MakeBoolFlag(
+ "New Label Package",
+ "newLabels",
+ "Alirie Gray",
+ false,
+ Temporary,
+ false,
+)
+
+// NewLabelPackage - Enables the refactored labels api
+func NewLabelPackage() BoolFlag {
+ return newLabels
+}
+
var all = []Flag{
backendExample,
frontendExample,
@@ -108,6 +122,7 @@ var all = []Flag{
newAuth,
sessionService,
pushDownGroupAggregateCount,
+ newLabels,
}
var byKey = map[string]Flag{
@@ -118,4 +133,5 @@ var byKey = map[string]Flag{
"newAuth": newAuth,
"sessionService": sessionService,
"pushDownGroupAggregateCount": pushDownGroupAggregateCount,
+ "newLabels": newLabels,
}
diff --git a/label/controller.go b/label/controller.go
new file mode 100644
index 0000000000..1d1a6d1b36
--- /dev/null
+++ b/label/controller.go
@@ -0,0 +1,94 @@
+package label
+
+import (
+ "context"
+
+ "github.com/influxdata/influxdb/v2"
+ "github.com/influxdata/influxdb/v2/kit/feature"
+)
+
+var _ influxdb.LabelService = (*LabelController)(nil)
+
+// LabelController is a temporary system for switching the label backend between
+// the old and refactored versions with a feature flag
+type LabelController struct {
+ flagger feature.Flagger
+ oldLabelService influxdb.LabelService
+ newLabelService influxdb.LabelService
+}
+
+func NewLabelController(flagger feature.Flagger, oldLabelService, newLabelService influxdb.LabelService) *LabelController {
+ return &LabelController{
+ flagger: flagger,
+ oldLabelService: oldLabelService,
+ newLabelService: newLabelService,
+ }
+}
+
+func (s *LabelController) useNew(ctx context.Context) bool {
+ return feature.NewLabelPackage().Enabled(ctx, s.flagger)
+}
+
+func (s *LabelController) CreateLabel(ctx context.Context, l *influxdb.Label) error {
+ if s.useNew(ctx) {
+ return s.newLabelService.CreateLabel(ctx, l)
+ }
+ return s.oldLabelService.CreateLabel(ctx, l)
+
+}
+
+func (s *LabelController) FindLabelByID(ctx context.Context, id influxdb.ID) (*influxdb.Label, error) {
+ if s.useNew(ctx) {
+ return s.newLabelService.FindLabelByID(ctx, id)
+ }
+ return s.oldLabelService.FindLabelByID(ctx, id)
+
+}
+
+func (s *LabelController) FindLabels(ctx context.Context, filter influxdb.LabelFilter, opt ...influxdb.FindOptions) ([]*influxdb.Label, error) {
+ if s.useNew(ctx) {
+ return s.newLabelService.FindLabels(ctx, filter, opt...)
+ }
+ return s.oldLabelService.FindLabels(ctx, filter, opt...)
+
+}
+
+func (s *LabelController) FindResourceLabels(ctx context.Context, filter influxdb.LabelMappingFilter) ([]*influxdb.Label, error) {
+ if s.useNew(ctx) {
+ return s.newLabelService.FindResourceLabels(ctx, filter)
+ }
+ return s.oldLabelService.FindResourceLabels(ctx, filter)
+
+}
+
+func (s *LabelController) UpdateLabel(ctx context.Context, id influxdb.ID, upd influxdb.LabelUpdate) (*influxdb.Label, error) {
+ if s.useNew(ctx) {
+ return s.newLabelService.UpdateLabel(ctx, id, upd)
+ }
+ return s.oldLabelService.UpdateLabel(ctx, id, upd)
+
+}
+
+func (s *LabelController) DeleteLabel(ctx context.Context, id influxdb.ID) error {
+ if s.useNew(ctx) {
+ return s.newLabelService.DeleteLabel(ctx, id)
+ }
+ return s.oldLabelService.DeleteLabel(ctx, id)
+
+}
+
+func (s *LabelController) CreateLabelMapping(ctx context.Context, m *influxdb.LabelMapping) error {
+ if s.useNew(ctx) {
+ return s.newLabelService.CreateLabelMapping(ctx, m)
+ }
+ return s.oldLabelService.CreateLabelMapping(ctx, m)
+
+}
+
+func (s *LabelController) DeleteLabelMapping(ctx context.Context, m *influxdb.LabelMapping) error {
+ if s.useNew(ctx) {
+ return s.newLabelService.DeleteLabelMapping(ctx, m)
+ }
+ return s.oldLabelService.DeleteLabelMapping(ctx, m)
+
+}
diff --git a/label/controller_test.go b/label/controller_test.go
new file mode 100644
index 0000000000..be702f4765
--- /dev/null
+++ b/label/controller_test.go
@@ -0,0 +1,55 @@
+package label_test
+
+import (
+ "context"
+ "testing"
+
+ "github.com/influxdata/influxdb/v2"
+ "github.com/influxdata/influxdb/v2/kit/feature"
+ "github.com/influxdata/influxdb/v2/kv"
+ "github.com/influxdata/influxdb/v2/label"
+ influxdbtesting "github.com/influxdata/influxdb/v2/testing"
+ "go.uber.org/zap/zaptest"
+)
+
+func TestLabelServiceController(t *testing.T) {
+ influxdbtesting.LabelService(initBoltLabelServiceController, t)
+}
+
+func initBoltLabelServiceController(f influxdbtesting.LabelFields, t *testing.T) (influxdb.LabelService, string, func()) {
+ newSvc, s, closer := initBoltLabelService(f, t)
+ st, _, _ := NewTestBoltStore(t)
+ oldSvc, _, _ := initOldLabelService(st, f, t)
+
+ flagger := feature.DefaultFlagger()
+ return label.NewLabelController(flagger, oldSvc, newSvc), s, closer
+}
+
+func initOldLabelService(s kv.Store, f influxdbtesting.LabelFields, t *testing.T) (influxdb.LabelService, string, func()) {
+ svc := kv.NewService(zaptest.NewLogger(t), s)
+ svc.IDGenerator = f.IDGenerator
+
+ ctx := context.Background()
+ if err := svc.Initialize(ctx); err != nil {
+ t.Fatalf("error initializing label service: %v", err)
+ }
+ for _, l := range f.Labels {
+ if err := svc.PutLabel(ctx, l); err != nil {
+ t.Fatalf("failed to populate labels: %v", err)
+ }
+ }
+
+ for _, m := range f.Mappings {
+ if err := svc.PutLabelMapping(ctx, m); err != nil {
+ t.Fatalf("failed to populate label mappings: %v", err)
+ }
+ }
+
+ return svc, kv.OpPrefix, func() {
+ for _, l := range f.Labels {
+ if err := svc.DeleteLabel(ctx, l.ID); err != nil {
+ t.Logf("failed to remove label: %v", err)
+ }
+ }
+ }
+}
diff --git a/label/middleware_auth.go b/label/middleware_auth.go
index fbd0ef6c98..2c117aaa03 100644
--- a/label/middleware_auth.go
+++ b/label/middleware_auth.go
@@ -6,19 +6,20 @@ import (
"github.com/influxdata/influxdb/v2"
"github.com/influxdata/influxdb/v2/authorizer"
- kithttp "github.com/influxdata/influxdb/v2/kit/transport/http"
)
var _ influxdb.LabelService = (*AuthedLabelService)(nil)
type AuthedLabelService struct {
- s influxdb.LabelService
+ s influxdb.LabelService
+ orgSvc authorizer.OrganizationService
}
// NewAuthedLabelService constructs an instance of an authorizing label serivce.
-func NewAuthedLabelService(s influxdb.LabelService) *AuthedLabelService {
+func NewAuthedLabelService(s influxdb.LabelService, orgSvc authorizer.OrganizationService) *AuthedLabelService {
return &AuthedLabelService{
- s: s,
+ s: s,
+ orgSvc: orgSvc,
}
}
func (s *AuthedLabelService) CreateLabel(ctx context.Context, l *influxdb.Label) error {
@@ -57,19 +58,22 @@ func (s *AuthedLabelService) FindResourceLabels(ctx context.Context, filter infl
if err := filter.ResourceType.Valid(); err != nil {
return nil, err
}
- // first fetch all labels for this resource
- ls, err := s.s.FindResourceLabels(ctx, filter)
+
+ if s.orgSvc == nil {
+ return nil, errors.New("failed to find orgSvc")
+ }
+ orgID, err := s.orgSvc.FindResourceOrganizationID(ctx, filter.ResourceType, filter.ResourceID)
if err != nil {
return nil, err
}
- // check the permissions for the resource by the org on the context
- orgID := kithttp.OrgIDFromContext(ctx)
- if orgID == nil {
- return nil, errors.New("failed to find orgID on context")
+ if _, _, err := authorizer.AuthorizeRead(ctx, filter.ResourceType, filter.ResourceID, orgID); err != nil {
+ return nil, err
}
- if _, _, err := authorizer.AuthorizeRead(ctx, filter.ResourceType, filter.ResourceID, *orgID); err != nil {
+ // first fetch all labels for this resource
+ ls, err := s.s.FindResourceLabels(ctx, filter)
+ if err != nil {
return nil, err
}
@@ -108,6 +112,7 @@ func (s *AuthedLabelService) CreateLabelMapping(ctx context.Context, m *influxdb
if err != nil {
return err
}
+
if _, _, err := authorizer.AuthorizeWrite(ctx, influxdb.LabelsResourceType, m.LabelID, l.OrgID); err != nil {
return err
}
diff --git a/label/middleware_auth_test.go b/label/middleware_auth_test.go
index b00c470b30..b9f382067b 100644
--- a/label/middleware_auth_test.go
+++ b/label/middleware_auth_test.go
@@ -9,7 +9,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/influxdata/influxdb/v2"
influxdbcontext "github.com/influxdata/influxdb/v2/context"
- kithttp "github.com/influxdata/influxdb/v2/kit/transport/http"
"github.com/influxdata/influxdb/v2/mock"
influxdbtesting "github.com/influxdata/influxdb/v2/testing"
)
@@ -20,6 +19,11 @@ const (
var (
orgOneInfluxID = influxdbtesting.MustIDBase16(orgOneID)
+ orgSvc = &mock.OrganizationService{
+ FindResourceOrganizationIDF: func(_ context.Context, _ influxdb.ResourceType, _ influxdb.ID) (influxdb.ID, error) {
+ return orgOneInfluxID, nil
+ },
+ }
)
var labelCmpOptions = cmp.Options{
@@ -112,7 +116,7 @@ func TestLabelService_FindLabelByID(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, []influxdb.Permission{tt.args.permission}))
@@ -267,9 +271,9 @@ func TestLabelService_FindLabels(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
- ctx := context.WithValue(context.Background(), kithttp.CtxOrgKey, orgOneInfluxID)
+ ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, []influxdb.Permission{tt.args.permission}))
labels, err := s.FindLabels(ctx, influxdb.LabelFilter{})
@@ -375,7 +379,7 @@ func TestLabelService_UpdateLabel(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, tt.args.permissions))
@@ -475,7 +479,7 @@ func TestLabelService_DeleteLabel(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, tt.args.permissions))
@@ -579,7 +583,7 @@ func TestLabelService_CreateLabel(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, []influxdb.Permission{tt.args.permission}))
@@ -811,9 +815,9 @@ func TestLabelService_FindResourceLabels(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
- ctx := context.WithValue(context.Background(), kithttp.CtxOrgKey, orgOneInfluxID)
+ ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, tt.args.permissions))
labels, err := s.FindResourceLabels(ctx, tt.args.filter)
@@ -963,7 +967,7 @@ func TestLabelService_CreateLabelMapping(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, tt.args.permissions))
@@ -1111,7 +1115,7 @@ func TestLabelService_DeleteLabelMapping(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- s := NewAuthedLabelService(tt.fields.LabelService)
+ s := NewAuthedLabelService(tt.fields.LabelService, orgSvc)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, mock.NewMockAuthorizer(false, tt.args.permissions))
diff --git a/label/service.go b/label/service.go
index 97dd326c0c..7785533950 100644
--- a/label/service.go
+++ b/label/service.go
@@ -2,6 +2,7 @@ package label
import (
"context"
+ "fmt"
"strings"
"github.com/influxdata/influxdb/v2"
@@ -144,13 +145,17 @@ func (s *Service) DeleteLabel(ctx context.Context, id influxdb.ID) error {
// CreateLabelMapping creates a new mapping between a resource and a label.
func (s *Service) CreateLabelMapping(ctx context.Context, m *influxdb.LabelMapping) error {
+ fmt.Println("creating label mapping")
err := s.store.View(ctx, func(tx kv.Tx) error {
if _, err := s.store.GetLabel(ctx, tx, m.LabelID); err != nil {
+ fmt.Println("could not get label")
return err
}
+
ls := []*influxdb.Label{}
err := s.store.FindResourceLabels(ctx, tx, influxdb.LabelMappingFilter{ResourceID: m.ResourceID, ResourceType: m.ResourceType}, &ls)
if err != nil {
+ fmt.Println("could not get resource labels")
return err
}
for i := 0; i < len(ls); i++ {
@@ -162,10 +167,12 @@ func (s *Service) CreateLabelMapping(ctx context.Context, m *influxdb.LabelMappi
return nil
})
if err != nil {
+ fmt.Println("could not .....?")
return err
}
return s.store.Update(ctx, func(tx kv.Tx) error {
+ fmt.Println("calling to store ")
return s.store.CreateLabelMapping(ctx, tx, m)
})
}