search

servicebinding / spec

Specification for binding services to k8s workloads
https://servicebinding.io
Apache License 2.0
92 stars 35 forks source link

Consider limiting containers scoping to names #127

Closed liggitt closed 3 years ago

liggitt commented 4 years ago

The current spec says:

The Service Binding resource MAY define .spec.application.containers, as a list of integers or strings

Referencing containers by index is fragile in the presence of admission webhooks that inject sidecar containers. Referencing by name would be much more stable.

scothis commented 4 years ago

I agree that we should encourage specifying containers by name, but there are two cases making supporting only names less convenient:

  1. some PodSpecable resources do not require that the container name be defined, a default value is set before ending up as a Pod. For example, the Knative Service doesn't require the container name to be defined.
  2. injecting into the first container regardless of name. Injected containers are typically appended assuming they appear on the target resource at all.

I'm 👍 on removing index based container selection if the community agrees.

arthurdm commented 4 years ago

thanks for opening this @liggitt - we discussed in today's hangout and agreed that we should augment the spec to include the recommendation of using names for the containers. PRs are welcome. :)

baijum commented 3 years ago

I have sent a PR: #134 Please review!

baijum commented 3 years ago

I have sent a PR: #134 Please review!

I closed the PR as it was not according to the meeting discussion.