If -apb is a required naming convention, should be enforced by apb-tools

[cunningt]

I think @eriknelson mentioned to me that a "-apb" suffix might be a necessary condition to creating a working apb.

If this is true (I don't know enough about apb to know whether it is or not), this seems like something that apbtools should enforce. Using the apbtools, I can do "apb init foobar / cd foobar / apb prepare / apb build" and waste a bunch of time creating an apb that would be problematic.

[rthallisey]

It's a good observation @cunningt, thanks for reporting it.

When you deploy the broker by default, the configmap sets the value of whitelist to -apb so it only looks for containers with the suffix -apb. https://github.com/openshift/ansible-service-broker/blob/master/templates/deploy-ansible-service-broker.template.yaml#L312

this seems like something that apbtools should enforce. Using the apbtools, I can do "apb init foobar / cd foobar / apb prepare / apb build" and waste a bunch of time creating an apb that would be problematic.

Because it's a configurable value I don't think it's necessary to enforce the -apb suffix, but I think we should make sure we're doing two things: 1) apb init foobar sets the name of the apb to foobar-apb 2) Document in the apb tool that the broker, by default, expects the suffix -apb

Do you agree with those action items @cunningt ?

[jwmatthews]

@rthallisey +1 to your suggestions

@rthallisey might be good to add a warning in future lint tool of apbs to raise warning if name doesn't ends in -apb.

I think of the *-apb name as a useful convention that is helping us and a best practice we recommend, yet if an advanced user wants to remove that I think we should allow with warnings so they aren't surprised.

[cunningt]

Sounds great to me (all of this). Probably be good to mention it in the getting started guide too.

[eriknelson]

@rthallisey @jwmatthews There seems to be a misconception that the -apb suffix is enforced by the whitelist (I had this impression as well). In fact, the broker enforces this suffix before it even hits the filters, throwing out names that don't end in -apb. It's a hard rule applied to all adapters regardless of configuration.

We need to re-evaluate whether or not this is something that should be in the broker (I know we had specific reasons for doing it in the first place), and if we feel it's still appropriate, I think @cunningt is right, this needs to be part of the apb tool validation.

[rthallisey]

Looks like we do enforce it. @shawn-hurley you may remember why we kept the -apb suffix hardcoded. My guess is we kept it because didn't default the whitelist to -apb when the whitelist patch merged. Maybe we should change that.

[shawn-hurley]

I think it was because we did not want to try and pull all of the image's metadata if some were not even close to APBs and decided that this was a way to prefilter. The filter code was written to focus on the name of the APB so if we had the names and knew they were APBs then we could filter them and then pull the metadata. The original discussion is here I think:

This maybe something that we want to revisit, and it sounds like we are not super happy with the adapter interface so this might be apart of that work. see https://github.com/openshift/ansible-service-broker/pull/682. I think that we should do this sooner rather than later as we will be exposing the adapter and registry stuff for the vendoring work.