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

Update user-provided service tags #474

Merged
merged 7 commits into from
Jun 9, 2015
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
8 changes: 6 additions & 2 deletions cf/api/fakes/fake_service_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type FakeServiceRepo struct {
Name string
PlanGuid string
Params map[string]interface{}
Tags []string
}
CreateServiceInstanceReturns struct {
Error error
Expand All @@ -49,6 +50,7 @@ type FakeServiceRepo struct {
InstanceGuid string
PlanGuid string
Params map[string]interface{}
Tags []string
}

UpdateServiceInstanceReturnsErr bool
Expand Down Expand Up @@ -142,22 +144,24 @@ func (repo *FakeServiceRepo) FindServiceOfferingsByLabel(name string) (offerings
return
}

func (repo *FakeServiceRepo) CreateServiceInstance(name, planGuid string, params map[string]interface{}) (apiErr error) {
func (repo *FakeServiceRepo) CreateServiceInstance(name, planGuid string, params map[string]interface{}, tags []string) (apiErr error) {
repo.CreateServiceInstanceArgs.Name = name
repo.CreateServiceInstanceArgs.PlanGuid = planGuid
repo.CreateServiceInstanceArgs.Params = params
repo.CreateServiceInstanceArgs.Tags = tags

return repo.CreateServiceInstanceReturns.Error
}

func (repo *FakeServiceRepo) UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}) (apiErr error) {
func (repo *FakeServiceRepo) UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}, tags []string) (apiErr error) {

if repo.UpdateServiceInstanceReturnsErr {
apiErr = errors.New("Error updating service instance")
} else {
repo.UpdateServiceInstanceArgs.InstanceGuid = instanceGuid
repo.UpdateServiceInstanceArgs.PlanGuid = planGuid
repo.UpdateServiceInstanceArgs.Params = params
repo.UpdateServiceInstanceArgs.Tags = tags
}

return
Expand Down
10 changes: 6 additions & 4 deletions cf/api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ type ServiceRepository interface {
GetAllServiceOfferings() (offerings models.ServiceOfferings, apiErr error)
GetServiceOfferingsForSpace(spaceGuid string) (offerings models.ServiceOfferings, apiErr error)
FindInstanceByName(name string) (instance models.ServiceInstance, apiErr error)
CreateServiceInstance(name, planGuid string, params map[string]interface{}) (apiErr error)
UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}) (apiErr error)
CreateServiceInstance(name, planGuid string, params map[string]interface{}, tags []string) (apiErr error)
UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}, tags []string) (apiErr error)
RenameService(instance models.ServiceInstance, newName string) (apiErr error)
DeleteService(instance models.ServiceInstance) (apiErr error)
FindServicePlanByDescription(planDescription resources.ServicePlanDescription) (planGuid string, apiErr error)
Expand Down Expand Up @@ -134,13 +134,14 @@ func (repo CloudControllerServiceRepository) FindInstanceByName(name string) (in
return
}

func (repo CloudControllerServiceRepository) CreateServiceInstance(name, planGuid string, params map[string]interface{}) (err error) {
func (repo CloudControllerServiceRepository) CreateServiceInstance(name, planGuid string, params map[string]interface{}, tags []string) (err error) {
path := "/v2/service_instances?accepts_incomplete=true"
request := models.ServiceInstanceCreateRequest{
Name: name,
PlanGuid: planGuid,
SpaceGuid: repo.config.SpaceFields().Guid,
Params: params,
Tags: tags,
}

jsonBytes, err := json.Marshal(request)
Expand All @@ -161,11 +162,12 @@ func (repo CloudControllerServiceRepository) CreateServiceInstance(name, planGui
return
}

func (repo CloudControllerServiceRepository) UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}) (err error) {
func (repo CloudControllerServiceRepository) UpdateServiceInstance(instanceGuid, planGuid string, params map[string]interface{}, tags []string) (err error) {
path := fmt.Sprintf("/v2/service_instances/%s?accepts_incomplete=true", instanceGuid)
request := models.ServiceInstanceUpdateRequest{
PlanGuid: planGuid,
Params: params,
Tags: tags,
}

jsonBytes, err := json.Marshal(request)
Expand Down
54 changes: 44 additions & 10 deletions cf/api/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ var _ = Describe("Services Repo", func() {
Response: testnet.TestResponse{Status: http.StatusCreated},
}))

err := repo.CreateServiceInstance("instance-name", "plan-guid", nil)
err := repo.CreateServiceInstance("instance-name", "plan-guid", nil, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of hate seeing these nils all over the place when we could use empty string array... But eh, that's just me.

Though hopefully we are testing for case when tags is empty array vs nil...

Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
Expand All @@ -220,7 +220,7 @@ var _ = Describe("Services Repo", func() {
paramsMap := make(map[string]interface{})
paramsMap["data"] = "hello"

err := repo.CreateServiceInstance("instance-name", "plan-guid", paramsMap)
err := repo.CreateServiceInstance("instance-name", "plan-guid", paramsMap, nil)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
Expand All @@ -230,12 +230,29 @@ var _ = Describe("Services Repo", func() {
paramsMap := make(map[string]interface{})
paramsMap["data"] = make(chan bool)

err := repo.CreateServiceInstance("instance-name", "plan-guid", paramsMap)
err := repo.CreateServiceInstance("instance-name", "plan-guid", paramsMap, nil)
Expect(err).To(MatchError("json: unsupported type: chan bool"))
})
})
})

