feat: --update-if-exists flag for create-service-broker

A very common pattern is to have scripting like: ``` cf create-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL || cf update-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL ``` This is not ideal because: - it's a duplicated pattern that the CF CLI could handle better - it does not check the error type - it results in a failure being logged, when in fact there was none Adding a --update-if-exists flag to the CF CLI allows the following alternative: ``` cf create-service-broker $SERVICE_BROKER_NAME $USERNAME $PASSWORD $APP_URL --update-if-exists ```
cloudfoundry · Apr 12, 2023 · a654414 · a654414
1 parent 014842c
commit a654414
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 7 deletions.
diff --git a/command/v7/create_service_broker_command.go b/command/v7/create_service_broker_command.go
@@ -3,6 +3,7 @@ package v7
 import (
 	"os"
 
+	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/command"
 	"code.cloudfoundry.org/cli/command/flag"
 	"code.cloudfoundry.org/cli/resources"
@@ -14,6 +15,7 @@ type CreateServiceBrokerCommand struct {
 
 	PositionalArgs  flag.ServiceBrokerArgs `positional-args:"yes"`
 	SpaceScoped     bool                   `long:"space-scoped" description:"Make the broker's service plans only visible within the targeted space"`
+	UpdateIfExists  bool                   `long:"update-if-exists" description:"If the broker already exists, update it rather than failing. Ignores --space-scoped."`
 	usage           any                    `usage:"CF_NAME create-service-broker SERVICE_BROKER USERNAME PASSWORD URL [--space-scoped]\n   CF_NAME create-service-broker SERVICE_BROKER USERNAME URL [--space-scoped] (omit password to specify interactively or via environment variable)\n\nWARNING:\n   Providing your password as a command line option is highly discouraged\n   Your password may be visible to others and may be recorded in your shell history"`
 	relatedCommands any                    `related_commands:"enable-service-access, service-brokers, target"`
 	envPassword     any                    `environmentName:"CF_BROKER_PASSWORD" environmentDescription:"Password associated with user. Overridden if PASSWORD argument is provided" environmentDefault:"password"`
@@ -35,6 +37,18 @@ func (cmd *CreateServiceBrokerCommand) Execute(args []string) error {
 		return err
 	}
 
+	if cmd.UpdateIfExists {
+		serviceBroker, warnings, err := cmd.Actor.GetServiceBrokerByName(brokerName)
+		cmd.UI.DisplayWarnings(warnings)
+		switch err.(type) {
+		case nil:
+			return updateServiceBroker(cmd.UI, cmd.Actor, user.Name, serviceBroker.GUID, brokerName, username, password, url)
+		case actionerror.ServiceBrokerNotFoundError: // do nothing
+		default:
+			return err
+		}
+	}
+
 	var space configv3.Space
 	if cmd.SpaceScoped {
 		space = cmd.Config.TargetedSpace()

diff --git a/command/v7/create_service_broker_command_test.go b/command/v7/create_service_broker_command_test.go
@@ -5,10 +5,12 @@ import (
 	"fmt"
 	"os"
 
+	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/actor/v7action"
 	"code.cloudfoundry.org/cli/command/commandfakes"
 	v7 "code.cloudfoundry.org/cli/command/v7"
 	"code.cloudfoundry.org/cli/command/v7/v7fakes"
+	"code.cloudfoundry.org/cli/resources"
 	"code.cloudfoundry.org/cli/util/configv3"
 	"code.cloudfoundry.org/cli/util/ui"
 	. "github.com/onsi/ginkgo"
@@ -225,4 +227,59 @@ var _ = Describe("create-service-broker Command", func() {
 			Expect(model.SpaceGUID).To(Equal(""))
 		})
 	})
+
+	When("the --update-if-exists flag is used", func() {
+		BeforeEach(func() {
+			setPositionalFlags(cmd, serviceBrokerName, username, password, url)
+			setFlag(cmd, "--update-if-exists")
+		})
+
+		Context("and the broker does not exist", func() {
+			BeforeEach(func() {
+				fakeActor.GetServiceBrokerByNameReturns(resources.ServiceBroker{}, v7action.Warnings{}, actionerror.ServiceBrokerNotFoundError{})
+			})
+
+			It("checks to see whether the broker exists", func() {
+				Expect(fakeActor.GetServiceBrokerByNameCallCount()).To(Equal(1))
+				Expect(fakeActor.GetServiceBrokerByNameArgsForCall(0)).To(Equal(serviceBrokerName))
+			})
+
+			It("creates a new service broker", func() {
+				Expect(fakeActor.CreateServiceBrokerCallCount()).To(Equal(1))
+
+				model := fakeActor.CreateServiceBrokerArgsForCall(0)
+
+				Expect(model.Name).To(Equal(serviceBrokerName))
+				Expect(model.Username).To(Equal(username))
+				Expect(model.Password).To(Equal(password))
+				Expect(model.URL).To(Equal(url))
+				Expect(model.SpaceGUID).To(Equal(""))
+			})
+		})
+
+		Context("and the broker already exists", func() {
+			const brokerGUID = "fake-broker-guid"
+
+			BeforeEach(func() {
+				fakeActor.GetServiceBrokerByNameReturns(resources.ServiceBroker{GUID: brokerGUID}, v7action.Warnings{}, nil)
+			})
+
+			It("checks to see whether the broker exists", func() {
+				Expect(fakeActor.GetServiceBrokerByNameCallCount()).To(Equal(1))
+				Expect(fakeActor.GetServiceBrokerByNameArgsForCall(0)).To(Equal(serviceBrokerName))
+			})
+
+			It("updates an existing service broker", func() {
+				Expect(fakeActor.UpdateServiceBrokerCallCount()).To(Equal(1))
+
+				guid, model := fakeActor.UpdateServiceBrokerArgsForCall(0)
+
+				Expect(guid).To(Equal(brokerGUID))
+				Expect(model.Username).To(Equal(username))
+				Expect(model.Password).To(Equal(password))
+				Expect(model.URL).To(Equal(url))
+				Expect(model.SpaceGUID).To(Equal(""))
+			})
+		})
+	})
 })
