-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Binding to a template with no expose annotation creates an empty secret #16726
Comments
I think the theory w/ making the nodejs ones bindable is that could represent a microservice you've deployed that you want to bind to. |
(hence them exposing their URI) |
I'm not sure it's clear to the user what creating a binding for the Node.js + MongoDB service, for example, would mean. It seems potentially confusing when a service creates more than one thing in my project. |
I don't see the harm in somehow indicating that a template is not bindable, but initially I'm hesitant to link "contains no expose annotation" and "not bindable". |
@jim-minter if it doesn't contain any expose annotations, then bind results in an empty secret, so i'm not sure what value there is in leaving it "bindable" in that case? |
I'm not saying these can't be overruled, but:
|
Fair enough, i think i'm reasonably convinced we should just make it an explicit decision. |
(but we should still probably remove expose from the existing quickstart templates, and mark them not bindable once we have a way to do that). |
Automatic merge from submit-queue. add template.openshift.io/bindable annotation, default is true fixes #16726 will update quickstarts and pull them in after api review (and update docs). @bparees @jorgemoralespou @spadgett
If I create a binding against a template that doesn't use the
template.openshift.io/expose
annotation, I get an empty secret. It would be better to instead disable binding altogether for the service class if not supported for the template.A lot of our template don't really make sense to bind to. For instance, sample apps like nodejs + mongo. We might consider disabling binding entirely on those to avoid confusion.
@jim-minter @ncameronbritt @sspeiche
The text was updated successfully, but these errors were encountered: