Skip to content
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

Remove integer based indexing to limit containers #171

Merged
merged 2 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,8 @@ Restricting service binding to resources within the same namespace is strongly *

A Service Binding resource **MUST** define a `.spec.application` which is an `ObjectReference`-like declaration. A `ServiceBinding` **MAY** define the application reference by-name or by-[label selector][ls]. A name and selector **MUST NOT** be defined in the same reference. A Service Binding resource **MUST** define a `.spec.service` which is an `ObjectReference`-like declaration to a Provisioned Service-able resource. Extensions and implementations **MAY** allow additional kinds of applications and services to be referenced.

The Service Binding resource **MAY** define `.spec.application.containers`, as a list of integers or strings, to limit which containers in the application are bound. Referencing containers by index is fragile in the presence of admission webhooks that inject sidecar containers. It is **RECOMMENDED** to match containers by name. Binding to a container is opt-in, unless `.spec.application.containers` is undefined then all containers **MUST** be bound. For each item in the containers list:
- if the value is an integer (`${containerInteger}`), the container matching by index (`.spec.template.spec.containers[${containerInteger}]`) **MUST** be bound. Init containers **MUST NOT** be bound
- if the value is a string (`${containerString}`), a container or init container matching by name (`.spec.template.spec.containers[?(@.name=='${containerString}')]` or `.spec.template.spec.initContainers[?(@.name=='${containerString}')]`) **MUST** be bound
The Service Binding resource **MAY** define `.spec.application.containers`, to limit which containers in the application are bound. If `.spec.application.containers` is defined, the value **MUST** be a list of strings. Binding to a container is opt-in, unless `.spec.application.containers` is undefined then all containers **MUST** be bound. For each item in the containers list:
- a container or init container matching by name (`.spec.template.spec.containers[?(@.name=='${containerString}')]` or `.spec.template.spec.initContainers[?(@.name=='${containerString}')]`) **MUST** be bound
- values that do not match a container or init container **SHOULD** be ignored

A Service Binding Resource **MAY** define a `.spec.env` which is an array of `EnvMapping`. An `EnvMapping` object **MUST** define `name` and `key` entries. The `key` of an `EnvMapping` **MUST** refer to a binding `Secret` key name. The value of this `Secret` entry **MUST** be configured as an environment variable on the resource represented by `application`.
Expand Down Expand Up @@ -295,7 +294,7 @@ spec:
kind: # string
name: # string, mutually exclusive with selector
selector: # metav1.LabelSelector, mutually exclusive with name
containers: # []intstr.IntOrString, optional
containers: # []string, optional

service: # Provisioned Service resource ObjectReference-like
apiVersion: # string
Expand Down Expand Up @@ -554,7 +553,7 @@ If a `ClusterApplicationResourceMapping` defines `containers`, the reconciler **

If a `ClusterApplicationResourceMapping` defines `.envs` and `.volumeMounts`, the reconciler **MUST** first resolve a set of candidate locations in the application resource addressed by the `ServiceBinding` for all available containers and then filter that collection by the `ServiceBinding` `.spec.application.containers` filter before applying the appropriate modification.

If a `ServiceBinding` specifies a `.spec.applications.containers` value, and the value contains an `Int`-based index, that index **MUST** be used to filter the first entry in the `.containers` list and all other entries in those lists are ineligible for mapping. If a `ServiceBinding` specifies a `.spec.applications.containers` value, and the value contains an `string`-based index that index **MUST** be used to filter all entries in the `.containers` list. If a `ServiceBinding` specifies a `.spec.applications.containers` value and `ClusterApplicationResourceMapping` for the mapped type defines `.envs` and `.volumeMounts`, the reconciler **MUST** fail to reconcile.
If a `ServiceBinding` specifies `.spec.applications.containers` value, that value **MUST** be used to filter all entries in the `.containers` list. If a `ServiceBinding` specifies a `.spec.applications.containers` value and `ClusterApplicationResourceMapping` for the mapped type defines `.envs` and `.volumeMounts`, the reconciler **MUST** fail to reconcile.

A reconciler **MUST** apply the appropriate modification to the application resource addressed by the `ServiceBinding` as defined by `.volumes`.

Expand Down
3 changes: 1 addition & 2 deletions internal/service.binding/v1alpha2/service_binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1alpha2

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

// ServiceBindingApplicationReference defines a subset of corev1.ObjectReference with extensions
Expand All @@ -34,7 +33,7 @@ type ServiceBindingApplicationReference struct {
// Selector is a query that selects the application or applications to bind the service to
Selector metav1.LabelSelector `json:"selector,omitempty"`
// Containers describes which containers in a Pod should be bound to
Containers []intstr.IntOrString `json:"containers,omitempty"`
Containers []string `json:"containers,omitempty"`
}

// ServiceBindingServiceReference defines a subset of corev1.ObjectReference
Expand Down
5 changes: 1 addition & 4 deletions service.binding_servicebindings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ spec:
containers:
description: Containers describes which containers in a Pod should be bound to
items:
anyOf:
- type: integer
- type: string
x-kubernetes-int-or-string: true
type: string
type: array
kind:
description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
Expand Down