Add basic register broker command to svcat (#2162)

- refactor pkg/svcat to use an interface
- begin adding unit tests to cmd/svcat w/ faked out pkg interface

cmd/svcat/broker/broker_suite_test.go

@@ -0,0 +1,31 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package broker_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+
+	_ "github.com/kubernetes-incubator/service-catalog/internal/test"
+)
+
+func TestBroker(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Broker Suite")
+}

cmd/svcat/broker/register_cmd.go

@@ -0,0 +1,78 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package broker
+
+import (
+	"fmt"
+
+	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/command"
+	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/output"
+	"github.com/spf13/cobra"
+)
+
+// RegisterCmd contains the information needed to register a broker
+type RegisterCmd struct {
+	BrokerName string
+	Context    *command.Context
+	URL        string
+}
+
+// NewRegisterCmd builds a "svcat register" command
+func NewRegisterCmd(cxt *command.Context) *cobra.Command {
+	registerCmd := &RegisterCmd{
+		Context: cxt,
+	}
+	cmd := &cobra.Command{
+		Use:   "register NAME --url URL",
+		Short: "Registers a new broker with service catalog",
+		Example: command.NormalizeExamples(`
+		svcat register mysqlbroker --url http://mysqlbroker.com
+		`),
+		PreRunE: command.PreRunE(registerCmd),
+		RunE:    command.RunE(registerCmd),
+	}
+	cmd.Flags().StringVar(&registerCmd.URL, "url", "",
+		"The broker URL (Required)")
+	cmd.MarkFlagRequired("url")
+	return cmd
+}
+
+// Validate checks that the required arguements have been provided
+func (c *RegisterCmd) Validate(args []string) error {
+	if len(args) == 0 {
+		return fmt.Errorf("a broker name is required")
+	}
+	c.BrokerName = args[0]
+
+	return nil
+}
+
+// Run runs the command
+func (c *RegisterCmd) Run() error {
+	return c.Register()
+}
+
+// Register calls out to the pkg lib to create the broker and displays the output
+func (c *RegisterCmd) Register() error {
+	broker, err := c.Context.App.Register(c.BrokerName, c.URL)
+	if err != nil {
+		return err
+	}
+
+	output.WriteBrokerDetails(c.Context.Output, broker)
+	return nil
+}

