From 03f10713a55b7cfe3c9dac45655ec92eff50d753 Mon Sep 17 00:00:00 2001 From: Anmol Babu Date: Sun, 18 Oct 2020 16:27:03 +0530 Subject: [PATCH] Allow passing username & password as flags to broker register This fix adds username and password as optional parameters to broker register command. If passed the kubernetes secret is auto created and used for broker register fixes: https://github.com/kubernetes-sigs/service-catalog/issues/2479 Signed-off-by: Anmol Babu --- cmd/svcat/broker/register_cmd.go | 46 ++++++++++++++++++- cmd/svcat/broker/register_cmd_test.go | 45 +++++++++++++++++- cmd/svcat/testdata/output/completion-bash.txt | 8 ++++ cmd/svcat/testdata/output/completion-zsh.txt | 8 ++++ pkg/svcat/service-catalog/sdk.go | 1 + pkg/svcat/service-catalog/secret.go | 5 ++ .../service-catalogfakes/fake_svcat_client.go | 31 +++++++++++++ 7 files changed, 141 insertions(+), 3 deletions(-) diff --git a/cmd/svcat/broker/register_cmd.go b/cmd/svcat/broker/register_cmd.go index 635e6613a6a..aa5a35fa015 100644 --- a/cmd/svcat/broker/register_cmd.go +++ b/cmd/svcat/broker/register_cmd.go @@ -17,6 +17,7 @@ limitations under the License. package broker import ( + "encoding/base64" "fmt" "os" "strings" @@ -27,6 +28,7 @@ import ( "github.com/kubernetes-sigs/service-catalog/pkg/apis/servicecatalog/v1beta1" servicecatalog "github.com/kubernetes-sigs/service-catalog/pkg/svcat/service-catalog" "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,6 +40,8 @@ type RegisterCmd struct { BasicSecret string BearerSecret string + UserName string + UserPassword string BrokerName string CAFile string ClassRestrictions []string @@ -66,6 +70,8 @@ func NewRegisterCmd(cxt *command.Context) *cobra.Command { } cmd.Flags().StringVar(®isterCmd.URL, "url", "", "The broker URL (Required)") + cmd.Flags().StringVarP(®isterCmd.UserName, "username", "u", "", "User name to create kubernetes secret with") + cmd.Flags().StringVarP(®isterCmd.UserPassword, "password", "p", "", "Password to create kubernetes secret with") cmd.MarkFlagRequired("url") cmd.Flags().StringVar(®isterCmd.BasicSecret, "basic-secret", "", "A secret containing basic auth (username/password) information to connect to the broker") @@ -97,8 +103,24 @@ func (c *RegisterCmd) Validate(args []string) error { } c.BrokerName = args[0] - if c.BasicSecret != "" && c.BearerSecret != "" { - return fmt.Errorf("cannot use both basic auth and bearer auth") + var isAuthParamSet bool + + if c.UserName != "" && c.UserPassword != "" { + isAuthParamSet = true + } + + if c.BasicSecret != "" { + if isAuthParamSet { + return fmt.Errorf("cannot have more than one authentication parameter passed. Please pass either basic-secret or bearer-secret or username and password") + } + isAuthParamSet = true + } + + if c.BearerSecret != "" { + if isAuthParamSet { + return fmt.Errorf("cannot have more than one authentication parameter passed. Please pass either basic-secret or bearer-secret or username and password") + } + isAuthParamSet = true } if c.CAFile != "" { @@ -118,6 +140,26 @@ func (c *RegisterCmd) Validate(args []string) error { // Run creates the broker and then displays the broker details func (c *RegisterCmd) Run() error { + if c.UserName != "" && c.UserPassword != "" { + secret, err := c.Context.App.CreateSecret( + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.BrokerName, + Namespace: c.Namespace, + }, + Data: map[string][]byte{ + "username": []byte(base64.StdEncoding.EncodeToString([]byte(c.UserName))), + "password": []byte(base64.StdEncoding.EncodeToString([]byte(c.UserPassword))), + }, + Type: corev1.SecretTypeBasicAuth, + }, + ) + if err != nil { + return err + } + + c.BasicSecret = secret.Name + } opts := &servicecatalog.RegisterOptions{ BasicSecret: c.BasicSecret, BearerSecret: c.BearerSecret, diff --git a/cmd/svcat/broker/register_cmd_test.go b/cmd/svcat/broker/register_cmd_test.go index 1b672f3f756..a658fbc96cb 100644 --- a/cmd/svcat/broker/register_cmd_test.go +++ b/cmd/svcat/broker/register_cmd_test.go @@ -111,7 +111,7 @@ var _ = Describe("Register Command", func() { } err := cmd.Validate([]string{"bananabroker", "http://bananabroker.com", "--basic-secret", basicSecret, "--bearer-secret", bearerSecret}) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("cannot use both basic auth and bearer auth")) + Expect(err.Error()).To(ContainSubstring("cannot have more than one authentication parameter passed")) }) It("errors if a provided CA file does not exist", func() { cmd := RegisterCmd{ @@ -250,6 +250,49 @@ var _ = Describe("Register Command", func() { Expect(output).To(ContainSubstring(brokerName)) Expect(output).To(ContainSubstring(brokerURL)) }) + It("Passes in the uesrname and password", func() { + // userName := "foobar" + // password := "foobar" + + brokerToReturn.Spec.AuthInfo.Basic = &v1beta1.ClusterBasicAuthConfig{ + SecretRef: &v1beta1.ObjectReference{ + Name: brokerToReturn.Name, + }, + } + + outputBuffer := &bytes.Buffer{} + fakeApp, _ := svcat.NewApp(nil, nil, namespace) + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RegisterReturns(brokerToReturn, nil) + fakeApp.SvcatClient = fakeSDK + cxt := svcattest.NewContext(outputBuffer, fakeApp) + cmd := RegisterCmd{ + BasicSecret: brokerToReturn.Name, + BrokerName: brokerName, + Namespaced: command.NewNamespaced(cxt), + Scoped: command.NewScoped(), + Waitable: command.NewWaitable(), + URL: brokerURL, + } + cmd.Namespaced.ApplyNamespaceFlags(&pflag.FlagSet{}) + cmd.Waitable.ApplyWaitFlags() + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeSDK.RegisterCallCount()).To(Equal(1)) + returnedName, returnedURL, returnedOpts, _ := fakeSDK.RegisterArgsForCall(0) + Expect(returnedName).To(Equal(brokerName)) + Expect(returnedURL).To(Equal(brokerURL)) + opts := servicecatalog.RegisterOptions{ + Namespace: namespace, + BasicSecret: brokerToReturn.Name, + } + Expect(*returnedOpts).To(Equal(opts)) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring(brokerName)) + Expect(output).To(ContainSubstring(brokerURL)) + }) It("Passes in the bearer secret", func() { bearerSecret := "foobarsecret" brokerToReturn.Spec.AuthInfo.Basic = nil diff --git a/cmd/svcat/testdata/output/completion-bash.txt b/cmd/svcat/testdata/output/completion-bash.txt index 4a04c282304..474732b092a 100644 --- a/cmd/svcat/testdata/output/completion-bash.txt +++ b/cmd/svcat/testdata/output/completion-bash.txt @@ -1226,6 +1226,10 @@ _svcat_register() two_word_flags+=("--namespace") two_word_flags+=("-n") local_nonpersistent_flags+=("--namespace=") + flags+=("--password=") + two_word_flags+=("--password") + two_word_flags+=("-p") + local_nonpersistent_flags+=("--password=") flags+=("--plan-restrictions=") two_word_flags+=("--plan-restrictions") local_nonpersistent_flags+=("--plan-restrictions=") @@ -1246,6 +1250,10 @@ _svcat_register() flags+=("--url=") two_word_flags+=("--url") local_nonpersistent_flags+=("--url=") + flags+=("--username=") + two_word_flags+=("--username") + two_word_flags+=("-u") + local_nonpersistent_flags+=("--username=") flags+=("--wait") local_nonpersistent_flags+=("--wait") flags+=("--context=") diff --git a/cmd/svcat/testdata/output/completion-zsh.txt b/cmd/svcat/testdata/output/completion-zsh.txt index 1c282efcdf0..6ffafd4f10e 100644 --- a/cmd/svcat/testdata/output/completion-zsh.txt +++ b/cmd/svcat/testdata/output/completion-zsh.txt @@ -1360,6 +1360,10 @@ _svcat_register() two_word_flags+=("--namespace") two_word_flags+=("-n") local_nonpersistent_flags+=("--namespace=") + flags+=("--password=") + two_word_flags+=("--password") + two_word_flags+=("-p") + local_nonpersistent_flags+=("--password=") flags+=("--plan-restrictions=") two_word_flags+=("--plan-restrictions") local_nonpersistent_flags+=("--plan-restrictions=") @@ -1380,6 +1384,10 @@ _svcat_register() flags+=("--url=") two_word_flags+=("--url") local_nonpersistent_flags+=("--url=") + flags+=("--username=") + two_word_flags+=("--username") + two_word_flags+=("-u") + local_nonpersistent_flags+=("--username=") flags+=("--wait") local_nonpersistent_flags+=("--wait") flags+=("--context=") diff --git a/pkg/svcat/service-catalog/sdk.go b/pkg/svcat/service-catalog/sdk.go index 2bd4254388e..5ec6bb70606 100644 --- a/pkg/svcat/service-catalog/sdk.go +++ b/pkg/svcat/service-catalog/sdk.go @@ -83,6 +83,7 @@ type SvcatClient interface { RetrievePlanByID(string, ScopeOptions) (Plan, error) RetrieveSecretByBinding(*apiv1beta1.ServiceBinding) (*apicorev1.Secret, error) + CreateSecret(secret *apicorev1.Secret) (*apicorev1.Secret, error) ServerVersion() (*version.Info, error) } diff --git a/pkg/svcat/service-catalog/secret.go b/pkg/svcat/service-catalog/secret.go index 0e81e3e8af2..98b22003df7 100644 --- a/pkg/svcat/service-catalog/secret.go +++ b/pkg/svcat/service-catalog/secret.go @@ -42,3 +42,8 @@ func (sdk *SDK) RetrieveSecretByBinding(binding *v1beta1.ServiceBinding) (*corev return secret, nil } + +// CreateSecret creates a secret +func (sdk *SDK) CreateSecret(secret *corev1.Secret) (*corev1.Secret, error) { + return sdk.Core().Secrets(secret.Namespace).Create(context.Background(), secret, metav1.CreateOptions{}) +} diff --git a/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go b/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go index 1c2a87fa27c..d5473434456 100644 --- a/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go +++ b/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go @@ -238,6 +238,19 @@ type FakeSvcatClient struct { result1 servicecatalog.Broker result2 error } + CreateSecretStub func(*v1.Secret) (*v1.Secret, error) + createSecretMutex sync.RWMutex + createSecretArgsForCall []struct { + arg1 *v1.Secret + } + createSecretReturns struct { + result1 *v1.Secret + result2 error + } + createSecretReturnsOnCall map[int]struct { + result1 *v1.Secret + result2 error + } RemoveBindingFinalizerByInstanceStub func(string, string) ([]types.NamespacedName, error) removeBindingFinalizerByInstanceMutex sync.RWMutex removeBindingFinalizerByInstanceArgsForCall []struct { @@ -1580,6 +1593,24 @@ func (fake *FakeSvcatClient) ProvisionReturnsOnCall(i int, result1 *v1beta1.Serv }{result1, result2} } +func (fake *FakeSvcatClient) CreateSecret(arg1 *v1.Secret) (*v1.Secret, error) { + fake.createSecretMutex.Lock() + ret, specificReturn := fake.createSecretReturnsOnCall[len(fake.createSecretArgsForCall)] + fake.createSecretArgsForCall = append(fake.createSecretArgsForCall, struct { + arg1 *v1.Secret + }{arg1}) + fake.recordInvocation("CreateSecret", []interface{}{arg1}) + fake.createSecretMutex.Unlock() + if fake.CreateSecretStub != nil { + return fake.CreateSecretStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.createSecretReturns + return fakeReturns.result1, fakeReturns.result2 +} + func (fake *FakeSvcatClient) Register(arg1 string, arg2 string, arg3 *servicecatalog.RegisterOptions, arg4 *servicecatalog.ScopeOptions) (servicecatalog.Broker, error) { fake.registerMutex.Lock() ret, specificReturn := fake.registerReturnsOnCall[len(fake.registerArgsForCall)]