Differentiate containers and init containers in the application resource mapping

[baijum]

Service Binding section treats containers and init containers differently.

For example, consider this normative text:

• 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

The Application Resource Mapping doesn't differentiate between containers and init containers. As a result, it is hard to follow the different rules for both containers and init containers.

I would propose to add an initContainers field to avoid ambiguity.

[baijum]

I came across this issue while trying to implement this feature for KubePreset. For the convenience of testing, I have created a custom resource: https://github.com/kubepreset/custompod/blob/main/api/v1beta1/custompod_types.go (I am calling it as Non-PodSpec-able Custom Pod)

[scothis]

This didn't make it into the spec (not sure why), but at the time my suggestion was to say that only the first path in the containers list should support index based binding; all entries support binding by container name. This issue could also be side stepped if we revisit #146

[baijum]

This issue could also be side stepped if we revisit #146

If we could remove the container matching by index, differentiating containers and init containers are not required. This way, we could retain the text of application resource mapping.

[baijum]

From today's meeting:

• Leading suggestion is to remove by-index and require by-name container identification
• Need to research uniqueness of name across both init-containers and ephemeral containers (cc. @scothis )

[baijum]

• Need to research uniqueness of name across both init-containers and ephemeral containers

The ephemeral containers is an alpha feature and not in the scope of of the spec. The spec deals with nomal containers and init containers. However, the name of all containers (normal, init, and ephemeral) is unique. Few references:

• https://github.com/kubernetes/kubernetes/blob/7a9c8940fddba4b415e2b0db338296df2d0011ea/staging/src/k8s.io/api/core/v1/types.go#L3429
• https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#detailed-behavior (Last paragraph under Detailed behavior section)
• https://github.com/kubernetes/kubernetes/blob/7a9c8940fddba4b415e2b0db338296df2d0011ea/staging/src/k8s.io/api/core/v1/types.go#L2956

[baijum]

This didn't make it into the spec (not sure why), but at the time my suggestion was to say that only the first path in the containers list should support index based binding;

I just noticed that its there in the spec (It was in the Reconciler Implementation section, and I missed it):

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.

We discussed three options (adding separate init container field, removing match by-index, restricting first entry for match by-index) and decided to go for removing match by-index, I hope we can still proceed with that decision. I think that would be better choice considering the spec already recommend to avoid by-index due to its fragile nature.

[baijum]

Based on the discussion, I have created a pull request: #171