diff --git a/command/v7/update_service_broker_command.go b/command/v7/update_service_broker_command.go
@@ -1,6 +1,7 @@
 package v7
 
 import (
+	"code.cloudfoundry.org/cli/command"
 	"code.cloudfoundry.org/cli/command/flag"
 	"code.cloudfoundry.org/cli/resources"
 )
@@ -35,28 +36,32 @@ func (cmd UpdateServiceBrokerCommand) Execute(args []string) error {
 		return err
 	}
 
-	cmd.UI.DisplayTextWithFlavor(
+	return updateServiceBroker(cmd.UI, cmd.Actor, user.Name, serviceBroker.GUID, brokerName, username, password, url)
+}
+
+func updateServiceBroker(ui command.UI, actor Actor, user, brokerGUID, brokerName, username, password, url string) error {
+	ui.DisplayTextWithFlavor(
 		"Updating service broker {{.ServiceBroker}} as {{.Username}}...",
 		map[string]any{
-			"Username":      user.Name,
+			"Username":      user,
 			"ServiceBroker": brokerName,
 		},
 	)
 
-	warnings, err = cmd.Actor.UpdateServiceBroker(
-		serviceBroker.GUID,
+	warnings, err := actor.UpdateServiceBroker(
+		brokerGUID,
 		resources.ServiceBroker{
 			Username: username,
 			Password: password,
 			URL:      url,
 		},
 	)
-	cmd.UI.DisplayWarnings(warnings)
+	ui.DisplayWarnings(warnings)
 	if err != nil {
 		return err
 	}
 
-	cmd.UI.DisplayOK()
+	ui.DisplayOK()
 
 	return nil
 }
diff --git a/integration/v7/isolated/create_service_broker_command_test.go b/integration/v7/isolated/create_service_broker_command_test.go
@@ -151,6 +151,55 @@ var _ = Describe("create-service-broker command", func() {
 					})
 				})
 			})
+
+			When("the broker already exists", func() {
+				var (
+					org        string
+					cfUsername string
+					broker     *servicebrokerstub.ServiceBrokerStub
+					newBroker  *servicebrokerstub.ServiceBrokerStub
+				)
+
+				BeforeEach(func() {
+					org = helpers.SetupCFWithGeneratedOrgAndSpaceNames()
+					cfUsername, _ = helpers.GetCredentials()
+					broker = servicebrokerstub.Register()
+					newBroker = servicebrokerstub.Create()
+				})
+
+				AfterEach(func() {
+					broker.Forget()
+					newBroker.Forget()
+					helpers.QuickDeleteOrg(org)
+				})
+
+				It("fails", func() {
+					session := helpers.CF("create-service-broker", broker.Name, newBroker.Username, newBroker.Password, newBroker.URL)
+					Eventually(session).Should(Exit(1), "expected duplicate create-service-broker to fail")
+
+					Expect(session.Out).To(SatisfyAll(
+						Say(`Creating service broker %s as %s...\n`, broker.Name, cfUsername),
+						Say(`FAILED\n`),
+					))
+					Expect(session.Err).To(Say("Name must be unique"))
+				})
+
+				When("the --update-if-exists flag is passed", func() {
+					It("updates the existing broker", func() {
+						session := helpers.CF("create-service-broker", broker.Name, newBroker.Username, newBroker.Password, newBroker.URL, "--update-if-exists")
+						Eventually(session).Should(Exit(0))
+
+						Expect(session.Out).To(SatisfyAll(
+							Say("Updating service broker %s as %s...", broker.Name, cfUsername),
+							Say("OK"),
+						))
+
+						By("checking the URL has been updated")
+						session = helpers.CF("service-brokers")
+						Eventually(session.Out).Should(Say("%s[[:space:]]+%s", broker.Name, newBroker.URL))
+					})
+				})
+			})
 		})
 	})
 
@@ -196,7 +245,8 @@ func expectToRenderCreateServiceBrokerHelp(s *Session) {
 		Say(`\s+csb`),
 
 		Say(`OPTIONS:`),
-		Say(`\s+--space-scoped      Make the broker's service plans only visible within the targeted space`),
+		Say(`\s+--space-scoped\s+Make the broker's service plans only visible within the targeted space`),
+		Say(`\s+--update-if-exists\s+If the broker already exists, update it rather than failing. Ignores --space-scoped.`),
 
 		Say(`ENVIRONMENT:`),
 		Say(`\s+CF_BROKER_PASSWORD=password\s+Password associated with user. Overridden if PASSWORD argument is provided`),