From 78d7c06c652dc6a4b171223a7fbfc6812d5840e6 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Fri, 17 Apr 2020 13:46:12 -0300 Subject: [PATCH] feat(rpaas/api): receive flavors, IP and plan template as plan parameter (#67) --- api/api.go | 31 ++- api/api_test.go | 96 +++++++++ api/service_handlers.go | 39 +--- api/service_handlers_test.go | 157 ++++++-------- go.mod | 2 + go.sum | 4 + internal/pkg/rpaas/fake/manager.go | 4 +- internal/pkg/rpaas/k8s.go | 238 +++++++++++++++------ internal/pkg/rpaas/k8s_test.go | 11 +- internal/pkg/rpaas/manager.go | 155 +++++++++++++- internal/pkg/rpaas/manager_test.go | 321 +++++++++++++++++++++++++++++ 11 files changed, 853 insertions(+), 205 deletions(-) create mode 100644 api/api_test.go create mode 100644 internal/pkg/rpaas/manager_test.go diff --git a/api/api.go b/api/api.go index b795d9ef0..a9c66b7b2 100644 --- a/api/api.go +++ b/api/api.go @@ -7,13 +7,16 @@ package api import ( "context" "fmt" + "io" "net/http" "os" "os/signal" + "strings" "sync" "syscall" "time" + "github.com/ajg/form" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" "github.com/tsuru/rpaas-operator/config" @@ -158,7 +161,7 @@ func errorMiddleware(next echo.HandlerFunc) echo.HandlerFunc { func newEcho() *echo.Echo { e := echo.New() - + e.Binder = new(requestBinder) e.HideBanner = true e.Use(middleware.Recover()) @@ -221,3 +224,29 @@ func newEcho() *echo.Echo { return e } + +type requestBinder struct{} + +func (b *requestBinder) Bind(i interface{}, c echo.Context) error { + req := c.Request() + + ctype := req.Header.Get(echo.HeaderContentType) + if !strings.HasPrefix(ctype, echo.MIMEApplicationForm) { + c.Response().Header().Set("Accept", echo.MIMEApplicationForm) + return echo.ErrUnsupportedMediaType + } + + if err := newDecoder(req.Body).Decode(i); err != nil { + return fmt.Errorf("cannot decode the parameters: %w", err) + } + defer req.Body.Close() + + return nil +} + +func newDecoder(r io.Reader) *form.Decoder { + decoder := form.NewDecoder(r) + decoder.IgnoreCase(true) + decoder.IgnoreUnknownKeys(true) + return decoder +} diff --git a/api/api_test.go b/api/api_test.go new file mode 100644 index 000000000..be99361d7 --- /dev/null +++ b/api/api_test.go @@ -0,0 +1,96 @@ +// Copyright 2020 tsuru authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequestBinder_Bind(t *testing.T) { + type t1 struct { + Name string `form:"name"` + Tags []string `form:"tags"` + Complex map[string]interface{} `form:"complex"` + Ignored bool `form:"-"` + } + + tests := []struct { + name string + c echo.Context + data interface{} + assert func(*testing.T, error, interface{}, echo.Context) + }{ + { + name: "when content-type is not application/x-www-form-urleconded", + c: func() echo.Context { + body := `{"name": "my-instance"}` + e := newEcho() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + return e.NewContext(req, httptest.NewRecorder()) + }(), + assert: func(t *testing.T, err error, d interface{}, c echo.Context) { + require.Error(t, err) + assert.EqualError(t, err, "code=415, message=Unsupported Media Type, internal=") + assert.Equal(t, "application/x-www-form-urlencoded", c.Response().Header().Get("Accept")) + }, + }, + { + name: "submitting a complex object", + c: func() echo.Context { + body := `name=my-instance&tags.0=tag1&tags.1=tag2&tags.2=tag3&ignored=true&complex.key1=1&complex.other.key=value` + e := newEcho() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationForm) + return e.NewContext(req, httptest.NewRecorder()) + }(), + data: &t1{}, + assert: func(t *testing.T, err error, d interface{}, c echo.Context) { + require.NoError(t, err) + assert.Equal(t, &t1{ + Name: "my-instance", + Tags: []string{"tag1", "tag2", "tag3"}, + Complex: map[string]interface{}{ + "key1": "1", + "other": map[string]interface{}{ + "key": "value", + }, + }, + }, d) + }, + }, + { + name: "when some error occurs on decode method", + c: func() echo.Context { + body := `name=my-instance&tags.0=tag1&tags.1=tag2&tags.2=tag3&ignored=true&complex.key1=1&complex.other.key=value` + e := newEcho() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationForm) + return e.NewContext(req, httptest.NewRecorder()) + }(), + data: func() string { return "cannot decode a function" }, + assert: func(t *testing.T, err error, d interface{}, c echo.Context) { + require.Error(t, err) + assert.EqualError(t, err, "cannot decode the parameters: func() string has unsupported kind func") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.NotNil(t, tt.assert) + b := &requestBinder{} + err := b.Bind(tt.data, tt.c) + tt.assert(t, err, tt.data, tt.c) + }) + } +} diff --git a/api/service_handlers.go b/api/service_handlers.go index 8bb4ae313..f20bea8c4 100644 --- a/api/service_handlers.go +++ b/api/service_handlers.go @@ -14,23 +14,20 @@ import ( ) func serviceCreate(c echo.Context) error { - if c.Request().ContentLength == 0 { - return echo.NewHTTPError(http.StatusBadRequest, "Request body can't be empty") - } - var args rpaas.CreateArgs - err := c.Bind(&args) - if err != nil { + if err := c.Bind(&args); err != nil { return err } + manager, err := getManager(c) if err != nil { return err } - err = manager.CreateInstance(c.Request().Context(), args) - if err != nil { + + if err = manager.CreateInstance(c.Request().Context(), args); err != nil { return err } + return c.NoContent(http.StatusCreated) } @@ -39,6 +36,7 @@ func serviceDelete(c echo.Context) error { if len(name) == 0 { return c.String(http.StatusBadRequest, "name is required") } + manager, err := getManager(c) if err != nil { return err @@ -51,10 +49,6 @@ func serviceDelete(c echo.Context) error { } func serviceUpdate(c echo.Context) error { - if c.Request().ContentLength == 0 { - return echo.NewHTTPError(http.StatusBadRequest, "Request body can't be empty") - } - var args rpaas.UpdateInstanceArgs if err := c.Bind(&args); err != nil { return err @@ -72,12 +66,6 @@ func serviceUpdate(c echo.Context) error { return c.NoContent(http.StatusOK) } -type plan struct { - Name string `json:"name"` - Description string `json:"description"` - Default bool `json:"default"` -} - func servicePlans(c echo.Context) error { manager, err := getManager(c) if err != nil { @@ -89,20 +77,11 @@ func servicePlans(c echo.Context) error { return err } - var result []plan - for _, p := range plans { - result = append(result, plan{ - Name: p.Name, - Description: p.Spec.Description, - Default: p.Spec.Default, - }) - } - - if result == nil { - result = []plan{} + if plans == nil { + plans = make([]rpaas.Plan, 0) } - return c.JSON(http.StatusOK, result) + return c.JSON(http.StatusOK, plans) } func serviceInfo(c echo.Context) error { diff --git a/api/service_handlers_test.go b/api/service_handlers_test.go index 23f9a2fec..ae55629c5 100644 --- a/api/service_handlers_test.go +++ b/api/service_handlers_test.go @@ -24,77 +24,54 @@ import ( func Test_serviceCreate(t *testing.T) { testCases := []struct { + name string requestBody string expectedCode int expectedBody string manager rpaas.RpaasManager }{ { - requestBody: "", - expectedCode: http.StatusBadRequest, - expectedBody: "Request body can't be empty", - manager: &fake.RpaasManager{}, - }, - { - requestBody: "name=", - expectedCode: http.StatusBadRequest, - expectedBody: "name is required", - manager: &fake.RpaasManager{ - FakeCreateInstance: func(rpaas.CreateArgs) error { - return rpaas.ValidationError{Msg: "name is required"} - }, - }, - }, - { - requestBody: "name=rpaas", + name: "when some error is returned", + requestBody: "foo=bar", expectedCode: http.StatusBadRequest, - expectedBody: "plan is required", + expectedBody: "some error message", manager: &fake.RpaasManager{ - FakeCreateInstance: func(rpaas.CreateArgs) error { - return rpaas.ValidationError{Msg: "plan is required"} + FakeCreateInstance: func(args rpaas.CreateArgs) error { + assert.Equal(t, rpaas.CreateArgs{}, args) + return rpaas.ValidationError{Msg: "some error message"} }, }, }, { - requestBody: "name=rpaas&plan=myplan", - expectedCode: http.StatusBadRequest, - expectedBody: "team name is required", - manager: &fake.RpaasManager{ - FakeCreateInstance: func(rpaas.CreateArgs) error { - return rpaas.ValidationError{Msg: "team name is required"} - }, - }, - }, - { - requestBody: "name=rpaas&plan=plan2&team=myteam", - expectedCode: http.StatusBadRequest, - expectedBody: "invalid plan", - manager: &fake.RpaasManager{ - FakeCreateInstance: func(rpaas.CreateArgs) error { - return rpaas.ValidationError{Msg: "invalid plan"} - }, - }, - }, - { - requestBody: "name=firstinstance&plan=myplan&team=myteam", - expectedCode: http.StatusConflict, - expectedBody: "firstinstance instance already exists", + name: "passing all create parameters on body", + requestBody: "name=my-instance&description=some%20description&plan=my-plan&team=my-team&tags.0=tsuru&tags.1=rpaas¶meters.flavors.0=orange¶meters.flavors.1=strawberry¶meters.flavors.2=blueberry", + expectedCode: http.StatusCreated, manager: &fake.RpaasManager{ - FakeCreateInstance: func(rpaas.CreateArgs) error { - return rpaas.ConflictError{Msg: "firstinstance instance already exists"} + FakeCreateInstance: func(args rpaas.CreateArgs) error { + expected := rpaas.CreateArgs{ + Name: "my-instance", + Description: "some description", + Plan: "my-plan", + Team: "my-team", + Tags: []string{"tsuru", "rpaas"}, + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "orange", + "1": "strawberry", + "2": "blueberry", + }, + }, + } + assert.Equal(t, expected, args) + assert.Equal(t, []string{"orange", "strawberry", "blueberry"}, args.Flavors()) + return nil }, }, }, - { - requestBody: "name=otherinstance&plan=myplan&team=myteam", - expectedCode: http.StatusCreated, - expectedBody: "", - manager: &fake.RpaasManager{}, - }, } for _, tt := range testCases { - t.Run("", func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { srv := newTestingServer(t, tt.manager) defer srv.Close() path := fmt.Sprintf("%s/resources", srv.URL) @@ -159,46 +136,44 @@ func Test_serviceUpdate(t *testing.T) { manager rpaas.RpaasManager }{ { - name: "when no body content is sent", + name: "when some error is returned", instance: "my-instance", + requestBody: "foo=bar", expectedCode: http.StatusBadRequest, - expectedBody: "Request body can't be empty", - manager: &fake.RpaasManager{}, + expectedBody: "some error", + manager: &fake.RpaasManager{ + FakeUpdateInstance: func(instanceName string, args rpaas.UpdateInstanceArgs) error { + assert.Equal(t, "my-instance", instanceName) + assert.Equal(t, rpaas.UpdateInstanceArgs{}, args) + return rpaas.ValidationError{Msg: "some error"} + }, + }, }, { - name: "when UpdateInstance returns no error", - instance: "my-instance", - requestBody: "description=some%20description&plan=huge&team=team-one&tags=tag1&tags=tag2", + name: "passing all update parameters on body", + instance: "other-instance", + requestBody: "description=some%20description&plan=huge&team=team-one&tags.0=tag1&tags.1=tag2¶meters.flavors.0=orange¶meters.flavors.1=mango", expectedCode: http.StatusOK, manager: &fake.RpaasManager{ FakeUpdateInstance: func(instanceName string, args rpaas.UpdateInstanceArgs) error { - assert.Equal(t, "my-instance", instanceName) + assert.Equal(t, "other-instance", instanceName) assert.Equal(t, rpaas.UpdateInstanceArgs{ Description: "some description", Plan: "huge", Tags: []string{"tag1", "tag2"}, Team: "team-one", + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "orange", + "1": "mango", + }, + }, }, args) + assert.Equal(t, []string{"orange", "mango"}, args.Flavors()) return nil }, }, }, - { - name: "when UpdateInstance returns a NotFound error", - instance: "my-instance2", - requestBody: "plan=not-found", - expectedCode: http.StatusNotFound, - expectedBody: "some error", - manager: &fake.RpaasManager{ - FakeUpdateInstance: func(instanceName string, args rpaas.UpdateInstanceArgs) error { - assert.Equal(t, "my-instance2", instanceName) - assert.Equal(t, rpaas.UpdateInstanceArgs{ - Plan: "not-found", - }, args) - return rpaas.NotFoundError{Msg: "some error"} - }, - }, - }, } for _, tt := range tests { @@ -222,7 +197,7 @@ func Test_servicePlans(t *testing.T) { name string expectedCode int expectedError string - expectedPlans []plan + expectedPlans []rpaas.Plan manager rpaas.RpaasManager }{ { @@ -230,7 +205,7 @@ func Test_servicePlans(t *testing.T) { expectedCode: http.StatusConflict, expectedError: "some error", manager: &fake.RpaasManager{ - FakeGetPlans: func() ([]v1alpha1.RpaasPlan, error) { + FakeGetPlans: func() ([]rpaas.Plan, error) { return nil, rpaas.ConflictError{Msg: "some error"} }, }, @@ -238,9 +213,9 @@ func Test_servicePlans(t *testing.T) { { name: "when has no plans", expectedCode: http.StatusOK, - expectedPlans: []plan{}, + expectedPlans: []rpaas.Plan{}, manager: &fake.RpaasManager{ - FakeGetPlans: func() ([]v1alpha1.RpaasPlan, error) { + FakeGetPlans: func() ([]rpaas.Plan, error) { return nil, nil }, }, @@ -248,32 +223,24 @@ func Test_servicePlans(t *testing.T) { { name: "when returns several plans", expectedCode: http.StatusOK, - expectedPlans: []plan{ + expectedPlans: []rpaas.Plan{ { Name: "my-plan", }, { Name: "my-default-plan", Description: "Some description about my-default-plan.", - Default: true, }, }, manager: &fake.RpaasManager{ - FakeGetPlans: func() ([]v1alpha1.RpaasPlan, error) { - return []v1alpha1.RpaasPlan{ + FakeGetPlans: func() ([]rpaas.Plan, error) { + return []rpaas.Plan{ { - ObjectMeta: metav1.ObjectMeta{ - Name: "my-plan", - }, + Name: "my-plan", }, { - ObjectMeta: metav1.ObjectMeta{ - Name: "my-default-plan", - }, - Spec: v1alpha1.RpaasPlanSpec{ - Description: "Some description about my-default-plan.", - Default: true, - }, + Name: "my-default-plan", + Description: "Some description about my-default-plan.", }, }, nil }, @@ -295,9 +262,9 @@ func Test_servicePlans(t *testing.T) { assert.Regexp(t, tt.expectedError, bodyContent(rsp)) return } - var result []plan + var result []rpaas.Plan require.NoError(t, json.Unmarshal([]byte(bodyContent(rsp)), &result)) - assert.Equal(t, result, tt.expectedPlans) + assert.Equal(t, tt.expectedPlans, result) }) } } diff --git a/go.mod b/go.mod index d4248eede..ac10bd97f 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/tsuru/rpaas-operator go 1.13 require ( + github.com/ajg/form v1.5.1 github.com/davecgh/go-spew v1.1.1 github.com/fsnotify/fsnotify v1.4.7 github.com/go-openapi/spec v0.19.2 @@ -13,6 +14,7 @@ require ( github.com/olekukonko/tablewriter v0.0.4 github.com/operator-framework/operator-sdk v0.13.0 github.com/pkg/errors v0.8.1 + github.com/pmorie/go-open-service-broker-client v0.0.0-20190909175253-906fa5f9c249 github.com/sirupsen/logrus v1.4.2 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.4.0 diff --git a/go.sum b/go.sum index 81b24352a..9c3d872ee 100644 --- a/go.sum +++ b/go.sum @@ -51,6 +51,8 @@ github.com/Rican7/retry v0.1.0/go.mod h1:FgOROf8P5bebcC1DS0PdOQiqGUridaZvikzUmkF github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= github.com/StackExchange/wmi v0.0.0-20170410192909-ea383cf3ba6e/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= +github.com/ajg/form v1.5.1 h1:t9c7v8JUKu/XxOGBU0yjNpaMloxGEJhUkqFRq0ibGeU= +github.com/ajg/form v1.5.1/go.mod h1:uL1WgH+h2mgNtvBq0339dVnzXdBETtL2LeUXaIv25UY= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/ant31/crd-validation v0.0.0-20180702145049-30f8a35d0ac2/go.mod h1:X0noFIik9YqfhGYBLEHg8LJKEwy7QIitLQuFMpKLcPk= @@ -509,6 +511,8 @@ github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/pmorie/go-open-service-broker-client v0.0.0-20190909175253-906fa5f9c249 h1:LzBGqIqO5Mhzjg1HaXTUOYWSCLZToEeY6rCks4ehX1s= +github.com/pmorie/go-open-service-broker-client v0.0.0-20190909175253-906fa5f9c249/go.mod h1:6d5FSWVMC68G2RoLKixGVhkoNlgoEC/phmruM0yHdjQ= github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA= github.com/pquerna/ffjson v0.0.0-20180717144149-af8b230fcd20/go.mod h1:YARuvh7BUWHNhzDq2OM5tzR2RiCcN2D7sapiKyCel/M= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= diff --git a/internal/pkg/rpaas/fake/manager.go b/internal/pkg/rpaas/fake/manager.go index 446302909..0dcbaf1e9 100644 --- a/internal/pkg/rpaas/fake/manager.go +++ b/internal/pkg/rpaas/fake/manager.go @@ -30,7 +30,7 @@ type RpaasManager struct { FakeInstanceAddress func(name string) (string, error) FakeInstanceStatus func(name string) (*nginxv1alpha1.Nginx, rpaas.PodStatusMap, error) FakeScale func(instanceName string, replicas int32) error - FakeGetPlans func() ([]v1alpha1.RpaasPlan, error) + FakeGetPlans func() ([]rpaas.Plan, error) FakeGetFlavors func() ([]rpaas.Flavor, error) FakeCreateExtraFiles func(instanceName string, files ...rpaas.File) error FakeDeleteExtraFiles func(instanceName string, filenames ...string) error @@ -148,7 +148,7 @@ func (m *RpaasManager) Scale(ctx context.Context, instanceName string, replicas return nil } -func (m *RpaasManager) GetPlans(ctx context.Context) ([]v1alpha1.RpaasPlan, error) { +func (m *RpaasManager) GetPlans(ctx context.Context) ([]rpaas.Plan, error) { if m.FakeGetPlans != nil { return m.FakeGetPlans() } diff --git a/internal/pkg/rpaas/k8s.go b/internal/pkg/rpaas/k8s.go index 8dd3c3aa5..6c0295dbd 100644 --- a/internal/pkg/rpaas/k8s.go +++ b/internal/pkg/rpaas/k8s.go @@ -22,6 +22,7 @@ import ( "time" "github.com/pkg/errors" + osb "github.com/pmorie/go-open-service-broker-client/v2" nginxv1alpha1 "github.com/tsuru/nginx-operator/pkg/apis/nginx/v1alpha1" "github.com/tsuru/rpaas-operator/config" nginxManager "github.com/tsuru/rpaas-operator/internal/pkg/rpaas/nginx" @@ -94,23 +95,28 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e instance := newRpaasInstance(args.Name) instance.Namespace = nsName - instance.Spec.PlanName = plan.Name - instance.Spec.Replicas = func(n int32) *int32 { return &n }(int32(1)) // one replica - instance.Spec.Service = &nginxv1alpha1.NginxService{ - Type: corev1.ServiceTypeLoadBalancer, - Annotations: instance.Annotations, - Labels: instance.Labels, - } - instance.Spec.PodTemplate = nginxv1alpha1.NginxPodTemplateSpec{ - Affinity: getAffinity(args.Team), - Annotations: instance.Annotations, - Labels: instance.Labels, + instance.Spec = v1alpha1.RpaasInstanceSpec{ + Replicas: func(n int32) *int32 { return &n }(int32(1)), + PlanName: plan.Name, + Flavors: args.Flavors(), + Service: &nginxv1alpha1.NginxService{ + Type: corev1.ServiceTypeLoadBalancer, + Annotations: instance.Annotations, + Labels: instance.Labels, + }, + PodTemplate: nginxv1alpha1.NginxPodTemplateSpec{ + Affinity: getAffinity(args.Team), + Annotations: instance.Annotations, + Labels: instance.Labels, + }, } setDescription(instance, args.Description) setTeamOwner(instance, args.Team) + setTags(instance, args.Tags) + setIP(instance, args.IP()) - if err := m.setTags(ctx, instance, args.Tags); err != nil { + if err = setPlanTemplate(instance, args.PlanOverride()); err != nil { return err } @@ -118,21 +124,32 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e } func (m *k8sRpaasManager) UpdateInstance(ctx context.Context, instanceName string, args UpdateInstanceArgs) error { - instance, err := m.GetInstance(ctx, instanceName) - if err != nil { + if err := m.validateUpdateInstanceArgs(ctx, args); err != nil { return err } - plan, err := m.getPlan(ctx, args.Plan) + instance, err := m.GetInstance(ctx, instanceName) if err != nil { return err } - instance.Spec.PlanName = plan.Name + if args.Plan != "" && args.Plan != instance.Spec.PlanName { + plan, err := m.getPlan(ctx, args.Plan) + if err != nil { + return err + } + + instance.Spec.PlanName = plan.Name + } + + instance.Spec.Flavors = args.Flavors() + setDescription(instance, args.Description) setTeamOwner(instance, args.Team) + setTags(instance, args.Tags) + setIP(instance, args.IP()) - if err = m.setTags(ctx, instance, args.Tags); err != nil { + if err := setPlanTemplate(instance, args.PlanOverride()); err != nil { return err } @@ -549,13 +566,39 @@ func (m *k8sRpaasManager) GetInstance(ctx context.Context, name string) (*v1alph return &instance, nil } -func (m *k8sRpaasManager) GetPlans(ctx context.Context) ([]v1alpha1.RpaasPlan, error) { +func (m *k8sRpaasManager) GetPlans(ctx context.Context) ([]Plan, error) { var planList v1alpha1.RpaasPlanList if err := m.cli.List(ctx, &planList, client.InNamespace(namespaceName())); err != nil { return nil, err } - return planList.Items, nil + flavors, err := m.GetFlavors(ctx) + if err != nil { + return nil, err + } + + var schemas *osb.Schemas + if p := buildServiceInstanceParametersForPlan(flavors); p != nil { + schemas = &osb.Schemas{ + ServiceInstance: &osb.ServiceInstanceSchema{ + Create: &osb.InputParametersSchema{Parameters: p}, + Update: &osb.InputParametersSchema{Parameters: p}, + }, + } + } + + var plans []Plan + for _, p := range planList.Items { + plans = append(plans, Plan{ + Name: p.Name, + Description: p.Spec.Description, + Schemas: schemas, + }) + } + + sort.SliceStable(plans, func(i, j int) bool { return plans[i].Name < plans[j].Name }) + + return plans, nil } func (m *k8sRpaasManager) GetFlavors(ctx context.Context) ([]Flavor, error) { @@ -589,14 +632,14 @@ func (m *k8sRpaasManager) getFlavors(ctx context.Context) ([]v1alpha1.RpaasFlavo return flavorList.Items, nil } -func (m *k8sRpaasManager) isFlavorAvailable(ctx context.Context, flavorName string) bool { +func (m *k8sRpaasManager) isFlavorAvailable(ctx context.Context, name string) bool { flavors, err := m.getFlavors(ctx) if err != nil { return false } - for _, flavor := range flavors { - if flavor.Name == flavorName { + for _, f := range flavors { + if f.Name == name { return true } } @@ -993,8 +1036,13 @@ func (m *k8sRpaasManager) getDefaultPlan(ctx context.Context) (*v1alpha1.RpaasPl var defaultPlans []v1alpha1.RpaasPlan for _, p := range plans { - if p.Spec.Default { - defaultPlans = append(defaultPlans, p) + pp, err := m.getPlan(ctx, p.Name) + if err != nil { + return nil, err + } + + if pp.Spec.Default { + defaultPlans = append(defaultPlans, *pp) } } @@ -1075,17 +1123,36 @@ func (m *k8sRpaasManager) validateCreate(ctx context.Context, args CreateArgs) e return ConflictError{Msg: fmt.Sprintf("rpaas instance named %q already exists", args.Name)} } + if err = m.validateFlavors(ctx, args.Flavors()); err != nil { + return err + } + return nil } -func parseTagArg(tags []string, name string, destination *string) { - for _, tag := range tags { - parts := strings.SplitN(tag, "=", 2) - if parts[0] == name { - *destination = parts[1] - break +func (m *k8sRpaasManager) validateUpdateInstanceArgs(ctx context.Context, args UpdateInstanceArgs) error { + if err := m.validateFlavors(ctx, args.Flavors()); err != nil { + return err + } + + return nil +} + +func (m *k8sRpaasManager) validateFlavors(ctx context.Context, flavors []string) error { + encountered := map[string]bool{} + for _, f := range flavors { + if !m.isFlavorAvailable(ctx, f) { + return ValidationError{Msg: fmt.Sprintf("flavor %q not found", f)} + } + + if _, duplicated := encountered[f]; duplicated { + return ValidationError{Msg: fmt.Sprintf("flavor %q only can be set once", f)} } + + encountered[f] = true } + + return nil } func isBlockTypeAllowed(bt v1alpha1.BlockType) bool { @@ -1301,62 +1368,47 @@ func setDescription(instance *v1alpha1.RpaasInstance, description string) { }) } -func (m *k8sRpaasManager) setTags(ctx context.Context, instance *v1alpha1.RpaasInstance, tags []string) error { +func setIP(instance *v1alpha1.RpaasInstance, ip string) { if instance == nil { - return nil + return } - sort.Strings(tags) - - instance.Annotations = mergeMap(instance.Annotations, map[string]string{ - labelKey("tags"): strings.Join(tags, ","), - }) - - var ip string - parseTagArg(tags, "ip", &ip) if instance.Spec.Service == nil { instance.Spec.Service = &nginxv1alpha1.NginxService{} } + instance.Spec.Service.LoadBalancerIP = ip +} - var flavor string - parseTagArg(tags, "flavor", &flavor) - if err := m.setFlavors(ctx, instance, strings.Split(flavor, ",")); err != nil { - return err +func setPlanTemplate(instance *v1alpha1.RpaasInstance, override string) error { + if instance == nil { + return nil } - var planOverride string - parseTagArg(tags, "plan-override", &planOverride) instance.Spec.PlanTemplate = nil - if planOverride == "" { + if override == "" { return nil } var planTemplate v1alpha1.RpaasPlanSpec - if err := json.Unmarshal([]byte(planOverride), &planTemplate); err != nil { - return errors.Wrapf(err, "unable to parse plan-override from data %q", planOverride) + if err := json.Unmarshal([]byte(override), &planTemplate); err != nil { + return fmt.Errorf("unable to unmarshal plan-override on plan spec: %w", err) } - instance.Spec.PlanTemplate = &planTemplate + instance.Spec.PlanTemplate = &planTemplate return nil } -func (m *k8sRpaasManager) setFlavors(ctx context.Context, instance *v1alpha1.RpaasInstance, flavorNames []string) error { - var flavors []string - for _, flavor := range flavorNames { - if flavor == "" { - break - } - - if !m.isFlavorAvailable(ctx, flavor) { - return NotFoundError{Msg: fmt.Sprintf("flavor %q not found", flavor)} - } - - flavors = append(flavors, flavor) +func setTags(instance *v1alpha1.RpaasInstance, tags []string) { + if instance == nil { + return } - instance.Spec.Flavors = flavors - return nil + sort.Strings(tags) + + instance.Annotations = mergeMap(instance.Annotations, map[string]string{ + labelKey("tags"): strings.Join(tags, ","), + }) } func setTeamOwner(instance *v1alpha1.RpaasInstance, team string) { @@ -1647,3 +1699,61 @@ func (m *k8sRpaasManager) getErrorsForPod(ctx context.Context, pod *corev1.Pod) return errors, nil } + +func buildServiceInstanceParametersForPlan(flavors []Flavor) interface{} { + return map[string]interface{}{ + "$id": "https://example.com/schema.json", + "$schema": "https://json-schema.org/draft-07/schema#", + "type": "object", + "properties": map[string]interface{}{ + "flavors": map[string]interface{}{ + "type": "array", + "items": map[string]interface{}{ + "$ref": "#/definitions/flavor", + }, + "description": formatFlavorsDescription(flavors), + "enum": flavorNames(flavors), + }, + "ip": map[string]interface{}{ + "type": "string", + "description": "IP address that will be assigned to load balancer.\nExamples:\n\tip=192.168.15.10", + }, + "plan-override": map[string]interface{}{ + "type": "object", + "description": "Allows an instance to change its plan parameters to specific ones.\nExamples:\n\tplan-override={\"config\": {\"cacheEnabled\": false}}\n\tplan-override={\"image\": \"tsuru/nginx:latest\"}", + }, + }, + "definitions": map[string]interface{}{ + "flavor": map[string]interface{}{ + "type": "string", + }, + }, + } +} + +func formatFlavorsDescription(flavors []Flavor) string { + var sb strings.Builder + sb.WriteString("Provides a self-contained set of features that can be enabled on this plan.\n") + + if len(flavors) == 0 { + return sb.String() + } + + sb.WriteString("\n") + sb.WriteString("Supported flavors:") + for _, f := range flavors { + sb.WriteString("\n") + sb.WriteString(fmt.Sprintf("\t- name: %s\n", f.Name)) + sb.WriteString(fmt.Sprintf("\t description: %s", f.Description)) + } + + return sb.String() +} + +func flavorNames(flavors []Flavor) (names []string) { + for _, f := range flavors { + names = append(names, f.Name) + } + + return +} diff --git a/internal/pkg/rpaas/k8s_test.go b/internal/pkg/rpaas/k8s_test.go index 94a65943a..71034bb29 100644 --- a/internal/pkg/rpaas/k8s_test.go +++ b/internal/pkg/rpaas/k8s_test.go @@ -2810,9 +2810,14 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) { }, { name: "invalid flavor", - args: CreateArgs{Name: "r1", Team: "t1", Tags: []string{"flavor=aaaaa"}}, + args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "aaaaa"}}}, expectedError: `flavor "aaaaa" not found`, }, + { + name: "duplicated flavor", + args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry", "1": "strawberry"}}}, + expectedError: `flavor "strawberry" only can be set once`, + }, { name: "instance already exists", args: CreateArgs{Name: "r0", Team: "t2"}, @@ -2925,7 +2930,7 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) { }, { name: "with flavor", - args: CreateArgs{Name: "r1", Team: "t1", Tags: []string{"flavor=strawberry"}}, + args: CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"flavors": map[string]interface{}{"0": "strawberry"}}}, expected: v1alpha1.RpaasInstance{ TypeMeta: metav1.TypeMeta{ Kind: "RpaasInstance", @@ -2936,8 +2941,8 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) { Namespace: "rpaasv2", ResourceVersion: "1", Annotations: map[string]string{ + "rpaas.extensions.tsuru.io/tags": "", "rpaas.extensions.tsuru.io/description": "", - "rpaas.extensions.tsuru.io/tags": "flavor=strawberry", "rpaas.extensions.tsuru.io/team-owner": "t1", }, Labels: map[string]string{ diff --git a/internal/pkg/rpaas/manager.go b/internal/pkg/rpaas/manager.go index c76ddf648..3ead3e2c6 100644 --- a/internal/pkg/rpaas/manager.go +++ b/internal/pkg/rpaas/manager.go @@ -10,7 +10,10 @@ import ( "crypto/tls" "encoding/json" "fmt" + "sort" + "strings" + osb "github.com/pmorie/go-open-service-broker-client/v2" nginxv1alpha1 "github.com/tsuru/nginx-operator/pkg/apis/nginx/v1alpha1" "github.com/tsuru/rpaas-operator/pkg/apis/extensions/v1alpha1" clientTypes "github.com/tsuru/rpaas-operator/pkg/rpaas/client/types" @@ -79,18 +82,44 @@ type RouteHandler interface { } type CreateArgs struct { - Name string `json:"name" form:"name"` - Plan string `json:"plan" form:"plan"` - Team string `json:"team" form:"team"` - Tags []string `json:"tags" form:"tags"` - Description string `json:"description" form:"description"` + Name string `form:"name"` + Team string `form:"team"` + Plan string `form:"plan"` + Description string `form:"description"` + Tags []string `form:"tags"` + Parameters map[string]interface{} `form:"parameters"` +} + +func (args CreateArgs) Flavors() []string { + return getFlavors(args.Parameters, args.Tags) +} + +func (args CreateArgs) IP() string { + return getIP(args.Parameters, args.Tags) +} + +func (args CreateArgs) PlanOverride() string { + return getPlanOverride(args.Parameters, args.Tags) } type UpdateInstanceArgs struct { - Description string `json:"description" form:"description"` - Plan string `json:"plan" form:"plan"` - Tags []string `json:"tags" form:"tags"` - Team string `json:"team" form:"team"` + Team string `form:"team"` + Description string `form:"description"` + Plan string `form:"plan"` + Tags []string `form:"tags"` + Parameters map[string]interface{} `form:"parameters"` +} + +func (args UpdateInstanceArgs) Flavors() []string { + return getFlavors(args.Parameters, args.Tags) +} + +func (args UpdateInstanceArgs) IP() string { + return getIP(args.Parameters, args.Tags) +} + +func (args UpdateInstanceArgs) PlanOverride() string { + return getPlanOverride(args.Parameters, args.Tags) } type PodStatusMap map[string]PodStatus @@ -117,6 +146,12 @@ type PurgeCacheArgs struct { PreservePath bool `json:"preserve_path" form:"preserve_path"` } +type Plan struct { + Name string `json:"name"` + Description string `json:"description"` + Schemas *osb.Schemas `json:"schemas,omitempty"` +} + type Flavor struct { Name string `json:"name"` Description string `json:"description"` @@ -145,7 +180,7 @@ type RpaasManager interface { GetInstanceAddress(ctx context.Context, name string) (string, error) GetInstanceStatus(ctx context.Context, name string) (*nginxv1alpha1.Nginx, PodStatusMap, error) Scale(ctx context.Context, name string, replicas int32) error - GetPlans(ctx context.Context) ([]v1alpha1.RpaasPlan, error) + GetPlans(ctx context.Context) ([]Plan, error) GetFlavors(ctx context.Context) ([]Flavor, error) BindApp(ctx context.Context, instanceName string, args BindAppArgs) error UnbindApp(ctx context.Context, instanceName, appName string) error @@ -158,3 +193,103 @@ type CertificateData struct { Certificate string `json:"certificate"` Key string `json:"key"` } + +func getFlavors(params map[string]interface{}, tags []string) (flavors []string) { + p, found := params["flavors"] + if !found { + return legacyGetFlavors(tags) + } + + flavorsParams, ok := p.(map[string]interface{}) + if !ok { + return + } + + var sortedKeys []string + for key := range flavorsParams { + sortedKeys = append(sortedKeys, key) + } + + sort.Strings(sortedKeys) + + for _, key := range sortedKeys { + flavors = append(flavors, flavorsParams[key].(string)) + } + + return +} + +func legacyGetFlavors(tags []string) (flavors []string) { + values := extractTagValues([]string{"flavor:", "flavor=", "flavors:", "flavors="}, tags) + if len(values) == 0 { + return nil + } + + return strings.Split(values[0], ",") +} + +func getIP(params map[string]interface{}, tags []string) string { + p, found := params["ip"] + if !found { + return legacyGetIP(tags) + } + + ip, ok := p.(string) + if !ok { + return "" + } + + return ip +} + +func legacyGetIP(tags []string) string { + values := extractTagValues([]string{"ip:", "ip="}, tags) + if len(values) == 0 { + return "" + } + + return values[0] +} + +func getPlanOverride(params map[string]interface{}, tags []string) string { + p, found := params["plan-override"] + if !found { + return legacyGetPlanOverride(tags) + } + + override, ok := p.(string) + if !ok { + return "" + } + + return override +} + +func legacyGetPlanOverride(tags []string) string { + values := extractTagValues([]string{"plan-override:", "plan-override="}, tags) + if len(values) == 0 { + return "" + } + + return values[0] +} + +func extractTagValues(prefixes, tags []string) []string { + for _, t := range tags { + for _, p := range prefixes { + if !strings.HasPrefix(t, p) { + continue + } + + separator := string(p[len(p)-1]) + parts := strings.SplitN(t, separator, 2) + if len(parts) == 1 { + return nil + } + + return parts[1:] + } + } + + return nil +} diff --git a/internal/pkg/rpaas/manager_test.go b/internal/pkg/rpaas/manager_test.go new file mode 100644 index 000000000..d45257b3a --- /dev/null +++ b/internal/pkg/rpaas/manager_test.go @@ -0,0 +1,321 @@ +// Copyright 2019 tsuru authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package rpaas + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCreateArgs_Flavors(t *testing.T) { + tests := []struct { + args CreateArgs + want []string + }{ + {}, + { + args: CreateArgs{ + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "strawberry", + "1": "blueberry", + }, + }, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavor:banana"}, + }, + want: []string{"banana"}, + }, + { + args: CreateArgs{ + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "strawberry", + "1": "blueberry", + }, + }, + Tags: []string{"flavors=banana"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavor:strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavors:strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavor=strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavors=strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: CreateArgs{ + Tags: []string{"flavor:banana", "flavors=strawberry,blueberry"}, + }, + want: []string{"banana"}, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.Flavors() + assert.Equal(t, tt.want, have) + }) + } +} + +func TestCreateArgs_IP(t *testing.T) { + tests := []struct { + args CreateArgs + want string + }{ + {}, + { + args: CreateArgs{ + Parameters: map[string]interface{}{"ip": "7.7.7.7"}, + }, + want: "7.7.7.7", + }, + { + args: CreateArgs{ + Parameters: map[string]interface{}{"ip": []string{"not valid"}}, + }, + }, + { + args: CreateArgs{Tags: []string{"ip:7.7.7.7"}}, + want: "7.7.7.7", + }, + { + args: CreateArgs{Tags: []string{"ip=7.7.7.7"}}, + want: "7.7.7.7", + }, + { + args: CreateArgs{Tags: []string{"ip:6.6.6.6", "ip=7.7.7.7"}}, + want: "6.6.6.6", + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.IP() + assert.Equal(t, tt.want, have) + }) + } +} + +func TestCreateArgs_PlanOverride(t *testing.T) { + tests := []struct { + args CreateArgs + want string + }{ + {}, + { + args: CreateArgs{ + Parameters: map[string]interface{}{"plan-override": `{"image": "nginx:alpine"}`}, + }, + want: `{"image": "nginx:alpine"}`, + }, + { + args: CreateArgs{ + Parameters: map[string]interface{}{"plan-override": []string{"not valid"}}, + }, + }, + { + args: CreateArgs{Tags: []string{`plan-override:{"config": {"cacheEnabled": false}}`}}, + want: `{"config": {"cacheEnabled": false}}`, + }, + { + args: CreateArgs{Tags: []string{`plan-override={"config": {"cacheEnabled": false}}`}}, + want: `{"config": {"cacheEnabled": false}}`, + }, + { + args: CreateArgs{Tags: []string{`plan-override={"image": "nginx:alpine"}`, `plan-override:{"config": {"cacheEnabled": false}}`}}, + want: `{"image": "nginx:alpine"}`, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.PlanOverride() + assert.Equal(t, tt.want, have) + }) + } +} + +func TestUpdateInstanceArgs_Flavors(t *testing.T) { + tests := []struct { + args UpdateInstanceArgs + want []string + }{ + {}, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "strawberry", + "1": "blueberry", + }, + }, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavor:banana"}, + }, + want: []string{"banana"}, + }, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{ + "flavors": map[string]interface{}{ + "0": "strawberry", + "1": "blueberry", + }, + }, + Tags: []string{"flavors=banana"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavor:strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavors:strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavor=strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavors=strawberry,blueberry"}, + }, + want: []string{"strawberry", "blueberry"}, + }, + { + args: UpdateInstanceArgs{ + Tags: []string{"flavor:banana", "flavors=strawberry,blueberry"}, + }, + want: []string{"banana"}, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.Flavors() + assert.Equal(t, tt.want, have) + }) + } +} + +func TestUpdateInstanceArgs_IP(t *testing.T) { + tests := []struct { + args UpdateInstanceArgs + want string + }{ + {}, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{ + "ip": "7.7.7.7", + }, + }, + want: "7.7.7.7", + }, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{ + "ip": []string{"not valid"}, + }, + }, + }, + { + args: UpdateInstanceArgs{Tags: []string{"ip:7.7.7.7"}}, + want: "7.7.7.7", + }, + { + args: UpdateInstanceArgs{Tags: []string{"ip=7.7.7.7"}}, + want: "7.7.7.7", + }, + { + args: UpdateInstanceArgs{Tags: []string{"ip:6.6.6.6", "ip=7.7.7.7"}}, + want: "6.6.6.6", + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.IP() + assert.Equal(t, tt.want, have) + }) + } +} + +func TestUpdateInstanceArgs_PlanOverride(t *testing.T) { + tests := []struct { + args UpdateInstanceArgs + want string + }{ + {}, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{"plan-override": `{"image": "nginx:alpine"}`}, + }, + want: `{"image": "nginx:alpine"}`, + }, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{"plan-override": []string{"not valid"}}, + }, + }, + { + args: UpdateInstanceArgs{Tags: []string{`plan-override:{"config": {"cacheEnabled": false}}`}}, + want: `{"config": {"cacheEnabled": false}}`, + }, + { + args: UpdateInstanceArgs{Tags: []string{`plan-override={"config": {"cacheEnabled": false}}`}}, + want: `{"config": {"cacheEnabled": false}}`, + }, + { + args: UpdateInstanceArgs{Tags: []string{`plan-override={"image": "nginx:alpine"}`, `plan-override:{"config": {"cacheEnabled": false}}`}}, + want: `{"image": "nginx:alpine"}`, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + have := tt.args.PlanOverride() + assert.Equal(t, tt.want, have) + }) + } +}