diff --git a/.travis.yml b/.travis.yml index 39c6c0477fd..8393d3d7a0f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -40,7 +40,10 @@ jobs: make verify-docs else echo "Running full build" - make verify build svcat build-integration build-e2e test-integration + # make sure code quality is good and proper + # generate the output binaries for server and client + # ensure the tests build + make verify build svcat build-integration build-e2e fi env: GO_VERSION=1.10 # Doc Site svc-cat.io diff --git a/Makefile.sc b/Makefile.sc index cbc6f170e40..6ffc05e69ff 100644 --- a/Makefile.sc +++ b/Makefile.sc @@ -146,42 +146,26 @@ $(BINDIR)/service-catalog: .init .generate_files cmd/service-catalog # This section contains the code generation stuff ################################################# -.generate_exes: $(BINDIR)/defaulter-gen \ - $(BINDIR)/deepcopy-gen \ - $(BINDIR)/conversion-gen \ - $(BINDIR)/client-gen \ - $(BINDIR)/lister-gen \ - $(BINDIR)/informer-gen \ - $(BINDIR)/openapi-gen - touch $@ - -$(BINDIR)/defaulter-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/defaulter-gen - -$(BINDIR)/deepcopy-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/deepcopy-gen - -$(BINDIR)/conversion-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/conversion-gen - -$(BINDIR)/client-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/client-gen +GENERATORS = $(addprefix $(BINDIR)/, defaulter-gen deepcopy-gen conversion-gen \ + client-gen lister-gen informer-gen openapi-gen) -$(BINDIR)/lister-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/lister-gen +.PHONY: generators +generators: $(GENERATORS) -$(BINDIR)/informer-gen: .init - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/informer-gen +.SECONDEXPANSION: -$(BINDIR)/openapi-gen: vendor/k8s.io/code-generator/cmd/openapi-gen - $(DOCKER_CMD) go build -o $@ $(SC_PKG)/$^ +# We specify broad dependencies for these generator binaries: each one depends +# on everything under its source tree as well as gengo's. This uses GNU Make's +# secondary expansion feature to pass $* to `find`. +$(BINDIR)/%-gen: $$(shell find vendor/k8s.io/code-generator/cmd/$$*-gen vendor/k8s.io/gengo) .init + $(DOCKER_CMD) go build -o $@ $(SC_PKG)/vendor/k8s.io/code-generator/cmd/$*-gen .PHONY: $(BINDIR)/e2e.test $(BINDIR)/e2e.test: .init $(DOCKER_CMD) go test -c -o $@ $(SC_PKG)/test/e2e # Regenerate all files if the gen exes changed or any "types.go" files changed -.generate_files: .init .generate_exes $(TYPES_FILES) +.generate_files: .init generators $(TYPES_FILES) # generate apiserver deps $(DOCKER_CMD) $(BUILD_DIR)/update-apiserver-gen.sh # generate all pkg/client contents @@ -242,10 +226,10 @@ verify-docs: .init @echo Running href checker$(SKIP_COMMENT): @$(DOCKER_CMD) verify-links.sh -s .pkg -s .bundler -s _plugins -s _includes -t $(SKIP_HTTP) . -verify-generated: .init .generate_exes +verify-generated: .init generators $(DOCKER_CMD) $(BUILD_DIR)/update-apiserver-gen.sh --verify-only -verify-client-gen: .init .generate_exes +verify-client-gen: .init generators $(DOCKER_CMD) $(BUILD_DIR)/verify-client-gen.sh format: .init @@ -302,7 +286,6 @@ clean: clean-bin clean-build-image clean-generated clean-coverage clean-bin: .init $(scBuildImageTarget) $(DOCKER_CMD) rm -rf $(BINDIR) - rm -f .generate_exes clean-build-image: .init $(scBuildImageTarget) $(DOCKER_CMD) rm -rf .pkg diff --git a/README.md b/README.md index d74431e9514..ea7715e7b18 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,9 @@ Interested in contributing? Check out the [contribution guidelines](./CONTRIBUTI Also see the [developer's guide](./docs/devguide.md) for information on how to build and test the code. +We have a mailing list available +[here](https://groups.google.com/forum/#!forum/kubernetes-sig-service-catalog). + We have weekly meetings - see [our SIG Readme](https://github.com/kubernetes/community/blob/master/sig-service-catalog/README.md#meetings) for details. For meeting agendas diff --git a/UPSTREAM-VERSION b/UPSTREAM-VERSION index ed8be60d7e7..fac8924a184 100644 --- a/UPSTREAM-VERSION +++ b/UPSTREAM-VERSION @@ -1 +1 @@ -v0.1.27 +v0.1.29 diff --git a/charts/catalog/Chart.yaml b/charts/catalog/Chart.yaml index 2d4108f83ed..39107c7a88d 100644 --- a/charts/catalog/Chart.yaml +++ b/charts/catalog/Chart.yaml @@ -1,3 +1,3 @@ name: catalog description: service-catalog API server and controller-manager helm chart -version: 0.1.27 +version: 0.1.29 diff --git a/charts/catalog/README.md b/charts/catalog/README.md index 2644955ed15..7ce1084a41d 100644 --- a/charts/catalog/README.md +++ b/charts/catalog/README.md @@ -40,7 +40,7 @@ chart and their default values. | Parameter | Description | Default | |-----------|-------------|---------| -| `image` | apiserver image to use | `quay.io/kubernetes-service-catalog/service-catalog:v0.1.27` | +| `image` | apiserver image to use | `quay.io/kubernetes-service-catalog/service-catalog:v0.1.29` | | `imagePullPolicy` | `imagePullPolicy` for the service catalog | `Always` | | `apiserver.annotations` | Annotations for apiserver pods | `{}` | | `apiserver.nodeSelector` | A nodeSelector value to apply to the apiserver pods. If not specified, no nodeSelector will be applied | | @@ -84,6 +84,7 @@ chart and their default values. | `rbacEnable` | If true, create & use RBAC resources | `true` | | `originatingIdentityEnabled` | Whether the OriginatingIdentity alpha feature should be enabled | `false` | | `asyncBindingOperationsEnabled` | Whether or not alpha support for async binding operations is enabled | `false` | +| `namespacedServiceBrokerDisabled` | Whether or not alpha support for namespace scoped brokers is disabled | `false` | Specify each parameter using the `--set key=value[,key=value]` argument to `helm install`. diff --git a/charts/catalog/templates/apiserver-deployment.yaml b/charts/catalog/templates/apiserver-deployment.yaml index 1555cbfae8d..4f2e112809f 100644 --- a/charts/catalog/templates/apiserver-deployment.yaml +++ b/charts/catalog/templates/apiserver-deployment.yaml @@ -39,7 +39,7 @@ spec: - {{ .Values.apiserver.audit.logPath }} {{- end}} - --enable-admission-plugins - - "KubernetesNamespaceLifecycle,DefaultServicePlan,ServiceBindingsLifecycle,ServicePlanChangeValidator,BrokerAuthSarCheck" + - "NamespaceLifecycle,DefaultServicePlan,ServiceBindingsLifecycle,ServicePlanChangeValidator,BrokerAuthSarCheck" - --secure-port - "8443" - --storage-type @@ -60,9 +60,9 @@ spec: - --feature-gates - OriginatingIdentity=true {{- end }} - {{- if .Values.namespacedServiceBrokerEnabled }} + {{- if .Values.namespacedServiceBrokerDisabled }} - --feature-gates - - NamespacedServiceBroker=true + - NamespacedServiceBroker=false {{- end }} {{- if .Values.apiserver.serveOpenAPISpec }} - --serve-openapi-spec diff --git a/charts/catalog/templates/controller-manager-deployment.yaml b/charts/catalog/templates/controller-manager-deployment.yaml index 1c11d223efb..54dd2e2b70c 100644 --- a/charts/catalog/templates/controller-manager-deployment.yaml +++ b/charts/catalog/templates/controller-manager-deployment.yaml @@ -81,9 +81,9 @@ spec: - --feature-gates - CatalogRestrictions=true {{- end }} - {{- if .Values.namespacedServiceBrokerEnabled }} + {{- if .Values.namespacedServiceBrokerDisabled }} - --feature-gates - - NamespacedServiceBroker=true + - NamespacedServiceBroker=false {{- end }} ports: - containerPort: 8444 diff --git a/charts/catalog/templates/rbac.yaml b/charts/catalog/templates/rbac.yaml index c0968649813..33083829431 100644 --- a/charts/catalog/templates/rbac.yaml +++ b/charts/catalog/templates/rbac.yaml @@ -106,7 +106,7 @@ items: - apiGroups: ["servicecatalog.k8s.io"] resources: ["clusterservicebrokers/status","clusterserviceclasses/status","clusterserviceplans/status","serviceinstances/status","serviceinstances/reference","servicebindings/status"] verbs: ["update"] - {{- if .Values.namespacedServiceBrokerEnabled }} + {{- if not .Values.namespacedServiceBrokerDisabled }} - apiGroups: ["servicecatalog.k8s.io"] resources: ["serviceclasses"] verbs: ["get","list","watch","create","patch","update","delete"] diff --git a/charts/catalog/values.yaml b/charts/catalog/values.yaml index 82d927af54f..9038549bedd 100644 --- a/charts/catalog/values.yaml +++ b/charts/catalog/values.yaml @@ -1,6 +1,6 @@ # Default values for Service Catalog # service-catalog image to use -image: quay.io/kubernetes-service-catalog/service-catalog:v0.1.27 +image: quay.io/kubernetes-service-catalog/service-catalog:v0.1.29 # imagePullPolicy for the service-catalog; valid values are "IfNotPresent", # "Never", and "Always" imagePullPolicy: Always @@ -152,5 +152,5 @@ controllerManager: originatingIdentityEnabled: false # Whether the AsyncBindingOperations alpha feature should be enabled asyncBindingOperationsEnabled: false -# Whether the NamespacedServiceBroker alpha feature should be enabled -namespacedServiceBrokerEnabled: false +# Whether the NamespacedServiceBroker alpha feature should be disabled +namespacedServiceBrokerDisabled: false diff --git a/charts/ups-broker/README.md b/charts/ups-broker/README.md index d40dfd0cdcb..2b3e6e91fab 100644 --- a/charts/ups-broker/README.md +++ b/charts/ups-broker/README.md @@ -34,7 +34,7 @@ Service Broker | Parameter | Description | Default | |-----------|-------------|---------| -| `image` | Image to use | `quay.io/kubernetes-service-catalog/user-broker:v0.1.27` | +| `image` | Image to use | `quay.io/kubernetes-service-catalog/user-broker:v0.1.29` | | `imagePullPolicy` | `imagePullPolicy` for the ups-broker | `Always` | Specify each parameter using the `--set key=value[,key=value]` argument to diff --git a/charts/ups-broker/values.yaml b/charts/ups-broker/values.yaml index d3b5c9efeac..d6970c88fd1 100644 --- a/charts/ups-broker/values.yaml +++ b/charts/ups-broker/values.yaml @@ -1,6 +1,6 @@ # Default values for User-Provided Service Broker # Image to use -image: quay.io/kubernetes-service-catalog/user-broker:v0.1.27 +image: quay.io/kubernetes-service-catalog/user-broker:v0.1.29 # ImagePullPolicy; valid values are "IfNotPresent", "Never", and "Always" imagePullPolicy: Always # Whether the broker should also log to stderr instead of to files only diff --git a/cmd/apiserver/app/server/plugins.go b/cmd/apiserver/app/server/plugins.go index a772d783058..3015566e6d3 100644 --- a/cmd/apiserver/app/server/plugins.go +++ b/cmd/apiserver/app/server/plugins.go @@ -24,7 +24,6 @@ import ( // Admission controllers "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/broker/authsarcheck" - "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/namespace/lifecycle" siclifecycle "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/servicebindings/lifecycle" "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/serviceplan/changevalidator" "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/serviceplan/defaultserviceplan" @@ -32,7 +31,6 @@ import ( // registerAllAdmissionPlugins registers all admission plugins func registerAllAdmissionPlugins(plugins *admission.Plugins) { - lifecycle.Register(plugins) defaultserviceplan.Register(plugins) siclifecycle.Register(plugins) changevalidator.Register(plugins) diff --git a/cmd/svcat/broker/describe_cmd_test.go b/cmd/svcat/broker/describe_cmd_test.go index 63e001c0bed..a0fb0d2751e 100644 --- a/cmd/svcat/broker/describe_cmd_test.go +++ b/cmd/svcat/broker/describe_cmd_test.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" k8sfake "k8s.io/client-go/kubernetes/fake" - "github.com/kubernetes-incubator/service-catalog/cmd/svcat/command" _ "github.com/kubernetes-incubator/service-catalog/internal/test" ) @@ -78,9 +77,8 @@ func TestDescribeCommand(t *testing.T) { cxt := svcattest.NewContext(output, fakeApp) // Initialize the command arguments - cmd := &getCmd{ - Context: cxt, - Formatted: command.NewFormatted(), + cmd := &describeCmd{ + Context: cxt, } cmd.name = tc.brokerName diff --git a/cmd/svcat/broker/get_cmd.go b/cmd/svcat/broker/get_cmd.go index fd598758c4d..b2e6060f225 100644 --- a/cmd/svcat/broker/get_cmd.go +++ b/cmd/svcat/broker/get_cmd.go @@ -19,33 +19,40 @@ package broker import ( "github.com/kubernetes-incubator/service-catalog/cmd/svcat/command" "github.com/kubernetes-incubator/service-catalog/cmd/svcat/output" + "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog" "github.com/spf13/cobra" ) type getCmd struct { - *command.Context + *command.Namespaced *command.Formatted + *command.Scoped name string } // NewGetCmd builds a "svcat get brokers" command func NewGetCmd(cxt *command.Context) *cobra.Command { getCmd := &getCmd{ - Context: cxt, - Formatted: command.NewFormatted(), + Namespaced: command.NewNamespaced(cxt), + Formatted: command.NewFormatted(), + Scoped: command.NewScoped(), } cmd := &cobra.Command{ Use: "brokers [NAME]", Aliases: []string{"broker", "brk"}, - Short: "List brokers, optionally filtered by name", + Short: "List brokers, optionally filtered by name, scope or namespace", Example: command.NormalizeExamples(` svcat get brokers - svcat get broker asb + svcat get brokers --scope=cluster + svcat get brokers --scope=all + svcat get broker minibroker `), PreRunE: command.PreRunE(getCmd), RunE: command.RunE(getCmd), } getCmd.AddOutputFlags(cmd.Flags()) + getCmd.AddScopedFlags(cmd.Flags(), true) + getCmd.AddNamespaceFlags(cmd.Flags(), true) return cmd } @@ -66,7 +73,11 @@ func (c *getCmd) Run() error { } func (c *getCmd) getAll() error { - brokers, err := c.App.RetrieveBrokers() + opts := servicecatalog.ScopeOptions{ + Namespace: c.Namespace, + Scope: c.Scope, + } + brokers, err := c.App.RetrieveBrokers(opts) if err != nil { return err } diff --git a/cmd/svcat/broker/get_cmd_test.go b/cmd/svcat/broker/get_cmd_test.go new file mode 100644 index 00000000000..ce9954697bd --- /dev/null +++ b/cmd/svcat/broker/get_cmd_test.go @@ -0,0 +1,158 @@ +/* +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 ( + "bytes" + + "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" + "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("Get Broker Command", func() { + Describe("NewGetBrokerCmd", func() { + It("Builds and returns a cobra command", func() { + cxt := &command.Context{} + cmd := NewGetCmd(cxt) + Expect(*cmd).NotTo(BeNil()) + Expect(cmd.Use).To(Equal("brokers [NAME]")) + Expect(cmd.Short).To(ContainSubstring("List brokers, optionally filtered by name, scope or namespace")) + Expect(cmd.Example).To(ContainSubstring("svcat get brokers")) + Expect(cmd.Example).To(ContainSubstring("svcat get brokers --scope=cluster")) + Expect(cmd.Example).To(ContainSubstring("svcat get brokers --scope=all")) + Expect(cmd.Example).To(ContainSubstring("svcat get broker minibroker")) + Expect(len(cmd.Aliases)).To(Equal(2)) + }) + }) + Describe("Validate", func() { + It("allows broker name arg to be empty", func() { + cmd := &getCmd{} + err := cmd.Validate([]string{}) + Expect(err).To(BeNil()) + }) + It("optionally parses the broker name argument", func() { + cmd := &getCmd{} + err := cmd.Validate([]string{"minibroker"}) + Expect(err).To(BeNil()) + Expect(cmd.name).To(Equal("minibroker")) + }) + }) + Describe("Run", func() { + It("Calls the pkg/svcat libs RetrieveBrokers with namespace scope and current namespace", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveBrokersReturns( + []servicecatalog.Broker{&v1beta1.ServiceBroker{ObjectMeta: v1.ObjectMeta{Name: "minibroker", Namespace: "default"}}}, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + Formatted: command.NewFormatted(), + } + cmd.Namespace = "default" + cmd.Scope = servicecatalog.NamespaceScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveBrokersArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "default", + Scope: servicecatalog.NamespaceScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("minibroker")) + }) + It("Calls the pkg/svcat libs RetrieveBrokers with namespace scope and all namespaces", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveBrokersReturns( + []servicecatalog.Broker{ + &v1beta1.ServiceBroker{ObjectMeta: v1.ObjectMeta{Name: "minibroker", Namespace: "default"}}, + &v1beta1.ServiceBroker{ObjectMeta: v1.ObjectMeta{Name: "ups-broker", Namespace: "test-ns"}}, + }, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + Formatted: command.NewFormatted(), + } + cmd.Namespace = "" + cmd.Scope = servicecatalog.NamespaceScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveBrokersArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "", + Scope: servicecatalog.NamespaceScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("minibroker")) + Expect(output).To(ContainSubstring("ups-broker")) + }) + It("Calls the pkg/svcat libs RetrieveBrokers with all scope and current namespaces", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveBrokersReturns( + []servicecatalog.Broker{ + &v1beta1.ClusterServiceBroker{ObjectMeta: v1.ObjectMeta{Name: "global-broker"}}, + &v1beta1.ServiceBroker{ObjectMeta: v1.ObjectMeta{Name: "minibroker", Namespace: "default"}}, + }, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + Formatted: command.NewFormatted(), + } + cmd.Namespace = "default" + cmd.Scope = servicecatalog.AllScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveBrokersArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "default", + Scope: servicecatalog.AllScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("global-broker")) + Expect(output).To(ContainSubstring("minibroker")) + }) + }) +}) diff --git a/cmd/svcat/class/create_cmd.go b/cmd/svcat/class/create_cmd.go new file mode 100644 index 00000000000..118e679ab95 --- /dev/null +++ b/cmd/svcat/class/create_cmd.go @@ -0,0 +1,81 @@ +/* +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 class + +import ( + "fmt" + + "github.com/kubernetes-incubator/service-catalog/cmd/svcat/command" + "github.com/kubernetes-incubator/service-catalog/cmd/svcat/output" + "github.com/spf13/cobra" +) + +// CreateCmd contains the information needed to create a new class +type CreateCmd struct { + *command.Context + Name string + From string +} + +// NewCreateCmd builds a "svcat create class" command +func NewCreateCmd(cxt *command.Context) *cobra.Command { + createCmd := &CreateCmd{Context: cxt} + cmd := &cobra.Command{ + Use: "class [NAME] --from [EXISTING_NAME]", + Short: "Copies an existing class into a new user-defined cluster-scoped class", + Example: command.NormalizeExamples(` + svcat create class newclass --from mysqldb +`), + PreRunE: command.PreRunE(createCmd), + RunE: command.RunE(createCmd), + } + cmd.Flags().StringVarP(&createCmd.From, "from", "f", "", + "Name from an existing class that will be copied (Required)", + ) + cmd.MarkFlagRequired("from") + + return cmd +} + +// Validate checks that the required arguments have been provided +func (c *CreateCmd) Validate(args []string) error { + if len(args) <= 0 { + return fmt.Errorf("new class name should be provided") + } + + c.Name = args[0] + + return nil +} + +// Run calls out to the pkg lib to create the class and displays the output +func (c *CreateCmd) Run() error { + class, err := c.App.RetrieveClassByName(c.From) + if err != nil { + return err + } + + class.Name = c.Name + + createdClass, err := c.App.CreateClass(class) + if err != nil { + return err + } + + output.WriteClassDetails(c.Output, createdClass) + return nil +} diff --git a/cmd/svcat/class/create_cmd_test.go b/cmd/svcat/class/create_cmd_test.go new file mode 100644 index 00000000000..d752367a3c0 --- /dev/null +++ b/cmd/svcat/class/create_cmd_test.go @@ -0,0 +1,99 @@ +/* +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 class_test + +import ( + "bytes" + + . "github.com/kubernetes-incubator/service-catalog/cmd/svcat/class" + "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" + servicecatalogfakes "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("Create Command", func() { + Describe("NewCreateCmd", func() { + It("Builds and returns a cobra command", func() { + cxt := &command.Context{} + cmd := NewCreateCmd(cxt) + Expect(*cmd).NotTo(BeNil()) + Expect(cmd.Use).To(Equal("class [NAME] --from [EXISTING_NAME]")) + Expect(cmd.Short).To(ContainSubstring("Copies an existing class into a new user-defined cluster-scoped class")) + Expect(cmd.Example).To(ContainSubstring("svcat create class newclass --from mysqldb")) + Expect(len(cmd.Aliases)).To(Equal(0)) + }) + }) + Describe("Validate name is provided", func() { + It("errors if a new class name is not provided", func() { + cmd := CreateCmd{ + Context: nil, + Name: "", + From: "existingclass", + } + err := cmd.Validate([]string{}) + Expect(err).To(HaveOccurred()) + }) + }) + Describe("Validate from is provided", func() { + It("errors if a existing class name is not provided using from", func() { + cmd := CreateCmd{ + Context: nil, + Name: "newclass", + From: "", + } + err := cmd.Validate([]string{}) + Expect(err).To(HaveOccurred()) + }) + }) + Describe("Create", func() { + It("Calls the pkg/svcat libs Create method with the passed in variables and prints output to the user", func() { + className := "newclass" + existingClassName := "existingclass" + + classToReturn := &v1beta1.ClusterServiceClass{ + ObjectMeta: v1.ObjectMeta{ + Name: className, + }, + } + + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.CreateClassReturns(classToReturn, nil) + fakeApp.SvcatClient = fakeSDK + cmd := CreateCmd{ + Context: svcattest.NewContext(outputBuffer, fakeApp), + Name: className, + From: existingClassName, + } + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + class := fakeSDK.CreateClassArgsForCall(0) + Expect(class.Name).To(Equal(className)) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring(className)) + }) + }) +}) diff --git a/cmd/svcat/class/get_cmd.go b/cmd/svcat/class/get_cmd.go index ac8ce9cfd41..c65d4a784f3 100644 --- a/cmd/svcat/class/get_cmd.go +++ b/cmd/svcat/class/get_cmd.go @@ -43,9 +43,11 @@ func NewGetCmd(cxt *command.Context) *cobra.Command { cmd := &cobra.Command{ Use: "classes [NAME]", Aliases: []string{"class", "cl"}, - Short: "List classes, optionally filtered by name", + Short: "List classes, optionally filtered by name, scope or namespace", Example: command.NormalizeExamples(` svcat get classes + svcat get classes --scope cluster + svcat get classes --scope namespace --namespace dev svcat get class mysqldb svcat get class --uuid 997b8372-8dac-40ac-ae65-758b4a5075a5 `), diff --git a/cmd/svcat/class/get_cmd_test.go b/cmd/svcat/class/get_cmd_test.go index 4ec1b11b00e..1526bc4a336 100644 --- a/cmd/svcat/class/get_cmd_test.go +++ b/cmd/svcat/class/get_cmd_test.go @@ -28,6 +28,9 @@ import ( svcatfake "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/fake" "github.com/kubernetes-incubator/service-catalog/pkg/svcat" "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog" + "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog/service-catalogfakes" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" k8sfake "k8s.io/client-go/kubernetes/fake" @@ -174,3 +177,126 @@ func TestListClasses(t *testing.T) { }) } } + +var _ = Describe("Get Classes Command", func() { + Describe("NewGetClassesCmd", func() { + It("Builds and returns a cobra command", func() { + cxt := &command.Context{} + cmd := NewGetCmd(cxt) + Expect(*cmd).NotTo(BeNil()) + Expect(cmd.Use).To(Equal("classes [NAME]")) + Expect(cmd.Short).To(ContainSubstring("List classes, optionally filtered by name, scope or namespace")) + Expect(cmd.Example).To(ContainSubstring("svcat get classes")) + Expect(cmd.Example).To(ContainSubstring("svcat get classes --scope cluster")) + Expect(cmd.Example).To(ContainSubstring("svcat get classes --scope namespace --namespace dev")) + Expect(len(cmd.Aliases)).To(Equal(2)) + }) + }) + Describe("Validate", func() { + It("allows class name arg to be empty", func() { + cmd := &getCmd{} + err := cmd.Validate([]string{}) + Expect(err).To(BeNil()) + }) + It("optionally parses the class name argument", func() { + cmd := &getCmd{} + err := cmd.Validate([]string{"mysqldb"}) + Expect(err).To(BeNil()) + Expect(cmd.name).To(Equal("mysqldb")) + }) + }) + Describe("Run", func() { + It("Calls the pkg/svcat libs RetrieveClasses with namespace scope and current namespace", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveClassesReturns( + []servicecatalog.Class{&v1beta1.ServiceClass{ObjectMeta: metav1.ObjectMeta{Name: "mysqldb", Namespace: "default"}}}, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + } + cmd.Namespace = "default" + cmd.Scope = servicecatalog.NamespaceScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveClassesArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "default", + Scope: servicecatalog.NamespaceScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("mysqldb")) + }) + It("Calls the pkg/svcat libs RetrieveClasses with namespace scope and all namespaces", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveClassesReturns( + []servicecatalog.Class{ + &v1beta1.ServiceClass{ObjectMeta: metav1.ObjectMeta{Name: "mysqldb", Namespace: "default"}}, + &v1beta1.ServiceClass{ObjectMeta: metav1.ObjectMeta{Name: "postgresdb", Namespace: "test-ns"}}, + }, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + } + cmd.Namespace = "" + cmd.Scope = servicecatalog.NamespaceScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveClassesArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "", + Scope: servicecatalog.NamespaceScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("mysqldb")) + Expect(output).To(ContainSubstring("postgresdb")) + }) + It("Calls the pkg/svcat libs RetrieveClasses with all scope and current namespaces", func() { + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RetrieveClassesReturns( + []servicecatalog.Class{ + &v1beta1.ClusterServiceClass{ObjectMeta: metav1.ObjectMeta{Name: "mysqldb"}}, + &v1beta1.ServiceClass{ObjectMeta: metav1.ObjectMeta{Name: "postgresdb", Namespace: "default"}}, + }, + nil) + fakeApp.SvcatClient = fakeSDK + cmd := getCmd{ + Namespaced: &command.Namespaced{Context: svcattest.NewContext(outputBuffer, fakeApp)}, + Scoped: command.NewScoped(), + } + cmd.Namespace = "default" + cmd.Scope = servicecatalog.AllScope + + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + scopeArg := fakeSDK.RetrieveClassesArgsForCall(0) + Expect(scopeArg).To(Equal(servicecatalog.ScopeOptions{ + Namespace: "default", + Scope: servicecatalog.AllScope, + })) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring("mysqldb")) + Expect(output).To(ContainSubstring("postgresdb")) + }) + }) +}) diff --git a/cmd/svcat/main.go b/cmd/svcat/main.go index 5ed5d261126..3804dca5697 100644 --- a/cmd/svcat/main.go +++ b/cmd/svcat/main.go @@ -115,6 +115,7 @@ func buildRootCommand(cxt *command.Context) *cobra.Command { cmd.PersistentFlags().StringVar(&opts.KubeContext, "context", "", "name of the kubeconfig context to use.") cmd.PersistentFlags().StringVar(&opts.KubeConfig, "kubeconfig", "", "path to kubeconfig file. Overrides $KUBECONFIG") + cmd.AddCommand(newCreateCmd(cxt)) cmd.AddCommand(newGetCmd(cxt)) cmd.AddCommand(newDescribeCmd(cxt)) cmd.AddCommand(broker.NewRegisterCmd(cxt)) @@ -144,6 +145,16 @@ func newSyncCmd(cxt *command.Context) *cobra.Command { return cmd } +func newCreateCmd(cxt *command.Context) *cobra.Command { + cmd := &cobra.Command{ + Use: "create", + Short: "Create a user-defined resource", + } + cmd.AddCommand(class.NewCreateCmd(cxt)) + + return cmd +} + func newGetCmd(cxt *command.Context) *cobra.Command { cmd := &cobra.Command{ Use: "get", diff --git a/cmd/svcat/output/broker.go b/cmd/svcat/output/broker.go index 9c326adaa2b..ee1eeb768a3 100644 --- a/cmd/svcat/output/broker.go +++ b/cmd/svcat/output/broker.go @@ -17,56 +17,55 @@ limitations under the License. package output import ( - "fmt" "io" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog" ) -func getBrokerStatusCondition(status v1beta1.ClusterServiceBrokerStatus) v1beta1.ServiceBrokerCondition { +func getBrokerStatusCondition(status v1beta1.CommonServiceBrokerStatus) v1beta1.ServiceBrokerCondition { if len(status.Conditions) > 0 { return status.Conditions[len(status.Conditions)-1] } return v1beta1.ServiceBrokerCondition{} } -func getBrokerStatusShort(status v1beta1.ClusterServiceBrokerStatus) string { +func getBrokerStatusShort(status v1beta1.CommonServiceBrokerStatus) string { lastCond := getBrokerStatusCondition(status) return formatStatusShort(string(lastCond.Type), lastCond.Status, lastCond.Reason) } -func getBrokerStatusFull(status v1beta1.ClusterServiceBrokerStatus) string { +func getBrokerStatusFull(status v1beta1.CommonServiceBrokerStatus) string { lastCond := getBrokerStatusCondition(status) return formatStatusFull(string(lastCond.Type), lastCond.Status, lastCond.Reason, lastCond.Message, lastCond.LastTransitionTime) } -func writeBrokerListTable(w io.Writer, brokers []v1beta1.ClusterServiceBroker) { +func writeBrokerListTable(w io.Writer, brokers []servicecatalog.Broker) { t := NewListTable(w) t.SetHeader([]string{ "Name", + "Namespace", "URL", "Status", }) for _, broker := range brokers { t.Append([]string{ - broker.Name, - broker.Spec.URL, - getBrokerStatusShort(broker.Status), + broker.GetName(), + broker.GetNamespace(), + broker.GetURL(), + getBrokerStatusShort(broker.GetStatus()), }) } t.Render() } // WriteBrokerList prints a list of brokers in the specified output format. -func WriteBrokerList(w io.Writer, outputFormat string, brokers ...v1beta1.ClusterServiceBroker) { - l := v1beta1.ClusterServiceBrokerList{ - Items: brokers, - } +func WriteBrokerList(w io.Writer, outputFormat string, brokers ...servicecatalog.Broker) { switch outputFormat { case FormatJSON: - writeJSON(w, l) + writeJSON(w, brokers) case FormatYAML: - writeYAML(w, l, 0) + writeYAML(w, brokers, 0) case FormatTable: writeBrokerListTable(w, brokers) } @@ -80,29 +79,18 @@ func WriteBroker(w io.Writer, outputFormat string, broker v1beta1.ClusterService case FormatYAML: writeYAML(w, broker, 0) case FormatTable: - writeBrokerListTable(w, []v1beta1.ClusterServiceBroker{broker}) + writeBrokerListTable(w, []servicecatalog.Broker{&broker}) } } -// WriteParentBroker prints identifying information for a parent broker. -func WriteParentBroker(w io.Writer, broker *v1beta1.ClusterServiceBroker) { - fmt.Fprintln(w, "\nBroker:") - t := NewDetailsTable(w) - t.AppendBulk([][]string{ - {"Name:", broker.Name}, - {"Status:", getBrokerStatusShort(broker.Status)}, - }) - t.Render() -} - // WriteBrokerDetails prints details for a single broker. -func WriteBrokerDetails(w io.Writer, broker *v1beta1.ClusterServiceBroker) { +func WriteBrokerDetails(w io.Writer, broker servicecatalog.Broker) { t := NewDetailsTable(w) t.AppendBulk([][]string{ - {"Name:", broker.Name}, - {"URL:", broker.Spec.URL}, - {"Status:", getBrokerStatusFull(broker.Status)}, + {"Name:", broker.GetName()}, + {"URL:", broker.GetURL()}, + {"Status:", getBrokerStatusFull(broker.GetStatus())}, }) t.Render() diff --git a/cmd/svcat/output/class.go b/cmd/svcat/output/class.go index a0c8d9bac0a..eb994d20134 100644 --- a/cmd/svcat/output/class.go +++ b/cmd/svcat/output/class.go @@ -33,11 +33,14 @@ func getClassStatusText(status v1beta1.ClusterServiceClassStatus) string { func writeClassListTable(w io.Writer, classes []servicecatalog.Class) { t := NewListTable(w) + t.SetHeader([]string{ "Name", "Namespace", "Description", }) + t.SetVariableColumn(3) + for _, class := range classes { t.Append([]string{ class.GetExternalName(), @@ -45,6 +48,7 @@ func writeClassListTable(w io.Writer, classes []servicecatalog.Class) { class.GetDescription(), }) } + t.Render() } diff --git a/cmd/svcat/output/instance.go b/cmd/svcat/output/instance.go index bd881f372e9..7077bac1be0 100644 --- a/cmd/svcat/output/instance.go +++ b/cmd/svcat/output/instance.go @@ -21,6 +21,7 @@ import ( "io" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/olekukonko/tablewriter" ) func getInstanceStatusCondition(status v1beta1.ServiceInstanceStatus) v1beta1.ServiceInstanceCondition { @@ -40,6 +41,15 @@ func getInstanceStatusShort(status v1beta1.ServiceInstanceStatus) string { return formatStatusShort(string(lastCond.Type), lastCond.Status, lastCond.Reason) } +func appendInstanceDashboardURL(status v1beta1.ServiceInstanceStatus, table *tablewriter.Table) { + if status.DashboardURL != nil { + dashboardURL := *status.DashboardURL + table.AppendBulk([][]string{ + {"DashboardURL:", dashboardURL}, + }) + } +} + func writeInstanceListTable(w io.Writer, instanceList *v1beta1.ServiceInstanceList) { t := NewListTable(w) t.SetHeader([]string{ @@ -133,6 +143,9 @@ func WriteInstanceDetails(w io.Writer, instance *v1beta1.ServiceInstance) { {"Name:", instance.Name}, {"Namespace:", instance.Namespace}, {"Status:", getInstanceStatusFull(instance.Status)}, + }) + appendInstanceDashboardURL(instance.Status, t) + t.AppendBulk([][]string{ {"Class:", instance.Spec.GetSpecifiedClusterServiceClass()}, {"Plan:", instance.Spec.GetSpecifiedClusterServicePlan()}, }) diff --git a/cmd/svcat/output/instance_test.go b/cmd/svcat/output/instance_test.go new file mode 100644 index 00000000000..17ea60d14f7 --- /dev/null +++ b/cmd/svcat/output/instance_test.go @@ -0,0 +1,55 @@ +/* +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 output + +import ( + "strings" + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/olekukonko/tablewriter" +) + +func Test_appendInstanceDashboardURL(t *testing.T) { + dashboardURL := "grafana.example.com" + table := &tablewriter.Table{} + + tests := []struct { + name string + status v1beta1.ServiceInstanceStatus + table *tablewriter.Table + expectedString string + }{ + {"dashboardURLOK", v1beta1.ServiceInstanceStatus{ + DashboardURL: &dashboardURL, + }, table, "DashboardURL: grafana.example.com"}, + {"dashboardURLEmpty", v1beta1.ServiceInstanceStatus{}, table, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var stringBuilder strings.Builder + tt.table = NewDetailsTable(&stringBuilder) + appendInstanceDashboardURL(tt.status, tt.table) + tt.table.Render() + actualString := strings.Trim(stringBuilder.String(), " \n") + + if actualString != tt.expectedString { + t.Fatalf("%v failed; expected %v; got %v", tt.name, tt.expectedString, actualString) + } + }) + } +} diff --git a/cmd/svcat/output/plan.go b/cmd/svcat/output/plan.go index 5df527d4c2c..631a5ec5480 100644 --- a/cmd/svcat/output/plan.go +++ b/cmd/svcat/output/plan.go @@ -64,6 +64,8 @@ func writePlanListTable(w io.Writer, plans []v1beta1.ClusterServicePlan, classNa plan.Spec.Description, }) } + t.SetVariableColumn(3) + t.Render() } diff --git a/cmd/svcat/output/table.go b/cmd/svcat/output/table.go index addeba9ea2a..c83ce729321 100644 --- a/cmd/svcat/output/table.go +++ b/cmd/svcat/output/table.go @@ -22,12 +22,127 @@ import ( "github.com/olekukonko/tablewriter" ) +// DefaultPageWidth is the page (screen) width to use when we need to twiddle +// the width of some table columns for better viewing. For now assume it's only +// 80, but if we can figure out a nice (quick) way to determine this for all +// platforms, include Windows, then we should probably use that value instead. +const DefaultPageWidth = 80 + +// ListTable is a proxy for 'tablewriter.Table' so we can support a variable +// width column that tries to fill up extra space on the line when needed. +// For each func on tablewriter.Table we use we'll need a proxy func. +// We save the headers and rows and only send them on when Render() is called +// because the tablewriter stuff won't respect the call to SetColMinWidth +// if it's called after some rows have been added. So we need to calc the +// value of our special column first, call SetColMinWidth and then add the +// headers/rows. +type ListTable struct { + table *tablewriter.Table + + columnWidths []int // Max width of data in column we've seen + variableColumn int // 0 == not set + pageWidth int // Defaults to 80 + headers []string + rows [][]string +} + +// SetBorder is a proxy/pass-thru to the tablewriter.Table's func +func (lt *ListTable) SetBorder(b bool) { lt.table.SetBorder(b) } + +// SetVariableColumn tells us which column, if any, should be of variable +// width so that the table takes up the width of the screen rather than +// wrapping cells in this column prematurely. +func (lt *ListTable) SetVariableColumn(c int) { lt.variableColumn = c } + +// SetColMinWidth is a proxy/pass-thru to the tablewriter.Table's func +func (lt *ListTable) SetColMinWidth(c, w int) { lt.table.SetColMinWidth(c, w) } + +// SetPageWidth allows us to change the screen/page width. +// Probably not used right now, so it's just for future need. +func (lt *ListTable) SetPageWidth(w int) { lt.pageWidth = w } + +// SetHeader tracks the width of each header value as we save them. +func (lt *ListTable) SetHeader(keys []string) { + // Expand our slice if needed + if tmp := (len(keys) - len(lt.columnWidths)); tmp > 0 { + lt.columnWidths = append(lt.columnWidths, make([]int, tmp)...) + } + + for i, header := range keys { + if tmp := len(header); tmp > lt.columnWidths[i] { + lt.columnWidths[i] = tmp + } + } + + // Save the headers for when we call Render + lt.headers = keys +} + +// Append will look at each column in the row to see if it's longer than any +// previous value, and save it if so. Then it saves the data for later +// rendering. +func (lt *ListTable) Append(row []string) { + // Expand our slice if needed + if tmp := (len(row) - len(lt.columnWidths)); tmp > 0 { + lt.columnWidths = append(lt.columnWidths, make([]int, tmp)...) + } + + // Look for a wider cell than what we've seen before + for i, cell := range row { + if tmp := len(cell); tmp > lt.columnWidths[i] { + lt.columnWidths[i] = tmp + } + } + + // Just save the row for when we call Render + lt.rows = append(lt.rows, row) +} + +// Render will calc the width of the variable column if asked to. +// Then pass our headers and rows to the real Render func to display it. +func (lt *ListTable) Render() { + // If the variableColumn is out of bounds, just ignore it and render + if lt.variableColumn > 0 && lt.variableColumn <= len(lt.columnWidths)+1 { + // Add up the width of all columns except our special one + total := 2 // 2 == left border + space + for i, w := range lt.columnWidths { + if i+1 != lt.variableColumn { + total = total + 3 + w // 2 == space before/after text + col-sep + } + } + + // Space left for our special column if we use the full page width + remaining := lt.pageWidth - total - 3 // 3 == space+border of this col + + // If this column doesn't push us past the page width then just + // set it to the max cell width so Render() doesn't chop it on us. + // Otherwise, chop it so we don't go past the page width. + colWidth := lt.columnWidths[lt.variableColumn-1] + if remaining >= colWidth { + lt.SetColMinWidth(lt.variableColumn-1, colWidth) + } else { + lt.SetColMinWidth(lt.variableColumn-1, remaining) + } + } + + // Pass along all of the data (header and rows) to the real tablewriter + lt.table.SetHeader(lt.headers) + for _, row := range lt.rows { + lt.table.Append(row) + } + lt.table.Render() +} + // NewListTable builds a table formatted to list a set of results. -func NewListTable(w io.Writer) *tablewriter.Table { +func NewListTable(w io.Writer) *ListTable { t := tablewriter.NewWriter(w) t.SetBorder(false) t.SetColumnSeparator(" ") - return t + + return &ListTable{ + table: t, + pageWidth: DefaultPageWidth, + } } // NewDetailsTable builds a table formatted to list details for a single result. diff --git a/cmd/svcat/svcat_test.go b/cmd/svcat/svcat_test.go index cd3d0d56446..9a858f8d15e 100644 --- a/cmd/svcat/svcat_test.go +++ b/cmd/svcat/svcat_test.go @@ -197,6 +197,7 @@ func TestCommandOutput(t *testing.T) { {name: "get class by uuid", cmd: "get class --uuid 4f6e6cf6-ffdd-425f-a2c7-3c9258ad2468", golden: "output/get-class.txt"}, {name: "describe class by name", cmd: "describe class user-provided-service", golden: "output/describe-class.txt"}, {name: "describe class uuid", cmd: "describe class --uuid 4f6e6cf6-ffdd-425f-a2c7-3c9258ad2468", golden: "output/describe-class.txt"}, + {name: "create class", cmd: "create class new-class --from user-provided-service", golden: "output/create-class.txt"}, {name: "list all plans", cmd: "get plans", golden: "output/get-plans.txt"}, {name: "list all plans (json)", cmd: "get plans -o json", golden: "output/get-plans.json"}, diff --git a/cmd/svcat/testdata/output/completion-bash.txt b/cmd/svcat/testdata/output/completion-bash.txt index dedda5349bf..f9122a40b5e 100644 --- a/cmd/svcat/testdata/output/completion-bash.txt +++ b/cmd/svcat/testdata/output/completion-bash.txt @@ -307,6 +307,56 @@ _svcat_completion() noun_aliases=() } +_svcat_create_class() +{ + last_command="svcat_create_class" + commands=() + + flags=() + two_word_flags=() + local_nonpersistent_flags=() + flags_with_completion=() + flags_completion=() + + flags+=("--from=") + two_word_flags+=("-f") + local_nonpersistent_flags+=("--from=") + flags+=("--context=") + flags+=("--kubeconfig=") + flags+=("--logtostderr") + flags+=("--v=") + two_word_flags+=("-v") + + must_have_one_flag=() + must_have_one_flag+=("--from=") + must_have_one_flag+=("-f") + must_have_one_noun=() + noun_aliases=() +} + +_svcat_create() +{ + last_command="svcat_create" + commands=() + commands+=("class") + + flags=() + two_word_flags=() + local_nonpersistent_flags=() + flags_with_completion=() + flags_completion=() + + flags+=("--context=") + flags+=("--kubeconfig=") + flags+=("--logtostderr") + flags+=("--v=") + two_word_flags+=("-v") + + must_have_one_flag=() + must_have_one_noun=() + noun_aliases=() +} + _svcat_deprovision() { last_command="svcat_deprovision" @@ -532,9 +582,16 @@ _svcat_get_brokers() flags_with_completion=() flags_completion=() + flags+=("--all-namespaces") + local_nonpersistent_flags+=("--all-namespaces") + flags+=("--namespace=") + two_word_flags+=("-n") + local_nonpersistent_flags+=("--namespace=") flags+=("--output=") two_word_flags+=("-o") local_nonpersistent_flags+=("--output=") + flags+=("--scope=") + local_nonpersistent_flags+=("--scope=") flags+=("--context=") flags+=("--kubeconfig=") flags+=("--logtostderr") @@ -952,6 +1009,7 @@ _svcat_root_command() commands=() commands+=("bind") commands+=("completion") + commands+=("create") commands+=("deprovision") commands+=("describe") commands+=("get") diff --git a/cmd/svcat/testdata/output/completion-zsh.txt b/cmd/svcat/testdata/output/completion-zsh.txt index 05e87d1800b..195e7c1ee31 100644 --- a/cmd/svcat/testdata/output/completion-zsh.txt +++ b/cmd/svcat/testdata/output/completion-zsh.txt @@ -441,6 +441,56 @@ _svcat_completion() noun_aliases=() } +_svcat_create_class() +{ + last_command="svcat_create_class" + commands=() + + flags=() + two_word_flags=() + local_nonpersistent_flags=() + flags_with_completion=() + flags_completion=() + + flags+=("--from=") + two_word_flags+=("-f") + local_nonpersistent_flags+=("--from=") + flags+=("--context=") + flags+=("--kubeconfig=") + flags+=("--logtostderr") + flags+=("--v=") + two_word_flags+=("-v") + + must_have_one_flag=() + must_have_one_flag+=("--from=") + must_have_one_flag+=("-f") + must_have_one_noun=() + noun_aliases=() +} + +_svcat_create() +{ + last_command="svcat_create" + commands=() + commands+=("class") + + flags=() + two_word_flags=() + local_nonpersistent_flags=() + flags_with_completion=() + flags_completion=() + + flags+=("--context=") + flags+=("--kubeconfig=") + flags+=("--logtostderr") + flags+=("--v=") + two_word_flags+=("-v") + + must_have_one_flag=() + must_have_one_noun=() + noun_aliases=() +} + _svcat_deprovision() { last_command="svcat_deprovision" @@ -666,9 +716,16 @@ _svcat_get_brokers() flags_with_completion=() flags_completion=() + flags+=("--all-namespaces") + local_nonpersistent_flags+=("--all-namespaces") + flags+=("--namespace=") + two_word_flags+=("-n") + local_nonpersistent_flags+=("--namespace=") flags+=("--output=") two_word_flags+=("-o") local_nonpersistent_flags+=("--output=") + flags+=("--scope=") + local_nonpersistent_flags+=("--scope=") flags+=("--context=") flags+=("--kubeconfig=") flags+=("--logtostderr") @@ -1086,6 +1143,7 @@ _svcat_root_command() commands=() commands+=("bind") commands+=("completion") + commands+=("create") commands+=("deprovision") commands+=("describe") commands+=("get") diff --git a/cmd/svcat/testdata/output/create-class.txt b/cmd/svcat/testdata/output/create-class.txt new file mode 100644 index 00000000000..c6e69c73c7c --- /dev/null +++ b/cmd/svcat/testdata/output/create-class.txt @@ -0,0 +1,6 @@ + Name: user-provided-service + Description: A user provided service + UUID: new-class + Status: Active + Tags: + Broker: ups-broker diff --git a/cmd/svcat/testdata/output/get-broker.txt b/cmd/svcat/testdata/output/get-broker.txt index 619157f198c..fef60a89335 100644 --- a/cmd/svcat/testdata/output/get-broker.txt +++ b/cmd/svcat/testdata/output/get-broker.txt @@ -1,3 +1,3 @@ - NAME URL STATUS -+------------+-----------------------------------------------------------+--------+ - ups-broker http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready + NAME NAMESPACE URL STATUS ++------------+-----------+-----------------------------------------------------------+--------+ + ups-broker http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready diff --git a/cmd/svcat/testdata/output/get-brokers.json b/cmd/svcat/testdata/output/get-brokers.json index aa8f14a965e..21bb14c1d23 100644 --- a/cmd/svcat/testdata/output/get-brokers.json +++ b/cmd/svcat/testdata/output/get-brokers.json @@ -1,37 +1,66 @@ -{ - "metadata": {}, - "items": [ - { - "metadata": { - "name": "ups-broker", - "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/clusterservicebrokers/ups-broker", - "uid": "7b0ce3d1-f711-11e7-aa44-0242ac110005", - "resourceVersion": "103", - "generation": 2, - "creationTimestamp": "2018-01-11T20:53:30Z", - "finalizers": [ - "kubernetes-incubator/service-catalog" - ] - }, - "spec": { - "url": "http://ups-broker-ups-broker.ups-broker.svc.cluster.local", - "relistBehavior": "Duration", - "relistDuration": "15m0s", - "relistRequests": 1 - }, - "status": { - "conditions": [ - { - "type": "Ready", - "status": "True", - "lastTransitionTime": "2018-01-11T20:53:31Z", - "reason": "FetchedCatalog", - "message": "Successfully fetched catalog entries from broker." - } - ], - "reconciledGeneration": 2, - "lastCatalogRetrievalTime": "2018-01-12T02:10:27Z" - } +[ + { + "metadata": { + "name": "ups-broker", + "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/clusterservicebrokers/ups-broker", + "uid": "7b0ce3d1-f711-11e7-aa44-0242ac110005", + "resourceVersion": "103", + "generation": 2, + "creationTimestamp": "2018-01-11T20:53:30Z", + "finalizers": [ + "kubernetes-incubator/service-catalog" + ] + }, + "spec": { + "url": "http://ups-broker-ups-broker.ups-broker.svc.cluster.local", + "relistBehavior": "Duration", + "relistDuration": "15m0s", + "relistRequests": 1 + }, + "status": { + "conditions": [ + { + "type": "Ready", + "status": "True", + "lastTransitionTime": "2018-01-11T20:53:31Z", + "reason": "FetchedCatalog", + "message": "Successfully fetched catalog entries from broker." + } + ], + "reconciledGeneration": 2, + "lastCatalogRetrievalTime": "2018-01-12T02:10:27Z" } - ] -} \ No newline at end of file + }, + { + "metadata": { + "name": "ups-broker", + "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/namespaces/default/servicebrokers/ups-broker", + "uid": "7b0ce3d1-f711-11e7-aa44-0242ac110005", + "resourceVersion": "103", + "generation": 2, + "creationTimestamp": "2018-01-11T20:53:30Z", + "finalizers": [ + "kubernetes-incubator/service-catalog" + ] + }, + "spec": { + "url": "http://ups-broker-ups-broker.svc.cluster.local", + "relistBehavior": "Duration", + "relistDuration": "15m0s", + "relistRequests": 1 + }, + "status": { + "conditions": [ + { + "type": "Ready", + "status": "True", + "lastTransitionTime": "2018-01-11T20:53:31Z", + "reason": "FetchedCatalog", + "message": "Successfully fetched catalog entries from broker." + } + ], + "reconciledGeneration": 2, + "lastCatalogRetrievalTime": "2018-01-12T02:10:27Z" + } + } +] \ No newline at end of file diff --git a/cmd/svcat/testdata/output/get-brokers.txt b/cmd/svcat/testdata/output/get-brokers.txt index 619157f198c..e4723a23884 100644 --- a/cmd/svcat/testdata/output/get-brokers.txt +++ b/cmd/svcat/testdata/output/get-brokers.txt @@ -1,3 +1,4 @@ - NAME URL STATUS -+------------+-----------------------------------------------------------+--------+ - ups-broker http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready + NAME NAMESPACE URL STATUS ++------------+-----------+-----------------------------------------------------------+--------+ + ups-broker http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready + ups-broker http://ups-broker-ups-broker.svc.cluster.local Ready diff --git a/cmd/svcat/testdata/output/get-brokers.yaml b/cmd/svcat/testdata/output/get-brokers.yaml index 9ca955d3742..472675c45a4 100644 --- a/cmd/svcat/testdata/output/get-brokers.yaml +++ b/cmd/svcat/testdata/output/get-brokers.yaml @@ -1,4 +1,3 @@ -items: - metadata: creationTimestamp: 2018-01-11T20:53:30Z finalizers: @@ -22,4 +21,26 @@ items: type: Ready lastCatalogRetrievalTime: 2018-01-12T02:10:27Z reconciledGeneration: 2 -metadata: {} +- metadata: + creationTimestamp: 2018-01-11T20:53:30Z + finalizers: + - kubernetes-incubator/service-catalog + generation: 2 + name: ups-broker + resourceVersion: "103" + selfLink: /apis/servicecatalog.k8s.io/v1beta1/namespaces/default/servicebrokers/ups-broker + uid: 7b0ce3d1-f711-11e7-aa44-0242ac110005 + spec: + relistBehavior: Duration + relistDuration: 15m0s + relistRequests: 1 + url: http://ups-broker-ups-broker.svc.cluster.local + status: + conditions: + - lastTransitionTime: 2018-01-11T20:53:31Z + message: Successfully fetched catalog entries from broker. + reason: FetchedCatalog + status: "True" + type: Ready + lastCatalogRetrievalTime: 2018-01-12T02:10:27Z + reconciledGeneration: 2 diff --git a/cmd/svcat/testdata/output/get-plans.json b/cmd/svcat/testdata/output/get-plans.json index a5df3b37325..b4ed84b8fae 100644 --- a/cmd/svcat/testdata/output/get-plans.json +++ b/cmd/svcat/testdata/output/get-plans.json @@ -80,7 +80,7 @@ "spec": { "externalName": "default", "externalID": "090b5eac-dfa4-49f3-827d-8bcaf3a5bd7c", - "description": "Another sample plan description", + "description": "Another sample plan description that's really really really really really, kinda, wide", "free": true, "clusterServiceBrokerName": "ups-broker", "clusterServiceClassRef": { diff --git a/cmd/svcat/testdata/output/get-plans.txt b/cmd/svcat/testdata/output/get-plans.txt index be1664e031c..17aaa850512 100644 --- a/cmd/svcat/testdata/output/get-plans.txt +++ b/cmd/svcat/testdata/output/get-plans.txt @@ -1,7 +1,8 @@ - NAME CLASS DESCRIPTION -+---------+--------------------------+--------------------------------+ - default user-provided-service Sample plan description - premium user-provided-service Premium plan - default another-provided-service Another sample plan - description - premium another-provided-service Another premium plan + NAME CLASS DESCRIPTION ++---------+--------------------------+----------------------------------------+ + default user-provided-service Sample plan description + premium user-provided-service Premium plan + default another-provided-service Another sample plan description that's + really really really really really, + kinda, wide + premium another-provided-service Another premium plan diff --git a/cmd/svcat/testdata/output/get-plans.yaml b/cmd/svcat/testdata/output/get-plans.yaml index 6f8feb4eed0..1eafb6ce844 100644 --- a/cmd/svcat/testdata/output/get-plans.yaml +++ b/cmd/svcat/testdata/output/get-plans.yaml @@ -57,7 +57,8 @@ items: clusterServiceBrokerName: ups-broker clusterServiceClassRef: name: f1a80068-e366-494e-92d6-a0782337945b - description: Another sample plan description + description: Another sample plan description that's really really really really + really, kinda, wide externalID: 090b5eac-dfa4-49f3-827d-8bcaf3a5bd7c externalName: default free: true diff --git a/cmd/svcat/testdata/plugin.yaml b/cmd/svcat/testdata/plugin.yaml index bf58839ad35..1e3b2585ea1 100644 --- a/cmd/svcat/testdata/plugin.yaml +++ b/cmd/svcat/testdata/plugin.yaml @@ -60,6 +60,20 @@ tree: Svcat shell completion\\nsource '$HOME/.svcat/svcat_completion.bash.inc'\\n\" >> $HOME/.bash_profile\n source $HOME/.bash_profile" command: ./svcat completion +- name: create + use: create + shortDesc: Create a user-defined resource + command: ./svcat create + tree: + - name: class + use: class [NAME] --from [EXISTING_NAME] + shortDesc: Copies an existing class into a new user-defined cluster-scoped class + example: ' svcat create class newclass --from mysqldb' + command: ./svcat create class + flags: + - name: from + shorthand: f + desc: Name from an existing class that will be copied (Required) - name: deprovision use: deprovision NAME shortDesc: Deletes an instance of a service @@ -146,21 +160,30 @@ tree: present, defaults to table - name: brokers use: brokers [NAME] - shortDesc: List brokers, optionally filtered by name + shortDesc: List brokers, optionally filtered by name, scope or namespace example: |2- svcat get brokers - svcat get broker asb + svcat get brokers --scope=cluster + svcat get brokers --scope=all + svcat get broker minibroker command: ./svcat get brokers flags: + - name: all-namespaces + desc: If present, list the requested object(s) across all namespaces. Namespace + in current context is ignored even if specified with --namespace - name: output shorthand: o desc: The output format to use. Valid options are table, json or yaml. If not present, defaults to table + - name: scope + desc: 'Limit the results to a particular scope: cluster, namespace or all' - name: classes use: classes [NAME] - shortDesc: List classes, optionally filtered by name + shortDesc: List classes, optionally filtered by name, scope or namespace example: |2- svcat get classes + svcat get classes --scope cluster + svcat get classes --scope namespace --namespace dev svcat get class mysqldb svcat get class --uuid 997b8372-8dac-40ac-ae65-758b4a5075a5 command: ./svcat get classes diff --git a/cmd/svcat/testdata/responses/catalog/clusterserviceplans.json b/cmd/svcat/testdata/responses/catalog/clusterserviceplans.json index 8371a10ac2c..b2b4f7db6ca 100644 --- a/cmd/svcat/testdata/responses/catalog/clusterserviceplans.json +++ b/cmd/svcat/testdata/responses/catalog/clusterserviceplans.json @@ -86,7 +86,7 @@ "clusterServiceBrokerName": "ups-broker", "externalName": "default", "externalID": "090b5eac-dfa4-49f3-827d-8bcaf3a5bd7c", - "description": "Another sample plan description", + "description": "Another sample plan description that's really really really really really, kinda, wide", "free": true, "clusterServiceClassRef": { "name": "f1a80068-e366-494e-92d6-a0782337945b" diff --git a/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers.json b/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers.json new file mode 100644 index 00000000000..c39af62d821 --- /dev/null +++ b/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers.json @@ -0,0 +1,42 @@ +{ + "kind": "ServiceBrokerList", + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "metadata": { + "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/servicebrokers", + "resourceVersion": "106" + }, + "items": [ + { + "metadata": { + "name": "ups-broker", + "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/namespaces/default/servicebrokers/ups-broker", + "uid": "7b0ce3d1-f711-11e7-aa44-0242ac110005", + "resourceVersion": "103", + "generation": 2, + "creationTimestamp": "2018-01-11T20:53:30Z", + "finalizers": [ + "kubernetes-incubator/service-catalog" + ] + }, + "spec": { + "url": "http://ups-broker-ups-broker.svc.cluster.local", + "relistBehavior": "Duration", + "relistDuration": "15m0s", + "relistRequests": 1 + }, + "status": { + "conditions": [ + { + "type": "Ready", + "status": "True", + "lastTransitionTime": "2018-01-11T20:53:31Z", + "reason": "FetchedCatalog", + "message": "Successfully fetched catalog entries from broker." + } + ], + "reconciledGeneration": 2, + "lastCatalogRetrievalTime": "2018-01-12T02:10:27Z" + } + } + ] +} diff --git a/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers/ups-broker.json b/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers/ups-broker.json new file mode 100644 index 00000000000..53441ae6a67 --- /dev/null +++ b/cmd/svcat/testdata/responses/catalog/namespaces/default/servicebrokers/ups-broker.json @@ -0,0 +1,34 @@ +{ + "kind": "ServiceBroker", + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "metadata": { + "name": "ups-broker", + "selfLink": "/apis/servicecatalog.k8s.io/v1beta1/namespaces/default/servicebrokers/ups-broker", + "uid": "7b0ce3d1-f711-11e7-aa44-0242ac110005", + "resourceVersion": "103", + "generation": 2, + "creationTimestamp": "2018-01-11T20:53:30Z", + "finalizers": [ + "kubernetes-incubator/service-catalog" + ] + }, + "spec": { + "url": "http://ups-broker-ups-broker.ups-broker.svc.cluster.local", + "relistBehavior": "Duration", + "relistDuration": "15m0s", + "relistRequests": 1 + }, + "status": { + "conditions": [ + { + "type": "Ready", + "status": "True", + "lastTransitionTime": "2018-01-11T20:53:31Z", + "reason": "FetchedCatalog", + "message": "Successfully fetched catalog entries from broker." + } + ], + "reconciledGeneration": 2, + "lastCatalogRetrievalTime": "2018-01-12T02:10:27Z" + } +} diff --git a/contrib/examples/consumer/main.go b/contrib/examples/consumer/main.go index 29d4286dd54..7f1e7318834 100644 --- a/contrib/examples/consumer/main.go +++ b/contrib/examples/consumer/main.go @@ -20,12 +20,13 @@ import ( "fmt" "github.com/kubernetes-incubator/service-catalog/pkg/svcat" + "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog" ) func main() { a, _ := svcat.NewApp(nil, nil, "") - brokers, _ := a.RetrieveBrokers() + brokers, _ := a.RetrieveBrokers(servicecatalog.ScopeOptions{Scope: servicecatalog.AllScope}) for _, b := range brokers { - fmt.Println(b.Name) + fmt.Println(b.GetName()) } } diff --git a/docs/cli.md b/docs/cli.md index cb0c911fa85..fb10e2987af 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -37,11 +37,34 @@ Below are some common tasks made easy with svcat. The example output assumes tha ## Find brokers installed on the cluster +This lists all brokers available in the current namespace and at the cluster scope. + ```console $ svcat get brokers - NAME URL STATUS -+------------+-----------------------------------------------------------+--------+ - ups-broker http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready + NAME NAMESPACE URL STATUS ++--------------------+------------+-----------------------------------------------------------+--------+ + minibroker http://minibroker-minibroker.minibroker.svc.cluster.local Ready + myminibroker myspace http://minibroker-minibroker.minibroker.svc.cluster.local Ready +``` + +Use the `--namespace` and `--all-namespaces` flags to control which namespace to view: + +```console +$ svcat get brokers --namespace default + NAME NAMESPACE URL STATUS ++--------------------+-----------+-----------------------------------------------------------+--------+ + minibroker http://minibroker-minibroker.minibroker.svc.cluster.local Ready + ups-broker default http://ups-broker-ups-broker.ups-broker.svc.cluster.local Ready +``` + +You can view only cluster-scoped brokers with the `--scope` flag: + +``` +$ svcat get brokers --scope cluster + NAME NAMESPACE URL STATUS ++-------------------+------------+-----------------------------------------------------------+--------+ + minibroker http://minibroker-minibroker.minibroker.svc.cluster.local Ready + ``` ## Trigger a sync of a broker's catalog @@ -109,6 +132,19 @@ Plans: premium Premium plan ``` +## Copies an exisitng class into a new user-defined class + +This copies an exisitng class specified by name into a new user-defined one with new specified name. +```console +$ svcat create class new-class --from user-provided-service + Name: user-provided-service + Description: A user provided service + UUID: new-class + Status: Active + Tags: + Broker: ups-broker +``` + ## Provision a service ```console diff --git a/docs/namespaced-broker-resources.md b/docs/namespaced-broker-resources.md index 69bfc91d499..1725178202e 100644 --- a/docs/namespaced-broker-resources.md +++ b/docs/namespaced-broker-resources.md @@ -71,21 +71,21 @@ services and plans, however, can be effectively combined with Kubernetes RBAC and Service Catalog [Catalog Restrictions](catalog-restrictions.md) in order to provide more granular control over service instance provisioning. -## Enabling Namespace Scoped Broker Resources +## Disabling Namespace Scoped Broker Resources Currently, namespace-scoped broker resources are an alpha-feature of Service -Catalog behind a feature flag. To start using these resources, you will need +Catalog that is on by default. To disable use of these resources, you will need to pass an argument to the API Server when you install Service Catalog: - `--feature-gates NamespacedServiceBroker=true`. + `--feature-gates NamespacedServiceBroker=false`. -If you are using Helm, you can use the `namespacedServiceBrokerEnabled` setting +If you are using Helm, you can use the `namespacedServiceBrokerDisabled` setting to control that flag: ```console helm install svc-cat/catalog \ --name catalog \ --namespace catalog \ - --set namespacedServiceBrokerEnabled=true + --set namespacedServiceBrokerDisabled=true ``` ## Using Namespace Scoped Broker Resources diff --git a/docs/resources.md b/docs/resources.md index 428dcf4caa7..895b6dcabf2 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -165,7 +165,7 @@ spec: ### Service Instance Parameters -Each `ServiceInstance` has a `paramters` field that you can add +Each `ServiceInstance` has a `parameters` field that you can add metadata to. Service Catalog passes this metadata directly through to the service broker. diff --git a/pkg/apis/servicecatalog/v1beta1/broker.go b/pkg/apis/servicecatalog/v1beta1/broker.go new file mode 100644 index 00000000000..f5ac5ac14a4 --- /dev/null +++ b/pkg/apis/servicecatalog/v1beta1/broker.go @@ -0,0 +1,57 @@ +/* +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 v1beta1 + +// GetName returns the broker's name. +func (b *ClusterServiceBroker) GetName() string { + return b.Name +} + +// GetName returns the broker's name. +func (b *ServiceBroker) GetName() string { + return b.Name +} + +// GetNamespace always returns "", because it's cluster-scoped. +func (b *ClusterServiceBroker) GetNamespace() string { + return "" +} + +// GetNamespace returns the broker's namespace. +func (b *ServiceBroker) GetNamespace() string { + return b.Namespace +} + +// GetURL returns the broker's endpoint URL. +func (b *ClusterServiceBroker) GetURL() string { + return b.Spec.URL +} + +// GetURL returns the broker's endpoint URL. +func (b *ServiceBroker) GetURL() string { + return b.Spec.URL +} + +// GetStatus returns the broker status. +func (b *ClusterServiceBroker) GetStatus() CommonServiceBrokerStatus { + return b.Status.CommonServiceBrokerStatus +} + +// GetStatus returns the broker status. +func (b *ServiceBroker) GetStatus() CommonServiceBrokerStatus { + return b.Status.CommonServiceBrokerStatus +} diff --git a/pkg/apis/servicecatalog/v1beta1/conversion.go b/pkg/apis/servicecatalog/v1beta1/conversion.go index b1bbb30f231..4615d935cf3 100644 --- a/pkg/apis/servicecatalog/v1beta1/conversion.go +++ b/pkg/apis/servicecatalog/v1beta1/conversion.go @@ -29,7 +29,8 @@ import ( // what it's given for the supported fields, and errors for unsupported. func ClusterServicePlanFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID", + case "metadata.name", + "spec.externalID", "spec.externalName", "spec.clusterServiceBrokerName", "spec.clusterServiceClassRef.name": @@ -43,7 +44,9 @@ func ClusterServicePlanFieldLabelConversionFunc(label, value string) (string, st // what it's given for the supported fields, and errors for unsupported. func ServicePlanFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID", + case "metadata.name", + "metadata.namespace", + "spec.externalID", "spec.externalName", "spec.serviceBrokerName", "spec.serviceClassRef.name": @@ -57,7 +60,9 @@ func ServicePlanFieldLabelConversionFunc(label, value string) (string, string, e // what it's given for the supported fields, and errors for unsupported. func ServiceClassFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID", + case "metadata.name", + "metadata.namespace", + "spec.externalID", "spec.externalName", "spec.serviceBrokerName": return label, value, nil @@ -70,7 +75,8 @@ func ServiceClassFieldLabelConversionFunc(label, value string) (string, string, // what it's given for the supported fields, and errors for unsupported. func ClusterServiceClassFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID", + case "metadata.name", + "spec.externalID", "spec.externalName", "spec.clusterServiceBrokerName": return label, value, nil @@ -83,7 +89,9 @@ func ClusterServiceClassFieldLabelConversionFunc(label, value string) (string, s // what it's given for the supported fields, and errors for unsupported. func ServiceInstanceFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID", + case "metadata.name", + "metadata.namespace", + "spec.externalID", "spec.clusterServiceClassRef.name", "spec.clusterServicePlanRef.name": return label, value, nil @@ -96,7 +104,9 @@ func ServiceInstanceFieldLabelConversionFunc(label, value string) (string, strin // what it's given for the supported fields, and errors for unsupported. func ServiceBindingFieldLabelConversionFunc(label, value string) (string, string, error) { switch label { - case "spec.externalID": + case "metadata.name", + "metadata.namespace", + "spec.externalID": return label, value, nil default: return "", "", fmt.Errorf("field label not supported: %s", label) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 0ec7f1e6171..82e707ee1a8 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -958,12 +958,14 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro // // The instance's Ready condition should already be False, so // we just need to record an event. - s := fmt.Sprintf("Error polling last operation: %v", err) - glog.V(4).Info(pcb.Message(s)) - c.recorder.Event(instance, corev1.EventTypeWarning, errorPollingLastOperationReason, s) + reason := errorPollingLastOperationReason + message := fmt.Sprintf("Error polling last operation: %v", err) + glog.V(4).Info(pcb.Message(message)) + c.recorder.Event(instance, corev1.EventTypeWarning, reason, message) if c.reconciliationRetryDurationExceeded(instance.Status.OperationStartTime) { - return c.processServiceInstancePollingFailureRetryTimeout(instance, nil) + readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, reason, message) + return c.processServiceInstancePollingFailureRetryTimeout(instance, readyCond) } return c.continuePollingServiceInstance(instance) @@ -1573,13 +1575,6 @@ func newServiceInstanceFailedCondition(status v1beta1.ConditionStatus, reason, m return newServiceInstanceCondition(status, v1beta1.ServiceInstanceConditionFailed, reason, message) } -// newServiceInstanceOrphanMitigationCondition is a helper function that returns -// an OrphanMitigation condition with the given status, reason and message, -// with its transition time set to now. -func newServiceInstanceOrphanMitigationCondition(status v1beta1.ConditionStatus, reason, message string) *v1beta1.ServiceInstanceCondition { - return newServiceInstanceCondition(status, v1beta1.ServiceInstanceConditionOrphanMitigation, reason, message) -} - // removeServiceInstanceCondition removes a condition of a given type from an // instance's status if it exists. func removeServiceInstanceCondition(toUpdate *v1beta1.ServiceInstance, diff --git a/pkg/features/features.go b/pkg/features/features.go index a993469c566..a24a625c510 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -84,7 +84,7 @@ var defaultServiceCatalogFeatureGates = map[utilfeature.Feature]utilfeature.Feat PodPreset: {Default: false, PreRelease: utilfeature.Alpha}, OriginatingIdentity: {Default: false, PreRelease: utilfeature.Alpha}, AsyncBindingOperations: {Default: false, PreRelease: utilfeature.Alpha}, - NamespacedServiceBroker: {Default: false, PreRelease: utilfeature.Alpha}, + NamespacedServiceBroker: {Default: true, PreRelease: utilfeature.Alpha}, ResponseSchema: {Default: false, PreRelease: utilfeature.Alpha}, UpdateDashboardURL: {Default: false, PreRelease: utilfeature.Alpha}, OriginatingIdentityLocking: {Default: true, PreRelease: utilfeature.Alpha}, diff --git a/pkg/registry/servicecatalog/binding/storage.go b/pkg/registry/servicecatalog/binding/storage.go index 1ab364c073c..c08a6644a50 100644 --- a/pkg/registry/servicecatalog/binding/storage.go +++ b/pkg/registry/servicecatalog/binding/storage.go @@ -90,15 +90,9 @@ func Match(label labels.Selector, field fields.Selector) storage.SelectionPredic func toSelectableFields(binding *servicecatalog.ServiceBinding) fields.Set { // If you add a new selectable field, you also need to modify // pkg/apis/servicecatalog/v1beta1/conversion[_test].go - objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&binding.ObjectMeta, true) - specFieldSet := make(fields.Set, 1) - - if binding.Spec.ExternalID != "" { - specFieldSet["spec.externalID"] = binding.Spec.ExternalID - } - - return generic.MergeFieldsSets(objectMetaFieldsSet, specFieldSet) + specFieldSet["spec.externalID"] = binding.Spec.ExternalID + return generic.AddObjectMetaFieldsSet(specFieldSet, &binding.ObjectMeta, true) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/registry/servicecatalog/clusterservicebroker/storage.go b/pkg/registry/servicecatalog/clusterservicebroker/storage.go index d08b169ee6e..4e3ee76ba80 100644 --- a/pkg/registry/servicecatalog/clusterservicebroker/storage.go +++ b/pkg/registry/servicecatalog/clusterservicebroker/storage.go @@ -89,8 +89,7 @@ func Match(label labels.Selector, field fields.Selector) storage.SelectionPredic // toSelectableFields returns a field set that represents the object for matching purposes. func toSelectableFields(broker *servicecatalog.ClusterServiceBroker) fields.Set { - objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&broker.ObjectMeta, true) - return generic.MergeFieldsSets(objectMetaFieldsSet, nil) + return generic.ObjectMetaFieldsSet(&broker.ObjectMeta, false) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/registry/servicecatalog/clusterserviceclass/storage.go b/pkg/registry/servicecatalog/clusterserviceclass/storage.go index 02b014a1826..d3f87f4424e 100644 --- a/pkg/registry/servicecatalog/clusterserviceclass/storage.go +++ b/pkg/registry/servicecatalog/clusterserviceclass/storage.go @@ -100,7 +100,7 @@ func toSelectableFields(clusterServiceClass *servicecatalog.ClusterServiceClass) cscSpecificFieldsSet["spec.clusterServiceBrokerName"] = clusterServiceClass.Spec.ClusterServiceBrokerName cscSpecificFieldsSet["spec.externalName"] = clusterServiceClass.Spec.ExternalName cscSpecificFieldsSet["spec.externalID"] = clusterServiceClass.Spec.ExternalID - return generic.AddObjectMetaFieldsSet(cscSpecificFieldsSet, &clusterServiceClass.ObjectMeta, true) + return generic.AddObjectMetaFieldsSet(cscSpecificFieldsSet, &clusterServiceClass.ObjectMeta, false) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/registry/servicecatalog/clusterserviceplan/storage.go b/pkg/registry/servicecatalog/clusterserviceplan/storage.go index 2c784206e33..891386939e6 100644 --- a/pkg/registry/servicecatalog/clusterserviceplan/storage.go +++ b/pkg/registry/servicecatalog/clusterserviceplan/storage.go @@ -99,7 +99,7 @@ func toSelectableFields(servicePlan *servicecatalog.ClusterServicePlan) fields.S spSpecificFieldsSet["spec.clusterServiceClassRef.name"] = servicePlan.Spec.ClusterServiceClassRef.Name spSpecificFieldsSet["spec.externalName"] = servicePlan.Spec.ExternalName spSpecificFieldsSet["spec.externalID"] = servicePlan.Spec.ExternalID - return generic.AddObjectMetaFieldsSet(spSpecificFieldsSet, &servicePlan.ObjectMeta, true) + return generic.AddObjectMetaFieldsSet(spSpecificFieldsSet, &servicePlan.ObjectMeta, false) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/registry/servicecatalog/instance/storage.go b/pkg/registry/servicecatalog/instance/storage.go index 6104001f499..a8dc61070de 100644 --- a/pkg/registry/servicecatalog/instance/storage.go +++ b/pkg/registry/servicecatalog/instance/storage.go @@ -90,23 +90,15 @@ func Match(label labels.Selector, field fields.Selector) storage.SelectionPredic func toSelectableFields(instance *servicecatalog.ServiceInstance) fields.Set { // If you add a new selectable field, you also need to modify // pkg/apis/servicecatalog/v1beta1/conversion[_test].go - objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&instance.ObjectMeta, true) - specFieldSet := make(fields.Set, 3) - if instance.Spec.ClusterServiceClassRef != nil { specFieldSet["spec.clusterServiceClassRef.name"] = instance.Spec.ClusterServiceClassRef.Name } - if instance.Spec.ClusterServicePlanRef != nil { specFieldSet["spec.clusterServicePlanRef.name"] = instance.Spec.ClusterServicePlanRef.Name } - - if instance.Spec.ExternalID != "" { - specFieldSet["spec.externalID"] = instance.Spec.ExternalID - } - - return generic.MergeFieldsSets(objectMetaFieldsSet, specFieldSet) + specFieldSet["spec.externalID"] = instance.Spec.ExternalID + return generic.AddObjectMetaFieldsSet(specFieldSet, &instance.ObjectMeta, true) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/registry/servicecatalog/servicebroker/storage.go b/pkg/registry/servicecatalog/servicebroker/storage.go index f61d82e60f1..c915546c5f0 100644 --- a/pkg/registry/servicecatalog/servicebroker/storage.go +++ b/pkg/registry/servicecatalog/servicebroker/storage.go @@ -89,8 +89,7 @@ func Match(label labels.Selector, field fields.Selector) storage.SelectionPredic // toSelectableFields returns a field set that represents the object for matching purposes. func toSelectableFields(broker *servicecatalog.ServiceBroker) fields.Set { - objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&broker.ObjectMeta, true) - return generic.MergeFieldsSets(objectMetaFieldsSet, nil) + return generic.ObjectMetaFieldsSet(&broker.ObjectMeta, true) } // GetAttrs returns labels and fields of a given object for filtering purposes. diff --git a/pkg/svcat/service-catalog/broker.go b/pkg/svcat/service-catalog/broker.go index 85435324c73..73bf292f75c 100644 --- a/pkg/svcat/service-catalog/broker.go +++ b/pkg/svcat/service-catalog/broker.go @@ -24,6 +24,22 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// Broker provides a unifying layer of cluster and namespace scoped broker resources. +type Broker interface { + + // GetName returns the broker's name. + GetName() string + + // GetNamespace returns the broker's namespace, or "" if it's cluster-scoped. + GetNamespace() string + + // GetURL returns the broker's URL. + GetURL() string + + // GetStatus returns the broker's status. + GetStatus() v1beta1.CommonServiceBrokerStatus +} + // Deregister deletes a broker func (sdk *SDK) Deregister(brokerName string) error { err := sdk.ServiceCatalog().ClusterServiceBrokers().Delete(brokerName, &v1.DeleteOptions{}) @@ -35,13 +51,36 @@ func (sdk *SDK) Deregister(brokerName string) error { } // RetrieveBrokers lists all brokers defined in the cluster. -func (sdk *SDK) RetrieveBrokers() ([]v1beta1.ClusterServiceBroker, error) { - brokers, err := sdk.ServiceCatalog().ClusterServiceBrokers().List(v1.ListOptions{}) - if err != nil { - return nil, fmt.Errorf("unable to list brokers (%s)", err) +func (sdk *SDK) RetrieveBrokers(opts ScopeOptions) ([]Broker, error) { + var brokers []Broker + + if opts.Scope.Matches(ClusterScope) { + csb, err := sdk.ServiceCatalog().ClusterServiceBrokers().List(v1.ListOptions{}) + if err != nil { + return nil, fmt.Errorf("unable to list cluster-scoped brokers (%s)", err) + } + for _, b := range csb.Items { + broker := b + brokers = append(brokers, &broker) + } + } + + if opts.Scope.Matches(NamespaceScope) { + sb, err := sdk.ServiceCatalog().ServiceBrokers(opts.Namespace).List(v1.ListOptions{}) + if err != nil { + // Gracefully handle when the feature-flag for namespaced broker resources isn't enabled on the server. + if errors.IsNotFound(err) { + return brokers, nil + } + return nil, fmt.Errorf("unable to list brokers in %q (%s)", opts.Namespace, err) + } + for _, b := range sb.Items { + broker := b + brokers = append(brokers, &broker) + } } - return brokers.Items, nil + return brokers, nil } // RetrieveBroker gets a broker by its name. diff --git a/pkg/svcat/service-catalog/broker_test.go b/pkg/svcat/service-catalog/broker_test.go index 1b51f1d129a..c18353703a3 100644 --- a/pkg/svcat/service-catalog/broker_test.go +++ b/pkg/svcat/service-catalog/broker_test.go @@ -35,14 +35,18 @@ var _ = Describe("Broker", func() { var ( sdk *SDK svcCatClient *fake.Clientset - sb *v1beta1.ClusterServiceBroker - sb2 *v1beta1.ClusterServiceBroker + csb *v1beta1.ClusterServiceBroker + csb2 *v1beta1.ClusterServiceBroker + sb *v1beta1.ServiceBroker + sb2 *v1beta1.ServiceBroker ) BeforeEach(func() { - sb = &v1beta1.ClusterServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "foobar"}} - sb2 = &v1beta1.ClusterServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "barbaz"}} - svcCatClient = fake.NewSimpleClientset(sb, sb2) + csb = &v1beta1.ClusterServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "foobar"}} + csb2 = &v1beta1.ClusterServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "barbaz"}} + sb = &v1beta1.ServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "foobar", Namespace: "default"}} + sb2 = &v1beta1.ServiceBroker{ObjectMeta: metav1.ObjectMeta{Name: "barbaz", Namespace: "ns2"}} + svcCatClient = fake.NewSimpleClientset(csb, csb2, sb, sb2) sdk = &SDK{ ServiceCatalogClient: svcCatClient, } @@ -76,36 +80,75 @@ var _ = Describe("Broker", func() { }) }) Describe("RetrieveBrokers", func() { - It("Calls the generated v1beta1 List method", func() { - brokers, err := sdk.RetrieveBrokers() + It("Calls the generated v1beta1 List methods", func() { + brokers, err := sdk.RetrieveBrokers(ScopeOptions{Scope: AllScope}) Expect(err).NotTo(HaveOccurred()) - Expect(brokers).Should(ConsistOf(*sb, *sb2)) + Expect(brokers).Should(ConsistOf(csb, csb2, sb, sb2)) + actions := svcCatClient.Actions() + Expect(len(actions)).To(Equal(2)) Expect(svcCatClient.Actions()[0].Matches("list", "clusterservicebrokers")).To(BeTrue()) + Expect(svcCatClient.Actions()[1].Matches("list", "servicebrokers")).To(BeTrue()) }) - It("Bubbles up errors", func() { + It("Filters by namespace scope", func() { + brokers, err := sdk.RetrieveBrokers(ScopeOptions{Scope: NamespaceScope, Namespace: "default"}) + + Expect(err).NotTo(HaveOccurred()) + Expect(brokers).Should(ConsistOf(sb)) + actions := svcCatClient.Actions() + Expect(len(actions)).To(Equal(1)) + Expect(actions[0].Matches("list", "servicebrokers")).To(BeTrue()) + }) + It("Filters by cluster scope", func() { + brokers, err := sdk.RetrieveBrokers(ScopeOptions{Scope: ClusterScope}) + + Expect(err).NotTo(HaveOccurred()) + Expect(brokers).Should(ConsistOf(csb, csb2)) + actions := svcCatClient.Actions() + Expect(len(actions)).To(Equal(1)) + Expect(actions[0].Matches("list", "clusterservicebrokers")).To(BeTrue()) + }) + It("Bubbles up cluster-scoped errors", func() { badClient := &fake.Clientset{} errorMessage := "error retrieving list" badClient.AddReactor("list", "clusterservicebrokers", func(action testing.Action) (bool, runtime.Object, error) { return true, nil, fmt.Errorf(errorMessage) }) sdk.ServiceCatalogClient = badClient - _, err := sdk.RetrieveBrokers() + _, err := sdk.RetrieveBrokers(ScopeOptions{Scope: AllScope}) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring(errorMessage)) + actions := badClient.Actions() + Expect(len(actions)).To(Equal(1)) + Expect(actions[0].Matches("list", "clusterservicebrokers")).To(BeTrue()) + }) + It("Bubbles up namespace-scoped errors", func() { + badClient := &fake.Clientset{} + errorMessage := "error retrieving list" + badClient.AddReactor("list", "servicebrokers", func(action testing.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf(errorMessage) + }) + sdk.ServiceCatalogClient = badClient + _, err := sdk.RetrieveBrokers(ScopeOptions{Scope: AllScope}) Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring(errorMessage)) - Expect(badClient.Actions()[0].Matches("list", "clusterservicebrokers")).To(BeTrue()) + actions := badClient.Actions() + Expect(len(actions)).To(Equal(2)) + Expect(actions[0].Matches("list", "clusterservicebrokers")).To(BeTrue()) + Expect(actions[1].Matches("list", "servicebrokers")).To(BeTrue()) }) }) Describe("RetrieveBroker", func() { It("Calls the generated v1beta1 List method with the passed in broker", func() { - broker, err := sdk.RetrieveBroker(sb.Name) + broker, err := sdk.RetrieveBroker(csb.Name) Expect(err).NotTo(HaveOccurred()) - Expect(broker).To(Equal(sb)) + Expect(broker).To(Equal(csb)) actions := svcCatClient.Actions() Expect(actions[0].Matches("get", "clusterservicebrokers")).To(BeTrue()) - Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(sb.Name)) + Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(csb.Name)) }) It("Bubbles up errors", func() { brokerName := "banana" @@ -122,14 +165,14 @@ var _ = Describe("Broker", func() { }) Describe("RetrieveBrokerByClass", func() { It("Calls the generated v1beta1 List method with the passed in class's parent broker", func() { - sc := &v1beta1.ClusterServiceClass{Spec: v1beta1.ClusterServiceClassSpec{ClusterServiceBrokerName: sb.Name}} + sc := &v1beta1.ClusterServiceClass{Spec: v1beta1.ClusterServiceClassSpec{ClusterServiceBrokerName: csb.Name}} broker, err := sdk.RetrieveBrokerByClass(sc) Expect(broker).NotTo(BeNil()) Expect(err).NotTo(HaveOccurred()) actions := svcCatClient.Actions() Expect(actions[0].Matches("get", "clusterservicebrokers")).To(BeTrue()) - Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(sb.Name)) + Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(csb.Name)) }) It("Bubbles up errors", func() { @@ -182,13 +225,13 @@ var _ = Describe("Broker", func() { }) Describe("Sync", func() { It("Useds the generated v1beta1 Retrieve method to get the broker, and then updates it with a new RelistRequests", func() { - err := sdk.Sync(sb.Name, 3) + err := sdk.Sync(csb.Name, 3) Expect(err).NotTo(HaveOccurred()) actions := svcCatClient.Actions() Expect(len(actions) >= 2).To(BeTrue()) Expect(actions[0].Matches("get", "clusterservicebrokers")).To(BeTrue()) - Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(sb.Name)) + Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(csb.Name)) Expect(actions[1].Matches("update", "clusterservicebrokers")).To(BeTrue()) Expect(actions[1].(testing.UpdateActionImpl).Object.(*v1beta1.ClusterServiceBroker).Spec.RelistRequests).Should(BeNumerically(">", 0)) diff --git a/pkg/svcat/service-catalog/class.go b/pkg/svcat/service-catalog/class.go index 7cf4cde2664..4469eae8970 100644 --- a/pkg/svcat/service-catalog/class.go +++ b/pkg/svcat/service-catalog/class.go @@ -116,3 +116,13 @@ func (sdk *SDK) RetrieveClassByPlan(plan *v1beta1.ClusterServicePlan, return class, nil } + +// CreateClass returns new created class +func (sdk *SDK) CreateClass(class *v1beta1.ClusterServiceClass) (*v1beta1.ClusterServiceClass, error) { + created, err := sdk.ServiceCatalog().ClusterServiceClasses().Create(class) + if err != nil { + return nil, fmt.Errorf("unable to create class (%s)", err) + } + + return created, nil +} diff --git a/pkg/svcat/service-catalog/class_test.go b/pkg/svcat/service-catalog/class_test.go index a90fa189902..0ca45b65d35 100644 --- a/pkg/svcat/service-catalog/class_test.go +++ b/pkg/svcat/service-catalog/class_test.go @@ -230,4 +230,27 @@ var _ = Describe("Class", func() { Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(fakeClassName)) }) }) + Describe("CreateClass", func() { + It("Calls the generated v1beta1 create method with the passed in class", func() { + className := "newclass" + csc.Name = className + class, err := sdk.CreateClass(csc) + + Expect(err).NotTo(HaveOccurred()) + Expect(class).To(Equal(csc)) + actions := svcCatClient.Actions() + Expect(actions[0].Matches("create", "clusterserviceclasses")).To(BeTrue()) + objectFromRequest := actions[0].(testing.CreateActionImpl).Object.(*v1beta1.ClusterServiceClass) + Expect(objectFromRequest.Name).To(Equal(className)) + }) + It("Bubbles up errors", func() { + class, err := sdk.CreateClass(csc) + + Expect(class).To(BeNil()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("unable to create class")) + actions := svcCatClient.Actions() + Expect(actions[0].Matches("create", "clusterserviceclasses")).To(BeTrue()) + }) + }) }) diff --git a/pkg/svcat/service-catalog/sdk.go b/pkg/svcat/service-catalog/sdk.go index 70098e48fce..18c74751bf7 100644 --- a/pkg/svcat/service-catalog/sdk.go +++ b/pkg/svcat/service-catalog/sdk.go @@ -45,7 +45,7 @@ type SvcatClient interface { WaitForBinding(string, string, time.Duration, *time.Duration) (*apiv1beta1.ServiceBinding, error) Deregister(string) error - RetrieveBrokers() ([]apiv1beta1.ClusterServiceBroker, error) + RetrieveBrokers(opts ScopeOptions) ([]Broker, error) RetrieveBroker(string) (*apiv1beta1.ClusterServiceBroker, error) RetrieveBrokerByClass(*apiv1beta1.ClusterServiceClass) (*apiv1beta1.ClusterServiceBroker, error) Register(string, string) (*apiv1beta1.ClusterServiceBroker, error) @@ -55,6 +55,7 @@ type SvcatClient interface { RetrieveClassByName(string) (*apiv1beta1.ClusterServiceClass, error) RetrieveClassByID(string) (*apiv1beta1.ClusterServiceClass, error) RetrieveClassByPlan(*apiv1beta1.ClusterServicePlan) (*apiv1beta1.ClusterServiceClass, error) + CreateClass(*apiv1beta1.ClusterServiceClass) (*apiv1beta1.ClusterServiceClass, error) Deprovision(string, string) error InstanceParentHierarchy(*apiv1beta1.ServiceInstance) (*apiv1beta1.ClusterServiceClass, *apiv1beta1.ClusterServicePlan, *apiv1beta1.ClusterServiceBroker, error) 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 7547c77f6ee..b7dd46aa961 100644 --- a/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go +++ b/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go @@ -195,15 +195,17 @@ type FakeSvcatClient struct { deregisterReturnsOnCall map[int]struct { result1 error } - RetrieveBrokersStub func() ([]apiv1beta1.ClusterServiceBroker, error) + RetrieveBrokersStub func(opts servicecatalog.ScopeOptions) ([]servicecatalog.Broker, error) retrieveBrokersMutex sync.RWMutex - retrieveBrokersArgsForCall []struct{} - retrieveBrokersReturns struct { - result1 []apiv1beta1.ClusterServiceBroker + retrieveBrokersArgsForCall []struct { + opts servicecatalog.ScopeOptions + } + retrieveBrokersReturns struct { + result1 []servicecatalog.Broker result2 error } retrieveBrokersReturnsOnCall map[int]struct { - result1 []apiv1beta1.ClusterServiceBroker + result1 []servicecatalog.Broker result2 error } RetrieveBrokerStub func(string) (*apiv1beta1.ClusterServiceBroker, error) @@ -310,6 +312,19 @@ type FakeSvcatClient struct { result1 *apiv1beta1.ClusterServiceClass result2 error } + CreateClassStub func(*apiv1beta1.ClusterServiceClass) (*apiv1beta1.ClusterServiceClass, error) + createClassMutex sync.RWMutex + createClassArgsForCall []struct { + arg1 *apiv1beta1.ClusterServiceClass + } + createClassReturns struct { + result1 *apiv1beta1.ClusterServiceClass + result2 error + } + createClassReturnsOnCall map[int]struct { + result1 *apiv1beta1.ClusterServiceClass + result2 error + } DeprovisionStub func(string, string) error deprovisionMutex sync.RWMutex deprovisionArgsForCall []struct { @@ -1199,14 +1214,16 @@ func (fake *FakeSvcatClient) DeregisterReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeSvcatClient) RetrieveBrokers() ([]apiv1beta1.ClusterServiceBroker, error) { +func (fake *FakeSvcatClient) RetrieveBrokers(opts servicecatalog.ScopeOptions) ([]servicecatalog.Broker, error) { fake.retrieveBrokersMutex.Lock() ret, specificReturn := fake.retrieveBrokersReturnsOnCall[len(fake.retrieveBrokersArgsForCall)] - fake.retrieveBrokersArgsForCall = append(fake.retrieveBrokersArgsForCall, struct{}{}) - fake.recordInvocation("RetrieveBrokers", []interface{}{}) + fake.retrieveBrokersArgsForCall = append(fake.retrieveBrokersArgsForCall, struct { + opts servicecatalog.ScopeOptions + }{opts}) + fake.recordInvocation("RetrieveBrokers", []interface{}{opts}) fake.retrieveBrokersMutex.Unlock() if fake.RetrieveBrokersStub != nil { - return fake.RetrieveBrokersStub() + return fake.RetrieveBrokersStub(opts) } if specificReturn { return ret.result1, ret.result2 @@ -1220,24 +1237,30 @@ func (fake *FakeSvcatClient) RetrieveBrokersCallCount() int { return len(fake.retrieveBrokersArgsForCall) } -func (fake *FakeSvcatClient) RetrieveBrokersReturns(result1 []apiv1beta1.ClusterServiceBroker, result2 error) { +func (fake *FakeSvcatClient) RetrieveBrokersArgsForCall(i int) servicecatalog.ScopeOptions { + fake.retrieveBrokersMutex.RLock() + defer fake.retrieveBrokersMutex.RUnlock() + return fake.retrieveBrokersArgsForCall[i].opts +} + +func (fake *FakeSvcatClient) RetrieveBrokersReturns(result1 []servicecatalog.Broker, result2 error) { fake.RetrieveBrokersStub = nil fake.retrieveBrokersReturns = struct { - result1 []apiv1beta1.ClusterServiceBroker + result1 []servicecatalog.Broker result2 error }{result1, result2} } -func (fake *FakeSvcatClient) RetrieveBrokersReturnsOnCall(i int, result1 []apiv1beta1.ClusterServiceBroker, result2 error) { +func (fake *FakeSvcatClient) RetrieveBrokersReturnsOnCall(i int, result1 []servicecatalog.Broker, result2 error) { fake.RetrieveBrokersStub = nil if fake.retrieveBrokersReturnsOnCall == nil { fake.retrieveBrokersReturnsOnCall = make(map[int]struct { - result1 []apiv1beta1.ClusterServiceBroker + result1 []servicecatalog.Broker result2 error }) } fake.retrieveBrokersReturnsOnCall[i] = struct { - result1 []apiv1beta1.ClusterServiceBroker + result1 []servicecatalog.Broker result2 error }{result1, result2} } @@ -1649,6 +1672,57 @@ func (fake *FakeSvcatClient) RetrieveClassByPlanReturnsOnCall(i int, result1 *ap }{result1, result2} } +func (fake *FakeSvcatClient) CreateClass(arg1 *apiv1beta1.ClusterServiceClass) (*apiv1beta1.ClusterServiceClass, error) { + fake.createClassMutex.Lock() + ret, specificReturn := fake.createClassReturnsOnCall[len(fake.createClassArgsForCall)] + fake.createClassArgsForCall = append(fake.createClassArgsForCall, struct { + arg1 *apiv1beta1.ClusterServiceClass + }{arg1}) + fake.recordInvocation("CreateClass", []interface{}{arg1}) + fake.createClassMutex.Unlock() + if fake.CreateClassStub != nil { + return fake.CreateClassStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fake.createClassReturns.result1, fake.createClassReturns.result2 +} + +func (fake *FakeSvcatClient) CreateClassCallCount() int { + fake.createClassMutex.RLock() + defer fake.createClassMutex.RUnlock() + return len(fake.createClassArgsForCall) +} + +func (fake *FakeSvcatClient) CreateClassArgsForCall(i int) *apiv1beta1.ClusterServiceClass { + fake.createClassMutex.RLock() + defer fake.createClassMutex.RUnlock() + return fake.createClassArgsForCall[i].arg1 +} + +func (fake *FakeSvcatClient) CreateClassReturns(result1 *apiv1beta1.ClusterServiceClass, result2 error) { + fake.CreateClassStub = nil + fake.createClassReturns = struct { + result1 *apiv1beta1.ClusterServiceClass + result2 error + }{result1, result2} +} + +func (fake *FakeSvcatClient) CreateClassReturnsOnCall(i int, result1 *apiv1beta1.ClusterServiceClass, result2 error) { + fake.CreateClassStub = nil + if fake.createClassReturnsOnCall == nil { + fake.createClassReturnsOnCall = make(map[int]struct { + result1 *apiv1beta1.ClusterServiceClass + result2 error + }) + } + fake.createClassReturnsOnCall[i] = struct { + result1 *apiv1beta1.ClusterServiceClass + result2 error + }{result1, result2} +} + func (fake *FakeSvcatClient) Deprovision(arg1 string, arg2 string) error { fake.deprovisionMutex.Lock() ret, specificReturn := fake.deprovisionReturnsOnCall[len(fake.deprovisionArgsForCall)] diff --git a/plugin/pkg/admission/namespace/lifecycle/admission.go b/plugin/pkg/admission/namespace/lifecycle/admission.go deleted file mode 100644 index 0eb1a3ae759..00000000000 --- a/plugin/pkg/admission/namespace/lifecycle/admission.go +++ /dev/null @@ -1,166 +0,0 @@ -/* -Copyright 2017 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 lifecycle - -import ( - "fmt" - "io" - "time" - - "github.com/golang/glog" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apiserver/pkg/admission" - kubeinformers "k8s.io/client-go/informers" - kubeclientset "k8s.io/client-go/kubernetes" - listerscorev1 "k8s.io/client-go/listers/core/v1" - - scadmission "github.com/kubernetes-incubator/service-catalog/pkg/apiserver/admission" -) - -// TODO nilebox: This looks like a copy-paste of k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle/admission.go -// Do we still need it given that API servers now have "default" admission controllers? - -const ( - // PluginName is name of admission plug-in - PluginName = "KubernetesNamespaceLifecycle" - // how long to wait for a missing namespace before re-checking the cache (and then doing a live lookup) - // this accomplishes two things: - // 1. It allows a watch-fed cache time to observe a namespace creation event - // 2. It allows time for a namespace creation to distribute to members of a storage cluster, - // so the live lookup has a better chance of succeeding even if it isn't performed against the leader. - missingNamespaceWait = 50 * time.Millisecond -) - -// Register registers a plugin -func Register(plugins *admission.Plugins) { - plugins.Register(PluginName, func(io.Reader) (admission.Interface, error) { - return NewLifecycle() - }) -} - -// lifecycle is an implementation of admission.Interface. -// It enforces life-cycle constraints around a Namespace depending on its Phase -type lifecycle struct { - *admission.Handler - client kubeclientset.Interface - namespaceLister listerscorev1.NamespaceLister -} - -type forceLiveLookupEntry struct { - expiry time.Time -} - -var _ = scadmission.WantsKubeInformerFactory(&lifecycle{}) -var _ = scadmission.WantsKubeClientSet(&lifecycle{}) - -func (l *lifecycle) Admit(a admission.Attributes) error { - // we need to wait for our caches to warm - if !l.WaitForReady() { - return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request")) - } - - var ( - exists bool - err error - ) - - namespace, err := l.namespaceLister.Get(a.GetNamespace()) - if err != nil { - if !errors.IsNotFound(err) { - return errors.NewInternalError(err) - } - } else { - exists = true - } - - // we only care about namespaced resources - if len(a.GetNamespace()) == 0 { - return nil - } - - if !exists && a.GetOperation() == admission.Create { - // give the cache time to observe the namespace before rejecting a create. - // this helps when creating a namespace and immediately creating objects within it. - time.Sleep(missingNamespaceWait) - namespace, err = l.namespaceLister.Get(a.GetNamespace()) - switch { - case errors.IsNotFound(err): - // no-op - case err != nil: - return errors.NewInternalError(err) - default: - exists = true - } - if exists { - glog.V(4).Infof("found namespace %q in cache after waiting", a.GetNamespace()) - } - } - - // refuse to operate on non-existent namespaces - if !exists { - // as a last resort, make a call directly to storage - namespace, err = l.client.CoreV1().Namespaces().Get(a.GetNamespace(), metav1.GetOptions{}) - switch { - case errors.IsNotFound(err): - return err - case err != nil: - return errors.NewInternalError(err) - } - glog.V(4).Infof("found namespace %q via storage lookup", a.GetNamespace()) - } - - // ensure that we're not trying to create objects in terminating namespaces - if a.GetOperation() == admission.Create { - if namespace.Status.Phase != corev1.NamespaceTerminating { - return nil - } - - return admission.NewForbidden(a, fmt.Errorf("unable to create new content in namespace %q because it is being terminated", a.GetNamespace())) - } - - return nil -} - -// NewLifecycle creates a new namespace lifecycle admission control handler -func NewLifecycle() (admission.Interface, error) { - return &lifecycle{ - Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete), - }, nil -} - -func (l *lifecycle) SetKubeInformerFactory(f kubeinformers.SharedInformerFactory) { - namespaceInformer := f.Core().V1().Namespaces() - l.namespaceLister = namespaceInformer.Lister() - l.SetReadyFunc(namespaceInformer.Informer().HasSynced) -} - -func (l *lifecycle) SetKubeClientSet(client kubeclientset.Interface) { - l.client = client -} - -func (l *lifecycle) ValidateInitialization() error { - if l.namespaceLister == nil { - return fmt.Errorf("missing namespaceLister") - } - if l.client == nil { - return fmt.Errorf("missing client") - } - return nil -} diff --git a/plugin/pkg/admission/namespace/lifecycle/admission_test.go b/plugin/pkg/admission/namespace/lifecycle/admission_test.go deleted file mode 100644 index 2014ec6d9f2..00000000000 --- a/plugin/pkg/admission/namespace/lifecycle/admission_test.go +++ /dev/null @@ -1,176 +0,0 @@ -/* -Copyright 2017 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 lifecycle - -import ( - "fmt" - "testing" - "time" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apiserver/pkg/admission" - kubefake "k8s.io/client-go/kubernetes/fake" - core "k8s.io/client-go/testing" - - kubeinformers "k8s.io/client-go/informers" - kubeclientset "k8s.io/client-go/kubernetes" - - "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" - "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/internalclientset" - "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/internalclientset/fake" - informers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/internalversion" - - scadmission "github.com/kubernetes-incubator/service-catalog/pkg/apiserver/admission" -) - -// newHandlerForTest returns a configured handler for testing. -func newHandlerForTest(internalClient internalclientset.Interface, kubeClient kubeclientset.Interface) (admission.Interface, informers.SharedInformerFactory, kubeinformers.SharedInformerFactory, error) { - f := informers.NewSharedInformerFactory(internalClient, 5*time.Minute) - kf := kubeinformers.NewSharedInformerFactory(kubeClient, 5*time.Minute) - handler, err := NewLifecycle() - if err != nil { - return nil, f, kf, err - } - pluginInitializer := scadmission.NewPluginInitializer(internalClient, f, kubeClient, kf) - pluginInitializer.Initialize(handler) - err = admission.ValidateInitialization(handler) - return handler, f, kf, err -} - -// newMockKubeClientForTest creates a mock client that returns a client -// configured for the specified list of namespaces with the specified phase. -func newMockKubeClientForTest(namespaces map[string]corev1.NamespacePhase) *kubefake.Clientset { - mockClient := &kubefake.Clientset{} - mockClient.AddReactor("list", "namespaces", func(action core.Action) (bool, runtime.Object, error) { - namespaceList := &corev1.NamespaceList{ - ListMeta: metav1.ListMeta{ - ResourceVersion: fmt.Sprintf("%d", len(namespaces)), - }, - } - index := 0 - for name, phase := range namespaces { - namespaceList.Items = append(namespaceList.Items, corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - ResourceVersion: fmt.Sprintf("%d", index), - }, - Status: corev1.NamespaceStatus{ - Phase: phase, - }, - }) - index++ - } - return true, namespaceList, nil - }) - return mockClient -} - -// newMockClientForTest creates a mock client. -func newMockClientForTest() *fake.Clientset { - mockClient := &fake.Clientset{} - return mockClient -} - -// newServiceInstance returns a new instance for the specified namespace. -func newServiceInstance(namespace string) servicecatalog.ServiceInstance { - return servicecatalog.ServiceInstance{ - ObjectMeta: metav1.ObjectMeta{Name: "instance", Namespace: namespace}, - } -} - -// TestAdmissionNamespaceDoesNotExist verifies instance is not admitted if namespace does not exist. -func TestAdmissionNamespaceDoesNotExist(t *testing.T) { - namespace := "test" - mockClient := newMockClientForTest() - mockKubeClient := newMockKubeClientForTest(map[string]corev1.NamespacePhase{}) - mockKubeClient.AddReactor("get", "namespaces", func(action core.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("nope, out of luck") - }) - handler, informerFactory, kubeInformerFactory, err := newHandlerForTest(mockClient, mockKubeClient) - if err != nil { - t.Errorf("unexpected error initializing handler: %v", err) - } - informerFactory.Start(wait.NeverStop) - kubeInformerFactory.Start(wait.NeverStop) - - instance := newServiceInstance(namespace) - err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(&instance, nil, servicecatalog.Kind("ServiceInstance").WithVersion("version"), instance.Namespace, instance.Name, servicecatalog.Resource("serviceinstances").WithVersion("version"), "", admission.Create, nil)) - if err == nil { - actions := "" - for _, action := range mockClient.Actions() { - actions = actions + action.GetVerb() + ":" + action.GetResource().Resource + ":" + action.GetSubresource() + ", " - } - t.Errorf("expected error returned from admission handler: %v", actions) - } -} - -// TestAdmissionNamespaceActive verifies a resource is admitted when the namespace is active. -func TestAdmissionNamespaceActive(t *testing.T) { - namespace := "test" - mockClient := newMockClientForTest() - mockKubeClient := newMockKubeClientForTest(map[string]corev1.NamespacePhase{ - namespace: corev1.NamespaceActive, - }) - handler, informerFactory, kubeInformerFactory, err := newHandlerForTest(mockClient, mockKubeClient) - if err != nil { - t.Errorf("unexpected error initializing handler: %v", err) - } - informerFactory.Start(wait.NeverStop) - kubeInformerFactory.Start(wait.NeverStop) - - instance := newServiceInstance(namespace) - err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(&instance, nil, servicecatalog.Kind("ServiceInstance").WithVersion("version"), instance.Namespace, instance.Name, servicecatalog.Resource("serviceinstances").WithVersion("version"), "", admission.Create, nil)) - if err != nil { - t.Errorf("unexpected error returned from admission handler") - } -} - -// TestAdmissionNamespaceTerminating verifies a resource is not created when the namespace is terminating. -func TestAdmissionNamespaceTerminating(t *testing.T) { - namespace := "test" - mockClient := newMockClientForTest() - mockKubeClient := newMockKubeClientForTest(map[string]corev1.NamespacePhase{ - namespace: corev1.NamespaceTerminating, - }) - handler, informerFactory, kubeInformerFactory, err := newHandlerForTest(mockClient, mockKubeClient) - if err != nil { - t.Errorf("unexpected error initializing handler: %v", err) - } - informerFactory.Start(wait.NeverStop) - kubeInformerFactory.Start(wait.NeverStop) - - instance := newServiceInstance(namespace) - err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(&instance, nil, servicecatalog.Kind("ServiceInstance").WithVersion("version"), instance.Namespace, instance.Name, servicecatalog.Resource("serviceinstances").WithVersion("version"), "", admission.Create, nil)) - if err == nil { - t.Errorf("Expected error rejecting creates in a namespace when it is terminating") - } - - // verify update operations in the namespace can proceed - err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(&instance, nil, servicecatalog.Kind("ServiceInstance").WithVersion("version"), instance.Namespace, instance.Name, servicecatalog.Resource("serviceinstances").WithVersion("version"), "", admission.Update, nil)) - if err != nil { - t.Errorf("Unexpected error returned from admission handler: %v", err) - } - - // verify delete operations in the namespace can proceed - err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(nil, nil, servicecatalog.Kind("ServiceInstance").WithVersion("version"), instance.Namespace, instance.Name, servicecatalog.Resource("serviceinstances").WithVersion("version"), "", admission.Delete, nil)) - if err != nil { - t.Errorf("Unexpected error returned from admission handler: %v", err) - } -} diff --git a/test/integration/clientset_test.go b/test/integration/clientset_test.go index f733f364206..e4571050b7d 100644 --- a/test/integration/clientset_test.go +++ b/test/integration/clientset_test.go @@ -174,11 +174,20 @@ func testNoName(client servicecatalogclient.Interface) error { if bi, e := scClient.ServiceBindings(ns).Create(&v1beta1.ServiceBinding{}); nil == e { return fmt.Errorf("needs a name (%s)", bi.Name) } + if br, e := scClient.ServiceBrokers(ns).Create(&v1beta1.ServiceBroker{}); nil == e { + return fmt.Errorf("needs a name (%s)", br.Name) + } + if sc, e := scClient.ServiceClasses(ns).Create(&v1beta1.ServiceClass{}); nil == e { + return fmt.Errorf("needs a name (%s)", sc.Name) + } + if sp, e := scClient.ServicePlans(ns).Create(&v1beta1.ServicePlan{}); nil == e { + return fmt.Errorf("needs a name (%s)", sp.Name) + } return nil } -// TestBrokerClient exercises the Broker client. -func TestBrokerClient(t *testing.T) { +// TestClusterServiceBrokerClient exercises the ClusterServiceBroker client. +func TestClusterServiceBrokerClient(t *testing.T) { const name = "test-broker" rootTestFunc := func(sType server.StorageType) func(t *testing.T) { return func(t *testing.T) { @@ -186,7 +195,7 @@ func TestBrokerClient(t *testing.T) { return &servicecatalog.ClusterServiceBroker{} }) defer shutdownServer() - if err := testBrokerClient(sType, client, name); err != nil { + if err := testClusterServiceBrokerClient(sType, client, name); err != nil { t.Fatal(err) } } @@ -198,7 +207,7 @@ func TestBrokerClient(t *testing.T) { } } -func testBrokerClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testClusterServiceBrokerClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { brokerClient := client.Servicecatalog().ClusterServiceBrokers() broker := &v1beta1.ClusterServiceBroker{ ObjectMeta: metav1.ObjectMeta{Name: name}, @@ -237,6 +246,14 @@ func testBrokerClient(sType server.StorageType, client servicecatalogclient.Inte return fmt.Errorf("should have exactly one broker, had %v brokers", len(brokers.Items)) } + brokers, err = brokerClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ClusterServiceBrokers: %v", err) + } + if 1 != len(brokers.Items) { + return fmt.Errorf("should have exactly one broker, had %v brokers", len(brokers.Items)) + } + brokerServer, err = brokerClient.Get(name, metav1.GetOptions{}) if err != nil { return fmt.Errorf("error getting broker %s (%s)", name, err) @@ -343,48 +360,424 @@ func testBrokerClient(sType server.StorageType, client servicecatalogclient.Inte return fmt.Errorf("broker should be deleted (%v)", brokerDeleted) } return nil -} - -// TestClusterServiceClassClient exercises the ClusterServiceClass client. -func TestClusterServiceClassClient(t *testing.T) { +} // TestNamespacedServiceBrokerClient exercises the namespaced ServiceBroker client. +func TestNamespacedServiceBrokerClient(t *testing.T) { + const name = "test-broker" + const namespace = "test-namespace" rootTestFunc := func(sType server.StorageType) func(t *testing.T) { return func(t *testing.T) { - const name = "test-serviceclass" + resetFeaturesFunc, err := enableNamespacedResources() + if err != nil { + t.Fatal(err) + } + defer resetFeaturesFunc() client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServiceClass{} + return &servicecatalog.ClusterServiceBroker{} }) defer shutdownServer() + if err := testNamespacedServiceBrokerClient(sType, client, namespace, name); err != nil { + t.Fatal(err) + } + } + } + for _, sType := range storageTypes { + if !t.Run(sType.String(), rootTestFunc(sType)) { + t.Errorf("%q test failed", sType) + } + } +} + +func testNamespacedServiceBrokerClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { + brokerClient := client.Servicecatalog().ServiceBrokers(namespace) + broker := &v1beta1.ServiceBroker{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1beta1.ServiceBrokerSpec{ + CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{ + URL: "https://example.com", + }, + }, + } + + // start from scratch + brokers, err := brokerClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing brokers (%s)", err) + } + if brokers.Items == nil { + return fmt.Errorf("Items field should not be set to nil") + } + if len(brokers.Items) > 0 { + return fmt.Errorf("brokers should not exist on start, had %v brokers", len(brokers.Items)) + } + + brokerServer, err := brokerClient.Create(broker) + if nil != err { + return fmt.Errorf("error creating the broker '%v' (%v)", broker, err) + } + if name != brokerServer.Name { + return fmt.Errorf("didn't get the same broker back from the server \n%+v\n%+v", broker, brokerServer) + } + + brokers, err = brokerClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing brokers (%s)", err) + } + if 1 != len(brokers.Items) { + return fmt.Errorf("should have exactly one broker, had %v brokers", len(brokers.Items)) + } + + brokers, err = brokerClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ServiceBrokers: %v", err) + } + if 1 != len(brokers.Items) { + return fmt.Errorf("should have exactly one broker, had %v brokers", len(brokers.Items)) + } + + brokers, err = brokerClient.List(metav1.ListOptions{FieldSelector: "metadata.namespace=" + namespace}) + if err != nil { + return fmt.Errorf("error listing ServiceBrokers: %v", err) + } + if 1 != len(brokers.Items) { + return fmt.Errorf("should have exactly one broker, had %v brokers", len(brokers.Items)) + } + + brokerServer, err = brokerClient.Get(name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error getting broker %s (%s)", name, err) + } + if name != brokerServer.Name && + broker.ResourceVersion == brokerServer.ResourceVersion { + return fmt.Errorf("didn't get the same broker back from the server \n%+v\n%+v", broker, brokerServer) + } + + // check that the broker is the same from get and list + brokerListed := &brokers.Items[0] + if !reflect.DeepEqual(brokerServer, brokerListed) { + return fmt.Errorf( + "Didn't get the same instance from list and get: diff: %v", + diff.ObjectReflectDiff(brokerServer, brokerListed), + ) + } + + authSecret := &v1beta1.LocalObjectReference{ + Name: "test-name", + } + + brokerServer.Spec.AuthInfo = &v1beta1.ServiceBrokerAuthInfo{ + Basic: &v1beta1.BasicAuthConfig{ + SecretRef: authSecret, + }, + } + + brokerUpdated, err := brokerClient.Update(brokerServer) + if nil != err || + "test-name" != brokerUpdated.Spec.AuthInfo.Basic.SecretRef.Name { + return fmt.Errorf("broker wasn't updated, %v, %v", brokerServer, brokerUpdated) + } + + readyConditionTrue := v1beta1.ServiceBrokerCondition{ + Type: v1beta1.ServiceBrokerConditionReady, + Status: v1beta1.ConditionTrue, + Reason: "ConditionReason", + Message: "ConditionMessage", + } + brokerUpdated.Status = v1beta1.ServiceBrokerStatus{ + CommonServiceBrokerStatus: v1beta1.CommonServiceBrokerStatus{ + Conditions: []v1beta1.ServiceBrokerCondition{ + readyConditionTrue, + }, + }, + } + brokerUpdated.Spec.URL = "http://shouldnotupdate.com" + + brokerUpdated2, err := brokerClient.UpdateStatus(brokerUpdated) + if nil != err || len(brokerUpdated2.Status.Conditions) != 1 { + return fmt.Errorf("broker status wasn't updated") + } + if e, a := readyConditionTrue, brokerUpdated2.Status.Conditions[0]; !reflect.DeepEqual(e, a) { + return fmt.Errorf("Didn't get matching ready conditions:\nexpected: %v\n\ngot: %v", e, a) + } + if e, a := "https://example.com", brokerUpdated2.Spec.URL; e != a { + return fmt.Errorf("Should not be able to update spec from status subresource") + } + + readyConditionFalse := v1beta1.ServiceBrokerCondition{ + Type: v1beta1.ServiceBrokerConditionReady, + Status: v1beta1.ConditionFalse, + Reason: "ConditionReason", + Message: "ConditionMessage", + } + brokerUpdated2.Status.Conditions[0] = readyConditionFalse + + brokerUpdated3, err := brokerClient.UpdateStatus(brokerUpdated2) + if nil != err || len(brokerUpdated3.Status.Conditions) != 1 { + return fmt.Errorf("broker status wasn't updated (%s)", err) + } + + brokerServer, err = brokerClient.Get(name, metav1.GetOptions{}) + if nil != err || + "test-name" != brokerServer.Spec.AuthInfo.Basic.SecretRef.Name { + return fmt.Errorf("broker wasn't updated (%v)", brokerServer) + } + if e, a := readyConditionFalse, brokerServer.Status.Conditions[0]; !reflect.DeepEqual(e, a) { + return fmt.Errorf("Didn't get matching ready conditions:\nexpected: %v\n\ngot: %v", e, a) + } + + err = brokerClient.Delete(name, &metav1.DeleteOptions{}) + if nil != err { + return fmt.Errorf("broker should be deleted (%s)", err) + } + + brokerDeleted, err := brokerClient.Get(name, metav1.GetOptions{}) + if nil != err { + return fmt.Errorf("broker should not be deleted (%v): %v", brokerDeleted, err) + } + + brokerDeleted.ObjectMeta.Finalizers = nil + _, err = brokerClient.UpdateStatus(brokerDeleted) + if nil != err { + return fmt.Errorf("broker should be deleted (%v): %v", brokerDeleted, err) + } + + brokerDeleted, err = brokerClient.Get("test-broker", metav1.GetOptions{}) + if nil == err { + return fmt.Errorf("broker should be deleted (%v)", brokerDeleted) + } + return nil +} + +// TestClusterServiceClassClient exercises the ClusterServiceClass client. +func TestClusterServiceClassClient(t *testing.T) { + const name = "test-serviceclass" + sType := server.StorageTypeEtcd + client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + return &servicecatalog.ClusterServiceClass{} + }) + defer shutdownServer() + + if err := testClusterServiceClassClient(sType, client, name); err != nil { + t.Fatal(err) + } +} + +func testClusterServiceClassClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { + serviceClassClient := client.Servicecatalog().ClusterServiceClasses() + + serviceClass := &v1beta1.ClusterServiceClass{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1beta1.ClusterServiceClassSpec{ + ClusterServiceBrokerName: "test-broker", + CommonServiceClassSpec: v1beta1.CommonServiceClassSpec{ + Bindable: true, + ExternalName: name, + ExternalID: "b8269ab4-7d2d-456d-8c8b-5aab63b321d1", + Description: "test description", + }, + }, + } + + // start from scratch + serviceClasses, err := serviceClassClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if serviceClasses.Items == nil { + return fmt.Errorf("Items field should not be set to nil") + } + if len(serviceClasses.Items) > 0 { + return fmt.Errorf( + "serviceClasses should not exist on start, had %v serviceClasses", + len(serviceClasses.Items), + ) + } + + serviceClassAtServer, err := serviceClassClient.Create(serviceClass) + if nil != err { + return fmt.Errorf("error creating the ClusterServiceClass (%v)", serviceClass) + } + if name != serviceClassAtServer.Name { + return fmt.Errorf( + "didn't get the same ClusterServiceClass back from the server \n%+v\n%+v", + serviceClass, + serviceClassAtServer, + ) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 1 != len(serviceClasses.Items) { + return fmt.Errorf("should have exactly one ClusterServiceClass, had %v ClusterServiceClasses", len(serviceClasses.Items)) + } + + serviceClassAtServer, err = serviceClassClient.Get(name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if serviceClassAtServer.Name != name && + serviceClass.ResourceVersion == serviceClassAtServer.ResourceVersion { + return fmt.Errorf( + "didn't get the same ClusterServiceClass back from the server \n%+v\n%+v", + serviceClass, + serviceClassAtServer, + ) + } + + // check that the broker is the same from get and list + serviceClassListed := &serviceClasses.Items[0] + if !reflect.DeepEqual(serviceClassAtServer, serviceClassListed) { + return fmt.Errorf( + "Didn't get the same instance from list and get: diff: %v", + diff.ObjectReflectDiff(serviceClassAtServer, serviceClassListed), + ) + } + + serviceClassAtServer.Spec.Bindable = false + _, err = serviceClassClient.Update(serviceClassAtServer) + if err != nil { + return fmt.Errorf("Error updating serviceClass: %v", err) + } + updated, err := serviceClassClient.Get(name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("Error getting serviceClass: %v", err) + } + if updated.Spec.Bindable { + return errors.New("Failed to update service class") + } + + // Test status subresource + updated.Status.RemovedFromBrokerCatalog = true + updated, err = serviceClassClient.UpdateStatus(updated) + if err != nil { + return fmt.Errorf("Error updating serviceClass status: %v", err) + } + if !updated.Status.RemovedFromBrokerCatalog { + return errors.New("Expected status.removedFromBrokerCatalog = true, got false") + } + + // Ok, let's verify the field selectors + sc2Name := name + "2" + sc2ID := "someotheridhere" + serviceClass2 := &v1beta1.ClusterServiceClass{ + ObjectMeta: metav1.ObjectMeta{Name: sc2Name}, + Spec: v1beta1.ClusterServiceClassSpec{ + ClusterServiceBrokerName: "test-broker", + CommonServiceClassSpec: v1beta1.CommonServiceClassSpec{ + Bindable: true, + ExternalName: sc2Name, + ExternalID: sc2ID, + Description: "test description 2", + }, + }, + } + _, err = serviceClassClient.Create(serviceClass2) + if nil != err { + return fmt.Errorf("error creating the ClusterServiceClass (%v) : %s", serviceClass2, err) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 2 != len(serviceClasses.Items) { + return fmt.Errorf("should have two ClusterServiceClasses, had %v ClusterServiceClasses", len(serviceClasses.Items)) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.externalName==" + sc2Name}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 1 != len(serviceClasses.Items) { + return fmt.Errorf("*should have one ClusterServiceClass, had %v ClusterServiceClassess : %+v", len(serviceClasses.Items), serviceClasses.Items) + } + + if serviceClasses.Items[0].Spec.ExternalID != sc2ID { + return fmt.Errorf("should have same externalID: %q, got %q", sc2ID, serviceClasses.Items[0].Spec.ExternalID) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.externalID==" + "b8269ab4-7d2d-456d-8c8b-5aab63b321d1"}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 1 != len(serviceClasses.Items) { + return fmt.Errorf("**should have one ClusterServiceClass, had %v ClusterServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) + } + + if serviceClasses.Items[0].Spec.ExternalName != name { + return fmt.Errorf("should have same externalName: %q, got %q", name, serviceClasses.Items[0].Spec.ExternalName) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.externalName==" + "crap"}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 0 != len(serviceClasses.Items) { + return fmt.Errorf("should have zero ClusterServiceClasses, had %v ClusterServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.clusterServiceBrokerName=" + "test-broker"}) + if err != nil { + return fmt.Errorf("error listing service classes (%s)", err) + } + if 2 != len(serviceClasses.Items) { + return fmt.Errorf("should have two ClusterServiceClasses, had %v ClusterServiceClasses", len(serviceClasses.Items)) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ClusterServiceClasses: %v", err) + } + + if 1 != len(serviceClasses.Items) { + return fmt.Errorf("should have exactly one ClusterServiceClass, had %v ClusterServiceClasses", len(serviceClasses.Items)) + } + + err = serviceClassClient.Delete(name, &metav1.DeleteOptions{}) + if nil != err { + return fmt.Errorf("serviceclass should be deleted (%s)", err) + } + + serviceClassDeleted, err := serviceClassClient.Get(name, metav1.GetOptions{}) + if nil == err { + return fmt.Errorf("serviceclass should be deleted (%v)", serviceClassDeleted) + } + + err = serviceClassClient.Delete(sc2Name, &metav1.DeleteOptions{}) + if nil != err { + return fmt.Errorf("serviceclass should be deleted (%s)", err) + } + return nil +} - if err := testClusterServiceClassClient(sType, client, name); err != nil { - t.Fatal(err) - } - } +// TestNamespacedServiceClassClient exercises the namespaced ServiceClass client. +func TestNamespacedServiceClassClient(t *testing.T) { + const name = "test-serviceclass" + const namespace = "test-namespace" + resetFeaturesFunc, err := enableNamespacedResources() + if err != nil { + t.Fatal(err) } - // TODO: Fix this for CRD. - // https://github.com/kubernetes-incubator/service-catalog/issues/1256 - // for _, sType := range storageTypes { - // if !t.Run(sType.String(), rootTestFunc(sType)) { - // t.Errorf("%q test failed", sType) - // } - // } - // for _, sType := range storageTypes { - // if !t.Run(sType.String(), rootTestFunc(sType)) { - // t.Errorf("%q test failed", sType) - // } - // } + defer resetFeaturesFunc() sType := server.StorageTypeEtcd - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) + client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + return &servicecatalog.ServiceClass{} + }) + defer shutdownServer() + + if err := testNamespacedServiceClassClient(sType, client, namespace, name); err != nil { + t.Fatal(err) } } -func testClusterServiceClassClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { - serviceClassClient := client.Servicecatalog().ClusterServiceClasses() +func testNamespacedServiceClassClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { + serviceClassClient := client.Servicecatalog().ServiceClasses(namespace) - serviceClass := &v1beta1.ClusterServiceClass{ + serviceClass := &v1beta1.ServiceClass{ ObjectMeta: metav1.ObjectMeta{Name: name}, - Spec: v1beta1.ClusterServiceClassSpec{ - ClusterServiceBrokerName: "test-broker", + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: "test-broker", CommonServiceClassSpec: v1beta1.CommonServiceClassSpec{ Bindable: true, ExternalName: name, @@ -411,11 +804,11 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal serviceClassAtServer, err := serviceClassClient.Create(serviceClass) if nil != err { - return fmt.Errorf("error creating the ClusterServiceClass (%v)", serviceClass) + return fmt.Errorf("error creating the ServiceClass (%v)", serviceClass) } if name != serviceClassAtServer.Name { return fmt.Errorf( - "didn't get the same ClusterServiceClass back from the server \n%+v\n%+v", + "didn't get the same ServiceClass back from the server \n%+v\n%+v", serviceClass, serviceClassAtServer, ) @@ -426,7 +819,7 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal return fmt.Errorf("error listing service classes (%s)", err) } if 1 != len(serviceClasses.Items) { - return fmt.Errorf("should have exactly one ClusterServiceClass, had %v ClusterServiceClasses", len(serviceClasses.Items)) + return fmt.Errorf("should have exactly one ServiceClass, had %v ServiceClasses", len(serviceClasses.Items)) } serviceClassAtServer, err = serviceClassClient.Get(name, metav1.GetOptions{}) @@ -436,13 +829,13 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal if serviceClassAtServer.Name != name && serviceClass.ResourceVersion == serviceClassAtServer.ResourceVersion { return fmt.Errorf( - "didn't get the same ClusterServiceClass back from the server \n%+v\n%+v", + "didn't get the same ServiceClass back from the server \n%+v\n%+v", serviceClass, serviceClassAtServer, ) } - // check that the broker is the same from get and list + // check that the service class is the same from get and list serviceClassListed := &serviceClasses.Items[0] if !reflect.DeepEqual(serviceClassAtServer, serviceClassListed) { return fmt.Errorf( @@ -477,10 +870,10 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal // Ok, let's verify the field selectors sc2Name := name + "2" sc2ID := "someotheridhere" - serviceClass2 := &v1beta1.ClusterServiceClass{ + serviceClass2 := &v1beta1.ServiceClass{ ObjectMeta: metav1.ObjectMeta{Name: sc2Name}, - Spec: v1beta1.ClusterServiceClassSpec{ - ClusterServiceBrokerName: "test-broker", + Spec: v1beta1.ServiceClassSpec{ + ServiceBrokerName: "test-broker", CommonServiceClassSpec: v1beta1.CommonServiceClassSpec{ Bindable: true, ExternalName: sc2Name, @@ -491,7 +884,7 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal } _, err = serviceClassClient.Create(serviceClass2) if nil != err { - return fmt.Errorf("error creating the ClusterServiceClass (%v) : %s", serviceClass2, err) + return fmt.Errorf("error creating the ServiceClass (%v) : %s", serviceClass2, err) } serviceClasses, err = serviceClassClient.List(metav1.ListOptions{}) @@ -499,7 +892,7 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal return fmt.Errorf("error listing service classes (%s)", err) } if 2 != len(serviceClasses.Items) { - return fmt.Errorf("should have two ClusterServiceClasses, had %v ClusterServiceClasses", len(serviceClasses.Items)) + return fmt.Errorf("should have two ServiceClasses, had %v ServiceClasses", len(serviceClasses.Items)) } serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.externalName==" + sc2Name}) @@ -507,7 +900,7 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal return fmt.Errorf("error listing service classes (%s)", err) } if 1 != len(serviceClasses.Items) { - return fmt.Errorf("*should have one ClusterServiceClass, had %v ClusterServiceClassess : %+v", len(serviceClasses.Items), serviceClasses.Items) + return fmt.Errorf("*should have one ServiceClass, had %v ServiceClassess : %+v", len(serviceClasses.Items), serviceClasses.Items) } if serviceClasses.Items[0].Spec.ExternalID != sc2ID { @@ -519,7 +912,7 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal return fmt.Errorf("error listing service classes (%s)", err) } if 1 != len(serviceClasses.Items) { - return fmt.Errorf("**should have one ClusterServiceClass, had %v ClusterServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) + return fmt.Errorf("**should have one ServiceClass, had %v ServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) } if serviceClasses.Items[0].Spec.ExternalName != name { @@ -531,15 +924,33 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal return fmt.Errorf("error listing service classes (%s)", err) } if 0 != len(serviceClasses.Items) { - return fmt.Errorf("should have zero ClusterServiceClasses, had %v ClusterServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) + return fmt.Errorf("should have zero ServiceClasses, had %v ServiceClasses : %+v", len(serviceClasses.Items), serviceClasses.Items) } - serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.clusterServiceBrokerName=" + "test-broker"}) + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "spec.serviceBrokerName=" + "test-broker"}) if err != nil { return fmt.Errorf("error listing service classes (%s)", err) } if 2 != len(serviceClasses.Items) { - return fmt.Errorf("should have two ClusterServiceClasses, had %v ClusterServiceClasses", len(serviceClasses.Items)) + return fmt.Errorf("should have two ServiceClasses, had %v ServiceClasses", len(serviceClasses.Items)) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ServiceClasses: %v", err) + } + + if 1 != len(serviceClasses.Items) { + return fmt.Errorf("should have exactly one ServiceClass, had %v ServiceClasses", len(serviceClasses.Items)) + } + + serviceClasses, err = serviceClassClient.List(metav1.ListOptions{FieldSelector: "metadata.namespace=" + namespace}) + if err != nil { + return fmt.Errorf("error listing ServiceClasses: %v", err) + } + + if 2 != len(serviceClasses.Items) { + return fmt.Errorf("should have exactly two ServiceClasses, had %v ServiceClasses", len(serviceClasses.Items)) } err = serviceClassClient.Delete(name, &metav1.DeleteOptions{}) @@ -561,29 +972,15 @@ func testClusterServiceClassClient(sType server.StorageType, client servicecatal // TestClusterServicePlanClient exercises the ClusterServicePlan client. func TestClusterServicePlanClient(t *testing.T) { - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - const name = "test-serviceplan" - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServicePlan{} - }) - defer shutdownServer() - - if err := testClusterServicePlanClient(sType, client, name); err != nil { - t.Fatal(err) - } - } - } - // TODO: Fix this for CRD. - // https://github.com/kubernetes-incubator/service-catalog/issues/1256 - // for _, sType := range storageTypes { - // if !t.Run(sType.String(), rootTestFunc(sType)) { - // t.Errorf("%q test failed", sType) - // } - // } + const name = "test-serviceplan" sType := server.StorageTypeEtcd - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) + client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + return &servicecatalog.ClusterServicePlan{} + }) + defer shutdownServer() + + if err := testClusterServicePlanClient(sType, client, name); err != nil { + t.Fatal(err) } } @@ -759,6 +1156,243 @@ func testClusterServicePlanClient(sType server.StorageType, client servicecatalo return fmt.Errorf("should have two ClusterServicePlans, had %v ClusterServicePlans : %+v", len(servicePlans.Items), servicePlans.Items) } + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ClusterServicePlans: %v", err) + } + + if 1 != len(servicePlans.Items) { + return fmt.Errorf("should have exactly one ClusterServicePlan, had %v ClusterServicePlans", len(servicePlans.Items)) + } + + err = servicePlanClient.Delete(name, &metav1.DeleteOptions{}) + if nil != err { + return fmt.Errorf("serviceplan should be deleted (%s)", err) + } + + servicePlanDeleted, err := servicePlanClient.Get(name, metav1.GetOptions{}) + if nil == err { + return fmt.Errorf("serviceplan should be deleted (%v)", servicePlanDeleted) + } + + err = servicePlanClient.Delete(sp2Name, &metav1.DeleteOptions{}) + if nil != err { + return fmt.Errorf("serviceplan should be deleted (%s)", err) + } + + return nil +} + +// TestNamespacedServicePlanClient exercises the namespaced ServicePlan client. +func TestNamespacedServicePlanClient(t *testing.T) { + const name = "test-serviceplan" + const namespace = "test-namespace" + resetFeaturesFunc, err := enableNamespacedResources() + if err != nil { + t.Fatal(err) + } + defer resetFeaturesFunc() + sType := server.StorageTypeEtcd + client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + return &servicecatalog.ServicePlan{} + }) + defer shutdownServer() + + if err := testNamespacedServicePlanClient(sType, client, namespace, name); err != nil { + t.Fatal(err) + } +} + +func testNamespacedServicePlanClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { + servicePlanClient := client.Servicecatalog().ServicePlans(namespace) + + bindable := true + servicePlan := &v1beta1.ServicePlan{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1beta1.ServicePlanSpec{ + ServiceBrokerName: "test-broker", + CommonServicePlanSpec: v1beta1.CommonServicePlanSpec{ + Bindable: &bindable, + ExternalName: name, + ExternalID: "b8269ab4-7d2d-456d-8c8b-5aab63b321d1", + Description: "test description", + }, + ServiceClassRef: v1beta1.LocalObjectReference{ + Name: "test-serviceclass", + }, + }, + } + + // start from scratch + servicePlans, err := servicePlanClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if servicePlans.Items == nil { + return fmt.Errorf("Items field should not be set to nil") + } + if len(servicePlans.Items) > 0 { + return fmt.Errorf( + "servicePlans should not exist on start, had %v servicePlans", + len(servicePlans.Items), + ) + } + + servicePlanAtServer, err := servicePlanClient.Create(servicePlan) + if nil != err { + return fmt.Errorf("error creating the Serviceplan (%v)", servicePlan) + } + if name != servicePlanAtServer.Name { + return fmt.Errorf( + "didn't get the same ServicePlan back from the server \n%+v\n%+v", + servicePlan, + servicePlanAtServer, + ) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 1 != len(servicePlans.Items) { + return fmt.Errorf("should have exactly one ServicePlan, had %v ServicePlans", len(servicePlans.Items)) + } + + servicePlanAtServer, err = servicePlanClient.Get(name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if servicePlanAtServer.Name != name && + servicePlan.ResourceVersion == servicePlanAtServer.ResourceVersion { + return fmt.Errorf( + "didn't get the same ServicePlan back from the server \n%+v\n%+v", + servicePlan, + servicePlanAtServer, + ) + } + + // check that the plan is the same from get and list + servicePlanListed := &servicePlans.Items[0] + if !reflect.DeepEqual(servicePlanAtServer, servicePlanListed) { + return fmt.Errorf( + "Didn't get the same instance from list and get: diff: %v", + diff.ObjectReflectDiff(servicePlanAtServer, servicePlanListed), + ) + } + + bindable = false + servicePlanAtServer.Spec.Bindable = &bindable + _, err = servicePlanClient.Update(servicePlanAtServer) + if err != nil { + return fmt.Errorf("Error updating servicePlan: %v", err) + } + updated, err := servicePlanClient.Get(name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("Error getting servicePlan: %v", err) + } + if *updated.Spec.Bindable { + return errors.New("Failed to update service class") + } + + // Test status subresource + updated.Status.RemovedFromBrokerCatalog = true + updated, err = servicePlanClient.UpdateStatus(updated) + if err != nil { + return fmt.Errorf("Error updating servicePlan status: %v", err) + } + if !updated.Status.RemovedFromBrokerCatalog { + return errors.New("Expected status.removedFromBrokerCatalog = true, got false") + } + + // Verify that field selectors work by listing. + sp2Name := name + "2" + sp2ID := "anotheridhere" + servicePlan2 := &v1beta1.ServicePlan{ + ObjectMeta: metav1.ObjectMeta{Name: sp2Name}, + Spec: v1beta1.ServicePlanSpec{ + ServiceBrokerName: "test-broker", + CommonServicePlanSpec: v1beta1.CommonServicePlanSpec{ + Bindable: &bindable, + ExternalName: sp2Name, + ExternalID: sp2ID, + Description: "test description 2", + }, + ServiceClassRef: v1beta1.LocalObjectReference{ + Name: "test-serviceclass", + }, + }, + } + _, err = servicePlanClient.Create(servicePlan2) + if nil != err { + return fmt.Errorf("error creating the second Serviceplan (%v)", servicePlan2) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 2 != len(servicePlans.Items) { + return fmt.Errorf("should have two ServicePlans, had %v ServicePlans", len(servicePlans.Items)) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "spec.externalName==" + sp2Name}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 1 != len(servicePlans.Items) { + return fmt.Errorf("should have one ServicePlan, had %v ServicePlans : %+v", len(servicePlans.Items), servicePlans.Items) + } + + if servicePlans.Items[0].Spec.ExternalID != sp2ID { + return fmt.Errorf("should have same externalID: %q, got %q", sp2ID, servicePlans.Items[0].Spec.ExternalID) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "spec.externalID==" + "b8269ab4-7d2d-456d-8c8b-5aab63b321d1"}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 1 != len(servicePlans.Items) { + return fmt.Errorf("should have one ServicePlan, had %v ServicePlans : %+v", len(servicePlans.Items), servicePlans.Items) + } + + if servicePlans.Items[0].Spec.ExternalName != name { + return fmt.Errorf("should have same externalName: %q, got %q", name, servicePlans.Items[0].Spec.ExternalName) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "spec.externalName==" + "crap"}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 0 != len(servicePlans.Items) { + return fmt.Errorf("should have zero ServicePlans, had %v ServicePlans : %+v", len(servicePlans.Items), servicePlans.Items) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "spec.serviceBrokerName=" + "test-broker"}) + if err != nil { + return fmt.Errorf("error listing service plans (%s)", err) + } + if 2 != len(servicePlans.Items) { + return fmt.Errorf("should have two ServicePlans, had %v ServicePlans : %+v", len(servicePlans.Items), servicePlans.Items) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing ServicePlans: %v", err) + } + + if 1 != len(servicePlans.Items) { + return fmt.Errorf("should have exactly one ServicePlan, had %v ServicePlans", len(servicePlans.Items)) + } + + servicePlans, err = servicePlanClient.List(metav1.ListOptions{FieldSelector: "metadata.namespace=" + namespace}) + if err != nil { + return fmt.Errorf("error listing ServicePlans: %v", err) + } + + if 2 != len(servicePlans.Items) { + return fmt.Errorf("should have exactly two ServicePlans, had %v ServicePlans", len(servicePlans.Items)) + } + err = servicePlanClient.Delete(name, &metav1.DeleteOptions{}) if nil != err { return fmt.Errorf("serviceplan should be deleted (%s)", err) @@ -992,6 +1626,15 @@ func testInstanceClient(sType server.StorageType, client servicecatalogclient.In return fmt.Errorf("should have exactly one instance, had %v instances", len(instances.Items)) } + instances, err = instanceClient.List(metav1.ListOptions{FieldSelector: "metadata.name=" + name}) + if err != nil { + return fmt.Errorf("error listing instances: %v", err) + } + + if 1 != len(instances.Items) { + return fmt.Errorf("should have exactly one instance, had %v instances", len(instances.Items)) + } + // update the instance's spec updateRequests := instanceServer.Spec.UpdateRequests + 1 expectedGeneration := instanceServer.Generation + 1 diff --git a/test/integration/util.go b/test/integration/util.go index e4ee4d847b1..3fa2b2f5dc3 100644 --- a/test/integration/util.go +++ b/test/integration/util.go @@ -16,6 +16,22 @@ limitations under the License. package integration +import ( + "time" + + "k8s.io/apiserver/pkg/util/feature" + + scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" +) + +var ( + // How often to poll for conditions + pollInterval = 2 * time.Second + + // Default time to wait for operations to complete + defaultTimeout = 30 * time.Second +) + // strPtr, String Pointer, returns the address of s. useful for filling struct // fields that require a *string (for json decoding purposes). func strPtr(s string) *string { @@ -33,3 +49,19 @@ func falsePtr() *bool { b := false return &b } + +func enableNamespacedResources() (resetFeaturesFunc func(), err error) { + previousFeatureGate := feature.DefaultFeatureGate + + newFeatureGate := feature.NewFeatureGate() + if err := newFeatureGate.Add(map[feature.Feature]feature.FeatureSpec{ + scfeatures.NamespacedServiceBroker: {Default: true, PreRelease: feature.Alpha}, + }); err != nil { + return nil, err + } + feature.DefaultFeatureGate = newFeatureGate + + return func() { + feature.DefaultFeatureGate = previousFeatureGate + }, nil +}