From a80fbd60e7babb5391313d791cd42fc33eb66544 Mon Sep 17 00:00:00 2001 From: Baiju Muthukadan Date: Fri, 25 Jun 2021 08:15:01 +0530 Subject: [PATCH 1/2] Remove integer based indexing to limit containers Signed-off-by: Baiju Muthukadan Closes #166 --- README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e89a8b6..3876412 100644 --- a/README.md +++ b/README.md @@ -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`. @@ -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`. From 97dcc712c36a70e7cca848d380e9e0d496e55ccd Mon Sep 17 00:00:00 2001 From: Baiju Muthukadan Date: Fri, 25 Jun 2021 19:54:17 +0530 Subject: [PATCH 2/2] update schema Signed-off-by: Baiju Muthukadan --- README.md | 2 +- internal/service.binding/v1alpha2/service_binding.go | 3 +-- service.binding_servicebindings.yaml | 5 +---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3876412..0c26b2f 100644 --- a/README.md +++ b/README.md @@ -294,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 diff --git a/internal/service.binding/v1alpha2/service_binding.go b/internal/service.binding/v1alpha2/service_binding.go index 9866abe..87464c3 100644 --- a/internal/service.binding/v1alpha2/service_binding.go +++ b/internal/service.binding/v1alpha2/service_binding.go @@ -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 @@ -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 diff --git a/service.binding_servicebindings.yaml b/service.binding_servicebindings.yaml index 2920886..0715ba4 100644 --- a/service.binding_servicebindings.yaml +++ b/service.binding_servicebindings.yaml @@ -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'