diff --git a/Gopkg.lock b/Gopkg.lock index e957d08d542..94d4d0d9fe1 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -831,6 +831,7 @@ "pkg/api/equality", "pkg/api/errors", "pkg/api/meta", + "pkg/api/meta/table", "pkg/api/resource", "pkg/api/testing", "pkg/api/testing/fuzzer", @@ -860,6 +861,7 @@ "pkg/util/cache", "pkg/util/clock", "pkg/util/diff", + "pkg/util/duration", "pkg/util/errors", "pkg/util/framer", "pkg/util/intstr", @@ -1219,6 +1221,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "1cf27edf2c8dd4bad7c5aa80f4bd5c7bec2652e61bfee5f4bd0f5a8eb9aba030" + inputs-digest = "18c14a2444b3d1edffdea1eba30f21a69343dd8331a58ce2b8899ccd7b25aa5c" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 14124a3d280..5af0704a6a0 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -13,8 +13,7 @@ # Force dep to vendor the code generators, which aren't imported just used at dev time. # Picking a subpackage with Go code won't be necessary once https://github.com/golang/dep/pull/1545 is merged. required = [ - "github.com/jteeuwen/go-bindata/go-bindata", - "k8s.io/code-generator/cmd/defaulter-gen", + "github.com/jteeuwen/go-bindata/go-bindata","k8s.io/code-generator/cmd/defaulter-gen", "k8s.io/code-generator/cmd/deepcopy-gen", "k8s.io/code-generator/cmd/conversion-gen", "k8s.io/code-generator/cmd/client-gen", diff --git a/UPSTREAM-VERSION b/UPSTREAM-VERSION index fac8924a184..3a121d002a0 100644 --- a/UPSTREAM-VERSION +++ b/UPSTREAM-VERSION @@ -1 +1 @@ -v0.1.29 +v0.1.30 diff --git a/charts/catalog/Chart.yaml b/charts/catalog/Chart.yaml index 39107c7a88d..d8488dd47c1 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.29 +version: 0.1.30 diff --git a/charts/catalog/README.md b/charts/catalog/README.md index 7ce1084a41d..36c91e33e19 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.29` | +| `image` | apiserver image to use | `quay.io/kubernetes-service-catalog/service-catalog:v0.1.30` | | `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 | | @@ -53,6 +53,8 @@ chart and their default values. | `apiserver.storage.type` | The storage backend to use; the only valid value is `etcd`, left for other storages support in future, e.g. `crd` | `etcd` | | `apiserver.storage.etcd.useEmbedded` | If storage type is `etcd`: Whether to embed an etcd container in the apiserver pod; THIS IS INADEQUATE FOR PRODUCTION USE! | `true` | | `apiserver.storage.etcd.servers` | If storage type is `etcd`: etcd URL(s); override this if NOT using embedded etcd. Only etcd v3 is supported. | `http://localhost:2379` | +| `apiserver.storage.etcd.image` | etcd image to use | `quay.io/coreos/etcd:latest` | +| `apiserver.storage.etcd.imagePullPolicy` | `imagePullPolicy` for etcd | `Always` | | `apiserver.storage.etcd.persistence.enabled` | Enable persistence using PVC | `false` | | `apiserver.storage.etcd.persistence.storageClass` | PVC Storage Class | `nil` (uses alpha storage class annotation) | | `apiserver.storage.etcd.persistence.accessMode` | PVC Access Mode | `ReadWriteOnce` | @@ -82,7 +84,7 @@ chart and their default values. | `controllerManager.resources` | Resources allocation (Requests and Limits) | `{requests: {cpu: 100m, memory: 20Mi}, limits: {cpu: 100m, memory: 30Mi}}` | | `useAggregator` | whether or not to set up the controller-manager to go through the main Kubernetes API server's API aggregator | `true` | | `rbacEnable` | If true, create & use RBAC resources | `true` | -| `originatingIdentityEnabled` | Whether the OriginatingIdentity alpha feature should be enabled | `false` | +| `originatingIdentityEnabled` | Whether the OriginatingIdentity feature should be enabled | `true` | | `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` | diff --git a/charts/catalog/templates/apiserver-deployment.yaml b/charts/catalog/templates/apiserver-deployment.yaml index 4f2e112809f..b8a43397d05 100644 --- a/charts/catalog/templates/apiserver-deployment.yaml +++ b/charts/catalog/templates/apiserver-deployment.yaml @@ -56,10 +56,8 @@ spec: {{- if not .Values.apiserver.auth.enabled }} - --disable-auth {{- end }} - {{- if .Values.originatingIdentityEnabled }} - --feature-gates - - OriginatingIdentity=true - {{- end }} + - OriginatingIdentity={{.Values.originatingIdentityEnabled}} {{- if .Values.namespacedServiceBrokerDisabled }} - --feature-gates - NamespacedServiceBroker=false @@ -67,12 +65,22 @@ spec: {{- if .Values.apiserver.serveOpenAPISpec }} - --serve-openapi-spec {{- end }} + {{- if .Values.apiserver.storage.etcd.tls.enabled }} + - --etcd-cafile=/var/run/etcd-client/etcd-client-ca.crt + - --etcd-certfile=/var/run/etcd-client/etcd-client.crt + - --etcd-keyfile=/var/run/etcd-client/etcd-client.key + {{- end }} ports: - containerPort: 8443 volumeMounts: - name: apiserver-cert mountPath: /var/run/kubernetes-service-catalog readOnly: true + {{- if .Values.apiserver.storage.etcd.tls.enabled }} + - name: etcd-client-cert + mountPath: /var/run/etcd-client + readOnly: true + {{- end }} {{- if .Values.apiserver.healthcheck.enabled }} readinessProbe: httpGet: @@ -97,8 +105,8 @@ spec: {{- end }} {{- if and (eq .Values.apiserver.storage.type "etcd") .Values.apiserver.storage.etcd.useEmbedded }} - name: etcd - image: quay.io/coreos/etcd:latest - imagePullPolicy: Always + image: {{ .Values.apiserver.storage.etcd.image }} + imagePullPolicy: {{ .Values.apiserver.storage.etcd.imagePullPolicy }} resources: {{ toYaml .Values.apiserver.storage.etcd.resources | indent 10 }} env: @@ -160,3 +168,8 @@ spec: emptyDir: {} {{- end }} {{- end }} + {{- if .Values.apiserver.storage.etcd.tls.enabled }} + - name: etcd-client-cert + secret: + secretName: {{ .Values.apiserver.storage.etcd.tls.clientCertSecretName }} + {{- end }} diff --git a/charts/catalog/templates/controller-manager-deployment.yaml b/charts/catalog/templates/controller-manager-deployment.yaml index 54dd2e2b70c..9e8a4d90aa9 100644 --- a/charts/catalog/templates/controller-manager-deployment.yaml +++ b/charts/catalog/templates/controller-manager-deployment.yaml @@ -69,10 +69,8 @@ spec: - --broker-relist-interval - {{ .Values.controllerManager.brokerRelistInterval }} {{- end }} - {{- if .Values.originatingIdentityEnabled }} - --feature-gates - - OriginatingIdentity=true - {{- end }} + - OriginatingIdentity={{.Values.originatingIdentityEnabled}} {{- if .Values.asyncBindingOperationsEnabled }} - --feature-gates - AsyncBindingOperations=true diff --git a/charts/catalog/values.yaml b/charts/catalog/values.yaml index 9038549bedd..c78e3de7274 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.29 +image: quay.io/kubernetes-service-catalog/service-catalog:v0.1.30 # imagePullPolicy for the service-catalog; valid values are "IfNotPresent", # "Never", and "Always" imagePullPolicy: Always @@ -58,11 +58,24 @@ apiserver: type: etcd # Further configuration for the etcd-based backend etcd: + # Whether to enable TLS communitation with etcd + tls: + enabled: false + ## If etcd tls is enabled you need to provide name of secret which stores 3 keys: + ## etcd-client-ca.crt - SSL Certificate Authority file used to secure etcd communication + ## etcd-client.crt - SSL certification file used to secure etcd communication. + ## etcd-client.key - SSL key file used to secure etcd communication. + clientCertSecretName: # Whether to embed an etcd container in the apiserver pod # THIS IS INADEQUATE FOR PRODUCTION USE! useEmbedded: true # etcd URL(s); override this if NOT using embedded etcd servers: http://localhost:2379 + # etcd image to use + image: quay.io/coreos/etcd:latest + # imagePullPolicy for the etcd; valid values are "IfNotPresent", + # "Never", and "Always" + imagePullPolicy: Always # etcd persistence options IF using embedded etcd persistence: enabled: false @@ -148,8 +161,8 @@ controllerManager: limits: cpu: 100m memory: 30Mi -# Whether the OriginatingIdentity alpha feature should be enabled -originatingIdentityEnabled: false +# Whether the OriginatingIdentity feature should be enabled +originatingIdentityEnabled: true # Whether the AsyncBindingOperations alpha feature should be enabled asyncBindingOperationsEnabled: false # Whether the NamespacedServiceBroker alpha feature should be disabled diff --git a/cmd/svcat/broker/register_cmd.go b/cmd/svcat/broker/register_cmd.go index 79f198ae340..e2292c7516a 100644 --- a/cmd/svcat/broker/register_cmd.go +++ b/cmd/svcat/broker/register_cmd.go @@ -18,23 +18,43 @@ package broker import ( "fmt" + "os" + "strings" + "time" "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/apis/servicecatalog/v1beta1" + servicecatalog "github.com/kubernetes-incubator/service-catalog/pkg/svcat/service-catalog" "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // RegisterCmd contains the information needed to register a broker type RegisterCmd struct { - BrokerName string - Context *command.Context - URL string + *command.Namespaced + *command.Waitable + + Context *command.Context + + BasicSecret string + BearerSecret string + BrokerName string + CAFile string + ClassRestrictions []string + PlanRestrictions []string + SkipTLS bool + RelistBehavior string + RelistDuration time.Duration + URL string } // NewRegisterCmd builds a "svcat register" command func NewRegisterCmd(cxt *command.Context) *cobra.Command { registerCmd := &RegisterCmd{ - Context: cxt, + Context: cxt, + Namespaced: command.NewNamespaced(cxt), + Waitable: command.NewWaitable(), } cmd := &cobra.Command{ Use: "register NAME --url URL", @@ -48,27 +68,73 @@ func NewRegisterCmd(cxt *command.Context) *cobra.Command { cmd.Flags().StringVar(®isterCmd.URL, "url", "", "The broker URL (Required)") cmd.MarkFlagRequired("url") + cmd.Flags().StringVar(®isterCmd.BasicSecret, "basic-secret", "", + "A secret containing basic auth (username/password) information to connect to the broker") + cmd.Flags().StringVar(®isterCmd.BearerSecret, "bearer-secret", "", + "A secret containing a bearer token to connect to the broker") + cmd.Flags().StringVar(®isterCmd.CAFile, "ca", "", + "A file containing the CA certificate to connect to the broker") + cmd.Flags().StringSliceVar(®isterCmd.ClassRestrictions, "class-restrictions", []string{}, + "A list of restrictions to apply to the classes allowed from the broker") + cmd.Flags().StringSliceVar(®isterCmd.PlanRestrictions, "plan-restrictions", []string{}, + "A list of restrictions to apply to the plans allowed from the broker") + cmd.Flags().StringVar(®isterCmd.RelistBehavior, "relist-behavior", "", + "Behavior for relisting the broker's catalog. Valid options are manual or duration. Defaults to duration with an interval of 15m.") + cmd.Flags().DurationVar(®isterCmd.RelistDuration, "relist-duration", 0*time.Second, + "Interval to refetch broker catalog when relist-behavior is set to duration, specified in human readable format: 30s, 1m, 1h") + cmd.Flags().BoolVar(®isterCmd.SkipTLS, "skip-tls", false, + "Disables TLS certificate verification when communicating with this broker. This is strongly discouraged. You should use --ca instead.") + registerCmd.AddNamespaceFlags(cmd.Flags(), false) + registerCmd.AddWaitFlags(cmd) + return cmd } // Validate checks that the required arguements have been provided func (c *RegisterCmd) Validate(args []string) error { - if len(args) == 0 { + if len(args) < 1 { return fmt.Errorf("a broker name is required") } c.BrokerName = args[0] + if c.BasicSecret != "" && c.BearerSecret != "" { + return fmt.Errorf("cannot use both basic auth and bearer auth") + } + + if c.CAFile != "" { + _, err := os.Stat(c.CAFile) + if err != nil { + return fmt.Errorf("error finding CA file: %v", err.Error()) + } + } + if c.RelistBehavior != "" { + c.RelistBehavior = strings.ToLower(c.RelistBehavior) + if c.RelistBehavior != "duration" && c.RelistBehavior != "manual" { + return fmt.Errorf("invalid --relist-duration value, allowed values are: duration, manual") + } + } return nil } -// Run runs the command +// Run creates the broker and then displays the broker details func (c *RegisterCmd) Run() error { - return c.Register() -} + opts := &servicecatalog.RegisterOptions{ + BasicSecret: c.BasicSecret, + BearerSecret: c.BearerSecret, + CAFile: c.CAFile, + ClassRestrictions: c.ClassRestrictions, + Namespace: c.Namespace, + PlanRestrictions: c.PlanRestrictions, + SkipTLS: c.SkipTLS, + } + if c.RelistBehavior == "duration" { + opts.RelistBehavior = v1beta1.ServiceBrokerRelistBehaviorDuration + opts.RelistDuration = &metav1.Duration{Duration: c.RelistDuration} + } else if c.RelistBehavior == "manual" { + opts.RelistBehavior = v1beta1.ServiceBrokerRelistBehaviorManual + } -// Register calls out to the pkg lib to create the broker and displays the output -func (c *RegisterCmd) Register() error { - broker, err := c.Context.App.Register(c.BrokerName, c.URL) + broker, err := c.Context.App.Register(c.BrokerName, c.URL, opts) if err != nil { return err } diff --git a/cmd/svcat/broker/register_cmd_test.go b/cmd/svcat/broker/register_cmd_test.go index f74cddd0a95..49d23510724 100644 --- a/cmd/svcat/broker/register_cmd_test.go +++ b/cmd/svcat/broker/register_cmd_test.go @@ -18,21 +18,25 @@ package broker_test import ( "bytes" + "time" . "github.com/kubernetes-incubator/service-catalog/cmd/svcat/broker" "github.com/kubernetes-incubator/service-catalog/cmd/svcat/command" "github.com/kubernetes-incubator/service-catalog/cmd/svcat/test" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" "github.com/kubernetes-incubator/service-catalog/pkg/svcat" + servicecatalog "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" + "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("Register Command", func() { Describe("NewRegisterCmd", func() { - It("Builds and returns a cobra command", func() { + It("Builds and returns a cobra command with the correct flags", func() { cxt := &command.Context{} cmd := NewRegisterCmd(cxt) Expect(*cmd).NotTo(BeNil()) @@ -40,23 +44,196 @@ var _ = Describe("Register Command", func() { Expect(cmd.Short).To(ContainSubstring("Registers a new broker with service catalog")) Expect(cmd.Example).To(ContainSubstring("svcat register mysqlbroker --url http://mysqlbroker.com")) Expect(len(cmd.Aliases)).To(Equal(0)) + + urlFlag := cmd.Flags().Lookup("url") + Expect(urlFlag).NotTo(BeNil()) + Expect(urlFlag.Usage).To(ContainSubstring("The broker URL (Required)")) + + basicSecretFlag := cmd.Flags().Lookup("basic-secret") + Expect(basicSecretFlag).NotTo(BeNil()) + Expect(basicSecretFlag.Usage).To(ContainSubstring("A secret containing basic auth (username/password) information to connect to the broker")) + + bearerSecretFlag := cmd.Flags().Lookup("bearer-secret") + Expect(bearerSecretFlag).NotTo(BeNil()) + Expect(bearerSecretFlag.Usage).To(ContainSubstring("A secret containing a bearer token to connect to the broker")) + + caFlag := cmd.Flags().Lookup("ca") + Expect(caFlag).NotTo(BeNil()) + Expect(caFlag.Usage).To(ContainSubstring("A file containing the CA certificate to connect to the broker")) + + classRestrictionFlag := cmd.Flags().Lookup("class-restrictions") + Expect(classRestrictionFlag).NotTo(BeNil()) + Expect(classRestrictionFlag.Usage).To(ContainSubstring("A list of restrictions to apply to the classes allowed from the broker")) + + planRestrictionFlag := cmd.Flags().Lookup("plan-restrictions") + Expect(planRestrictionFlag).NotTo(BeNil()) + Expect(planRestrictionFlag.Usage).To(ContainSubstring("A list of restrictions to apply to the plans allowed from the broker")) + + relistBehaviorFlag := cmd.Flags().Lookup("relist-behavior") + Expect(relistBehaviorFlag).NotTo(BeNil()) + Expect(relistBehaviorFlag.Usage).To(ContainSubstring("Behavior for relisting the broker's catalog. Valid options are manual or duration. Defaults to duration with an interval of 15m.")) + + relistDurationFlag := cmd.Flags().Lookup("relist-duration") + Expect(relistDurationFlag).NotTo(BeNil()) + Expect(relistDurationFlag.Usage).To(ContainSubstring("Interval to refetch broker catalog when relist-behavior is set to duration, specified in human readable format: 30s, 1m, 1h")) + + skipTLSFlag := cmd.Flags().Lookup("skip-tls") + Expect(skipTLSFlag).NotTo(BeNil()) + Expect(skipTLSFlag.Usage).To(ContainSubstring("Disables TLS certificate verification when communicating with this broker. This is strongly discouraged. You should use --ca instead.")) + + waitFlag := cmd.Flags().Lookup("wait") + Expect(waitFlag).NotTo(BeNil()) + timeoutFlag := cmd.Flags().Lookup("timeout") + Expect(timeoutFlag).NotTo(BeNil()) + intervalFlag := cmd.Flags().Lookup("interval") + Expect(intervalFlag).NotTo(BeNil()) }) }) + Describe("Validate", func() { + It("succeeds if a broker name and url are provided", func() { + cmd := RegisterCmd{} + err := cmd.Validate([]string{"bananabroker", "http://bananabroker.com"}) + Expect(err).NotTo(HaveOccurred()) + }) It("errors if a broker name is not provided", func() { + cmd := RegisterCmd{} + err := cmd.Validate([]string{}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("a broker name is required")) + }) + It("errors if both basic-secret and bearer-secret are provided", func() { + basicSecret := "basicsecret" + bearerSecret := "bearersecret" cmd := RegisterCmd{ - BrokerName: "", - Context: nil, - URL: "http://bananabroker.com", + BasicSecret: basicSecret, + BearerSecret: bearerSecret, } - err := cmd.Validate([]string{}) + err := cmd.Validate([]string{"bananabroker", "http://bananabroker.com", "--basic-secret", basicSecret, "--bearer-secret", bearerSecret}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot use both basic auth and bearer auth")) + }) + It("errors if a provided CA file does not exist", func() { + cmd := RegisterCmd{ + CAFile: "/not/a/real/file", + } + err := cmd.Validate([]string{"bananabroker", "http://bananabroker.com"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error finding CA file")) + }) + It("only allows valid values for relist behavior", func() { + cmd := RegisterCmd{ + RelistBehavior: "foobar", + } + err := cmd.Validate([]string{"bananabroker", "http://bananabroker.com"}) Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid --relist-duration value, allowed values are: duration, manual")) + + cmd = RegisterCmd{ + RelistBehavior: "Duration", + } + err = cmd.Validate([]string{"bananabroker", "http://bananabroker.com"}) + Expect(err).NotTo(HaveOccurred()) + + cmd = RegisterCmd{ + RelistBehavior: "MANUAL", + } + err = cmd.Validate([]string{"bananabroker", "http://bananabroker.com"}) + Expect(err).NotTo(HaveOccurred()) }) }) - Describe("Register", func() { + Describe("Run", func() { It("Calls the pkg/svcat libs Register method with the passed in variables and prints output to the user", func() { brokerName := "foobarbroker" brokerURL := "http://foobar.com" + basicSecret := "foobarsecret" + certFile := "register_cmd_test.go" + certFileContents := []byte("foobarCA") + classRestrictions := []string{"foobarclassa", "foobarclassb"} + namespace := "foobarnamespace" + planRestrictions := []string{"foobarplana", "foobarplanb"} + skipTLS := true + relistBehavior := "duration" + relistBehaviorConst := v1beta1.ServiceBrokerRelistBehaviorDuration + relistDuration := 10 * time.Minute + metaRelistDuration := &metav1.Duration{Duration: relistDuration} + + brokerToReturn := &v1beta1.ClusterServiceBroker{ + ObjectMeta: v1.ObjectMeta{ + Name: brokerName, + }, + Spec: v1beta1.ClusterServiceBrokerSpec{ + CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{ + CABundle: certFileContents, + InsecureSkipTLSVerify: skipTLS, + RelistBehavior: relistBehaviorConst, + RelistDuration: metaRelistDuration, + URL: brokerURL, + CatalogRestrictions: &v1beta1.CatalogRestrictions{ + ServiceClass: classRestrictions, + ServicePlan: planRestrictions, + }, + }, + AuthInfo: &v1beta1.ClusterServiceBrokerAuthInfo{ + Basic: &v1beta1.ClusterBasicAuthConfig{ + SecretRef: &v1beta1.ObjectReference{ + Name: basicSecret, + Namespace: namespace, + }, + }, + }, + }, + } + + outputBuffer := &bytes.Buffer{} + + fakeApp, _ := svcat.NewApp(nil, nil, namespace) + fakeSDK := new(servicecatalogfakes.FakeSvcatClient) + fakeSDK.RegisterReturns(brokerToReturn, nil) + fakeApp.SvcatClient = fakeSDK + cxt := svcattest.NewContext(outputBuffer, fakeApp) + cmd := RegisterCmd{ + Context: cxt, + BasicSecret: basicSecret, + BrokerName: brokerName, + CAFile: certFile, + ClassRestrictions: classRestrictions, + Namespaced: command.NewNamespaced(cxt), + PlanRestrictions: planRestrictions, + RelistBehavior: relistBehavior, + RelistDuration: relistDuration, + SkipTLS: skipTLS, + URL: brokerURL, + } + cmd.Namespaced.ApplyNamespaceFlags(&pflag.FlagSet{}) + err := cmd.Run() + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeSDK.RegisterCallCount()).To(Equal(1)) + returnedName, returnedURL, returnedOpts := fakeSDK.RegisterArgsForCall(0) + Expect(returnedName).To(Equal(brokerName)) + Expect(returnedURL).To(Equal(brokerURL)) + opts := servicecatalog.RegisterOptions{ + BasicSecret: basicSecret, + CAFile: certFile, + ClassRestrictions: classRestrictions, + Namespace: namespace, + PlanRestrictions: planRestrictions, + RelistBehavior: relistBehaviorConst, + RelistDuration: metaRelistDuration, + SkipTLS: skipTLS, + } + Expect(*returnedOpts).To(Equal(opts)) + + output := outputBuffer.String() + Expect(output).To(ContainSubstring(brokerName)) + Expect(output).To(ContainSubstring(brokerURL)) + }) + It("Passes in the bearer secret", func() { + brokerName := "foobarbroker" + brokerURL := "http://foobar.com" + bearerSecret := "foobarsecret" + namespace := "foobarnamespace" brokerToReturn := &v1beta1.ClusterServiceBroker{ ObjectMeta: v1.ObjectMeta{ @@ -66,26 +243,43 @@ var _ = Describe("Register Command", func() { CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{ URL: brokerURL, }, + AuthInfo: &v1beta1.ClusterServiceBrokerAuthInfo{ + Basic: &v1beta1.ClusterBasicAuthConfig{ + SecretRef: &v1beta1.ObjectReference{ + Name: bearerSecret, + }, + }, + }, }, } outputBuffer := &bytes.Buffer{} - fakeApp, _ := svcat.NewApp(nil, nil, "default") + fakeApp, _ := svcat.NewApp(nil, nil, namespace) fakeSDK := new(servicecatalogfakes.FakeSvcatClient) fakeSDK.RegisterReturns(brokerToReturn, nil) fakeApp.SvcatClient = fakeSDK + cxt := svcattest.NewContext(outputBuffer, fakeApp) cmd := RegisterCmd{ - Context: svcattest.NewContext(outputBuffer, fakeApp), - BrokerName: brokerName, - URL: brokerURL, + Context: cxt, + BearerSecret: bearerSecret, + BrokerName: brokerName, + Namespaced: command.NewNamespaced(cxt), + URL: brokerURL, } - err := cmd.Register() + cmd.Namespaced.ApplyNamespaceFlags(&pflag.FlagSet{}) + err := cmd.Run() Expect(err).NotTo(HaveOccurred()) - returnedName, returnedURL := fakeSDK.RegisterArgsForCall(0) + Expect(fakeSDK.RegisterCallCount()).To(Equal(1)) + returnedName, returnedURL, returnedOpts := fakeSDK.RegisterArgsForCall(0) Expect(returnedName).To(Equal(brokerName)) Expect(returnedURL).To(Equal(brokerURL)) + opts := servicecatalog.RegisterOptions{ + Namespace: namespace, + BearerSecret: bearerSecret, + } + Expect(*returnedOpts).To(Equal(opts)) output := outputBuffer.String() Expect(output).To(ContainSubstring(brokerName)) diff --git a/cmd/svcat/testdata/output/completion-bash.txt b/cmd/svcat/testdata/output/completion-bash.txt index f9122a40b5e..16f3c310505 100644 --- a/cmd/svcat/testdata/output/completion-bash.txt +++ b/cmd/svcat/testdata/output/completion-bash.txt @@ -838,8 +838,33 @@ _svcat_register() flags_with_completion=() flags_completion=() + flags+=("--basic-secret=") + local_nonpersistent_flags+=("--basic-secret=") + flags+=("--bearer-secret=") + local_nonpersistent_flags+=("--bearer-secret=") + flags+=("--ca=") + local_nonpersistent_flags+=("--ca=") + flags+=("--class-restrictions=") + local_nonpersistent_flags+=("--class-restrictions=") + flags+=("--interval=") + local_nonpersistent_flags+=("--interval=") + flags+=("--namespace=") + two_word_flags+=("-n") + local_nonpersistent_flags+=("--namespace=") + flags+=("--plan-restrictions=") + local_nonpersistent_flags+=("--plan-restrictions=") + flags+=("--relist-behavior=") + local_nonpersistent_flags+=("--relist-behavior=") + flags+=("--relist-duration=") + local_nonpersistent_flags+=("--relist-duration=") + flags+=("--skip-tls") + local_nonpersistent_flags+=("--skip-tls") + flags+=("--timeout=") + local_nonpersistent_flags+=("--timeout=") flags+=("--url=") local_nonpersistent_flags+=("--url=") + flags+=("--wait") + local_nonpersistent_flags+=("--wait") flags+=("--context=") flags+=("--kubeconfig=") flags+=("--logtostderr") diff --git a/cmd/svcat/testdata/output/completion-zsh.txt b/cmd/svcat/testdata/output/completion-zsh.txt index 195e7c1ee31..711c9cb7b5c 100644 --- a/cmd/svcat/testdata/output/completion-zsh.txt +++ b/cmd/svcat/testdata/output/completion-zsh.txt @@ -972,8 +972,33 @@ _svcat_register() flags_with_completion=() flags_completion=() + flags+=("--basic-secret=") + local_nonpersistent_flags+=("--basic-secret=") + flags+=("--bearer-secret=") + local_nonpersistent_flags+=("--bearer-secret=") + flags+=("--ca=") + local_nonpersistent_flags+=("--ca=") + flags+=("--class-restrictions=") + local_nonpersistent_flags+=("--class-restrictions=") + flags+=("--interval=") + local_nonpersistent_flags+=("--interval=") + flags+=("--namespace=") + two_word_flags+=("-n") + local_nonpersistent_flags+=("--namespace=") + flags+=("--plan-restrictions=") + local_nonpersistent_flags+=("--plan-restrictions=") + flags+=("--relist-behavior=") + local_nonpersistent_flags+=("--relist-behavior=") + flags+=("--relist-duration=") + local_nonpersistent_flags+=("--relist-duration=") + flags+=("--skip-tls") + local_nonpersistent_flags+=("--skip-tls") + flags+=("--timeout=") + local_nonpersistent_flags+=("--timeout=") flags+=("--url=") local_nonpersistent_flags+=("--url=") + flags+=("--wait") + local_nonpersistent_flags+=("--wait") flags+=("--context=") flags+=("--kubeconfig=") flags+=("--logtostderr") diff --git a/cmd/svcat/testdata/plugin.yaml b/cmd/svcat/testdata/plugin.yaml index 1e3b2585ea1..4441dd186e7 100644 --- a/cmd/svcat/testdata/plugin.yaml +++ b/cmd/svcat/testdata/plugin.yaml @@ -300,8 +300,36 @@ tree: example: ' svcat register mysqlbroker --url http://mysqlbroker.com' command: ./svcat register flags: + - name: basic-secret + desc: A secret containing basic auth (username/password) information to connect + to the broker + - name: bearer-secret + desc: A secret containing a bearer token to connect to the broker + - name: ca + desc: A file containing the CA certificate to connect to the broker + - name: class-restrictions + desc: A list of restrictions to apply to the classes allowed from the broker + - name: interval + desc: 'Poll interval for --wait, specified in human readable format: 30s, 1m, + 1h' + - name: plan-restrictions + desc: A list of restrictions to apply to the plans allowed from the broker + - name: relist-behavior + desc: Behavior for relisting the broker's catalog. Valid options are manual or + duration. Defaults to duration with an interval of 15m. + - name: relist-duration + desc: 'Interval to refetch broker catalog when relist-behavior is set to duration, + specified in human readable format: 30s, 1m, 1h' + - name: skip-tls + desc: Disables TLS certificate verification when communicating with this broker. + This is strongly discouraged. You should use --ca instead. + - name: timeout + desc: 'Timeout for --wait, specified in human readable format: 30s, 1m, 1h. Specify + -1 to wait indefinitely.' - name: url desc: The broker URL (Required) + - name: wait + desc: Wait until the operation completes. - name: sync use: sync shortDesc: Syncs service catalog for a service broker diff --git a/docs/devguide.md b/docs/devguide.md index 37eb21ed1db..b195d9b8f47 100644 --- a/docs/devguide.md +++ b/docs/devguide.md @@ -317,6 +317,29 @@ Counterfeiter by running `go get github.com/maxbrunsfeld/counterfeiter`. Then regenerate the fake with `counterfeiter ./pkg/svcat/service-catalog SvcatClient` and manually paste the boilerplate copyright comment into the the generated file. +## FeatureGates +Feature gates are a set of key=value pairs that describe experimental features +and can be turned on or off by specifying the value when launching the Service +Catalog executable (typically done in the Helm chart). A new feature gate +should be created when introducing new features that may break existing +functionality or introduce instability. See [FeatureGates](feature-gates.md) +for more details. + +When adding a FeatureGate to Helm charts, define the variable +`fooEnabled` with a value of `false` in [values.yaml](https://github.com/kubernetes-incubator/service-catalog/blob/master/charts/catalog/values.yaml). In the [API Server](https://github.com/kubernetes-incubator/service-catalog/blob/master/charts/catalog/templates/apiserver-deployment.yaml) and [Controller](https://github.com/kubernetes-incubator/service-catalog/blob/master/charts/catalog/templates/controller-manager-deployment.yaml) +templates, add the new FeatureGate: +{% raw %} +```yaml + - --feature-gates + - Foo={{.Values.fooEnabled}} +``` +{% endraw %} + +When the feature has had enough testing and the community agrees to change the +default to true, update [features.go](https://github.com/kubernetes-incubator/service-catalog/blob/master/pkg/features/features.go) and `values.yaml` changing the default for +feature foo to `true`. And lastly update the appropriate information in the +[FeatureGates doc](feature-gates.md). + ## Documentation Our documentation site is located at [svc-cat.io](https://svc-cat.io). The content files are located diff --git a/docs/feature-gates.md b/docs/feature-gates.md new file mode 100644 index 00000000000..7c645b0ee34 --- /dev/null +++ b/docs/feature-gates.md @@ -0,0 +1,97 @@ +--- +title: Feature Gates +layout: docwithnav +--- + +## Overview + +Feature gates are a set of key=value pairs that describe alpha or experimental +features and can be turned on or off by specifying the key=value pair in the +arguments list when launching the Service Catalog executable. A new geature +gate should be created when introducing new features that may break existing +functionality or introduce instability. + +The following table is a summary of the feature gates that you can set on +different Service Catalog. + +- The "Since" column contains the Service Catalog release when a feature is + introduced or its release stage is changed. +- The "Until" column, if not empty, contains the last Service Catalog release in + which you can still use a feature gate. + +| Feature | Default | Stage | Since | Until | +|---------|---------|-------|-------|-------| +| `AsyncBindingOperations` | `false` | Alpha | v0.1.7 | | +| `NamespacedServiceBroker` | `false` | Alpha | v0.1.10 | v0.1.28 | +| `NamespacedServiceBroker` | `true` | GA | v0.1.29 | | +| `OriginatingIdentity` | `false` | Alpha | v0.1.7 | v0.1.29 | +| `OriginatingIdentity` | `true` | GA | v0.1.30 | | +| `OriginatingIdentityLocking` | `true` | Alpha | v0.1.14 | | +| `PodPreset` | `false` | Alpha | v0.1.6 | | +| `ResponseSchema` | `false` | Alpha | v0.1.12 | | +| `UpdateDashboardURL` | `false` | Alpha | v0.1.13 | | + + +## Using a Feature + +### Feature Stages + +A feature can be in *Alpha*, *Beta* or *GA* stage. +An *Alpha* feature means: + +* Disabled by default. +* Might be buggy. Enabling the feature may expose bugs. +* Support for feature may be dropped at any time without notice. +* The API may change in incompatible ways in a later software release without + notice. +* Recommended for use only in short-lived testing clusters, due to increased + risk of bugs and lack of long-term support. + +A *Beta* feature means: + +* Enabled by default. +* The feature is well tested. Enabling the feature is considered safe. +* Support for the overall feature will not be dropped, though details may change. +* The schema and/or semantics of objects may change in incompatible ways in a + subsequent beta or stable release. When this happens, we will provide + instructions for migrating to the next version. This may require deleting, + editing, and re-creating API objects. The editing process may require some + thought. This may require downtime for applications that rely on the feature. +* Recommended for only non-business-critical uses because of potential for + incompatible changes in subsequent releases. If you have multiple clusters + that can be upgraded independently, you may be able to relax this restriction. + +**Note:** Please do try *Beta* features and give feedback on them! +After they exit beta, it may not be practical for us to make more changes. + +A *GA* feature is also referred to as a *stable* feature. It means: + +* The corresponding feature gate is no longer needed. +* Stable versions of features will appear in released software for many + subsequent versions. + +### Feature Gates + +Each feature gate is designed for enabling/disabling a specific feature: + +- `AsyncBindingOperations`: Controls whether the controller should attempt + asynchronous binding operations + +- `NamespacedServiceBroker`: Enables namespaced variants of ServiceBrokers, +ServiceClasses, and ServicePlans. + +- `OriginatingIdentity`: Controls whether the controller should include +originating identity in the header of requests sent to brokers + +- `OriginatingIdentityLocking`: Controls whether we lock OSB API resources +for updating while we are still processing the current spec. + + - `PodPreset`: Controls whether PodPreset resource is enabled or not in the + API server. + +- `ResponseSchema`: Enables the storage of the binding response schema in +ServicePlans + +- `UpdateDashboardURL`: Enables the update of DashboardURL in response to +update service instance requests to brokers. + diff --git a/docs/install.md b/docs/install.md index edb8a7ed6ca..105eb66dbcb 100644 --- a/docs/install.md +++ b/docs/install.md @@ -189,6 +189,12 @@ our canary (master) builds, and tags using the following prefixes: * Previous canary builds: https://download.svcat.sh/cli/VERSION-GITDESCRIBE where `GITDESCRIBE` is the result of calling `git describe --tags`, for example `v0.1.20-1-g203c8ad`. +## MacOS with Homebrew + +``` +brew update +brew install kubernetes-service-catalog-client +``` ## MacOS diff --git a/docsite/_data/devguide.yaml b/docsite/_data/devguide.yaml index 85d69b336cd..14011e98311 100644 --- a/docsite/_data/devguide.yaml +++ b/docsite/_data/devguide.yaml @@ -15,6 +15,8 @@ toc: path: /docs/devguide/#building - title: Testing path: /docs/devguide/#testing +- title: FeatureGates + path: "#featuregates" - title: Documentation path: "#documentation" - title: Making a Contribution diff --git a/pkg/controller/controller_binding.go b/pkg/controller/controller_binding.go index af9eb3e7337..9e5e5792780 100644 --- a/pkg/controller/controller_binding.go +++ b/pkg/controller/controller_binding.go @@ -1217,6 +1217,14 @@ func (c *controller) prepareBindRequest( } appGUID := string(ns.UID) + clusterID := c.getClusterID() + + requestContext := map[string]interface{}{ + "platform": ContextProfilePlatformKubernetes, + "namespace": instance.Namespace, + clusterIdentifierKey: clusterID, + } + request := &osb.BindRequest{ BindingID: binding.Spec.ExternalID, InstanceID: instance.Spec.ExternalID, @@ -1225,6 +1233,7 @@ func (c *controller) prepareBindRequest( AppGUID: &appGUID, Parameters: parameters, BindResource: &osb.BindResource{AppGUID: &appGUID}, + Context: requestContext, } // Asynchronous binding operations are currently ALPHA and not diff --git a/pkg/controller/controller_binding_ns_test.go b/pkg/controller/controller_binding_ns_test.go index 5c073f3ee5a..94d8827704b 100644 --- a/pkg/controller/controller_binding_ns_test.go +++ b/pkg/controller/controller_binding_ns_test.go @@ -134,6 +134,7 @@ func TestReconcileServiceBindingWithParametersNamespacedRefs(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() diff --git a/pkg/controller/controller_binding_test.go b/pkg/controller/controller_binding_test.go index c2e21175563..439d9552e7d 100644 --- a/pkg/controller/controller_binding_test.go +++ b/pkg/controller/controller_binding_test.go @@ -29,6 +29,7 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" v1beta1informers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions/servicecatalog/v1beta1" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" osb "github.com/pmorie/go-open-service-broker-client/v2" fakeosb "github.com/pmorie/go-open-service-broker-client/v2/fake" corev1 "k8s.io/api/core/v1" @@ -359,6 +360,7 @@ func TestReconcileServiceBindingWithSecretConflict(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -482,6 +484,7 @@ func TestReconcileServiceBindingWithParameters(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -617,6 +620,7 @@ func TestReconcileServiceBindingWithSecretTransform(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -799,6 +803,7 @@ func TestReconcileServiceBindingNonbindableClusterServiceClassBindablePlan(t *te BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -1654,6 +1659,7 @@ func TestReconcileServiceBindingWithServiceBindingCallFailure(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) events := getRecordedEvents(testController) @@ -1728,6 +1734,7 @@ func TestReconcileServiceBindingWithServiceBindingFailure(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) events := getRecordedEvents(testController) @@ -2026,10 +2033,8 @@ func TestReconcileUnbindingWithClusterServiceBrokerHTTPError(t *testing.T) { func TestReconcileBindingUsingOriginatingIdentity(t *testing.T) { for _, tc := range originatingIdentityTestCases { func() { - if tc.enableOriginatingIdentity { - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) - } + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, tc.enableOriginatingIdentity) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ BindReaction: &fakeosb.BindReaction{ @@ -2085,13 +2090,8 @@ func TestReconcileBindingUsingOriginatingIdentity(t *testing.T) { func TestReconcileBindingDeleteUsingOriginatingIdentity(t *testing.T) { for _, tc := range originatingIdentityTestCases { func() { - if tc.enableOriginatingIdentity { - err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - if err != nil { - t.Fatalf("Failed to enable originating identity feature: %v", err) - } - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) - } + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, tc.enableOriginatingIdentity) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ UnbindReaction: &fakeosb.UnbindReaction{ @@ -2189,6 +2189,7 @@ func TestReconcileBindingSuccessOnFinalRetry(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -2314,6 +2315,7 @@ func TestReconcileBindingWithSecretConflictFailedAfterFinalRetry(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -2500,6 +2502,7 @@ func TestReconcileServiceBindingWithSecretParameters(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) actions := fakeCatalogClient.Actions() @@ -2675,6 +2678,7 @@ func TestReconcileBindingWithSetOrphanMitigation(t *testing.T) { BindResource: &osb.BindResource{ AppGUID: strPtr(testNamespaceGUID), }, + Context: testContext, }) kubeActions := fakeKubeClient.Actions() @@ -3102,6 +3106,7 @@ func TestReconcileServiceBindingAsynchronousBind(t *testing.T) { AppGUID: strPtr(testNamespaceGUID), }, AcceptsIncomplete: true, + Context: testContext, }) // Kube actions diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 82e707ee1a8..56bed44be92 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -115,8 +115,8 @@ type backoffEntry struct { type instanceOperationBackoff struct { // lock to be used for accessing retry map mutex sync.RWMutex - instances map[string]backoffEntry - rateLimiter workqueue.RateLimiter // used to calculate next retry time + instances map[string]backoffEntry // Key is K8s metadata UID + rateLimiter workqueue.RateLimiter // used to calculate next retry time, key is UID } // ServiceInstance handlers and control-loop @@ -377,14 +377,9 @@ func (c *controller) initOrphanMitigationCondition(instance *v1beta1.ServiceInst // purgeExpiredRetryEntries() or when the operation is successful. func (c *controller) setRetryBackoffRequired(instance *v1beta1.ServiceInstance) { pcb := pretty.NewInstanceContextBuilder(instance) - key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(instance) - if err != nil { - glog.Errorf(pcb.Messagef("Couldn't create a key for object %+v: %v", instance, err)) - return - } - c.instanceOperationRetryQueue.mutex.Lock() defer c.instanceOperationRetryQueue.mutex.Unlock() + key := string(instance.GetUID()) retryEntry, found := c.instanceOperationRetryQueue.instances[key] if !found || retryEntry.generation != instance.Generation { retryEntry.generation = instance.Generation @@ -396,7 +391,7 @@ func (c *controller) setRetryBackoffRequired(instance *v1beta1.ServiceInstance) } retryEntry.dirty = true c.instanceOperationRetryQueue.instances[key] = retryEntry - glog.V(4).Info(pcb.Messagef("added %v generation %v to backoffBeforeRetrying map", key, instance.Generation)) + glog.V(4).Info(pcb.Messagef("BrokerOpRetry: added %v (%v/%v) generation %v to backoffBeforeRetrying map", key, instance.GetNamespace(), instance.GetName(), instance.Generation)) } // backoffAndRequeueIfRetrying returns true if this is a retry and a backoff @@ -406,11 +401,7 @@ func (c *controller) setRetryBackoffRequired(instance *v1beta1.ServiceInstance) // backoff delay. func (c *controller) backoffAndRequeueIfRetrying(instance *v1beta1.ServiceInstance, operation string) bool { pcb := pretty.NewInstanceContextBuilder(instance) - key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(instance) - if err != nil { - glog.Errorf(pcb.Messagef("Couldn't create a key for object %+v: %v", instance, err)) - return false - } + key := string(instance.GetUID()) delay := time.Millisecond * 0 // if there is a pending delay, calculate it and clear the dirty bit @@ -430,7 +421,7 @@ func (c *controller) backoffAndRequeueIfRetrying(instance *v1beta1.ServiceInstan retryEntry.calculatedRetryTime = time.Now().Add(c.instanceOperationRetryQueue.rateLimiter.When(key)) retryEntry.dirty = false c.instanceOperationRetryQueue.instances[key] = retryEntry - glog.V(4).Infof(pcb.Messagef("generation %v retryTime calculated as %v", instance.Generation, retryEntry.calculatedRetryTime)) + glog.V(4).Infof(pcb.Messagef("BrokerOpRetry: generation %v retryTime calculated as %v", instance.Generation, retryEntry.calculatedRetryTime)) } now := time.Now() @@ -439,7 +430,7 @@ func (c *controller) backoffAndRequeueIfRetrying(instance *v1beta1.ServiceInstan if delay > 0 { msg := fmt.Sprintf("Delaying %s retry, next attempt will be after %s", operation, retryEntry.calculatedRetryTime) c.recorder.Event(instance, corev1.EventTypeWarning, "RetryBackoff", msg) - glog.V(2).Info(pcb.Message(msg)) + glog.V(2).Info(pcb.Messagef("BrokerOpRetry: %s", msg)) // add back to worker queue to retry at the specified time c.instanceAddAfter(instance, delay) @@ -465,29 +456,25 @@ func (c *controller) purgeExpiredRetryEntries() { purgedEntries := 0 for k, v := range c.instanceOperationRetryQueue.instances { if v.calculatedRetryTime.Before(overDue) { - glog.V(5).Infof("removing %s from instanceOperationRetryQueue which had retry time of %v", k, v.calculatedRetryTime) + glog.V(5).Infof("BrokerOpRetry: removing %s from instanceOperationRetryQueue which had retry time of %v", k, v.calculatedRetryTime) delete(c.instanceOperationRetryQueue.instances, k) c.instanceOperationRetryQueue.rateLimiter.Forget(k) purgedEntries++ } } - glog.V(5).Infof("purged %v expired entries from instanceOperationRetryQueue.instances, number of entries remaining: %v", purgedEntries, len(c.instanceOperationRetryQueue.instances)) + glog.V(5).Infof("BrokerOpRetry: purged %v expired entries from instanceOperationRetryQueue.instances, number of entries remaining: %v", purgedEntries, len(c.instanceOperationRetryQueue.instances)) } // removeInstanceFromRetryMap removes the instance from the retry & ratelimter maps func (c *controller) removeInstanceFromRetryMap(instance *v1beta1.ServiceInstance) { pcb := pretty.NewInstanceContextBuilder(instance) - key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(instance) - if err != nil { - glog.Errorf(pcb.Messagef("Couldn't create a key for object %+v: %v", instance, err)) - return - } + key := string(instance.GetUID()) c.instanceOperationRetryQueue.mutex.Lock() defer c.instanceOperationRetryQueue.mutex.Unlock() delete(c.instanceOperationRetryQueue.instances, key) c.instanceOperationRetryQueue.rateLimiter.Forget(key) - glog.V(4).Infof(pcb.Message("removed from instanceOperationRetryQueue")) + glog.V(4).Infof(pcb.Message("BrokerOpRetry: removed %v from instanceOperationRetryQueue"), key) } // reconcileServiceInstanceAdd is responsible for handling the provisioning @@ -538,6 +525,16 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan } // recordStartOfServiceInstanceOperation has updated the instance, so we need to continue in the next iteration return nil + } else if instance.Status.DeprovisionStatus != v1beta1.ServiceInstanceDeprovisionStatusRequired { + instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + instance, err = c.updateServiceInstanceStatus(instance) + if err != nil { + // There has been an update to the instance. Start reconciliation + // over with a fresh view of the instance. + return err + } + // instance was updated, we will to continue in the next iteration + return nil } var prettyClass string @@ -2305,6 +2302,7 @@ func (c *controller) processServiceInstanceGracefulDeletionSuccess(instance *v1b pcb := pretty.NewInstanceContextBuilder(instance) glog.Info(pcb.Message("Cleared finalizer")) + c.removeInstanceFromRetryMap(instance) return nil } diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index f068197d939..d3b8149b9c2 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -42,6 +42,7 @@ import ( scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" "github.com/kubernetes-incubator/service-catalog/test/fake" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" corev1 "k8s.io/api/core/v1" clientgotesting "k8s.io/client-go/testing" ) @@ -3632,13 +3633,8 @@ func TestUpdateServiceInstanceCondition(t *testing.T) { func TestReconcileInstanceUsingOriginatingIdentity(t *testing.T) { for _, tc := range originatingIdentityTestCases { func() { - if tc.enableOriginatingIdentity { - err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - if err != nil { - t.Fatalf("Failed to enable originating identity feature: %v", err) - } - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) - } + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, tc.enableOriginatingIdentity) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ ProvisionReaction: &fakeosb.ProvisionReaction{ @@ -3692,10 +3688,9 @@ func TestReconcileInstanceUsingOriginatingIdentity(t *testing.T) { func TestReconcileInstanceDeleteUsingOriginatingIdentity(t *testing.T) { for _, tc := range originatingIdentityTestCases { func() { - if tc.enableOriginatingIdentity { - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) - } + + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, tc.enableOriginatingIdentity) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) _, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ DeprovisionReaction: &fakeosb.DeprovisionReaction{ @@ -3760,10 +3755,8 @@ func TestReconcileInstanceDeleteUsingOriginatingIdentity(t *testing.T) { func TestPollInstanceUsingOriginatingIdentity(t *testing.T) { for _, tc := range originatingIdentityTestCases { func() { - if tc.enableOriginatingIdentity { - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) - } + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, tc.enableOriginatingIdentity) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) _, _, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ diff --git a/pkg/features/features.go b/pkg/features/features.go index a24a625c510..95eb932f1cb 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -82,7 +82,7 @@ func init() { // available throughout service catalog binaries. var defaultServiceCatalogFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{ PodPreset: {Default: false, PreRelease: utilfeature.Alpha}, - OriginatingIdentity: {Default: false, PreRelease: utilfeature.Alpha}, + OriginatingIdentity: {Default: true, PreRelease: utilfeature.GA}, AsyncBindingOperations: {Default: false, PreRelease: utilfeature.Alpha}, NamespacedServiceBroker: {Default: true, PreRelease: utilfeature.Alpha}, ResponseSchema: {Default: false, PreRelease: utilfeature.Alpha}, diff --git a/pkg/registry/servicecatalog/binding/storage.go b/pkg/registry/servicecatalog/binding/storage.go index c08a6644a50..d313ccb133e 100644 --- a/pkg/registry/servicecatalog/binding/storage.go +++ b/pkg/registry/servicecatalog/binding/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -138,6 +140,38 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage, error) { DeleteStrategy: bindingRESTStrategies, EnableGarbageCollection: true, + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "Service-Instance", Type: "string"}, + {Name: "Secret-Name", Type: "string"}, + {Name: "Status", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + getStatus := func(status servicecatalog.ServiceBindingStatus) string { + if len(status.Conditions) > 0 { + condition := status.Conditions[len(status.Conditions)-1] + if condition.Status == servicecatalog.ConditionTrue { + return string(condition.Type) + } + return condition.Reason + } + return "" + } + + binding := obj.(*servicecatalog.ServiceBinding) + cells := []interface{}{ + name, + binding.Spec.ServiceInstanceRef.Name, + binding.Spec.SecretName, + getStatus(binding.Status), + age, + } + return cells, nil + }, + ), + Storage: storageInterface, DestroyFunc: dFunc, } diff --git a/pkg/registry/servicecatalog/binding/strategy_test.go b/pkg/registry/servicecatalog/binding/strategy_test.go index 812d3153319..a255790ac81 100644 --- a/pkg/registry/servicecatalog/binding/strategy_test.go +++ b/pkg/registry/servicecatalog/binding/strategy_test.go @@ -17,12 +17,10 @@ limitations under the License. package binding import ( - "context" "fmt" "testing" - "k8s.io/apiserver/pkg/authentication/user" - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" utilfeature "k8s.io/apiserver/pkg/util/feature" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" @@ -51,14 +49,6 @@ func getTestInstanceCredential() *servicecatalog.ServiceBinding { } } -func contextWithUserName(userName string) context.Context { - ctx := genericapirequest.NewContext() - userInfo := &user.DefaultInfo{ - Name: userName, - } - return genericapirequest.WithUser(ctx, userInfo) -} - // TODO: Un-comment "spec-change" test case when there is a field // in the spec to which the reconciler allows a change. @@ -89,8 +79,10 @@ func TestInstanceCredentialUpdate(t *testing.T) { // shouldGenerationIncrement: true, // }, } + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for _, tc := range cases { - bindingRESTStrategies.PrepareForUpdate(nil, tc.newer, tc.older) + bindingRESTStrategies.PrepareForUpdate(createContext, tc.newer, tc.older) expectedGeneration := tc.older.Generation if tc.shouldGenerationIncrement { @@ -106,12 +98,12 @@ func TestInstanceCredentialUpdate(t *testing.T) { // as the user changes for different modifications of the instance credential. func TestInstanceCredentialUserInfo(t *testing.T) { // Enable the OriginatingIdentity feature - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, true) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) creatorUserName := "creator" createdInstanceCredential := getTestInstanceCredential() - createContext := contextWithUserName(creatorUserName) + createContext := sctestutil.ContextWithUserName(creatorUserName) bindingRESTStrategies.PrepareForCreate(createContext, createdInstanceCredential) if e, a := creatorUserName, createdInstanceCredential.Spec.UserInfo.Username; e != a { @@ -123,7 +115,7 @@ func TestInstanceCredentialUserInfo(t *testing.T) { // updaterUserName := "updater" // updatedInstanceCredential := getTestInstanceCredential() - // updateContext := contextWithUserName(updaterUserName) + // updateContext := sctestutil.ContextWithUserName(updaterUserName) // bindingRESTStrategies.PrepareForUpdate(updateContext, updatedInstanceCredential, createdInstanceCredential) // if e, a := updaterUserName, updatedInstanceCredential.Spec.UserInfo.Username; e != a { @@ -132,7 +124,7 @@ func TestInstanceCredentialUserInfo(t *testing.T) { deleterUserName := "deleter" deletedInstanceCredential := getTestInstanceCredential() - deleteContext := contextWithUserName(deleterUserName) + deleteContext := sctestutil.ContextWithUserName(deleterUserName) bindingRESTStrategies.CheckGracefulDelete(deleteContext, deletedInstanceCredential, nil) if e, a := deleterUserName, deletedInstanceCredential.Spec.UserInfo.Username; e != a { @@ -143,7 +135,9 @@ func TestInstanceCredentialUserInfo(t *testing.T) { // TestExternalIDSet checks that we set the ExternalID if the user doesn't provide it. func TestExternalIDSet(t *testing.T) { createdInstanceCredential := getTestInstanceCredential() - bindingRESTStrategies.PrepareForCreate(nil, createdInstanceCredential) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + bindingRESTStrategies.PrepareForCreate(createContext, createdInstanceCredential) if createdInstanceCredential.Spec.ExternalID == "" { t.Error("Expected an ExternalID to be set, but got none") @@ -155,7 +149,9 @@ func TestExternalIDUserProvided(t *testing.T) { userExternalID := "my-id" createdInstanceCredential := getTestInstanceCredential() createdInstanceCredential.Spec.ExternalID = userExternalID - bindingRESTStrategies.PrepareForCreate(nil, createdInstanceCredential) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + bindingRESTStrategies.PrepareForCreate(createContext, createdInstanceCredential) if createdInstanceCredential.Spec.ExternalID != userExternalID { t.Errorf("Modified user provided ExternalID to %q", createdInstanceCredential.Spec.ExternalID) diff --git a/pkg/registry/servicecatalog/clusterservicebroker/storage.go b/pkg/registry/servicecatalog/clusterservicebroker/storage.go index 4e3ee76ba80..b5e1043281d 100644 --- a/pkg/registry/servicecatalog/clusterservicebroker/storage.go +++ b/pkg/registry/servicecatalog/clusterservicebroker/storage.go @@ -24,8 +24,10 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -134,6 +136,35 @@ func NewStorage(opts server.Options) (clusterServiceBrokers, clusterServiceBroke DeleteStrategy: clusterServiceBrokerRESTStrategies, EnableGarbageCollection: true, + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "URL", Type: "string"}, + {Name: "Status", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + getStatus := func(status servicecatalog.CommonServiceBrokerStatus) string { + if len(status.Conditions) > 0 { + condition := status.Conditions[len(status.Conditions)-1] + if condition.Status == servicecatalog.ConditionTrue { + return string(condition.Type) + } + return condition.Reason + } + return "" + } + broker := obj.(*servicecatalog.ClusterServiceBroker) + cells := []interface{}{ + name, + broker.Spec.URL, + getStatus(broker.Status.CommonServiceBrokerStatus), + age, + } + return cells, nil + }, + ), + Storage: storageInterface, DestroyFunc: dFunc, } diff --git a/pkg/registry/servicecatalog/clusterservicebroker/strategy_test.go b/pkg/registry/servicecatalog/clusterservicebroker/strategy_test.go index 762a91b6f04..e90fc816479 100644 --- a/pkg/registry/servicecatalog/clusterservicebroker/strategy_test.go +++ b/pkg/registry/servicecatalog/clusterservicebroker/strategy_test.go @@ -20,6 +20,7 @@ import ( "testing" sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -82,8 +83,10 @@ func TestClusterServiceBroker(t *testing.T) { }, } + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) // Canonicalize the broker - clusterServiceBrokerRESTStrategies.PrepareForCreate(nil, broker) + clusterServiceBrokerRESTStrategies.PrepareForCreate(createContext, broker) if broker.Status.Conditions == nil { t.Fatalf("Fresh clusterservicebroker should have empty status") @@ -115,9 +118,10 @@ func TestClusterServiceBrokerUpdate(t *testing.T) { shouldGenerationIncrement: true, }, } - + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for i := range cases { - clusterServiceBrokerRESTStrategies.PrepareForUpdate(nil, cases[i].newer, cases[i].older) + clusterServiceBrokerRESTStrategies.PrepareForUpdate(createContext, cases[i].newer, cases[i].older) if cases[i].shouldGenerationIncrement { if e, a := cases[i].older.Generation+1, cases[i].newer.Generation; e != a { @@ -171,8 +175,9 @@ func TestClusterServiceBrokerUpdateForRelistRequests(t *testing.T) { newClusterServiceBroker := clusterServiceBrokerWithOldSpec() newClusterServiceBroker.Spec.RelistRequests = tc.newValue - - clusterServiceBrokerRESTStrategies.PrepareForUpdate(nil, newClusterServiceBroker, oldBroker) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + clusterServiceBrokerRESTStrategies.PrepareForUpdate(createContext, newClusterServiceBroker, oldBroker) if e, a := tc.expectedValue, newClusterServiceBroker.Spec.RelistRequests; e != a { t.Errorf("%s: got unexpected RelistRequests: expected %v, got %v", tc.name, e, a) diff --git a/pkg/registry/servicecatalog/clusterserviceclass/storage.go b/pkg/registry/servicecatalog/clusterserviceclass/storage.go index d3f87f4424e..8a18cc9f152 100644 --- a/pkg/registry/servicecatalog/clusterserviceclass/storage.go +++ b/pkg/registry/servicecatalog/clusterserviceclass/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -144,8 +146,28 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage) { CreateStrategy: clusterServiceClassRESTStrategies, UpdateStrategy: clusterServiceClassRESTStrategies, DeleteStrategy: clusterServiceClassRESTStrategies, - Storage: storageInterface, - DestroyFunc: dFunc, + + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "External-Name", Type: "string"}, + {Name: "Broker", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + class := obj.(*servicecatalog.ClusterServiceClass) + cells := []interface{}{ + name, + class.Spec.ExternalName, + class.Spec.ClusterServiceBrokerName, + age, + } + return cells, nil + }, + ), + + Storage: storageInterface, + DestroyFunc: dFunc, } options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs} diff --git a/pkg/registry/servicecatalog/clusterserviceplan/storage.go b/pkg/registry/servicecatalog/clusterserviceplan/storage.go index 891386939e6..3eb2d8bea70 100644 --- a/pkg/registry/servicecatalog/clusterserviceplan/storage.go +++ b/pkg/registry/servicecatalog/clusterserviceplan/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -142,8 +144,30 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage) { CreateStrategy: clusterServicePlanRESTStrategies, UpdateStrategy: clusterServicePlanRESTStrategies, DeleteStrategy: clusterServicePlanRESTStrategies, - Storage: storageInterface, - DestroyFunc: dFunc, + + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "External-Name", Type: "string"}, + {Name: "Broker", Type: "string"}, + {Name: "Class", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + plan := obj.(*servicecatalog.ClusterServicePlan) + cells := []interface{}{ + name, + plan.Spec.ExternalName, + plan.Spec.ClusterServiceBrokerName, + plan.Spec.ClusterServiceClassRef.Name, + age, + } + return cells, nil + }, + ), + + Storage: storageInterface, + DestroyFunc: dFunc, } statusStore := store diff --git a/pkg/registry/servicecatalog/instance/storage.go b/pkg/registry/servicecatalog/instance/storage.go index a8dc61070de..b7d4bb226ee 100644 --- a/pkg/registry/servicecatalog/instance/storage.go +++ b/pkg/registry/servicecatalog/instance/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -143,6 +145,48 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage, rest.Storage) DeleteStrategy: instanceRESTStrategies, EnableGarbageCollection: true, + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "Class", Type: "string"}, + {Name: "Plan", Type: "string"}, + {Name: "Status", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + getStatus := func(status servicecatalog.ServiceInstanceStatus) string { + if len(status.Conditions) > 0 { + condition := status.Conditions[len(status.Conditions)-1] + if condition.Status == servicecatalog.ConditionTrue { + return string(condition.Type) + } + return condition.Reason + } + return "" + } + + instance := obj.(*servicecatalog.ServiceInstance) + + var class, plan string + if instance.Spec.ClusterServiceClassSpecified() && instance.Spec.ClusterServicePlanSpecified() { + class = fmt.Sprintf("ClusterServiceClass/%s", instance.Spec.GetSpecifiedClusterServiceClass()) + plan = instance.Spec.GetSpecifiedClusterServicePlan() + } else { + class = fmt.Sprintf("ServiceClass/%s", instance.Spec.GetSpecifiedServiceClass()) + plan = instance.Spec.GetSpecifiedServicePlan() + } + + cells := []interface{}{ + name, + class, + plan, + getStatus(instance.Status), + age, + } + return cells, nil + }, + ), + Storage: storageInterface, DestroyFunc: dFunc, } diff --git a/pkg/registry/servicecatalog/instance/strategy_test.go b/pkg/registry/servicecatalog/instance/strategy_test.go index 5875f68d3d0..ccd1d6db80c 100644 --- a/pkg/registry/servicecatalog/instance/strategy_test.go +++ b/pkg/registry/servicecatalog/instance/strategy_test.go @@ -17,17 +17,15 @@ limitations under the License. package instance import ( - "context" "fmt" "testing" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apiserver/pkg/authentication/user" - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) func getTestInstance() *servicecatalog.ServiceInstance { @@ -57,14 +55,6 @@ func getTestInstance() *servicecatalog.ServiceInstance { } } -func contextWithUserName(userName string) context.Context { - ctx := genericapirequest.NewContext() - userInfo := &user.DefaultInfo{ - Name: userName, - } - return genericapirequest.WithUser(ctx, userInfo) -} - // TestInstanceUpdate tests that updates to the spec of an Instance. func TestInstanceUpdate(t *testing.T) { cases := []struct { @@ -147,9 +137,10 @@ func TestInstanceUpdate(t *testing.T) { shouldPlanRefClear: true, }, } - + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for _, tc := range cases { - instanceRESTStrategies.PrepareForUpdate(nil, tc.newer, tc.older) + instanceRESTStrategies.PrepareForUpdate(createContext, tc.newer, tc.older) expectedGeneration := tc.older.Generation if tc.shouldGenerationIncrement { @@ -175,12 +166,12 @@ func TestInstanceUpdate(t *testing.T) { // as the user changes for different modifications of the instance. func TestInstanceUserInfo(t *testing.T) { // Enable the OriginatingIdentity feature - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, true) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) creatorUserName := "creator" createdInstance := getTestInstance() - createContext := contextWithUserName(creatorUserName) + createContext := sctestutil.ContextWithUserName(creatorUserName) instanceRESTStrategies.PrepareForCreate(createContext, createdInstance) if e, a := creatorUserName, createdInstance.Spec.UserInfo.Username; e != a { @@ -190,7 +181,7 @@ func TestInstanceUserInfo(t *testing.T) { updaterUserName := "updater" updatedInstance := getTestInstance() updatedInstance.Spec.UpdateRequests = updatedInstance.Spec.UpdateRequests + 1 - updateContext := contextWithUserName(updaterUserName) + updateContext := sctestutil.ContextWithUserName(updaterUserName) instanceRESTStrategies.PrepareForUpdate(updateContext, updatedInstance, createdInstance) if e, a := updaterUserName, updatedInstance.Spec.UserInfo.Username; e != a { @@ -199,7 +190,7 @@ func TestInstanceUserInfo(t *testing.T) { deleterUserName := "deleter" deletedInstance := getTestInstance() - deleteContext := contextWithUserName(deleterUserName) + deleteContext := sctestutil.ContextWithUserName(deleterUserName) instanceRESTStrategies.CheckGracefulDelete(deleteContext, deletedInstance, nil) if e, a := deleterUserName, deletedInstance.Spec.UserInfo.Username; e != a { @@ -241,6 +232,8 @@ func TestInstanceUpdateForUpdateRequests(t *testing.T) { expectedValue: 2, }, } + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for _, tc := range cases { oldInstance := getTestInstance() oldInstance.Spec.UpdateRequests = tc.oldValue @@ -248,7 +241,7 @@ func TestInstanceUpdateForUpdateRequests(t *testing.T) { newInstance := getTestInstance() newInstance.Spec.UpdateRequests = tc.newValue - instanceRESTStrategies.PrepareForUpdate(nil, newInstance, oldInstance) + instanceRESTStrategies.PrepareForUpdate(createContext, newInstance, oldInstance) if e, a := tc.expectedValue, newInstance.Spec.UpdateRequests; e != a { t.Errorf("%s: got unexpected UpdateRequests: expected %v, got %v", tc.name, e, a) @@ -259,7 +252,9 @@ func TestInstanceUpdateForUpdateRequests(t *testing.T) { // TestExternalIDSet checks that we set the ExternalID if the user doesn't provide it. func TestExternalIDSet(t *testing.T) { createdInstanceCredential := getTestInstance() - instanceRESTStrategies.PrepareForCreate(nil, createdInstanceCredential) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + instanceRESTStrategies.PrepareForCreate(createContext, createdInstanceCredential) if createdInstanceCredential.Spec.ExternalID == "" { t.Error("Expected an ExternalID to be set, but got none") @@ -271,7 +266,9 @@ func TestExternalIDUserProvided(t *testing.T) { userExternalID := "my-id" createdInstanceCredential := getTestInstance() createdInstanceCredential.Spec.ExternalID = userExternalID - instanceRESTStrategies.PrepareForCreate(nil, createdInstanceCredential) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + instanceRESTStrategies.PrepareForCreate(createContext, createdInstanceCredential) if createdInstanceCredential.Spec.ExternalID != userExternalID { t.Errorf("Modified user provided ExternalID to %q", createdInstanceCredential.Spec.ExternalID) diff --git a/pkg/registry/servicecatalog/servicebroker/storage.go b/pkg/registry/servicecatalog/servicebroker/storage.go index c915546c5f0..ca0ecf7e7aa 100644 --- a/pkg/registry/servicecatalog/servicebroker/storage.go +++ b/pkg/registry/servicecatalog/servicebroker/storage.go @@ -24,8 +24,10 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -134,6 +136,35 @@ func NewStorage(opts server.Options) (serviceBrokers, serviceBrokerStatus rest.S DeleteStrategy: serviceBrokerRESTStrategies, EnableGarbageCollection: true, + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "URL", Type: "string"}, + {Name: "Status", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + getStatus := func(status servicecatalog.CommonServiceBrokerStatus) string { + if len(status.Conditions) > 0 { + condition := status.Conditions[len(status.Conditions)-1] + if condition.Status == servicecatalog.ConditionTrue { + return string(condition.Type) + } + return condition.Reason + } + return "" + } + broker := obj.(*servicecatalog.ServiceBroker) + cells := []interface{}{ + name, + broker.Spec.URL, + getStatus(broker.Status.CommonServiceBrokerStatus), + age, + } + return cells, nil + }, + ), + Storage: storageInterface, DestroyFunc: dFunc, } diff --git a/pkg/registry/servicecatalog/servicebroker/strategy_test.go b/pkg/registry/servicecatalog/servicebroker/strategy_test.go index b67d350ca9f..7cee44cd1d2 100644 --- a/pkg/registry/servicecatalog/servicebroker/strategy_test.go +++ b/pkg/registry/servicecatalog/servicebroker/strategy_test.go @@ -20,6 +20,7 @@ import ( "testing" sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -83,7 +84,9 @@ func TestServiceBroker(t *testing.T) { } // Canonicalize the broker - serviceBrokerRESTStrategies.PrepareForCreate(nil, broker) + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) + serviceBrokerRESTStrategies.PrepareForCreate(createContext, broker) if broker.Status.Conditions == nil { t.Fatalf("Fresh servicebroker should have empty status") @@ -115,9 +118,10 @@ func TestServiceBrokerUpdate(t *testing.T) { shouldGenerationIncrement: true, }, } - + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for i := range cases { - serviceBrokerRESTStrategies.PrepareForUpdate(nil, cases[i].newer, cases[i].older) + serviceBrokerRESTStrategies.PrepareForUpdate(createContext, cases[i].newer, cases[i].older) if cases[i].shouldGenerationIncrement { if e, a := cases[i].older.Generation+1, cases[i].newer.Generation; e != a { @@ -165,6 +169,8 @@ func TestServiceBrokerUpdateForRelistRequests(t *testing.T) { expectedValue: 2, }, } + creatorUserName := "creator" + createContext := sctestutil.ContextWithUserName(creatorUserName) for _, tc := range cases { oldBroker := serviceBrokerWithOldSpec() oldBroker.Spec.RelistRequests = tc.oldValue @@ -172,7 +178,7 @@ func TestServiceBrokerUpdateForRelistRequests(t *testing.T) { newServiceBroker := serviceBrokerWithOldSpec() newServiceBroker.Spec.RelistRequests = tc.newValue - serviceBrokerRESTStrategies.PrepareForUpdate(nil, newServiceBroker, oldBroker) + serviceBrokerRESTStrategies.PrepareForUpdate(createContext, newServiceBroker, oldBroker) if e, a := tc.expectedValue, newServiceBroker.Spec.RelistRequests; e != a { t.Errorf("%s: got unexpected RelistRequests: expected %v, got %v", tc.name, e, a) diff --git a/pkg/registry/servicecatalog/serviceclass/storage.go b/pkg/registry/servicecatalog/serviceclass/storage.go index f98bc57ea8a..77742ceaa4d 100644 --- a/pkg/registry/servicecatalog/serviceclass/storage.go +++ b/pkg/registry/servicecatalog/serviceclass/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -144,8 +146,28 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage) { CreateStrategy: serviceClassRESTStrategies, UpdateStrategy: serviceClassRESTStrategies, DeleteStrategy: serviceClassRESTStrategies, - Storage: storageInterface, - DestroyFunc: dFunc, + + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "External-Name", Type: "string"}, + {Name: "Broker", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + class := obj.(*servicecatalog.ServiceClass) + cells := []interface{}{ + name, + class.Spec.ExternalName, + class.Spec.ServiceBrokerName, + age, + } + return cells, nil + }, + ), + + Storage: storageInterface, + DestroyFunc: dFunc, } options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs} diff --git a/pkg/registry/servicecatalog/serviceplan/storage.go b/pkg/registry/servicecatalog/serviceplan/storage.go index 8395536ad8e..eee3ae7759d 100644 --- a/pkg/registry/servicecatalog/serviceplan/storage.go +++ b/pkg/registry/servicecatalog/serviceplan/storage.go @@ -24,7 +24,9 @@ import ( scmeta "github.com/kubernetes-incubator/service-catalog/pkg/api/meta" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" + "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/tableconvertor" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -142,8 +144,30 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage) { CreateStrategy: servicePlanRESTStrategies, UpdateStrategy: servicePlanRESTStrategies, DeleteStrategy: servicePlanRESTStrategies, - Storage: storageInterface, - DestroyFunc: dFunc, + + TableConvertor: tableconvertor.NewTableConvertor( + []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Format: "name"}, + {Name: "External-Name", Type: "string"}, + {Name: "Broker", Type: "string"}, + {Name: "Class", Type: "string"}, + {Name: "Age", Type: "string"}, + }, + func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { + plan := obj.(*servicecatalog.ServicePlan) + cells := []interface{}{ + name, + plan.Spec.ExternalName, + plan.Spec.ServiceBrokerName, + plan.Spec.ServiceClassRef.Name, + age, + } + return cells, nil + }, + ), + + Storage: storageInterface, + DestroyFunc: dFunc, } statusStore := store diff --git a/pkg/registry/servicecatalog/tableconvertor/tableconvertor.go b/pkg/registry/servicecatalog/tableconvertor/tableconvertor.go new file mode 100644 index 00000000000..ae988be499e --- /dev/null +++ b/pkg/registry/servicecatalog/tableconvertor/tableconvertor.go @@ -0,0 +1,64 @@ +/* +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 tableconvertor + +import ( + "context" + + "k8s.io/apimachinery/pkg/api/meta" + metatable "k8s.io/apimachinery/pkg/api/meta/table" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/registry/rest" +) + +// RowFunction is a function that maps an object (e.g. ServiceInstance) +// to an array of values that get printed in columnar form when a user +// runs kubectl get. +type RowFunction func(obj runtime.Object, meta metav1.Object, name, age string) ([]interface{}, error) + +// NewTableConvertor creates a TableConvertor with the specified columns +// and RowFunction, which is used to map an object to those columns. +func NewTableConvertor(columnDefinitions []metav1beta1.TableColumnDefinition, rowFunction RowFunction) rest.TableConvertor { + return &convertor{columnDefinitions, rowFunction} +} + +type convertor struct { + columnDefinitions []metav1beta1.TableColumnDefinition + rowFunction RowFunction +} + +func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1beta1.Table, error) { + table := &metav1beta1.Table{ + ColumnDefinitions: c.columnDefinitions, + } + if m, err := meta.ListAccessor(obj); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + table.Continue = m.GetContinue() + } else { + if m, err := meta.CommonAccessor(obj); err == nil { + table.ResourceVersion = m.GetResourceVersion() + table.SelfLink = m.GetSelfLink() + } + } + + var err error + table.Rows, err = metatable.MetaToTableRow(obj, c.rowFunction) + return table, err +} diff --git a/pkg/svcat/service-catalog/assets/ca b/pkg/svcat/service-catalog/assets/ca new file mode 100644 index 00000000000..257cc5642cb --- /dev/null +++ b/pkg/svcat/service-catalog/assets/ca @@ -0,0 +1 @@ +foo diff --git a/pkg/svcat/service-catalog/broker.go b/pkg/svcat/service-catalog/broker.go index 73bf292f75c..a5f1c47bb72 100644 --- a/pkg/svcat/service-catalog/broker.go +++ b/pkg/svcat/service-catalog/broker.go @@ -18,6 +18,7 @@ package servicecatalog import ( "fmt" + "io/ioutil" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" "k8s.io/apimachinery/pkg/api/errors" @@ -104,18 +105,55 @@ func (sdk *SDK) RetrieveBrokerByClass(class *v1beta1.ClusterServiceClass, return broker, nil } -// Register creates a broker -func (sdk *SDK) Register(brokerName string, url string) (*v1beta1.ClusterServiceBroker, error) { +//Register creates a broker +func (sdk *SDK) Register(brokerName string, url string, opts *RegisterOptions) (*v1beta1.ClusterServiceBroker, error) { + var err error + var caBytes []byte + if opts.CAFile != "" { + caBytes, err = ioutil.ReadFile(opts.CAFile) + if err != nil { + return nil, fmt.Errorf("Error opening CA file: %v", err.Error()) + } + + } request := &v1beta1.ClusterServiceBroker{ ObjectMeta: v1.ObjectMeta{ Name: brokerName, }, Spec: v1beta1.ClusterServiceBrokerSpec{ CommonServiceBrokerSpec: v1beta1.CommonServiceBrokerSpec{ - URL: url, + CABundle: caBytes, + InsecureSkipTLSVerify: opts.SkipTLS, + RelistBehavior: opts.RelistBehavior, + RelistDuration: opts.RelistDuration, + URL: url, + CatalogRestrictions: &v1beta1.CatalogRestrictions{ + ServiceClass: opts.ClassRestrictions, + ServicePlan: opts.PlanRestrictions, + }, }, }, } + if opts.BasicSecret != "" { + request.Spec.AuthInfo = &v1beta1.ClusterServiceBrokerAuthInfo{ + Basic: &v1beta1.ClusterBasicAuthConfig{ + SecretRef: &v1beta1.ObjectReference{ + Name: opts.BasicSecret, + Namespace: opts.Namespace, + }, + }, + } + } else if opts.BearerSecret != "" { + request.Spec.AuthInfo = &v1beta1.ClusterServiceBrokerAuthInfo{ + Bearer: &v1beta1.ClusterBearerTokenAuthConfig{ + SecretRef: &v1beta1.ObjectReference{ + Name: opts.BearerSecret, + Namespace: opts.Namespace, + }, + }, + } + } + result, err := sdk.ServiceCatalog().ClusterServiceBrokers().Create(request) if err != nil { return nil, fmt.Errorf("register request failed (%s)", err) diff --git a/pkg/svcat/service-catalog/broker_test.go b/pkg/svcat/service-catalog/broker_test.go index c18353703a3..794184879df 100644 --- a/pkg/svcat/service-catalog/broker_test.go +++ b/pkg/svcat/service-catalog/broker_test.go @@ -18,6 +18,7 @@ package servicecatalog_test import ( "fmt" + "time" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/fake" @@ -192,8 +193,58 @@ var _ = Describe("Broker", func() { It("creates a broker by calling the v1beta1 Create method with the passed in arguements", func() { brokerName := "potato_broker" url := "http://potato.com" + basicSecret := "potatobasicsecret" + caFile := "assets/ca" + namespace := "potatonamespace" + planRestrictions := []string{"potatoplana", "potatoplanb"} + relistBehavior := v1beta1.ServiceBrokerRelistBehaviorDuration + relistDuration := &metav1.Duration{Duration: 10 * time.Minute} + skipTLS := true + + opts := &RegisterOptions{ + BasicSecret: basicSecret, + CAFile: caFile, + Namespace: namespace, + PlanRestrictions: planRestrictions, + RelistBehavior: relistBehavior, + RelistDuration: relistDuration, + SkipTLS: skipTLS, + } + broker, err := sdk.Register(brokerName, url, opts) - broker, err := sdk.Register(brokerName, url) + Expect(err).NotTo(HaveOccurred()) + Expect(broker).NotTo(BeNil()) + Expect(broker.Name).To(Equal(brokerName)) + Expect(broker.Spec.URL).To(Equal(url)) + Expect(broker.Spec.CABundle).To(Equal([]byte("foo\n"))) + Expect(broker.Spec.InsecureSkipTLSVerify).To(BeTrue()) + Expect(broker.Spec.RelistBehavior).To(Equal(relistBehavior)) + Expect(broker.Spec.RelistDuration).To(Equal(relistDuration)) + Expect(broker.Spec.CatalogRestrictions.ServicePlan).To(Equal(planRestrictions)) + + actions := svcCatClient.Actions() + Expect(actions[0].Matches("create", "clusterservicebrokers")).To(BeTrue()) + objectFromRequest := actions[0].(testing.CreateActionImpl).Object.(*v1beta1.ClusterServiceBroker) + Expect(objectFromRequest.ObjectMeta.Name).To(Equal(brokerName)) + Expect(objectFromRequest.Spec.URL).To(Equal(url)) + Expect(objectFromRequest.Spec.AuthInfo.Basic.SecretRef.Name).To(Equal(basicSecret)) + Expect(objectFromRequest.Spec.CABundle).To(Equal([]byte("foo\n"))) + Expect(objectFromRequest.Spec.InsecureSkipTLSVerify).To(BeTrue()) + Expect(objectFromRequest.Spec.RelistBehavior).To(Equal(relistBehavior)) + Expect(objectFromRequest.Spec.RelistDuration).To(Equal(relistDuration)) + Expect(objectFromRequest.Spec.CatalogRestrictions.ServicePlan).To(Equal(planRestrictions)) + }) + It("creates a broker with a bearer secret", func() { + brokerName := "potato_broker" + url := "http://potato.com" + namespace := "potatonamespace" + bearerSecret := "potatobearersecret" + opts := &RegisterOptions{ + Namespace: namespace, + BearerSecret: bearerSecret, + } + + broker, err := sdk.Register(brokerName, url, opts) Expect(err).NotTo(HaveOccurred()) Expect(broker).NotTo(BeNil()) @@ -205,6 +256,7 @@ var _ = Describe("Broker", func() { objectFromRequest := actions[0].(testing.CreateActionImpl).Object.(*v1beta1.ClusterServiceBroker) Expect(objectFromRequest.ObjectMeta.Name).To(Equal(brokerName)) Expect(objectFromRequest.Spec.URL).To(Equal(url)) + Expect(objectFromRequest.Spec.AuthInfo.Bearer.SecretRef.Name).To(Equal(bearerSecret)) }) It("Bubbles up errors", func() { errorMessage := "error provisioning broker" @@ -216,7 +268,7 @@ var _ = Describe("Broker", func() { }) sdk.ServiceCatalogClient = badClient - broker, err := sdk.Register(brokerName, url) + broker, err := sdk.Register(brokerName, url, &RegisterOptions{}) Expect(broker).To(BeNil()) Expect(err).To(HaveOccurred()) diff --git a/pkg/svcat/service-catalog/options.go b/pkg/svcat/service-catalog/options.go index d8c32b7f043..be6ecd22377 100644 --- a/pkg/svcat/service-catalog/options.go +++ b/pkg/svcat/service-catalog/options.go @@ -16,7 +16,25 @@ limitations under the License. package servicecatalog +import ( + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + // FilterOptions allows for optional filtering fields to be passed to `Retrieve` methods. type FilterOptions struct { ClassID string } + +// RegisterOptions allows for passing of optional fields to the broker Register method. +type RegisterOptions struct { + BasicSecret string + BearerSecret string + CAFile string + ClassRestrictions []string + Namespace string + PlanRestrictions []string + RelistBehavior v1beta1.ServiceBrokerRelistBehavior + RelistDuration *metav1.Duration + SkipTLS bool +} diff --git a/pkg/svcat/service-catalog/sdk.go b/pkg/svcat/service-catalog/sdk.go index 18c74751bf7..678a394fbf9 100644 --- a/pkg/svcat/service-catalog/sdk.go +++ b/pkg/svcat/service-catalog/sdk.go @@ -48,7 +48,7 @@ type SvcatClient interface { RetrieveBrokers(opts ScopeOptions) ([]Broker, error) RetrieveBroker(string) (*apiv1beta1.ClusterServiceBroker, error) RetrieveBrokerByClass(*apiv1beta1.ClusterServiceClass) (*apiv1beta1.ClusterServiceBroker, error) - Register(string, string) (*apiv1beta1.ClusterServiceBroker, error) + Register(string, string, *RegisterOptions) (*apiv1beta1.ClusterServiceBroker, error) Sync(string, int) error RetrieveClasses(ScopeOptions) ([]Class, 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 b7dd46aa961..f35bebe948b 100644 --- a/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go +++ b/pkg/svcat/service-catalog/service-catalogfakes/fake_svcat_client.go @@ -234,11 +234,12 @@ type FakeSvcatClient struct { result1 *apiv1beta1.ClusterServiceBroker result2 error } - RegisterStub func(string, string) (*apiv1beta1.ClusterServiceBroker, error) + RegisterStub func(string, string, *servicecatalog.RegisterOptions) (*apiv1beta1.ClusterServiceBroker, error) registerMutex sync.RWMutex registerArgsForCall []struct { arg1 string arg2 string + arg3 *servicecatalog.RegisterOptions } registerReturns struct { result1 *apiv1beta1.ClusterServiceBroker @@ -1367,17 +1368,18 @@ func (fake *FakeSvcatClient) RetrieveBrokerByClassReturnsOnCall(i int, result1 * }{result1, result2} } -func (fake *FakeSvcatClient) Register(arg1 string, arg2 string) (*apiv1beta1.ClusterServiceBroker, error) { +func (fake *FakeSvcatClient) Register(arg1 string, arg2 string, arg3 *servicecatalog.RegisterOptions) (*apiv1beta1.ClusterServiceBroker, error) { fake.registerMutex.Lock() ret, specificReturn := fake.registerReturnsOnCall[len(fake.registerArgsForCall)] fake.registerArgsForCall = append(fake.registerArgsForCall, struct { arg1 string arg2 string - }{arg1, arg2}) - fake.recordInvocation("Register", []interface{}{arg1, arg2}) + arg3 *servicecatalog.RegisterOptions + }{arg1, arg2, arg3}) + fake.recordInvocation("Register", []interface{}{arg1, arg2, arg3}) fake.registerMutex.Unlock() if fake.RegisterStub != nil { - return fake.RegisterStub(arg1, arg2) + return fake.RegisterStub(arg1, arg2, arg3) } if specificReturn { return ret.result1, ret.result2 @@ -1391,10 +1393,10 @@ func (fake *FakeSvcatClient) RegisterCallCount() int { return len(fake.registerArgsForCall) } -func (fake *FakeSvcatClient) RegisterArgsForCall(i int) (string, string) { +func (fake *FakeSvcatClient) RegisterArgsForCall(i int) (string, string, *servicecatalog.RegisterOptions) { fake.registerMutex.RLock() defer fake.registerMutex.RUnlock() - return fake.registerArgsForCall[i].arg1, fake.registerArgsForCall[i].arg2 + return fake.registerArgsForCall[i].arg1, fake.registerArgsForCall[i].arg2, fake.registerArgsForCall[i].arg3 } func (fake *FakeSvcatClient) RegisterReturns(result1 *apiv1beta1.ClusterServiceBroker, result2 error) { diff --git a/test/integration/controller_instance_test.go b/test/integration/controller_instance_test.go index 2d7a8aa8067..742efbd294d 100644 --- a/test/integration/controller_instance_test.go +++ b/test/integration/controller_instance_test.go @@ -964,6 +964,91 @@ func TestUpdateServiceInstanceUpdateParameters(t *testing.T) { } } +// TestCreateServiceInstanceWithRetries tests creating a ServiceInstance +// with/without retries. +func TestCreateServiceInstanceWithRetries(t *testing.T) { + cases := []struct { + name string + setup func(ct *controllerTest) + }{ + { + name: "no retry", + setup: func(ct *controllerTest) { + ct.osbClient.ProvisionReaction = &fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + }, + } + }, + }, + { + name: "retry after temporary error without orphan mitigation", + setup: func(ct *controllerTest) { + ct.osbClient.ProvisionReaction = fakeosb.DynamicProvisionReaction(getProvisionResponseByPollCountReactions(2, + []fakeosb.ProvisionReaction{ + fakeosb.ProvisionReaction{ + Error: osb.HTTPStatusCodeError{ + StatusCode: http.StatusUnauthorized, + ErrorMessage: strPtr("unauthorized; retry later"), + Description: strPtr("temporary error that can be retried without orphan mitigation"), + }, + }, + fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + }, + }, + })) + }, + }, + { + name: "retry after temporary error with orphan mitigation", + setup: func(ct *controllerTest) { + ct.osbClient.ProvisionReaction = fakeosb.DynamicProvisionReaction(getProvisionResponseByPollCountReactions(2, + []fakeosb.ProvisionReaction{ + fakeosb.ProvisionReaction{ + Error: osb.HTTPStatusCodeError{ + StatusCode: http.StatusInternalServerError, + ErrorMessage: strPtr("something is broken"), + Description: strPtr("temporary error that can be retried after orphan mitigation"), + }, + }, + fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + }, + }, + })) + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + ct := &controllerTest{ + t: t, + broker: getTestBroker(), + instance: getTestInstance(), + } + ct.setup = tc.setup + ct.run(func(ct *controllerTest) { + verifyCondition := v1beta1.ServiceInstanceCondition{ + Type: v1beta1.ServiceInstanceConditionReady, + Status: v1beta1.ConditionTrue, + Reason: "ProvisionedSuccessfully", + } + if err := util.WaitForInstanceCondition(ct.client, testNamespace, testInstanceName, verifyCondition); err != nil { + t.Fatalf("error waiting for instance condition: %v", err) + } + if ct.instance.Status.ProvisionStatus != v1beta1.ServiceInstanceProvisionStatusProvisioned { + t.Fatalf("expected provisioned status after successful provisioning, but got %s", ct.instance.Status.ProvisionStatus) + } + if ct.instance.Status.DeprovisionStatus != v1beta1.ServiceInstanceDeprovisionStatusRequired { + t.Fatalf("expected deprovision to be required after successful provisioning, but got %s", ct.instance.Status.DeprovisionStatus) + } + }) + }) + } +} + // TestCreateServiceInstanceWithInvalidParameters tests creating a ServiceInstance // with invalid parameters. func TestCreateServiceInstanceWithInvalidParameters(t *testing.T) { diff --git a/test/integration/controller_test.go b/test/integration/controller_test.go index 0cc4ee6520c..4cce0f4c80f 100644 --- a/test/integration/controller_test.go +++ b/test/integration/controller_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + sctestutil "github.com/kubernetes-incubator/service-catalog/test/util" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -230,8 +231,8 @@ func verifyUsernameInLastBrokerAction(t *testing.T, osbClient *fakeosb.FakeClien // as part of originating identity included in broker requests. func TestBasicFlowsWithOriginatingIdentity(t *testing.T) { // Enable the OriginatingIdentity feature - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.OriginatingIdentity)) - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.OriginatingIdentity)) + prevOrigIDEnablement := sctestutil.EnableOriginatingIdentity(t, true) + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, prevOrigIDEnablement)) createChangeUsernameFunc := func(username string) func(*controllerTest) { return func(ct *controllerTest) { diff --git a/test/util/util.go b/test/util/util.go index b84fc4395fc..b08e5afac45 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "context" "fmt" "testing" "time" @@ -26,9 +27,13 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/authentication/user" "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" v1beta1servicecatalog "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset/typed/servicecatalog/v1beta1" + scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) // WaitForBrokerCondition waits for the status of the named broker to contain @@ -382,3 +387,25 @@ func AssertServiceInstanceConditionFalseOrAbsent(t *testing.T, instance *v1beta1 } } } + +// EnableOriginatingIdentity enables the OriginatingIdentity feature gate. Returns +// the prior state of the gate. +func EnableOriginatingIdentity(t *testing.T, enabled bool) (previousState bool) { + prevOrigIdentEnablement := utilfeature.DefaultFeatureGate.Enabled(scfeatures.OriginatingIdentity) + if prevOrigIdentEnablement != enabled { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=%v", scfeatures.OriginatingIdentity, enabled)) + if err != nil { + t.Fatalf("Failed to enable originating identity feature: %v", err) + } + } + return prevOrigIdentEnablement +} + +// ContextWithUserName creates a Context with the specified userName +func ContextWithUserName(userName string) context.Context { + ctx := genericapirequest.NewContext() + userInfo := &user.DefaultInfo{ + Name: userName, + } + return genericapirequest.WithUser(ctx, userInfo) +} diff --git a/vendor/k8s.io/apimachinery/pkg/api/meta/table/table.go b/vendor/k8s.io/apimachinery/pkg/api/meta/table/table.go new file mode 100644 index 00000000000..2144a77cb19 --- /dev/null +++ b/vendor/k8s.io/apimachinery/pkg/api/meta/table/table.go @@ -0,0 +1,71 @@ +/* +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 table + +import ( + "time" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/duration" +) + +// MetaToTableRow converts a list or object into one or more table rows. The provided rowFn is invoked for +// each accessed item, with name and age being passed to each. +func MetaToTableRow(obj runtime.Object, rowFn func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error)) ([]metav1beta1.TableRow, error) { + if meta.IsListType(obj) { + rows := make([]metav1beta1.TableRow, 0, 16) + err := meta.EachListItem(obj, func(obj runtime.Object) error { + nestedRows, err := MetaToTableRow(obj, rowFn) + if err != nil { + return err + } + rows = append(rows, nestedRows...) + return nil + }) + if err != nil { + return nil, err + } + return rows, nil + } + + rows := make([]metav1beta1.TableRow, 0, 1) + m, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + row := metav1beta1.TableRow{ + Object: runtime.RawExtension{Object: obj}, + } + row.Cells, err = rowFn(obj, m, m.GetName(), ConvertToHumanReadableDateType(m.GetCreationTimestamp())) + if err != nil { + return nil, err + } + rows = append(rows, row) + return rows, nil +} + +// ConvertToHumanReadableDateType returns the elapsed time since timestamp in +// human-readable approximation. +func ConvertToHumanReadableDateType(timestamp metav1.Time) string { + if timestamp.IsZero() { + return "" + } + return duration.ShortHumanDuration(time.Now().Sub(timestamp.Time)) +} diff --git a/vendor/k8s.io/apimachinery/pkg/util/duration/duration.go b/vendor/k8s.io/apimachinery/pkg/util/duration/duration.go new file mode 100644 index 00000000000..00404c6cdda --- /dev/null +++ b/vendor/k8s.io/apimachinery/pkg/util/duration/duration.go @@ -0,0 +1,43 @@ +/* +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 duration + +import ( + "fmt" + "time" +) + +// ShortHumanDuration returns a succint representation of the provided duration +// with limited precision for consumption by humans. +func ShortHumanDuration(d time.Duration) string { + // Allow deviation no more than 2 seconds(excluded) to tolerate machine time + // inconsistence, it can be considered as almost now. + if seconds := int(d.Seconds()); seconds < -1 { + return fmt.Sprintf("") + } else if seconds < 0 { + return fmt.Sprintf("0s") + } else if seconds < 60 { + return fmt.Sprintf("%ds", seconds) + } else if minutes := int(d.Minutes()); minutes < 60 { + return fmt.Sprintf("%dm", minutes) + } else if hours := int(d.Hours()); hours < 24 { + return fmt.Sprintf("%dh", hours) + } else if hours < 24*365 { + return fmt.Sprintf("%dd", hours/24) + } + return fmt.Sprintf("%dy", int(d.Hours()/24/365)) +}