Context("when there are tags", func() {
It("sends the tags as part of the request body", func() {
setupTestServer(testapi.NewCloudControllerTestRequest(testnet.TestRequest{
Method: "POST",
Path: "/v2/service_instances?accepts_incomplete=true",
Matcher: testnet.RequestBodyMatcher(`{"name":"instance-name","service_plan_guid":"plan-guid","space_guid":"my-space-guid","tags": ["foo", "bar"]}`),
Response: testnet.TestResponse{Status: http.StatusCreated},
}))

tags := []string{"foo", "bar"}

err := repo.CreateServiceInstance("instance-name", "plan-guid", nil, tags)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
})

Context("when the name is taken but an identical service exists", func() {
BeforeEach(func() {
setupTestServer(
Expand All @@ -252,7 +269,7 @@ var _ = Describe("Services Repo", func() {
})

It("returns a ModelAlreadyExistsError if the plan is the same", func() {
err := repo.CreateServiceInstance("my-service", "plan-guid", nil)
err := repo.CreateServiceInstance("my-service", "plan-guid", nil, nil)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).To(BeAssignableToTypeOf(&errors.ModelAlreadyExistsError{}))
})
Expand All @@ -274,7 +291,7 @@ var _ = Describe("Services Repo", func() {
})

It("fails if the plan is different", func() {
err := repo.CreateServiceInstance("my-service", "different-plan-guid", nil)
err := repo.CreateServiceInstance("my-service", "different-plan-guid", nil, nil)

Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).To(HaveOccurred())
Expand All @@ -292,7 +309,7 @@ var _ = Describe("Services Repo", func() {
Response: testnet.TestResponse{Status: http.StatusOK},
}))

err := repo.UpdateServiceInstance("instance-guid", "plan-guid", nil)
err := repo.UpdateServiceInstance("instance-guid", "plan-guid", nil, nil)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
Expand All @@ -306,7 +323,7 @@ var _ = Describe("Services Repo", func() {
Response: testnet.TestResponse{Status: http.StatusNotFound},
}))

err := repo.UpdateServiceInstance("instance-guid", "plan-guid", nil)
err := repo.UpdateServiceInstance("instance-guid", "plan-guid", nil, nil)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).To(HaveOccurred())
})
Expand All @@ -317,13 +334,13 @@ var _ = Describe("Services Repo", func() {
setupTestServer(testapi.NewCloudControllerTestRequest(testnet.TestRequest{
Method: "PUT",
Path: "/v2/service_instances/instance-guid?accepts_incomplete=true",
Matcher: testnet.RequestBodyMatcher(`{"service_plan_guid":"plan-guid", "parameters": {"foo": "bar"}}`),
Matcher: testnet.RequestBodyMatcher(`{"parameters": {"foo": "bar"}}`),
Response: testnet.TestResponse{Status: http.StatusOK},
}))

paramsMap := map[string]interface{}{"foo": "bar"}

err := repo.UpdateServiceInstance("instance-guid", "plan-guid", paramsMap)
err := repo.UpdateServiceInstance("instance-guid", "", paramsMap, nil)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
Expand All @@ -333,11 +350,28 @@ var _ = Describe("Services Repo", func() {
paramsMap := make(map[string]interface{})
paramsMap["data"] = make(chan bool)

err := repo.UpdateServiceInstance("instance-guid", "plan-guid", paramsMap)
err := repo.UpdateServiceInstance("instance-guid", "", paramsMap, nil)
Expect(err).To(MatchError("json: unsupported type: chan bool"))
})
})
})

