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.

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

fix(dbrpv2): lookup by org name #18331

Merged
merged 1 commit into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

1. [18279](https://github.com/influxdata/influxdb/pull/18279): Make all pkg applications stateful via stacks

### Bug Fixes

1. [18331](https://github.com/influxdata/influxdb/pull/18331): Support organization name in addition to ID in DBRP operations

## v2.0.0-beta.11 [2020-05-26]

### Features
Expand Down
12 changes: 10 additions & 2 deletions dbrp/http_client_dbrp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

func setup(t *testing.T) (*dbrp.Client, func()) {
t.Helper()
svc := &mock.DBRPMappingServiceV2{
dbrpSvc := &mock.DBRPMappingServiceV2{
CreateFn: func(ctx context.Context, dbrp *influxdb.DBRPMappingV2) error {
dbrp.ID = 1
return nil
Expand All @@ -34,7 +34,15 @@ func setup(t *testing.T) (*dbrp.Client, func()) {
return []*influxdb.DBRPMappingV2{}, 0, nil
},
}
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc))
orgSvc := &mock.OrganizationService{
FindOrganizationF: func(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) {
return &influxdb.Organization{
ID: *filter.ID,
Name: "org",
}, nil
},
}
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), dbrpSvc, orgSvc))
client, err := httpc.New(httpc.WithAddr(server.URL), httpc.WithStatusFn(http.CheckError))
if err != nil {
t.Fatal(err)
Expand Down
48 changes: 39 additions & 9 deletions dbrp/http_server_dbrp.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ type Handler struct {
api *kithttp.API
log *zap.Logger
dbrpSvc influxdb.DBRPMappingServiceV2
orgSvc influxdb.OrganizationService
}

// NewHTTPHandler constructs a new http server.
func NewHTTPHandler(log *zap.Logger, dbrpSvc influxdb.DBRPMappingServiceV2) *Handler {
func NewHTTPHandler(log *zap.Logger, dbrpSvc influxdb.DBRPMappingServiceV2, orgSvc influxdb.OrganizationService) *Handler {
h := &Handler{
api: kithttp.NewAPI(kithttp.WithLog(log)),
log: log,
dbrpSvc: dbrpSvc,
orgSvc: orgSvc,
}

r := chi.NewRouter()
Expand Down Expand Up @@ -57,6 +59,7 @@ type createDBRPRequest struct {
Database string `json:"database"`
RetentionPolicy string `json:"retention_policy"`
Default bool `json:"default"`
Org string `json:"organization"`
OrganizationID influxdb.ID `json:"organization_id"`
BucketID influxdb.ID `json:"bucket_id"`
}
Expand All @@ -73,6 +76,21 @@ func (h *Handler) handlePostDBRP(w http.ResponseWriter, r *http.Request) {
return
}

if !req.OrganizationID.Valid() {
if req.Org == "" {
h.api.Err(w, r, influxdb.ErrInvalidID)
return
}
org, err := h.orgSvc.FindOrganization(r.Context(), influxdb.OrganizationFilter{
Name: &req.Org,
})
if err != nil {
h.api.Err(w, r, influxdb.ErrOrgNotFound)
return
}
req.OrganizationID = org.ID
}

dbrp := &influxdb.DBRPMappingV2{
Database: req.Database,
RetentionPolicy: req.RetentionPolicy,
Expand All @@ -92,7 +110,7 @@ type getDBRPsResponse struct {
}

func (h *Handler) handleGetDBRPs(w http.ResponseWriter, r *http.Request) {
filter, err := getFilterFromHTTPRequest(r)
filter, err := h.getFilterFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -129,7 +147,7 @@ func (h *Handler) handleGetDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -168,7 +186,7 @@ func (h *Handler) handlePatchDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -226,7 +244,7 @@ func (h *Handler) handleDeleteDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand All @@ -240,9 +258,9 @@ func (h *Handler) handleDeleteDBRP(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
}

func getFilterFromHTTPRequest(r *http.Request) (f influxdb.DBRPMappingFilterV2, err error) {
func (h *Handler) getFilterFromHTTPRequest(r *http.Request) (f influxdb.DBRPMappingFilterV2, err error) {
// Always provide OrgID.
f.OrgID, err = mustGetOrgIDFromHTTPRequest(r)
f.OrgID, err = h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
return f, err
}
Expand Down Expand Up @@ -289,13 +307,25 @@ func getIDFromHTTPRequest(r *http.Request, key string) (*influxdb.ID, error) {
return &id, nil
}

func mustGetOrgIDFromHTTPRequest(r *http.Request) (*influxdb.ID, error) {
// mustGetOrgIDFromHTTPRequest returns the org ID parameter from the request, falling
// back to looking up the org ID by org name if the ID parameter is not present.
func (h *Handler) mustGetOrgIDFromHTTPRequest(r *http.Request) (*influxdb.ID, error) {
orgID, err := getIDFromHTTPRequest(r, "orgID")
if err != nil {
return nil, err
}
if orgID == nil {
return nil, influxdb.ErrOrgNotFound
name := r.URL.Query().Get("org")
if name == "" {
return nil, influxdb.ErrOrgNotFound
}
org, err := h.orgSvc.FindOrganization(r.Context(), influxdb.OrganizationFilter{
Name: &name,
})
if err != nil {
return nil, influxdb.ErrOrgNotFound
}
orgID = &org.ID
}
return orgID, nil
}
Expand Down
78 changes: 76 additions & 2 deletions dbrp/http_server_dbrp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,25 @@ func initHttpService(t *testing.T) (influxdb.DBRPMappingServiceV2, *httptest.Ser
t.Helper()
ctx := context.Background()
bucketSvc := mock.NewBucketService()
orgSvc := &mock.OrganizationService{
FindOrganizationF: func(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) {
if filter.Name == nil || *filter.Name != "org" {
return nil, errors.New("not found")
}
return &influxdb.Organization{
Name: "org",
ID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
}, nil
},
}

s := inmem.NewKVStore()
svc, err := dbrp.NewService(ctx, bucketSvc, s)
if err != nil {
t.Fatal(err)
}

server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc))
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc, orgSvc))
return svc, server, func() {
server.Close()
}
Expand All @@ -52,6 +63,19 @@ func Test_handlePostDBRP(t *testing.T) {
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
},
},
{
Name: "Create valid dbrp by org name",
Input: strings.NewReader(`{
"bucket_id": "5555f7ed2a035555",
"organization": "org",
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Expand All @@ -72,6 +96,17 @@ func Test_handlePostDBRP(t *testing.T) {
Err: influxdb.ErrInvalidID.Err,
},
},
{
Name: "Create with invalid org name",
Input: strings.NewReader(`{
"bucket_id": "5555f7ed2a035555",
"organization": "invalid",
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedErr: influxdb.ErrOrgNotFound,
},
}

for _, tt := range table {
Expand Down Expand Up @@ -106,7 +141,7 @@ func Test_handlePostDBRP(t *testing.T) {
}

if !dbrp.ID.Valid() {
t.Fatalf("expected invalid id, got an invalid one %s", dbrp.ID.String())
t.Fatalf("expected valid id, got an invalid one %s", dbrp.ID.String())
}

if dbrp.OrganizationID != tt.ExpectedDBRP.OrganizationID {
Expand Down Expand Up @@ -186,6 +221,20 @@ func Test_handleGetDBRPs(t *testing.T) {
},
},
},
{
Name: "org name",
QueryParams: "org=org",
ExpectedDBRPs: []influxdb.DBRPMappingV2{
{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
BucketID: influxdbtesting.MustIDBase16("5555f7ed2a035555"),
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Database: "mydb",
RetentionPolicy: "autogen",
Default: true,
},
},
},
}

ctx := context.Background()
Expand Down Expand Up @@ -262,6 +311,22 @@ func Test_handlePatchDBRP(t *testing.T) {
Input: strings.NewReader(`{
"retention_policy": "updaterp",
"database": "wont_change"
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
BucketID: influxdbtesting.MustIDBase16("5555f7ed2a035555"),
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Database: "mydb",
RetentionPolicy: "updaterp",
Default: true,
},
},
{
Name: "happy path update by org name",
URLSuffix: "/1111111111111111?org=org",
Input: strings.NewReader(`{
"retention_policy": "updaterp",
"database": "wont_change"
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
Expand Down Expand Up @@ -368,6 +433,10 @@ func Test_handleDeleteDBRP(t *testing.T) {
Name: "delete",
URLSuffix: "/1111111111111111?orgID=059af7ed2a034000",
},
{
Name: "delete by org name",
URLSuffix: "/1111111111111111?org=org",
},
{
Name: "invalid org",
URLSuffix: "/1111111111111111?orgID=invalid",
Expand All @@ -376,6 +445,11 @@ func Test_handleDeleteDBRP(t *testing.T) {
Msg: "invalid ID",
},
},
{
Name: "invalid org name",
URLSuffix: "/1111111111111111?org=invalid",
ExpectedErr: influxdb.ErrOrgNotFound,
},
{
Name: "no org",
URLSuffix: "/1111111111111111",
Expand Down
2 changes: 1 addition & 1 deletion http/api_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
backupBackend.BackupService = authorizer.NewBackupService(backupBackend.BackupService)
h.Mount(prefixBackup, NewBackupHandler(backupBackend))

h.Mount(dbrp.PrefixDBRP, dbrp.NewHTTPHandler(b.Logger, b.DBRPService))
h.Mount(dbrp.PrefixDBRP, dbrp.NewHTTPHandler(b.Logger, b.DBRPService, b.OrganizationService))

writeBackend := NewWriteBackend(b.Logger.With(zap.String("handler", "write")), b)
h.Mount(prefixWrite, NewWriteHandler(b.Logger, writeBackend,
Expand Down
4 changes: 4 additions & 0 deletions http/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11683,6 +11683,7 @@ components:
DBRP:
required:
- orgID
- org
- bucketID
- database
- retention_policy
Expand All @@ -11694,6 +11695,9 @@ components:
orgID:
type: string
description: the organization ID that owns this mapping.
org:
type: string
description: the organization that owns this mapping.
bucketID:
type: string
description: the bucket ID used as target for the translation.
Expand Down