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 mappings from the ServiceBinding resource #154

Merged
merged 3 commits into from
Jun 14, 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
58 changes: 7 additions & 51 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ Behavior within the project is governed by the [Contributor Covenant Code of Con
- [Resource Type Schema](#resource-type-schema-1)
- [Minimal Example Resource](#minimal-example-resource)
- [Label Selector Example Resource](#label-selector-example-resource)
- [Mappings Example Resource](#mappings-example-resource)
- [Environment Variables Example Resource](#environment-variables-example-resource)
- [Reconciler Implementation](#reconciler-implementation)
- [Ready Condition Status](#ready-condition-status)
Expand Down Expand Up @@ -115,6 +114,8 @@ An implementation is not compliant if it fails to satisfy one or more of the MUS

A Provisioned Service resource **MUST** define a `.status.binding` which is a `LocalObjectReference`-able (containing a single field `name`) to a `Secret`. The `Secret` **MUST** be in the same namespace as the resource. The `Secret` data **SHOULD** contain a `type` entry with a value that identifies the abstract classification of the binding. The `Secret` type (`.type` verses `.data.type`) **SHOULD** reflect this value as `service.binding/{type}`, replacing `{type}` with the `Secret` data type. It is **RECOMMENDED** that the `Secret` data also contain a `provider` entry with a value that identifies the provider of the binding. The `Secret` data **MAY** contain any other entry. To facilitate discoverability, it is **RECOMMENDED** that a `CustomResourceDefinition` exposing a Provisioned Service add `service.binding/provisioned-service: "true"` as a label.

> Note: While the Provisioned Service referenced `Secret` data should contain a `type` entry, the `type` must be defined before it is projected into an application workload. This allows a mapping to enrich an existing secret.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a mapping to enrich

@scothis
Are you referring to the "mappings" as described in #145?
Alternatively, the .spec.type (optional) value available in the ServiceBinding resource can be used in the binding Secret, right? Should we add that info into the note here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.spec.type is removed by this PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that now :-)

The removal of .spec.type and .spec.provider should be called out in the PR description because I don't see mentioning it in #145.

BTW, I am fine with removing .spec.type and .spec.provider. That way, the reconciler can mount the Secret resource into the application workload, and no need to create an intermediate Secret resource.

I hope soon the extensions mentioned in #145 will become a reality!

Copy link
Contributor Author

@scothis scothis May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of .spec.type and .spec.provider should be called out in the PR description

updated the PR subject and overview

I hope soon the extensions mentioned in #145 will become a reality!

nothing to wait for. Start creating and using them today; they're just not part of the spec, yet(?)

Copy link
Member

@arthurdm arthurdm Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed some key words (should / must) weren't in the proper format, and I think the wording was a bit confusing in terms of what the spec is saying must be present (which is the type in the projection, not the secret)

Suggested change
> Note: While the Provisioned Service referenced `Secret` data should contain a `type` entry, the `type` must be defined before it is projected into an application workload. This allows a mapping to enrich an existing secret.
> Note: While the Provisioned Service referenced `Secret` data **SHOULD** contain a `type` entry, the `type` **MUST** be present in the application projection.

Copy link
Contributor Author

@scothis scothis Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a non-normative block, so it intentionally uses unstylized "should" and "must". The spec requirements are captured in the Application Projection section.

The driver for this note is #155


Extensions and implementations **MAY** define additional mechanisms to consume a Provisioned Service that does not conform to the duck type.

## Resource Type Schema
Expand Down Expand Up @@ -196,11 +197,11 @@ rules:

# Application Projection

A Binding `Secret` **MUST** be volume mounted into a container at `$SERVICE_BINDING_ROOT/<binding-name>` with directory names matching the name of the binding. Binding names **MUST** match `[a-z0-9\-\.]{1,253}`. The `$SERVICE_BINDING_ROOT` environment variable **MUST** be declared and can point to any valid file system location.
A projected binding **MUST** be volume mounted into a container at `$SERVICE_BINDING_ROOT/<binding-name>` with directory names matching the name of the binding. Binding names **MUST** match `[a-z0-9\-\.]{1,253}`. The `$SERVICE_BINDING_ROOT` environment variable **MUST** be declared and can point to any valid file system location.

The `Secret` data **MUST** contain a `type` entry with a value that identifies the abstract classification of the binding. The `Secret` type (`.type` verses `.data.type`) **MUST** reflect this value as `service.binding/{type}`, replacing `{type}` with the `Secret` data type. It is **RECOMMENDED** that the `Secret` data also contain a `provider` entry with a value that identifies the provider of the binding. The `Secret` data **MAY** contain any other entry.
The projected binding **MUST** contain a `type` entry with a value that identifies the abstract classification of the binding. It is **RECOMMENDED** that the projected binding also contain a `provider` entry with a value that identifies the provider of the binding. The projected binding data **MAY** contain any other entry.

The name of a secret entry file name **SHOULD** match `[a-z0-9\-\.]{1,253}`. The contents of a secret entry may be anything representable as bytes on the file system including, but not limited to, a literal string value (e.g. `db-password`), a language-specific binary (e.g. a Java `KeyStore` with a private key and X.509 certificate), or an indirect pointer to another system for value resolution (e.g. `vault://production-database/password`).
The name of a binding entry file name **SHOULD** match `[a-z0-9\-\.]{1,253}`. The contents of a binding entry may be anything representable as bytes on the file system including, but not limited to, a literal string value (e.g. `db-password`), a language-specific binary (e.g. a Java `KeyStore` with a private key and X.509 certificate), or an indirect pointer to another system for value resolution (e.g. `vault://production-database/password`).

The collection of files within the directory **MAY** change between container launches. The collection of files within the directory **SHOULD NOT** change during the lifetime of the container.

Expand Down Expand Up @@ -263,9 +264,7 @@ The Service Binding resource **MAY** define `.spec.application.containers`, as a
- 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
- values that do not match a container or init container **SHOULD** be ignored

A Service Binding Resource **MAY** define a `.spec.mappings` which is an array of `Mapping` objects. A `Mapping` object **MUST** define `name` and `value` entries. The `value` of a `Mapping` **MUST** be handled as a [Go Template][gt] exposing binding `Secret` keys for substitution. The executed output of the template **MUST** be added to the `Secret` exposed to the resource represented by `application` as the key specified by the `name` of the `Mapping`. If the `name` of a `Mapping` matches that of a Provisioned Service `Secret` key, the value from `Mapping` **MUST** be used for binding.

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 including any key defined by a `Mapping`. The value of this `Secret` entry **MUST** be configured as an environment variable on the resource represented by `application`.
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`.

A Service Binding resource **MUST** define `.status.conditions` which is an array of `Condition` objects as defined in [meta/v1 Condition][mv1c]. At least one condition containing a `type` of `Ready` **MUST** be defined. The `Ready` condition **SHOULD** contain appropriate values defined by the implementation. As label selectors are inherently queries that return zero-to-many resources, it is **RECOMMENDED** that `ServiceBinding` authors use a combination of labels that yield a single resource, but implementors **MUST** handle each matching resource as if it was specified by name in a distinct `ServiceBinding` resource. Partial failures **MUST** be aggregated and reported on the binding status's `Ready` condition. A Service Binding resource **SHOULD** reflect the secret projected into the application as `.status.binding.name`.

Expand Down Expand Up @@ -300,10 +299,6 @@ spec:
kind: # string
name: # string

mappings: # []Mapping, optional
- name: # string
value: # string

env: # []EnvMapping, optional
- name: # string
key: # string
Expand Down Expand Up @@ -374,39 +369,6 @@ status:
lastTransitionTime: '2021-01-20T17:00:00Z'
```

## Mappings Example Resource

```yaml
apiVersion: service.binding/v1alpha2
kind: ServiceBinding
metadata:
name: account-service
spec:
application:
apiVersion: apps/v1
kind: Deployment
name: online-banking

service:
apiVersion: com.example/v1alpha1
kind: AccountService
name: prod-account-service

mappings:
- name: accountServiceUri
value: https://{{ urlquery .username }}:{{ urlquery .password }}@{{ .host }}:{{ .port }}/{{ .path }}

status:
binding:
name: prod-account-service-projection
conditions:
- type: Ready
status: 'True'
reason: 'Projected'
message: ''
lastTransitionTime: '2021-01-20T17:00:00Z'
```

## Environment Variables Example Resource

```yaml
Expand All @@ -425,19 +387,13 @@ spec:
kind: AccountService
name: prod-account-service

mappings:
- name: accountServiceUri
value: https://{{ urlquery .username }}:{{ urlquery .password }}@{{ .host }}:{{ .port }}/{{ .path }}

env:
- name: ACCOUNT_SERVICE_HOST
key: host
- name: ACCOUNT_SERVICE_USERNAME
key: username
- name: ACCOUNT_SERVICE_PASSWORD
key: password
- name: ACCOUNT_SERVICE_URI
key: accountServiceUri

status:
binding:
Expand All @@ -460,7 +416,7 @@ If the `$SERVICE_BINDING_ROOT` environment variable has already been configured

The `$SERVICE_BINDING_ROOT` environment variable **MUST NOT** be reset if it is already configured on the resource represented by `application`.

If a `.spec.type` is set, the `type` entry in the binding `Secret` **MUST** be set to its value overriding any existing value. If a `.spec.provider` is set, the `provider` entry in the binding `Secret` **MUST** be set to its value overriding any existing value.
If a `.spec.type` is set, the `type` entry in the application projection **MUST** be set to its value overriding any existing value. If a `.spec.provider` is set, the `provider` entry in the application projection **MUST** be set to its value overriding any existing value.

### Ready Condition Status

Expand Down
11 changes: 0 additions & 11 deletions internal/service.binding/v1alpha2/service_binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,6 @@ type EnvMapping struct {
Key string `json:"key"`
}

// ServiceBindingMapping defines a mapping from the existing collection of Secret values to a new Secret entry.
type ServiceBindingMapping struct {
// Name is the name of the mapped Secret entry
Name string `json:"name"`
// Value is the value of the new Secret entry. Contents may be a Go template and refer to the other secret entries
// by name.
Value string `json:"value"`
}

// ServiceBindingSpec defines the desired state of ServiceBinding
type ServiceBindingSpec struct {
// Name is the name of the service as projected into the application container. Defaults to .metadata.name.
Expand All @@ -87,8 +78,6 @@ type ServiceBindingSpec struct {
Service ServiceBindingServiceReference `json:"service"`
// Env is the collection of mappings from Secret entries to environment variables
Env []EnvMapping `json:"env,omitempty"`
// Mappings is the collection of mappings from existing Secret entries to new Secret entries
Mappings []ServiceBindingMapping `json:"mappings,omitempty"`
}

// These are valid conditions of ServiceBinding.
Expand Down
16 changes: 0 additions & 16 deletions service.binding_servicebindings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,22 +112,6 @@ spec:
- name
type: object
type: array
mappings:
description: Mappings is the collection of mappings from existing Secret entries to new Secret entries
items:
description: ServiceBindingMapping defines a mapping from the existing collection of Secret values to a new Secret entry.
properties:
name:
description: Name is the name of the mapped Secret entry
type: string
value:
description: Value is the value of the new Secret entry. Contents may be a Go template and refer to the other secret entries by name.
type: string
required:
- name
- value
type: object
type: array
name:
description: Name is the name of the service as projected into the application container. Defaults to .metadata.name.
type: string
Expand Down