Context("when there are tags", func() {
It("sends the tags as part of the request body", func() {
setupTestServer(testapi.NewCloudControllerTestRequest(testnet.TestRequest{
Method: "PUT",
Path: "/v2/service_instances/instance-guid?accepts_incomplete=true",
Matcher: testnet.RequestBodyMatcher(`{"tags": ["foo", "bar"]}`),
Response: testnet.TestResponse{Status: http.StatusOK},
}))

tags := []string{"foo", "bar"}

err := repo.UpdateServiceInstance("instance-guid", "", nil, tags)
Expect(testHandler).To(HaveAllRequestsCalled())
Expect(err).NotTo(HaveOccurred())
})
})
})

Describe("finding service instances by name", func() {
Expand Down
47 changes: 24 additions & 23 deletions cf/commands/service/bind_service.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package service

import (
"strings"

"github.com/cloudfoundry/cli/cf"
"github.com/cloudfoundry/cli/cf/api"
"github.com/cloudfoundry/cli/cf/command_metadata"
Expand Down Expand Up @@ -36,37 +38,36 @@ func NewBindService(ui terminal.UI, config core_config.Reader, serviceBindingRep
}

func (cmd *BindService) Metadata() command_metadata.CommandMetadata {
return command_metadata.CommandMetadata{
Name: "bind-service",
ShortName: "bs",
Description: T("Bind a service instance to an app"),
Usage: T(`CF_NAME bind-service APP_NAME SERVICE_INSTANCE [-c PARAMETERS_AS_JSON]
baseUsage := T("CF_NAME bind-service APP_NAME SERVICE_INSTANCE [-c PARAMETERS_AS_JSON]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks for the nicely formatted string!

paramsUsage := T(` Optionally provide service-specific configuration parameters in a valid JSON object in-line:

Optionally provide service-specific configuration parameters in a valid JSON object in-line.
CF_NAME bind-service APP_NAME SERVICE_INSTANCE -c '{"name":"value","name":"value"}'
CF_NAME bind-service APP_NAME SERVICE_INSTANCE -c '{"name":"value","name":"value"}'

Optionally provide a file containing service-specific configuration parameters in a valid JSON object. The path to the parameters file can be an absolute or relative path to a file.
CF_NAME bind-service APP_NAME SERVICE_INSTANCE -c PATH_TO_FILE
Optionally provide a file containing service-specific configuration parameters in a valid JSON object.
The path to the parameters file can be an absolute or relative path to a file.
CF_NAME bind-service APP_NAME SERVICE_INSTANCE -c PATH_TO_FILE

Example of valid JSON object:
{
"cluster_nodes": {
"count": 5,
"memory_mb": 1024
}
}

EXAMPLE:
Linux/Mac:
CF_NAME bind-service myapp mydb -c '{"permissions":"read-only"}'
"permissions": "read-only"
}`)
exampleUsage := T(`EXAMPLE:
Linux/Mac:
CF_NAME bind-service myapp mydb -c '{"permissions":"read-only"}'

Windows Command Line
CF_NAME bind-service myapp mydb -c "{\"permissions\":\"read-only\"}"
Windows Command Line:
CF_NAME bind-service myapp mydb -c "{\"permissions\":\"read-only\"}"

Windows PowerShell
CF_NAME bind-service myapp mydb -c '{\"permissions\":\"read-only\"}'
Windows PowerShell:
CF_NAME bind-service myapp mydb -c '{\"permissions\":\"read-only\"}'

CF_NAME bind-service myapp mydb -c ~/workspace/tmp/instance_config.json`),
CF_NAME bind-service myapp mydb -c ~/workspace/tmp/instance_config.json`)

return command_metadata.CommandMetadata{
Name: "bind-service",
ShortName: "bs",
Description: T("Bind a service instance to an app"),
Usage: strings.Join([]string{baseUsage, paramsUsage, exampleUsage}, "\n\n"),
Flags: []cli.Flag{
flag_helpers.NewStringFlag("c", T("Valid JSON object containing service-specific configuration parameters, provided either in-line or in a file. For a list of supported configuration parameters, see documentation for the particular service offering.")),
},
Expand Down
15 changes: 11 additions & 4 deletions cf/commands/service/create_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cloudfoundry/cli/cf/models"
"github.com/cloudfoundry/cli/cf/requirements"
"github.com/cloudfoundry/cli/cf/terminal"
"github.com/cloudfoundry/cli/cf/ui_helpers"
"github.com/cloudfoundry/cli/json"
"github.com/codegangsta/cli"
)
Expand All @@ -35,7 +36,7 @@ func (cmd CreateService) Metadata() command_metadata.CommandMetadata {
Name: "create-service",
ShortName: "cs",
Description: T("Create a service instance"),
Usage: T(`CF_NAME create-service SERVICE PLAN SERVICE_INSTANCE [-c PARAMETERS_AS_JSON]
Usage: T(`CF_NAME create-service SERVICE PLAN SERVICE_INSTANCE [-c PARAMETERS_AS_JSON] [-t TAGS]

Optionally provide service-specific configuration parameters in a valid JSON object in-line:
CF_NAME create-service SERVICE PLAN SERVICE_INSTANCE -c '{"name":"value","name":"value"}'
Expand Down Expand Up @@ -63,10 +64,13 @@ EXAMPLE:

CF_NAME create-service db-service silver mydb -c ~/workspace/tmp/instance_config.json

CF_NAME create-service dbaas silver mydb -t "list, of, tags"

TIP:
Use 'CF_NAME create-user-provided-service' to make user-provided services available to cf apps`),
Flags: []cli.Flag{
flag_helpers.NewStringFlag("c", T("Valid JSON object containing service-specific configuration parameters, provided either in-line or in a file. For a list of supported configuration parameters, see documentation for the particular service offering.")),
flag_helpers.NewStringFlag("t", T("User provided tags")),
},
}
}
Expand All @@ -89,6 +93,9 @@ func (cmd CreateService) Run(c *cli.Context) {
planName := c.Args()[1]
serviceInstanceName := c.Args()[2]
params := c.String("c")
tags := c.String("t")

tagsList := ui_helpers.ParseTags(tags)

paramsMap, err := json.ParseJsonFromFileOrString(params)
if err != nil {
Expand All @@ -103,7 +110,7 @@ func (cmd CreateService) Run(c *cli.Context) {
"CurrentUser": terminal.EntityNameColor(cmd.config.Username()),
}))

plan, err := cmd.CreateService(serviceName, planName, serviceInstanceName, paramsMap)
plan, err := cmd.CreateService(serviceName, planName, serviceInstanceName, paramsMap, tagsList)

switch err.(type) {
case nil:
Expand All @@ -130,7 +137,7 @@ func (cmd CreateService) Run(c *cli.Context) {
}
}

func (cmd CreateService) CreateService(serviceName, planName, serviceInstanceName string, params map[string]interface{}) (models.ServicePlanFields, error) {
func (cmd CreateService) CreateService(serviceName, planName, serviceInstanceName string, params map[string]interface{}, tags []string) (models.ServicePlanFields, error) {
offerings, apiErr := cmd.serviceBuilder.GetServicesByNameForSpaceWithPlans(cmd.config.SpaceFields().Guid, serviceName)
if apiErr != nil {
return models.ServicePlanFields{}, apiErr
Expand All @@ -141,7 +148,7 @@ func (cmd CreateService) CreateService(serviceName, planName, serviceInstanceNam
return plan, apiErr
}

apiErr = cmd.serviceRepo.CreateServiceInstance(serviceInstanceName, plan.Guid, params)
apiErr = cmd.serviceRepo.CreateServiceInstance(serviceInstanceName, plan.Guid, params, tags)
return plan, apiErr
}

Expand Down
12 changes: 12 additions & 0 deletions cf/commands/service/create_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ var _ = Describe("create-service command", func() {
Expect(serviceRepo.CreateServiceInstanceArgs.PlanGuid).To(Equal("cleardb-spark-guid"))
})

Context("when passing in tags", func() {
It("sucessfully creates a service and passes the tags as json", func() {
callCreateService([]string{"cleardb", "spark", "my-cleardb-service", "-t", "tag1, tag2,tag3, tag4"})

Expect(ui.Outputs).To(ContainSubstrings(
[]string{"Creating service instance", "my-cleardb-service", "my-org", "my-space", "my-user"},
[]string{"OK"},
))
Expect(serviceRepo.CreateServiceInstanceArgs.Tags).To(ConsistOf("tag1", "tag2", "tag3", "tag4"))
})
})

Context("when passing arbitrary params", func() {
Context("as a json string", func() {
It("successfully creates a service and passes the params as a json string", func() {
Expand Down
Loading