Enable namespaced brokers by default

[jberkhahn]

This is ready to be turned on per #2244.

[MHBauer]

/lgtm
/approve

[eriknelson]

/lgtm

[jboyd01]

Looks like we need to update this as well so it's on by default when installing with Helm:

https://github.com/kubernetes-incubator/service-catalog/blob/f6f0e41fae6c4f1bdc5c507942ee7f1719555a19/charts/catalog/values.yaml#L155-L156

[MHBauer]

@jberkhahn the chart needs to change too. Give it another scrub.
/hold

[MHBauer]

conditionals in templates need to be cleaned up or removed.

charts/catalog/templates/controller-manager-deployment.yaml
83-        {{- end }}
84:        {{- if .Values.namespacedServiceBrokerEnabled }}
85-        - --feature-gates
86:        - NamespacedServiceBroker=true
87-        {{- end }}

charts/catalog/templates/rbac.yaml
108-    verbs:     ["update"]
109:  {{- if .Values.namespacedServiceBrokerEnabled }}
110-  - apiGroups: ["servicecatalog.k8s.io"]

charts/catalog/templates/apiserver-deployment.yaml
62-        {{- end }}
63:        {{- if .Values.namespacedServiceBrokerEnabled }}
64-        - --feature-gates
65:        - NamespacedServiceBroker=true
66-        {{- end }}

docs too

docs/namespaced-broker-resources.md
78-to pass an argument to the API Server when you install Service Catalog:
79: `--feature-gates NamespacedServiceBroker=true`.
80-
81:If you are using Helm, you can use the `namespacedServiceBrokerEnabled` setting
82- to control that flag:
--
87-   --namespace catalog \
88:   --set namespacedServiceBrokerEnabled=true
89-```

[MHBauer]

/lgtm cancel
/approve cancel
/woof

[k8s-ci-robot]

@MHBauer:

In response to this:

/lgtm cancel
/approve cancel
/woof

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[MHBauer]

see comments

[jberkhahn]

Why did that stuff not get done for originating identity?

[MHBauer]

I am not sure what you are looking at specifically. Feel free to file an issue if you see a problem.

[jboyd01]

@jberkhahn It probably should be done for Originating Identity. I created a new issue for it: #2252

[jboyd01]

@jberkhahn I looked at this too quickly - - that is the Originating Identity Locking locking that is defaulted to on. So I'm wrong, OI was never enabled by default. I think it should be, but I'm wrong in here where I said it was done already. I believe all the comments above are correct though in terms of what needs to be cleaned up for the NamespaceScoped broker feature gate.

Fixed it

[jberkhahn]

Ok, fixed the charts and added a bit to the docs about it.

[jboyd01]

My quick read on this is if the value is set to false or not defined, it will set the NamespacedServiceBroker to false. I'd prefer to see this only set it to false if the value is explicitly set to false in values.yaml. I was originally thinking we'd remove the NSB variable from values.yaml, but I guess as long as its present and set to true it's going to do the right thing. WDYT? Probably not worth changing, ok as is.

[jboyd01]

same as above

[jboyd01]

this is unintentional and should be reverted right?

[jboyd01]

I think this looks good, thanks for doing it. Looks like an accidental change of the repo that should not be in here though.

[jberkhahn]

Hmm, it seems Jay was right. If you don't pass in an option to Helm, it interprets it as being false. So if you don't pass in EnableNamespaced brokers, it would evaluate to false and set the option to false. So I had to change it to an affirmative DisableNamespacedBrokers flag.

also, cleaned up the typo.

Thoughts?

the repo issue was addressed, thanks!

[jboyd01]

/LGTM
Thanks for hashing through this @jberkhahn!

[MHBauer]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MHBauer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [MHBauer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[MHBauer]

/hold cancel