cmd/svcat/broker/register_cmd_test.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package broker_test
+
+import (
+	"bytes"
+
+	. "github.com/kubernetes-incubator/service-catalog/cmd/svcat/broker"
+	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/command"
+	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/test"
+	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
+	"github.com/kubernetes-incubator/service-catalog/pkg/svcat"
+	"github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog/service-catalogfakes"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+var _ = Describe("Register Command", func() {
+	Describe("NewRegisterCmd", func() {
+		It("Builds and returns a cobra command", func() {
+			cxt := &command.Context{}
+			cmd := NewRegisterCmd(cxt)
+			Expect(*cmd).NotTo(BeNil())
+			Expect(cmd.Use).To(Equal("register NAME --url URL"))
+			Expect(cmd.Short).To(ContainSubstring("Registers a new broker with service catalog"))
+			Expect(cmd.Example).To(ContainSubstring("svcat register mysqlbroker --url http://mysqlbroker.com"))
+			Expect(len(cmd.Aliases)).To(Equal(0))
+		})
+	})
+	Describe("Validate", func() {
+		It("errors if a broker name is not provided", func() {
+			cmd := RegisterCmd{
+				BrokerName: "",
+				Context:    nil,
+				URL:        "http://bananabroker.com",
+			}
+			err := cmd.Validate([]string{})
+			Expect(err).To(HaveOccurred())
+		})
+	})
+	Describe("Register", func() {
+		It("Calls the pkg/svcat libs Register method with the passed in variables and prints output to the user", func() {
+			brokerName := "foobarbroker"
+			brokerURL := "http://foobar.com"
+
+			brokerToReturn := &v1beta1.ClusterServiceBroker{
+				ObjectMeta: v1.ObjectMeta{
+					Name: brokerName,
+				},
+				Spec: v1beta1.ClusterServiceBrokerSpec{
+					CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{
+						URL: brokerURL,
+					},
+				},
+			}
+
+			outputBuffer := &bytes.Buffer{}
+
+			fakeApp, _ := svcat.NewApp(nil, nil, "default")
+			fakeSDK := new(servicecatalogfakes.FakeSvcatClient)
+			fakeSDK.RegisterReturns(brokerToReturn, nil)
+			fakeApp.SvcatClient = fakeSDK
+			cmd := RegisterCmd{
+				Context:    svcattest.NewContext(outputBuffer, fakeApp),
+				BrokerName: brokerName,
+				URL:        brokerURL,
+			}
+			err := cmd.Register()
+
+			Expect(err).NotTo(HaveOccurred())
+			returnedName, returnedURL := fakeSDK.RegisterArgsForCall(0)
+			Expect(returnedName).To(Equal(brokerName))
+			Expect(returnedURL).To(Equal(brokerURL))
+
+			output := outputBuffer.String()
+			Expect(output).To(ContainSubstring(brokerName))
+			Expect(output).To(ContainSubstring(brokerURL))
+		})
+	})
+})

cmd/svcat/main.go

@@ -117,6 +117,7 @@ func buildRootCommand(cxt *command.Context) *cobra.Command {
 
 	cmd.AddCommand(newGetCmd(cxt))
 	cmd.AddCommand(newDescribeCmd(cxt))
+	cmd.AddCommand(broker.NewRegisterCmd(cxt))
 	cmd.AddCommand(instance.NewProvisionCmd(cxt))
 	cmd.AddCommand(instance.NewDeprovisionCmd(cxt))
 	cmd.AddCommand(binding.NewBindCmd(cxt))

cmd/svcat/svcat_test.go

@@ -103,6 +103,7 @@ func TestCommandOutput(t *testing.T) {
 		{name: "get broker (json)", cmd: "get broker ups-broker -o json", golden: "output/get-broker.json"},
 		{name: "get broker (yaml)", cmd: "get broker ups-broker -o yaml", golden: "output/get-broker.yaml"},
 		{name: "describe broker", cmd: "describe broker ups-broker", golden: "output/describe-broker.txt"},
+		{name: "register broker", cmd: "register ups-broker --url http://upsbroker.com", golden: "output/register-broker.txt"},
 
 		{name: "list all classes", cmd: "get classes", golden: "output/get-classes.txt"},
 		{name: "list all classes (json)", cmd: "get classes -o json", golden: "output/get-classes.json"},
@@ -249,7 +250,7 @@ func TestNamespacedCommands(t *testing.T) {
 			cxt := newContext()
 			cxt.App = &svcat.App{
 				CurrentNamespace: contextNS,
-				SDK:              &servicecatalog.SDK{ServiceCatalogClient: fakeClient},
+				SvcatClient:      &servicecatalog.SDK{ServiceCatalogClient: fakeClient},
 			}
 			cxt.Output = ioutil.Discard
 
@@ -304,7 +305,7 @@ func TestParametersForBinding(t *testing.T) {
 
 			cxt := newContext()
 			cxt.App = &svcat.App{
-				SDK: &servicecatalog.SDK{ServiceCatalogClient: fakeClient},
+				SvcatClient: &servicecatalog.SDK{ServiceCatalogClient: fakeClient},
 			}
 			cxt.Output = ioutil.Discard
 

cmd/svcat/testdata/output/completion-bash.txt

@@ -763,6 +763,31 @@ _svcat_provision()
     noun_aliases=()
 }
 
+_svcat_register()
+{
+    last_command="svcat_register"
+    commands=()
+
+    flags=()
+    two_word_flags=()
+    local_nonpersistent_flags=()
+    flags_with_completion=()
+    flags_completion=()
+
+    flags+=("--url=")
+    local_nonpersistent_flags+=("--url=")
+    flags+=("--context=")
+    flags+=("--kubeconfig=")
+    flags+=("--logtostderr")
+    flags+=("--v=")
+    two_word_flags+=("-v")
+
+    must_have_one_flag=()
+    must_have_one_flag+=("--url=")
+    must_have_one_noun=()
+    noun_aliases=()
+}
+
 _svcat_sync_broker()
 {
     last_command="svcat_sync_broker"
@@ -925,6 +950,7 @@ _svcat_root_command()
     commands+=("get")
     commands+=("install")
     commands+=("provision")
+    commands+=("register")
     commands+=("sync")
     commands+=("touch")
     commands+=("unbind")

cmd/svcat/testdata/output/completion-zsh.txt

@@ -897,6 +897,31 @@ _svcat_provision()
     noun_aliases=()
 }
 
+_svcat_register()
+{
+    last_command="svcat_register"
+    commands=()
+
+    flags=()
+    two_word_flags=()
+    local_nonpersistent_flags=()
+    flags_with_completion=()
+    flags_completion=()
+
+    flags+=("--url=")
+    local_nonpersistent_flags+=("--url=")
+    flags+=("--context=")
+    flags+=("--kubeconfig=")
+    flags+=("--logtostderr")
+    flags+=("--v=")
+    two_word_flags+=("-v")
+
+    must_have_one_flag=()
+    must_have_one_flag+=("--url=")
+    must_have_one_noun=()
+    noun_aliases=()
+}
+
 _svcat_sync_broker()
 {
     last_command="svcat_sync_broker"
@@ -1059,6 +1084,7 @@ _svcat_root_command()
     commands+=("get")
     commands+=("install")
     commands+=("provision")
+    commands+=("register")
     commands+=("sync")
     commands+=("touch")
     commands+=("unbind")

cmd/svcat/testdata/output/register-broker.txt

@@ -0,0 +1,3 @@
+  Name:     ups-broker            
+  URL:      http://upsbroker.com  
+  Status:                         

cmd/svcat/testdata/plugin.yaml

@@ -266,6 +266,14 @@ tree:
       -1 to wait indefinitely.'
   - name: wait
     desc: Wait until the operation completes.
+- name: register
+  use: register NAME --url URL
+  shortDesc: Registers a new broker with service catalog
+  example: '  svcat register mysqlbroker --url http://mysqlbroker.com'
+  command: ./svcat register
+  flags:
+  - name: url
+    desc: The broker URL (Required)
 - name: sync
   use: sync
   shortDesc: Syncs service catalog for a service broker

docs/devguide.md

@@ -310,6 +310,13 @@ Keep in mind that golden files help catch errors when the output unexpectedly ch
 It's up to you to judge when you should run the tests with -update, 
 and to diff the changes in the golden file to ensure that the new output is correct.
 
+### Counterfeiter
+Certain tests use fakes generated with [Counterfeiter](http://github.com/maxbrunsfeld/counterfeiter). If you add a method
+to an interface (such as SvcatClient in pkg/svcat/service-catalog) you may need to regenerate the fake. You can install
+Counterfeiter by running `go get http://github.com/maxbrunsfeld/counterfeiter && go install http://github.com/maxbrunsfeld/countefeiter`.
+Then regenerate the fake with `counterfeiter ./path/to/dir/with/interface InterfaceName`. You may have to manually paste the boilerplate
+comment into the fakes.
+
 ## Documentation
 
 Our documentation site is located at [svc-cat.io](https://svc-cat.io). The content files are located

pkg/svcat/service-catalog/binding.go

@@ -239,18 +239,18 @@ func (sdk *SDK) WaitForBinding(ns, name string, interval time.Duration, timeout
 	return binding, err
 }
 
-// IsBindingReady returns if the instance is in the Ready status.
+// IsBindingReady returns true if the instance is in the Ready status.
 func (sdk *SDK) IsBindingReady(binding *v1beta1.ServiceBinding) bool {
-	return sdk.BindingHasStatus(binding, v1beta1.ServiceBindingConditionReady)
+	return sdk.bindingHasStatus(binding, v1beta1.ServiceBindingConditionReady)
 }
 
-// IsBindingFailed returns if the instance is in the Failed status.
+// IsBindingFailed returns true if the instance is in the Failed status.
 func (sdk *SDK) IsBindingFailed(binding *v1beta1.ServiceBinding) bool {
-	return sdk.BindingHasStatus(binding, v1beta1.ServiceBindingConditionFailed)
+	return sdk.bindingHasStatus(binding, v1beta1.ServiceBindingConditionFailed)
 }
 
 // BindingHasStatus returns if the instance is in the specified status.
-func (sdk *SDK) BindingHasStatus(binding *v1beta1.ServiceBinding, status v1beta1.ServiceBindingConditionType) bool {
+func (sdk *SDK) bindingHasStatus(binding *v1beta1.ServiceBinding, status v1beta1.ServiceBindingConditionType) bool {
 	if binding == nil {
 		return false
 	}