From 2525a56127f1c0bc9d8a35ba0245e0b8d0fa6146 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Tue, 16 Jun 2020 20:12:37 -0700 Subject: [PATCH] feat(pkger): extend stacks with update functionality references: #18548 --- cmd/influx/pkg_test.go | 8 + cmd/influxd/launcher/pkger_test.go | 73 +++++- http/swagger.yml | 175 +++++++------ pkger/http_remote_service.go | 91 +++++-- pkger/http_server.go | 155 ++++++++---- pkger/http_server_test.go | 391 +++++++++++++++++++++++++---- pkger/service.go | 40 +++ pkger/service_auth.go | 26 ++ pkger/service_logging.go | 39 +++ pkger/service_metrics.go | 12 + pkger/service_test.go | 91 +++++++ pkger/service_tracing.go | 12 + pkger/store.go | 2 +- 13 files changed, 921 insertions(+), 194 deletions(-) diff --git a/cmd/influx/pkg_test.go b/cmd/influx/pkg_test.go index 5b3327deed5..76d79b9ddac 100644 --- a/cmd/influx/pkg_test.go +++ b/cmd/influx/pkg_test.go @@ -733,6 +733,14 @@ func (f *fakePkgSVC) ExportStack(ctx context.Context, orgID, stackID influxdb.ID panic("not implemented") } +func (f *fakePkgSVC) ReadStack(ctx context.Context, id influxdb.ID) (pkger.Stack, error) { + panic("not implemented") +} + +func (f *fakePkgSVC) UpdateStack(ctx context.Context, upd pkger.StackUpdate) (pkger.Stack, error) { + panic("not implemented") +} + func (f *fakePkgSVC) CreatePkg(ctx context.Context, setters ...pkger.CreatePkgSetFn) (*pkger.Pkg, error) { if f.createFn != nil { return f.createFn(ctx, setters...) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index b2a2a6d6372..f1493206086 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -56,11 +56,15 @@ func TestLauncher_Pkger(t *testing.T) { newStack, err := svc.InitStack(ctx, l.User.ID, stack) require.NoError(t, err) - assert.NotZero(t, newStack.OrgID) + assert.Equal(t, l.Org.ID, newStack.OrgID) assert.Equal(t, stack.Name, newStack.Name) assert.Equal(t, stack.Description, newStack.Description) - assert.Equal(t, stack.URLs, newStack.URLs) - assert.NotNil(t, newStack.Resources) + assert.NotNil(t, newStack.Resources, "failed to match stack resorces") + expectedURLs := stack.URLs + if expectedURLs == nil { + expectedURLs = []string{} + } + assert.Equal(t, expectedURLs, newStack.URLs, "failed to match stack URLs") assert.NotZero(t, newStack.CRUDLog) return newStack, func() { @@ -371,6 +375,65 @@ func TestLauncher_Pkger(t *testing.T) { }) }) + t.Run("read a stack", func(t *testing.T) { + stacks, err := svc.ListStacks(ctx, l.Org.ID, pkger.ListFilter{}) + require.NoError(t, err) + require.Empty(t, stacks) + + newStack1, cleanup1 := newStackFn(t, pkger.Stack{ + Name: "first stack", + }) + defer cleanup1() + + newStack2, cleanup2 := newStackFn(t, pkger.Stack{ + Name: "second stack", + }) + defer cleanup2() + + actual, err := svc.ReadStack(ctx, newStack1.ID) + require.NoError(t, err) + assert.Equal(t, newStack1, actual) + + actual, err = svc.ReadStack(ctx, newStack2.ID) + require.NoError(t, err) + assert.Equal(t, newStack2, actual) + + _, err = svc.ReadStack(ctx, influxdb.ID(9000)) + require.Equal(t, influxdb.ENotFound, influxdb.ErrorCode(err)) + }) + + t.Run("updating a stack", func(t *testing.T) { + stack, cleanup := newStackFn(t, pkger.Stack{ + OrgID: l.Org.ID, + Name: "first name", + Description: "first desc", + URLs: []string{}, + }) + defer cleanup() + + assertStack := func(t *testing.T, st pkger.Stack) { + t.Helper() + assert.Equal(t, stack.ID, st.ID) + assert.Equal(t, "2nd name", st.Name) + assert.Equal(t, "2nd desc", st.Description) + assert.Equal(t, []string{"http://example.com"}, st.URLs) + assert.True(t, st.UpdatedAt.After(stack.UpdatedAt)) + } + + updStack, err := svc.UpdateStack(ctx, pkger.StackUpdate{ + ID: stack.ID, + Name: strPtr("2nd name"), + Description: strPtr("2nd desc"), + URLs: []string{"http://example.com"}, + }) + require.NoError(t, err) + assertStack(t, updStack) + + readStack, err := svc.ReadStack(ctx, stack.ID) + require.NoError(t, err) + assertStack(t, readStack) + }) + t.Run("apply with only a stackID succeeds when stack has URLs", func(t *testing.T) { svr := httptest.NewServer(nethttp.HandlerFunc(func(w nethttp.ResponseWriter, r *nethttp.Request) { pkg := newPkg(newBucketObject("bucket-0", "", "")) @@ -3664,3 +3727,7 @@ func sortLabels(labels []pkger.SummaryLabel) { return labels[i].Name < labels[j].Name }) } + +func strPtr(s string) *string { + return &s +} diff --git a/http/swagger.yml b/http/swagger.yml index 11d58ab40be..39c0eb3de2c 100644 --- a/http/swagger.yml +++ b/http/swagger.yml @@ -4725,52 +4725,7 @@ paths: stacks: type: array items: - type: object - properties: - id: - type: string - orgID: - type: string - name: - type: string - description: - type: string - sources: - type: array - items: - type: string - urls: - type: array - items: - type: string - createdAt: - type: string - format: date-time - readOnly: true - updatedAt: - type: string - format: date-time - readOnly: true - resources: - type: object - properties: - apiVersion: - type: string - resourceID: - type: string - kind: - type: string - pkgName: - type: string - associations: - type: array - items: - type: object - properties: - kind: - type: string - pkgName: - type: string + $ref: "#/components/schemas/Stack" default: description: Unexpected error content: @@ -4781,9 +4736,9 @@ paths: operationId: CreateStack tags: - InfluxPackages - summary: Create a new Influx package + summary: Create a new stack requestBody: - description: Influx package to create. + description: Influx stack to create. required: true content: application/json: @@ -4806,28 +4761,7 @@ paths: content: application/json: schema: - type: object - properties: - id: - type: string - orgID: - type: string - name: - type: string - description: - type: string - urls: - type: array - items: - type: string - createdAt: - type: string - format: date-time - readOnly: true - updatedAt: - type: string - format: date-time - readOnly: true + $ref: "#/components/schemas/Stack" default: description: Unexpected error content: @@ -4835,6 +4769,58 @@ paths: schema: $ref: "#/components/schemas/Error" /packages/stacks/{stack_id}: + get: + operationId: ReadStack + tags: + - InfluxPackages + summary: Grab a stack by its ID + responses: + "200": + description: Read an influx stack by ID + content: + application/json: + schema: + $ref: "#/components/schemas/Stack" + default: + description: Unexpected error + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + post: + operationId: UpdateStack + tags: + - InfluxPackages + summary: Update a an Influx Stack + requestBody: + description: Influx stack to update. + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + description: + type: string + urls: + type: array + items: + type: string + responses: + "200": + description: Influx stack updated + content: + application/json: + schema: + $ref: "#/components/schemas/Stack" + default: + description: Unexpected error + content: + application/json: + schema: + $ref: "#/components/schemas/Error" delete: operationId: DeleteStack tags: @@ -8395,6 +8381,55 @@ components: type: integer properties: # field name is properties $ref: "#/components/schemas/ViewProperties" + Stack: + type: object + properties: + id: + type: string + orgID: + type: string + name: + type: string + description: + type: string + sources: + type: array + items: + type: string + resources: + type: array + items: + type: object + properties: + apiVersion: + type: string + resourceID: + type: string + kind: + type: string + pkgName: + type: string + associations: + type: array + items: + type: object + properties: + kind: + type: string + pkgName: + type: string + urls: + type: array + items: + type: string + createdAt: + type: string + format: date-time + readOnly: true + updatedAt: + type: string + format: date-time + readOnly: true Runs: type: object properties: diff --git a/pkger/http_remote_service.go b/pkger/http_remote_service.go index 1a5797bf1dc..fe1eb7439ef 100644 --- a/pkger/http_remote_service.go +++ b/pkger/http_remote_service.go @@ -23,7 +23,7 @@ func (s *HTTPRemoteService) InitStack(ctx context.Context, userID influxdb.ID, s URLs: stack.URLs, } - var respBody RespCreateStack + var respBody RespStack err := s.Client. PostJSON(reqBody, RoutePrefix, "/stacks"). DecodeJSON(&respBody). @@ -32,27 +32,7 @@ func (s *HTTPRemoteService) InitStack(ctx context.Context, userID influxdb.ID, s return Stack{}, err } - newStack := Stack{ - Name: respBody.Name, - Description: respBody.Description, - URLs: respBody.URLs, - Resources: make([]StackResource, 0), - CRUDLog: respBody.CRUDLog, - } - - id, err := influxdb.IDFromString(respBody.ID) - if err != nil { - return Stack{}, err - } - newStack.ID = *id - - orgID, err := influxdb.IDFromString(respBody.OrgID) - if err != nil { - return Stack{}, err - } - newStack.OrgID = *orgID - - return newStack, nil + return convertRespStackToStack(respBody) } func (s *HTTPRemoteService) DeleteStack(ctx context.Context, identifiers struct{ OrgID, UserID, StackID influxdb.ID }) error { @@ -104,7 +84,47 @@ func (s *HTTPRemoteService) ListStacks(ctx context.Context, orgID influxdb.ID, f if err != nil { return nil, err } - return resp.Stacks, nil + + out := make([]Stack, 0, len(resp.Stacks)) + for _, st := range resp.Stacks { + stack, err := convertRespStackToStack(st) + if err != nil { + continue + } + out = append(out, stack) + } + return out, nil +} + +func (s *HTTPRemoteService) ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) { + var respBody RespStack + err := s.Client. + Get(RoutePrefix, "/stacks", id.String()). + DecodeJSON(&respBody). + Do(ctx) + if err != nil { + return Stack{}, err + } + return convertRespStackToStack(respBody) +} + +func (s *HTTPRemoteService) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) { + reqBody := ReqUpdateStack{ + Name: upd.Name, + Description: upd.Description, + URLs: upd.URLs, + } + + var respBody RespStack + err := s.Client. + PutJSON(reqBody, RoutePrefix, "/stacks", upd.ID.String()). + DecodeJSON(&respBody). + Do(ctx) + if err != nil { + return Stack{}, err + } + + return convertRespStackToStack(respBody) } // CreatePkg will produce a pkg from the parameters provided. @@ -215,3 +235,28 @@ func (s *HTTPRemoteService) apply(ctx context.Context, orgID influxdb.ID, dryRun return impact, NewParseError(resp.Errors...) } + +func convertRespStackToStack(respStack RespStack) (Stack, error) { + newStack := Stack{ + Name: respStack.Name, + Description: respStack.Description, + Sources: respStack.Sources, + URLs: respStack.URLs, + Resources: respStack.Resources, + CRUDLog: respStack.CRUDLog, + } + + id, err := influxdb.IDFromString(respStack.ID) + if err != nil { + return Stack{}, err + } + newStack.ID = *id + + orgID, err := influxdb.IDFromString(respStack.OrgID) + if err != nil { + return Stack{}, err + } + newStack.OrgID = *orgID + + return newStack, nil +} diff --git a/pkger/http_server.go b/pkger/http_server.go index 754ef1ab35e..6aeef99dce3 100644 --- a/pkger/http_server.go +++ b/pkger/http_server.go @@ -38,20 +38,25 @@ func NewHTTPServer(log *zap.Logger, svc SVC) *HTTPServer { svc: svc, } + exportAllowContentTypes := middleware.AllowContentType("text/yml", "application/x-yaml", "application/json") + setJSONContentType := middleware.SetHeader("Content-Type", "application/json; charset=utf-8") + r := chi.NewRouter() { - r.With(middleware.AllowContentType("text/yml", "application/x-yaml", "application/json")). - Post("/", svr.createPkg) - - r.With(middleware.SetHeader("Content-Type", "application/json; charset=utf-8")). - Post("/apply", svr.applyPkg) + r.With(exportAllowContentTypes).Post("/", svr.createPkg) + r.With(setJSONContentType).Post("/apply", svr.applyPkg) r.Route("/stacks", func(r chi.Router) { r.Post("/", svr.createStack) r.Get("/", svr.listStacks) - r.Delete("/{stack_id}", svr.deleteStack) - r.With(middleware.AllowContentType("text/yml", "application/x-yaml", "application/json")). - Get("/{stack_id}/export", svr.exportStack) + + r.Route("/{stack_id}", func(r chi.Router) { + r.Get("/", svr.readStack) + r.Delete("/", svr.deleteStack) + r.Put("/", svr.updateStack) + r.With(exportAllowContentTypes).Get("/export", svr.exportStack) + }) + }) } @@ -64,9 +69,21 @@ func (s *HTTPServer) Prefix() string { return RoutePrefix } +// RespStack is the response body for a stack. +type RespStack struct { + ID string `json:"id"` + OrgID string `json:"orgID"` + Name string `json:"name"` + Description string `json:"description"` + Resources []StackResource `json:"resources"` + Sources []string `json:"sources"` + URLs []string `json:"urls"` + influxdb.CRUDLog +} + // RespListStacks is the HTTP response for a stack list call. type RespListStacks struct { - Stacks []Stack `json:"stacks"` + Stacks []RespStack `json:"stacks"` } func (s *HTTPServer) listStacks(w http.ResponseWriter, r *http.Request) { @@ -118,8 +135,13 @@ func (s *HTTPServer) listStacks(w http.ResponseWriter, r *http.Request) { stacks = []Stack{} } + out := make([]RespStack, 0, len(stacks)) + for _, st := range stacks { + out = append(out, convertStackToRespStack(st)) + } + s.api.Respond(w, r, http.StatusOK, RespListStacks{ - Stacks: stacks, + Stacks: out, }) } @@ -157,16 +179,6 @@ func (r *ReqCreateStack) orgID() influxdb.ID { return *orgID } -// RespCreateStack is the response body for the create stack call. -type RespCreateStack struct { - ID string `json:"id"` - OrgID string `json:"orgID"` - Name string `json:"name"` - Description string `json:"description"` - URLs []string `json:"urls"` - influxdb.CRUDLog -} - func (s *HTTPServer) createStack(w http.ResponseWriter, r *http.Request) { var reqBody ReqCreateStack if err := s.api.DecodeJSON(r.Body, &reqBody); err != nil { @@ -192,14 +204,7 @@ func (s *HTTPServer) createStack(w http.ResponseWriter, r *http.Request) { return } - s.api.Respond(w, r, http.StatusCreated, RespCreateStack{ - ID: stack.ID.String(), - OrgID: stack.OrgID.String(), - Name: stack.Name, - Description: stack.Description, - URLs: stack.URLs, - CRUDLog: stack.CRUDLog, - }) + s.api.Respond(w, r, http.StatusCreated, convertStackToRespStack(stack)) } func (s *HTTPServer) deleteStack(w http.ResponseWriter, r *http.Request) { @@ -209,13 +214,9 @@ func (s *HTTPServer) deleteStack(w http.ResponseWriter, r *http.Request) { return } - stackID, err := influxdb.IDFromString(chi.URLParam(r, "stack_id")) + stackID, err := stackIDFromReq(r) if err != nil { - s.api.Err(w, r, &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "the stack id provided in the path was invalid", - Err: err, - }) + s.api.Err(w, r, err) return } @@ -229,7 +230,7 @@ func (s *HTTPServer) deleteStack(w http.ResponseWriter, r *http.Request) { err = s.svc.DeleteStack(r.Context(), struct{ OrgID, UserID, StackID influxdb.ID }{ OrgID: orgID, UserID: userID, - StackID: *stackID, + StackID: stackID, }) if err != nil { s.api.Err(w, r, err) @@ -246,17 +247,13 @@ func (s *HTTPServer) exportStack(w http.ResponseWriter, r *http.Request) { return } - stackID, err := influxdb.IDFromString(chi.URLParam(r, "stack_id")) + stackID, err := stackIDFromReq(r) if err != nil { - s.api.Err(w, r, &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "the stack id provided in the path was invalid", - Err: err, - }) + s.api.Err(w, r, err) return } - pkg, err := s.svc.ExportStack(r.Context(), orgID, *stackID) + pkg, err := s.svc.ExportStack(r.Context(), orgID, stackID) if err != nil { s.api.Err(w, r, err) return @@ -280,6 +277,67 @@ func (s *HTTPServer) exportStack(w http.ResponseWriter, r *http.Request) { s.api.Write(w, http.StatusOK, b) } +func (s *HTTPServer) readStack(w http.ResponseWriter, r *http.Request) { + stackID, err := stackIDFromReq(r) + if err != nil { + s.api.Err(w, r, err) + return + } + + stack, err := s.svc.ReadStack(r.Context(), stackID) + if err != nil { + s.api.Err(w, r, err) + return + } + + s.api.Respond(w, r, http.StatusOK, convertStackToRespStack(stack)) +} + +type ReqUpdateStack struct { + Name *string `json:"name"` + Description *string `json:"description"` + URLs []string `json:"urls"` +} + +func (s *HTTPServer) updateStack(w http.ResponseWriter, r *http.Request) { + var req ReqUpdateStack + if err := s.api.DecodeJSON(r.Body, &req); err != nil { + s.api.Err(w, r, err) + return + } + + stackID, err := stackIDFromReq(r) + if err != nil { + s.api.Err(w, r, err) + return + } + + stack, err := s.svc.UpdateStack(r.Context(), StackUpdate{ + ID: stackID, + Name: req.Name, + Description: req.Description, + URLs: req.URLs, + }) + if err != nil { + s.api.Err(w, r, err) + return + } + + s.api.Respond(w, r, http.StatusOK, convertStackToRespStack(stack)) +} + +func stackIDFromReq(r *http.Request) (influxdb.ID, error) { + stackID, err := influxdb.IDFromString(chi.URLParam(r, "stack_id")) + if err != nil { + return 0, &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: "the stack id provided in the path was invalid", + Err: err, + } + } + return *stackID, nil +} + func getRequiredOrgIDFromQuery(q url.Values) (influxdb.ID, error) { orgIDRaw := q.Get("orgID") if orgIDRaw == "" { @@ -672,3 +730,16 @@ func newDecodeErr(encoding string, err error) *influxdb.Error { Err: err, } } + +func convertStackToRespStack(st Stack) RespStack { + return RespStack{ + ID: st.ID.String(), + OrgID: st.OrgID.String(), + Name: st.Name, + Description: st.Description, + Resources: append([]StackResource{}, st.Resources...), + Sources: append([]string{}, st.Sources...), + URLs: append([]string{}, st.URLs...), + CRUDLog: st.CRUDLog, + } +} diff --git a/pkger/http_server_test.go b/pkger/http_server_test.go index d1708d593d8..5411577cec9 100644 --- a/pkger/http_server_test.go +++ b/pkger/http_server_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -515,7 +516,7 @@ func TestPkgerHTTPServer(t *testing.T) { t.Run("create a stack", func(t *testing.T) { t.Run("should successfully return with valid req body", func(t *testing.T) { svc := &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { stack.ID = 3 stack.CreatedAt = time.Now() stack.UpdatedAt = time.Now() @@ -538,7 +539,7 @@ func TestPkgerHTTPServer(t *testing.T) { Do(svr). ExpectStatus(http.StatusCreated). ExpectBody(func(buf *bytes.Buffer) { - var resp pkger.RespCreateStack + var resp pkger.RespStack decodeBody(t, buf, &resp) assert.NotZero(t, resp.ID) @@ -577,7 +578,7 @@ func TestPkgerHTTPServer(t *testing.T) { name: "translates svc conflict error", reqBody: pkger.ReqCreateStack{OrgID: influxdb.ID(3).String()}, svc: &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { return pkger.Stack{}, &influxdb.Error{Code: influxdb.EConflict} }, }, @@ -587,7 +588,7 @@ func TestPkgerHTTPServer(t *testing.T) { name: "translates svc internal error", reqBody: pkger.ReqCreateStack{OrgID: influxdb.ID(3).String()}, svc: &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { return pkger.Stack{}, &influxdb.Error{Code: influxdb.EInternal} }, }, @@ -600,7 +601,7 @@ func TestPkgerHTTPServer(t *testing.T) { svc := tt.svc if svc == nil { svc = &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { return stack, nil }, } @@ -623,7 +624,7 @@ func TestPkgerHTTPServer(t *testing.T) { t.Run("list a stack", func(t *testing.T) { t.Run("should successfully return with valid req body", func(t *testing.T) { - const expectedOrgID = 3 + const expectedOrgID influxdb.ID = 3 svc := &fakeSVC{ listStacksFn: func(ctx context.Context, orgID influxdb.ID, filter pkger.ListFilter) ([]pkger.Stack, error) { @@ -667,49 +668,64 @@ func TestPkgerHTTPServer(t *testing.T) { tests := []struct { name string queryArgs string - expectedStacks []pkger.Stack + expectedStacks []pkger.RespStack }{ { name: "with org ID that has stacks", - queryArgs: "orgID=" + influxdb.ID(expectedOrgID).String(), - expectedStacks: []pkger.Stack{{ - ID: 1, - OrgID: expectedOrgID, - Name: "stack_1", + queryArgs: "orgID=" + expectedOrgID.String(), + expectedStacks: []pkger.RespStack{{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "stack_1", + Resources: []pkger.StackResource{}, + Sources: []string{}, + URLs: []string{}, }}, }, { name: "with orgID with no stacks", queryArgs: "orgID=" + influxdb.ID(9000).String(), - expectedStacks: []pkger.Stack{}, + expectedStacks: []pkger.RespStack{}, }, { name: "with names", queryArgs: "name=name_stack&name=threeve&orgID=" + influxdb.ID(expectedOrgID).String(), - expectedStacks: []pkger.Stack{ + expectedStacks: []pkger.RespStack{ { - ID: 1, - OrgID: expectedOrgID, - Name: "name_stack", + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "name_stack", + Resources: []pkger.StackResource{}, + Sources: []string{}, + URLs: []string{}, }, { - ID: 2, - OrgID: expectedOrgID, - Name: "threeve", + ID: influxdb.ID(2).String(), + OrgID: expectedOrgID.String(), + Name: "threeve", + Resources: []pkger.StackResource{}, + Sources: []string{}, + URLs: []string{}, }, }, }, { name: "with ids", queryArgs: fmt.Sprintf("stackID=%s&stackID=%s&orgID=%s", influxdb.ID(1), influxdb.ID(2), influxdb.ID(expectedOrgID)), - expectedStacks: []pkger.Stack{ + expectedStacks: []pkger.RespStack{ { - ID: 1, - OrgID: expectedOrgID, + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Resources: []pkger.StackResource{}, + Sources: []string{}, + URLs: []string{}, }, { - ID: 2, - OrgID: expectedOrgID, + ID: influxdb.ID(2).String(), + OrgID: expectedOrgID.String(), + Resources: []pkger.StackResource{}, + Sources: []string{}, + URLs: []string{}, }, }, }, @@ -733,48 +749,297 @@ func TestPkgerHTTPServer(t *testing.T) { t.Run(tt.name, fn) } }) + }) + + t.Run("read a stack", func(t *testing.T) { + t.Run("should successfully return with valid req body", func(t *testing.T) { + const expectedOrgID influxdb.ID = 3 + + tests := []struct { + name string + stub pkger.Stack + expectedStack pkger.RespStack + }{ + { + name: "for stack that has all fields available", + stub: pkger.Stack{ + ID: 1, + OrgID: expectedOrgID, + Name: "name", + Description: "desc", + Sources: []string{"threeve"}, + URLs: []string{"http://example.com"}, + Resources: []pkger.StackResource{ + { + APIVersion: pkger.APIVersion, + ID: 3, + Kind: pkger.KindBucket, + PkgName: "rucketeer", + }, + }, + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "name", + Description: "desc", + Sources: []string{"threeve"}, + URLs: []string{"http://example.com"}, + Resources: []pkger.StackResource{ + { + APIVersion: pkger.APIVersion, + ID: 3, + Kind: pkger.KindBucket, + PkgName: "rucketeer", + }, + }, + }, + }, + { + name: "for stack that has missing resources urls and sources", + stub: pkger.Stack{ + ID: 1, + OrgID: expectedOrgID, + Name: "name", + Description: "desc", + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "name", + Description: "desc", + Sources: []string{}, + URLs: []string{}, + Resources: []pkger.StackResource{}, + }, + }, + { + name: "for stack that has no set fields", + stub: pkger.Stack{ + ID: 1, + OrgID: expectedOrgID, + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Sources: []string{}, + URLs: []string{}, + Resources: []pkger.StackResource{}, + }, + }, + } + + for _, tt := range tests { + fn := func(t *testing.T) { + svc := &fakeSVC{ + readStackFn: func(ctx context.Context, id influxdb.ID) (pkger.Stack, error) { + return tt.stub, nil + }, + } + pkgHandler := pkger.NewHTTPServer(zap.NewNop(), svc) + svr := newMountedHandler(pkgHandler, 1) + + testttp. + Get(t, "/api/v2/packages/stacks/"+tt.stub.ID.String()). + Do(svr). + ExpectStatus(http.StatusOK). + ExpectBody(func(buf *bytes.Buffer) { + var resp pkger.RespStack + decodeBody(t, buf, &resp) + + assert.Equal(t, tt.expectedStack, resp) + }) + } + + t.Run(tt.name, fn) + } + }) t.Run("error cases", func(t *testing.T) { tests := []struct { name string - reqBody pkger.ReqCreateStack + stackIDPath string expectedStatus int svc pkger.SVC }{ { - name: "bad org id", - reqBody: pkger.ReqCreateStack{ - OrgID: "invalid id", - }, + name: "bad stack id path", + stackIDPath: "badID", expectedStatus: http.StatusBadRequest, }, { - name: "bad url", - reqBody: pkger.ReqCreateStack{ - OrgID: influxdb.ID(3).String(), - URLs: []string{"invalid @% url"}, + name: "stack not found", + stackIDPath: influxdb.ID(1).String(), + svc: &fakeSVC{ + readStackFn: func(ctx context.Context, id influxdb.ID) (pkger.Stack, error) { + return pkger.Stack{}, &influxdb.Error{Code: influxdb.ENotFound} + }, + }, + expectedStatus: http.StatusNotFound, + }, + } + + for _, tt := range tests { + fn := func(t *testing.T) { + svc := tt.svc + if svc == nil { + svc = &fakeSVC{ + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + return stack, nil + }, + } + } + + pkgHandler := pkger.NewHTTPServer(zap.NewNop(), svc) + svr := newMountedHandler(pkgHandler, 1) + + testttp. + Get(t, "/api/v2/packages/stacks/"+tt.stackIDPath). + Headers("Content-Type", "application/json"). + Do(svr). + ExpectStatus(tt.expectedStatus) + } + + t.Run(tt.name, fn) + } + }) + }) + + t.Run("update a stack", func(t *testing.T) { + t.Run("should successfully update with valid req body", func(t *testing.T) { + const expectedOrgID influxdb.ID = 3 + + tests := []struct { + name string + input pkger.ReqUpdateStack + expectedStack pkger.RespStack + }{ + { + name: "update name field", + input: pkger.ReqUpdateStack{ + Name: strPtr("name"), + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "name", + Sources: []string{}, + URLs: []string{}, + Resources: []pkger.StackResource{}, }, - expectedStatus: http.StatusBadRequest, }, { - name: "translates svc conflict error", - reqBody: pkger.ReqCreateStack{OrgID: influxdb.ID(3).String()}, - svc: &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { - return pkger.Stack{}, &influxdb.Error{Code: influxdb.EConflict} - }, + name: "update desc field", + input: pkger.ReqUpdateStack{ + Description: strPtr("desc"), + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Description: "desc", + Sources: []string{}, + URLs: []string{}, + Resources: []pkger.StackResource{}, }, - expectedStatus: http.StatusUnprocessableEntity, }, { - name: "translates svc internal error", - reqBody: pkger.ReqCreateStack{OrgID: influxdb.ID(3).String()}, + name: "update urls field", + input: pkger.ReqUpdateStack{ + URLs: []string{"http://example.com"}, + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Sources: []string{}, + URLs: []string{"http://example.com"}, + Resources: []pkger.StackResource{}, + }, + }, + { + name: "update all fields", + input: pkger.ReqUpdateStack{ + Name: strPtr("name"), + Description: strPtr("desc"), + URLs: []string{"http://example.com"}, + }, + expectedStack: pkger.RespStack{ + ID: influxdb.ID(1).String(), + OrgID: expectedOrgID.String(), + Name: "name", + Description: "desc", + Sources: []string{}, + URLs: []string{"http://example.com"}, + Resources: []pkger.StackResource{}, + }, + }, + } + + for _, tt := range tests { + fn := func(t *testing.T) { + id, err := influxdb.IDFromString(tt.expectedStack.ID) + require.NoError(t, err) + + svc := &fakeSVC{ + updateStackFn: func(ctx context.Context, upd pkger.StackUpdate) (pkger.Stack, error) { + if upd.ID != *id { + return pkger.Stack{}, errors.New("unexpected stack ID: " + upd.ID.String()) + } + st := pkger.Stack{ + ID: *id, + OrgID: expectedOrgID, + } + if upd.Name != nil { + st.Name = *upd.Name + } + if upd.Description != nil { + st.Description = *upd.Description + } + if upd.URLs != nil { + st.URLs = upd.URLs + } + return st, nil + }, + } + pkgHandler := pkger.NewHTTPServer(zap.NewNop(), svc) + svr := newMountedHandler(pkgHandler, 1) + + testttp. + PutJSON(t, "/api/v2/packages/stacks/"+tt.expectedStack.ID, tt.input). + Do(svr). + ExpectStatus(http.StatusOK). + ExpectBody(func(buf *bytes.Buffer) { + var resp pkger.RespStack + decodeBody(t, buf, &resp) + + assert.Equal(t, tt.expectedStack, resp) + }) + } + + t.Run(tt.name, fn) + } + }) + + t.Run("error cases", func(t *testing.T) { + tests := []struct { + name string + stackIDPath string + expectedStatus int + svc pkger.SVC + }{ + { + name: "bad stack id path", + stackIDPath: "badID", + expectedStatus: http.StatusBadRequest, + }, + { + name: "stack not found", + stackIDPath: influxdb.ID(1).String(), svc: &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { - return pkger.Stack{}, &influxdb.Error{Code: influxdb.EInternal} + readStackFn: func(ctx context.Context, id influxdb.ID) (pkger.Stack, error) { + return pkger.Stack{}, &influxdb.Error{Code: influxdb.ENotFound} }, }, - expectedStatus: http.StatusInternalServerError, + expectedStatus: http.StatusNotFound, }, } @@ -783,7 +1048,7 @@ func TestPkgerHTTPServer(t *testing.T) { svc := tt.svc if svc == nil { svc = &fakeSVC{ - initStack: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { + initStackFn: func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { return stack, nil }, } @@ -793,7 +1058,7 @@ func TestPkgerHTTPServer(t *testing.T) { svr := newMountedHandler(pkgHandler, 1) testttp. - PostJSON(t, "/api/v2/packages/stacks", tt.reqBody). + Get(t, "/api/v2/packages/stacks/"+tt.stackIDPath). Headers("Content-Type", "application/json"). Do(svr). ExpectStatus(tt.expectedStatus) @@ -887,19 +1152,21 @@ func decodeBody(t *testing.T, r io.Reader, v interface{}) { } type fakeSVC struct { - initStack func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) - listStacksFn func(ctx context.Context, orgID influxdb.ID, filter pkger.ListFilter) ([]pkger.Stack, error) - dryRunFn func(ctx context.Context, orgID, userID influxdb.ID, opts ...pkger.ApplyOptFn) (pkger.PkgImpactSummary, error) - applyFn func(ctx context.Context, orgID, userID influxdb.ID, opts ...pkger.ApplyOptFn) (pkger.PkgImpactSummary, error) + initStackFn func(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) + listStacksFn func(ctx context.Context, orgID influxdb.ID, filter pkger.ListFilter) ([]pkger.Stack, error) + readStackFn func(ctx context.Context, id influxdb.ID) (pkger.Stack, error) + updateStackFn func(ctx context.Context, upd pkger.StackUpdate) (pkger.Stack, error) + dryRunFn func(ctx context.Context, orgID, userID influxdb.ID, opts ...pkger.ApplyOptFn) (pkger.PkgImpactSummary, error) + applyFn func(ctx context.Context, orgID, userID influxdb.ID, opts ...pkger.ApplyOptFn) (pkger.PkgImpactSummary, error) } var _ pkger.SVC = (*fakeSVC)(nil) func (f *fakeSVC) InitStack(ctx context.Context, userID influxdb.ID, stack pkger.Stack) (pkger.Stack, error) { - if f.initStack == nil { + if f.initStackFn == nil { panic("not implemented") } - return f.initStack(ctx, userID, stack) + return f.initStackFn(ctx, userID, stack) } func (f *fakeSVC) DeleteStack(ctx context.Context, identifiers struct{ OrgID, UserID, StackID influxdb.ID }) error { @@ -917,6 +1184,20 @@ func (f *fakeSVC) ListStacks(ctx context.Context, orgID influxdb.ID, filter pkge return f.listStacksFn(ctx, orgID, filter) } +func (f *fakeSVC) ReadStack(ctx context.Context, id influxdb.ID) (pkger.Stack, error) { + if f.readStackFn != nil { + return f.readStackFn(ctx, id) + } + panic("not implemented") +} + +func (f *fakeSVC) UpdateStack(ctx context.Context, upd pkger.StackUpdate) (pkger.Stack, error) { + if f.updateStackFn != nil { + return f.updateStackFn(ctx, upd) + } + panic("not implemented") +} + func (f *fakeSVC) CreatePkg(ctx context.Context, setters ...pkger.CreatePkgSetFn) (*pkger.Pkg, error) { panic("not implemented") } diff --git a/pkger/service.go b/pkger/service.go index cbf4b46a2d6..a2c67dacc92 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -56,6 +56,14 @@ type ( Kind Kind `json:"kind"` PkgName string `json:"pkgName"` } + + // StackUpdate provides a means to update an existing stack. + StackUpdate struct { + ID influxdb.ID + Name *string + Description *string + URLs []string + } ) const ResourceTypeStack influxdb.ResourceType = "stack" @@ -66,6 +74,8 @@ type SVC interface { DeleteStack(ctx context.Context, identifiers struct{ OrgID, UserID, StackID influxdb.ID }) error ExportStack(ctx context.Context, orgID, stackID influxdb.ID) (*Pkg, error) ListStacks(ctx context.Context, orgID influxdb.ID, filter ListFilter) ([]Stack, error) + ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) + UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) CreatePkg(ctx context.Context, setters ...CreatePkgSetFn) (*Pkg, error) DryRun(ctx context.Context, orgID, userID influxdb.ID, opts ...ApplyOptFn) (PkgImpactSummary, error) @@ -526,6 +536,36 @@ func (s *Service) ListStacks(ctx context.Context, orgID influxdb.ID, f ListFilte return s.store.ListStacks(ctx, orgID, f) } +// ReadStack returns a stack that matches the given id. +func (s *Service) ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) { + return s.store.ReadStackByID(ctx, id) +} + +// UpdateStack updates the stack by the given parameters. +func (s *Service) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) { + existing, err := s.ReadStack(ctx, upd.ID) + if err != nil { + return Stack{}, err + } + + if upd.Name != nil { + existing.Name = *upd.Name + } + if upd.Description != nil { + existing.Description = *upd.Description + } + if upd.URLs != nil { + existing.URLs = upd.URLs + } + existing.UpdatedAt = s.timeGen.Now() + + if err := s.store.UpdateStack(ctx, existing); err != nil { + return Stack{}, err + } + + return existing, nil +} + type ( // CreatePkgSetFn is a functional input for setting the pkg fields. CreatePkgSetFn func(opt *CreateOpt) error diff --git a/pkger/service_auth.go b/pkger/service_auth.go index 72f633dc5b5..1df1cd164c9 100644 --- a/pkger/service_auth.go +++ b/pkger/service_auth.go @@ -60,6 +60,32 @@ func (s *authMW) ListStacks(ctx context.Context, orgID influxdb.ID, f ListFilter return s.next.ListStacks(ctx, orgID, f) } +func (s *authMW) ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) { + st, err := s.next.ReadStack(ctx, id) + if err != nil { + return Stack{}, err + } + + err = s.authAgent.OrgPermissions(ctx, st.OrgID, influxdb.ReadAction) + if err != nil { + return Stack{}, err + } + return st, nil +} + +func (s *authMW) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) { + stack, err := s.next.ReadStack(ctx, upd.ID) + if err != nil { + return Stack{}, err + } + + err = s.authAgent.IsWritable(ctx, stack.OrgID, ResourceTypeStack) + if err != nil { + return Stack{}, err + } + return s.next.UpdateStack(ctx, upd) +} + func (s *authMW) CreatePkg(ctx context.Context, setters ...CreatePkgSetFn) (*Pkg, error) { return s.next.CreatePkg(ctx, setters...) } diff --git a/pkger/service_logging.go b/pkger/service_logging.go index 2bfafc86204..d5188251bf6 100644 --- a/pkger/service_logging.go +++ b/pkger/service_logging.go @@ -101,6 +101,45 @@ func (s *loggingMW) ListStacks(ctx context.Context, orgID influxdb.ID, f ListFil return s.next.ListStacks(ctx, orgID, f) } +func (s *loggingMW) ReadStack(ctx context.Context, id influxdb.ID) (st Stack, err error) { + defer func(start time.Time) { + if err != nil { + s.logger.Error("failed to read stack", + zap.Error(err), + zap.String("id", id.String()), + zap.Duration("took", time.Since(start)), + ) + return + } + }(time.Now()) + return s.next.ReadStack(ctx, id) +} + +func (s *loggingMW) UpdateStack(ctx context.Context, upd StackUpdate) (_ Stack, err error) { + defer func(start time.Time) { + if err != nil { + fields := []zap.Field{ + zap.Error(err), + zap.String("id", upd.ID.String()), + } + if upd.Name != nil { + fields = append(fields, zap.String("name", *upd.Name)) + } + if upd.Description != nil { + fields = append(fields, zap.String("desc", *upd.Description)) + } + fields = append(fields, + zap.Strings("urls", upd.URLs), + zap.Duration("took", time.Since(start)), + ) + + s.logger.Error("failed to update stack", fields...) + return + } + }(time.Now()) + return s.next.UpdateStack(ctx, upd) +} + func (s *loggingMW) CreatePkg(ctx context.Context, setters ...CreatePkgSetFn) (pkg *Pkg, err error) { defer func(start time.Time) { dur := zap.Duration("took", time.Since(start)) diff --git a/pkger/service_metrics.go b/pkger/service_metrics.go index fbb8f53749a..140fda7472f 100644 --- a/pkger/service_metrics.go +++ b/pkger/service_metrics.go @@ -54,6 +54,18 @@ func (s *mwMetrics) ListStacks(ctx context.Context, orgID influxdb.ID, f ListFil return stacks, rec(err) } +func (s *mwMetrics) ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) { + rec := s.rec.Record("read_stack") + stack, err := s.next.ReadStack(ctx, id) + return stack, rec(err) +} + +func (s *mwMetrics) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) { + rec := s.rec.Record("update_stack") + stack, err := s.next.UpdateStack(ctx, upd) + return stack, rec(err) +} + func (s *mwMetrics) CreatePkg(ctx context.Context, setters ...CreatePkgSetFn) (*Pkg, error) { rec := s.rec.Record("create_pkg") pkg, err := s.next.CreatePkg(ctx, setters...) diff --git a/pkger/service_test.go b/pkger/service_test.go index 330d451d4af..51a68708b81 100644 --- a/pkger/service_test.go +++ b/pkger/service_test.go @@ -3214,6 +3214,97 @@ func TestService(t *testing.T) { } }) }) + + t.Run("UpdateStack", func(t *testing.T) { + now := time.Time{}.Add(10 * 24 * time.Hour) + + t.Run("when updating valid stack", func(t *testing.T) { + tests := []struct { + name string + input StackUpdate + expected Stack + }{ + { + name: "update nothing", + input: StackUpdate{}, + expected: Stack{}, + }, + { + name: "update name", + input: StackUpdate{ + Name: strPtr("name"), + }, + expected: Stack{ + Name: "name", + }, + }, + { + name: "update desc", + input: StackUpdate{ + Description: strPtr("desc"), + }, + expected: Stack{ + Description: "desc", + }, + }, + { + name: "update URLs", + input: StackUpdate{ + URLs: []string{"http://example.com"}, + }, + expected: Stack{ + URLs: []string{"http://example.com"}, + }, + }, + { + name: "update all", + input: StackUpdate{ + Name: strPtr("name"), + Description: strPtr("desc"), + URLs: []string{"http://example.com"}, + }, + expected: Stack{ + Name: "name", + Description: "desc", + URLs: []string{"http://example.com"}, + }, + }, + } + + for _, tt := range tests { + fn := func(t *testing.T) { + svc := newTestService( + WithTimeGenerator(newTimeGen(now)), + WithStore(&fakeStore{ + readFn: func(ctx context.Context, id influxdb.ID) (Stack, error) { + if id != 33 { + return Stack{}, errors.New("wrong id: " + id.String()) + } + return Stack{ID: id, OrgID: 3}, nil + }, + updateFn: func(ctx context.Context, stack Stack) error { + return nil + }, + }), + ) + + tt.input.ID = 33 + stack, err := svc.UpdateStack(context.Background(), tt.input) + require.NoError(t, err) + + assert.Equal(t, influxdb.ID(33), stack.ID) + assert.Equal(t, influxdb.ID(3), stack.OrgID) + assert.Zero(t, stack.CreatedAt) // should always zero value in these tests + assert.Equal(t, tt.expected.Name, stack.Name) + assert.Equal(t, tt.expected.Description, stack.Description) + assert.Equal(t, tt.expected.URLs, stack.URLs) + assert.Equal(t, now, stack.UpdatedAt) + } + + t.Run(tt.name, fn) + } + }) + }) } func Test_normalizeRemoteSources(t *testing.T) { diff --git a/pkger/service_tracing.go b/pkger/service_tracing.go index 4ab564d126c..1dbb8c56f95 100644 --- a/pkger/service_tracing.go +++ b/pkger/service_tracing.go @@ -53,6 +53,18 @@ func (s *traceMW) ListStacks(ctx context.Context, orgID influxdb.ID, f ListFilte return stacks, err } +func (s *traceMW) ReadStack(ctx context.Context, id influxdb.ID) (Stack, error) { + span, ctx := tracing.StartSpanFromContext(ctx) + defer span.Finish() + return s.next.ReadStack(ctx, id) +} + +func (s *traceMW) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, error) { + span, ctx := tracing.StartSpanFromContext(ctx) + defer span.Finish() + return s.next.UpdateStack(ctx, upd) +} + func (s *traceMW) CreatePkg(ctx context.Context, setters ...CreatePkgSetFn) (pkg *Pkg, err error) { span, ctx := tracing.StartSpanFromContext(ctx) defer span.Finish() diff --git a/pkger/store.go b/pkger/store.go index c805459c23a..3d77577a11d 100644 --- a/pkger/store.go +++ b/pkger/store.go @@ -54,7 +54,7 @@ var _ Store = (*StoreKV)(nil) // want to init it if you want to have this init donezo at startup. If not it'll lazy // load the buckets as they are used. func NewStoreKV(store kv.Store) *StoreKV { - const resource = "pkg stack" + const resource = "stack" storeKV := &StoreKV{ kvStore: store,