Skip to content

Commit

Permalink
chore(kv): remove rule service behaviours from kv
Browse files Browse the repository at this point in the history
This also introduces the org id resolver type. Which is transplanted
from the kv service. As this one function coupled all resource
capabilities onto the kv service. Making removing these capabilities
impossible. Moving this type out into its own package which depends on
each service explicitly ensures we don't have one type which has to
implement all the service contracts.
  • Loading branch information
GeorgeMac committed Oct 24, 2020
1 parent f375730 commit 17c9f8e
Show file tree
Hide file tree
Showing 13 changed files with 250 additions and 709 deletions.
2 changes: 1 addition & 1 deletion authorizer/authorize_find.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func AuthorizeFindChecks(ctx context.Context, rs []influxdb.Check) ([]influxdb.C
}

// AuthorizeFindUserResourceMappings takes the given items and returns only the ones that the user is authorized to read.
func AuthorizeFindUserResourceMappings(ctx context.Context, os OrganizationService, rs []*influxdb.UserResourceMapping) ([]*influxdb.UserResourceMapping, int, error) {
func AuthorizeFindUserResourceMappings(ctx context.Context, os OrgIDResolver, rs []*influxdb.UserResourceMapping) ([]*influxdb.UserResourceMapping, int, error) {
// This filters without allocating
// https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating
rrs := rs[:0]
Expand Down
17 changes: 7 additions & 10 deletions authorizer/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package authorizer

import (
"context"
"errors"

"github.com/influxdata/influxdb/v2"
)
Expand All @@ -12,16 +11,16 @@ var _ influxdb.LabelService = (*LabelService)(nil)
// LabelService wraps a influxdb.LabelService and authorizes actions
// against it appropriately.
type LabelService struct {
s influxdb.LabelService
orgSvc OrganizationService
s influxdb.LabelService
orgIDResolver OrgIDResolver
}

// NewLabelServiceWithOrg constructs an instance of an authorizing label serivce.
// Replaces NewLabelService.
func NewLabelServiceWithOrg(s influxdb.LabelService, orgSvc OrganizationService) *LabelService {
func NewLabelServiceWithOrg(s influxdb.LabelService, orgIDResolver OrgIDResolver) *LabelService {
return &LabelService{
s: s,
orgSvc: orgSvc,
s: s,
orgIDResolver: orgIDResolver,
}
}

Expand Down Expand Up @@ -55,10 +54,8 @@ func (s *LabelService) FindResourceLabels(ctx context.Context, filter influxdb.L
if err := filter.ResourceType.Valid(); err != nil {
return nil, err
}
if s.orgSvc == nil {
return nil, errors.New("failed to find orgSvc")
}
orgID, err := s.orgSvc.FindResourceOrganizationID(ctx, filter.ResourceType, filter.ResourceID)

orgID, err := s.orgIDResolver.FindResourceOrganizationID(ctx, filter.ResourceType, filter.ResourceID)
if err != nil {
return nil, err
}
Expand Down
18 changes: 9 additions & 9 deletions authorizer/urm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import (
"github.com/influxdata/influxdb/v2"
)

type OrganizationService interface {
type OrgIDResolver interface {
FindResourceOrganizationID(ctx context.Context, rt influxdb.ResourceType, id influxdb.ID) (influxdb.ID, error)
}

type URMService struct {
s influxdb.UserResourceMappingService
orgService OrganizationService
s influxdb.UserResourceMappingService
orgIDResolver OrgIDResolver
}

func NewURMService(orgSvc OrganizationService, s influxdb.UserResourceMappingService) *URMService {
func NewURMService(orgIDResolver OrgIDResolver, s influxdb.UserResourceMappingService) *URMService {
return &URMService{
s: s,
orgService: orgSvc,
s: s,
orgIDResolver: orgIDResolver,
}
}

Expand All @@ -27,11 +27,11 @@ func (s *URMService) FindUserResourceMappings(ctx context.Context, filter influx
if err != nil {
return nil, 0, err
}
return AuthorizeFindUserResourceMappings(ctx, s.orgService, urms)
return AuthorizeFindUserResourceMappings(ctx, s.orgIDResolver, urms)
}

func (s *URMService) CreateUserResourceMapping(ctx context.Context, m *influxdb.UserResourceMapping) error {
orgID, err := s.orgService.FindResourceOrganizationID(ctx, m.ResourceType, m.ResourceID)
orgID, err := s.orgIDResolver.FindResourceOrganizationID(ctx, m.ResourceType, m.ResourceID)
if err != nil {
return err
}
Expand All @@ -49,7 +49,7 @@ func (s *URMService) DeleteUserResourceMapping(ctx context.Context, resourceID i
}

for _, urm := range urms {
orgID, err := s.orgService.FindResourceOrganizationID(ctx, urm.ResourceType, urm.ResourceID)
orgID, err := s.orgIDResolver.FindResourceOrganizationID(ctx, urm.ResourceType, urm.ResourceID)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions authorizer/urm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (s *OrgService) FindResourceOrganizationID(ctx context.Context, rt influxdb
func TestURMService_FindUserResourceMappings(t *testing.T) {
type fields struct {
UserResourceMappingService influxdb.UserResourceMappingService
OrgService authorizer.OrganizationService
OrgService authorizer.OrgIDResolver
}
type args struct {
permission influxdb.Permission
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestURMService_FindUserResourceMappings(t *testing.T) {
func TestURMService_WriteUserResourceMapping(t *testing.T) {
type fields struct {
UserResourceMappingService influxdb.UserResourceMappingService
OrgService authorizer.OrganizationService
OrgService authorizer.OrgIDResolver
}
type args struct {
permission influxdb.Permission
Expand Down
22 changes: 21 additions & 1 deletion cmd/influxd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
iqlquery "github.com/influxdata/influxdb/v2/influxql/query"
"github.com/influxdata/influxdb/v2/inmem"
"github.com/influxdata/influxdb/v2/internal/fs"
"github.com/influxdata/influxdb/v2/internal/resource"
"github.com/influxdata/influxdb/v2/kit/cli"
"github.com/influxdata/influxdb/v2/kit/feature"
overrideflagger "github.com/influxdata/influxdb/v2/kit/feature/override"
Expand Down Expand Up @@ -1129,6 +1130,25 @@ func (m *Launcher) run(ctx context.Context) (err error) {
onboardSvc = tenant.NewOnboardingMetrics(m.reg, onboardSvc, metric.WithSuffix("new")) // with metrics
onboardSvc = tenant.NewOnboardingLogger(m.log.With(zap.String("handler", "onboard")), onboardSvc) // with logging

// orgIDResolver is a deprecated type which combines the lookups
// of multiple resources into one type, used to resolve the resources
// associated org ID. It is a stop-gap while we move this behaviour
// off of *kv.Service to aid in reducing the coupling on this type.
orgIDResolver := &resource.OrgIDResolver{
AuthorizationFinder: authSvc,
BucketFinder: ts.BucketService,
OrganizationFinder: ts.OrganizationService,
DashboardFinder: dashboardSvc,
SourceFinder: sourceSvc,
TaskFinder: taskSvc,
TelegrafConfigFinder: telegrafSvc,
VariableFinder: variableSvc,
TargetFinder: scraperTargetSvc,
CheckFinder: checkSvc,
NotificationEndpointFinder: notificationEndpointStore,
NotificationRuleFinder: notificationRuleSvc,
}

m.apibackend = &http.APIBackend{
AssetsPath: m.assetsPath,
HTTPErrorHandler: kithttp.ErrorHandler(0),
Expand Down Expand Up @@ -1177,7 +1197,7 @@ func (m *Launcher) run(ctx context.Context) (err error) {
SecretService: secretSvc,
LookupService: lookupSvc,
DocumentService: m.kvService,
OrgLookupService: m.kvService,
OrgLookupService: orgIDResolver,
WriteEventRecorder: infprom.NewEventRecorder("write"),
QueryEventRecorder: infprom.NewEventRecorder("query"),
Flagger: m.flagger,
Expand Down
2 changes: 1 addition & 1 deletion http/api_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ type APIBackend struct {
SecretService influxdb.SecretService
LookupService influxdb.LookupService
ChronografService *server.Service
OrgLookupService authorizer.OrganizationService
OrgLookupService authorizer.OrgIDResolver
DocumentService influxdb.DocumentService
NotificationRuleStore influxdb.NotificationRuleStore
NotificationEndpointService influxdb.NotificationEndpointService
Expand Down
9 changes: 8 additions & 1 deletion http/document_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,14 @@ func setup(t *testing.T) (func(auth influxdb.Authorizer) *httptest.Server, func(
if err := ds.CreateDocument(ctx, adoc); err != nil {
panic(err)
}

// Organizations are needed only for creation.
// Need to cleanup for comparison later.
adoc.Organizations = nil
backend := NewMockDocumentBackend(t)
backend.HTTPErrorHandler = http.ErrorHandler(0)
backend.DocumentService = authorizer.NewDocumentService(svc)
backend.LabelService = authorizer.NewLabelServiceWithOrg(svc, svc)
backend.LabelService = authorizer.NewLabelServiceWithOrg(svc, staticOrgIDResolver(org.ID))
serverFn := func(auth influxdb.Authorizer) *httptest.Server {
handler := httpmock.NewAuthMiddlewareHandler(NewDocumentHandler(backend), auth)
return httptest.NewServer(handler)
Expand Down Expand Up @@ -751,3 +752,9 @@ func DeleteLabel(t *testing.T) {
}
})
}

type staticOrgIDResolver influxdb.ID

func (s staticOrgIDResolver) FindResourceOrganizationID(ctx context.Context, rt influxdb.ResourceType, id influxdb.ID) (influxdb.ID, error) {
return (influxdb.ID)(s), nil
}
194 changes: 194 additions & 0 deletions internal/resource/org_id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
package resource

import (
"context"
"fmt"

"github.com/influxdata/influxdb/v2"
)

// OrgIDResolver is a type which combines multiple resource services
// in order to resolve the resources associated org ID.
// Ideally you do not need to use this type, it is mostly a stop-gap
// while we migrate responsibilities off of *kv.Service.
// Consider it deprecated.
type OrgIDResolver struct {
AuthorizationFinder interface {
FindAuthorizationByID(context.Context, influxdb.ID) (*influxdb.Authorization, error)
}
BucketFinder interface {
FindBucketByID(context.Context, influxdb.ID) (*influxdb.Bucket, error)
}
OrganizationFinder interface {
FindOrganizationByID(context.Context, influxdb.ID) (*influxdb.Organization, error)
}
DashboardFinder interface {
FindDashboardByID(context.Context, influxdb.ID) (*influxdb.Dashboard, error)
}
SourceFinder interface {
FindSourceByID(context.Context, influxdb.ID) (*influxdb.Source, error)
}
TaskFinder interface {
FindTaskByID(context.Context, influxdb.ID) (*influxdb.Task, error)
}
TelegrafConfigFinder interface {
FindTelegrafConfigByID(context.Context, influxdb.ID) (*influxdb.TelegrafConfig, error)
}
VariableFinder interface {
FindVariableByID(context.Context, influxdb.ID) (*influxdb.Variable, error)
}
TargetFinder interface {
GetTargetByID(context.Context, influxdb.ID) (*influxdb.ScraperTarget, error)
}
CheckFinder interface {
FindCheckByID(context.Context, influxdb.ID) (influxdb.Check, error)
}
NotificationEndpointFinder interface {
FindNotificationEndpointByID(context.Context, influxdb.ID) (influxdb.NotificationEndpoint, error)
}
NotificationRuleFinder interface {
FindNotificationRuleByID(context.Context, influxdb.ID) (influxdb.NotificationRule, error)
}
}

// FindResourceOrganizationID is used to find the organization that a resource belongs to five the id of a resource and a resource type.
func (o *OrgIDResolver) FindResourceOrganizationID(ctx context.Context, rt influxdb.ResourceType, id influxdb.ID) (influxdb.ID, error) {
switch rt {
case influxdb.AuthorizationsResourceType:
if o.AuthorizationFinder == nil {
break
}

r, err := o.AuthorizationFinder.FindAuthorizationByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrgID, nil
case influxdb.BucketsResourceType:
if o.BucketFinder == nil {
break
}

r, err := o.BucketFinder.FindBucketByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrgID, nil
case influxdb.OrgsResourceType:
if o.OrganizationFinder == nil {
break
}

r, err := o.OrganizationFinder.FindOrganizationByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.ID, nil
case influxdb.DashboardsResourceType:
if o.DashboardFinder == nil {
break
}

r, err := o.DashboardFinder.FindDashboardByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrganizationID, nil
case influxdb.SourcesResourceType:
if o.SourceFinder == nil {
break
}

r, err := o.SourceFinder.FindSourceByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrganizationID, nil
case influxdb.TasksResourceType:
if o.TaskFinder == nil {
break
}

r, err := o.TaskFinder.FindTaskByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrganizationID, nil
case influxdb.TelegrafsResourceType:
if o.TelegrafConfigFinder == nil {
break
}

r, err := o.TelegrafConfigFinder.FindTelegrafConfigByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrgID, nil
case influxdb.VariablesResourceType:
if o.VariableFinder == nil {
break
}

r, err := o.VariableFinder.FindVariableByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrganizationID, nil
case influxdb.ScraperResourceType:
if o.TargetFinder == nil {
break
}

r, err := o.TargetFinder.GetTargetByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.OrgID, nil
case influxdb.ChecksResourceType:
if o.CheckFinder == nil {
break
}

r, err := o.CheckFinder.FindCheckByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.GetOrgID(), nil
case influxdb.NotificationEndpointResourceType:
if o.NotificationEndpointFinder == nil {
break
}

r, err := o.NotificationEndpointFinder.FindNotificationEndpointByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.GetOrgID(), nil
case influxdb.NotificationRuleResourceType:
if o.NotificationRuleFinder == nil {
break
}

r, err := o.NotificationRuleFinder.FindNotificationRuleByID(ctx, id)
if err != nil {
return influxdb.InvalidID(), err
}

return r.GetOrgID(), nil
}

return influxdb.InvalidID(), &influxdb.Error{
Msg: fmt.Sprintf("unsupported resource type %s", rt),
}
}
Loading

0 comments on commit 17c9f8e

Please sign in to comment.