This creates a GitHubBinding.

[mattmoor]

/hold
I don't plan to land this for 0.14, but want to start socializing this for 0.15 and beyond.

For (much) broader context, see here.

This PR creates a binding to pair with GitHubSource in the spirit of the above document. This is also loosely based on prior work in github.com/mattmoor/bindings, and which we've been using through knobots for several months.

Generally the idea is that a user could configure:

apiVersion: bindings.knative.dev/v1alpha1
kind: GitHubBinding
metadata: ...
spec:
  subject:
    apiVersion: serving.knative.dev/v1
    kind: Service
    selector:
      matchLabels:
        github: "true"  # or something

  # This matches GitHubSource
  accessToken:
    secretKeyRef:
      name: my-secret
      key: accessToken

Then in my Service, I would simply need todo the following to get an authenticated client:

import "knative.dev/eventing-contrib/github"

client, err := github.New(ctx)
if err != nil { ... }

// Use the client.

• 🎁 Add new feature

Release Note

Introduce GitHubBinding

Docs

Needs docs, but see my note about targeting 0.15.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-contrib-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
github/pkg/apis/bindings/v1alpha1/githubbinding_defaults.go	Do not exist	100.0%
github/pkg/apis/bindings/v1alpha1/githubbinding_lifecycle.go	Do not exist	90.6%
github/pkg/apis/bindings/v1alpha1/githubbinding_validation.go	Do not exist	100.0%
github/pkg/apis/bindings/v1alpha1/register.go	Do not exist	100.0%

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging 9fb3d9d into 3e94a36 - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable

fixed alerts:

• 1 for Useless assignment to local variable

[lionelvillard]

Related work: https://github.com/redhat-developer/service-binding-operator

The question is whether we want to provide a binding per resource or do we use something more generic like the Service Binding Operator. I'm not sure it covers all our use cases though.

[mattmoor]

@lionelvillard I'm familiar with that work (IIRC it's referenced from the original Bindings docs), and I'd bugged @sspeiche about coming to our Kubecon NA2019 talk and syncing about this, but I think it ended up getting lost in the holidays. Still very happy to, and I suspect there's a larger group that may be interested in collaborating on this (see also)

Last I looked that approach was purely controller-based, which means it suffers several of the flaws called out in the Binding doc, off the top of my head:

1. It doesn't work with immutable resources like Jobs,
2. It doesn't work with resources owned by other controllers.

It'd be interesting to discuss a "one size fits all" binding strategy, but in general I don't think that's tenable. I think you end up in one of two situations:

1. You force the user to configure a fair amount more in spec (e.g. what values go in what env vars or volumes, what sidecars to inject), in the limit you are back at manually configuring PodSpec (or worse!).
2. You are prescriptive (in a global way) about how the information is projected into the container, which is what Cloud Foundry does and it severely limits what you can do.

This approach allows the information to be surfaced in a manner suitable to the context (and in our case consistent with the Source). It allows you to project the information into the container in idiomatic ways (see some of my examples here) which lets them enable unmodified tooling to run with strictly less configuration.

An easy example of enabling unmodified tooling would be binding Google Application Default credentials into a Job, so what the user could just write:

apiVersion: batch/v1
kind: Job
metadata:
  name: blah
spec:
  template:
    spec:
      restartPolicy: Never
      containers:
      - name: blah
         image: google/cloud-sdk
         args: ["some", "maintenance", "task"]

[sbose78]

Heya! I'm Shoubhik from the Service Binding Operator project. I'll go through the details and follow up :)

[sbose78]

Semi-related, I've been dabbling with Associate binding secret to a custom schema path in the workload
for non-pod-spec workloads.

[mattmoor]

@sbose78 What non-PodSpec workloads are you thinking about?

[lionelvillard]

it seems to me another limitation of a pure controller-based approach is the fact that it cannot deal with mandatory values, since the injection is done after the resource has been accepted by the admission controllers. @sbose78 is that right?

[sbose78]

What non-PodSpec workloads are you thinking about?

Greetings @mattmoor !
I don't have a confirmed 'customer' on that yet, but it had come up in a KafkaSource discussion where we wanted to 'inject' information into an object at an arbitrary path.

Here's an over-simplication:
redhat-developer/service-binding-operator#322 (comment)

And in general, the idea of binding is to have a managed secret whose contents are managed by the service binding controller based on the information sources. So, technically anything which references a secret should be able to consume it. Along that lines, the project added support for "generic binding" ( always poor at naming ), where the service binding controller doesn't inject the secret anywhere. All it does is creates a secret and manages it.

We are working on making the dust settle on how strimzi would consume it strimzi/strimzi-kafka-operator#2753

it cannot deal with mandatory values, since the injection is done after the resource has been accepted by the admission controllers.

Correct @lionelvillard .

[mattmoor]

And in general, the idea of binding is to have a managed secret whose contents are managed by the service binding controller based on the information sources

Right, the focus here is just on the last mile of getting the information into the workload. Provisioning services and managing the secrets that provide access to them is a separate problem. This space is being explored in a variety of ways on k8s (svc-cat, crossplane), but most prior art in the space of getting the credential data into the workload has fallen short (though the openshift service binding was certainly the closest).

With the tools to bind information into workloads folks can build what you are talking about. e.g. imagine if when provisioning a Kafka instance thru strimzi there were a way to provide labeled workloads with some level of access? It could create the managed credential and a binding like:

apiVersion: bindings.knative.dev/v1alpha1
kind: KafkaBinding
metadata:
  name: kafka-deployment-binding
spec:
  subject:
    apiVersion: apps/v1
    kind: Deployment
    selector:
      matchLabels: # from user

  # reference to managed creds

So I totally agree that to complete the experience, you need more than this, but this seems like a critical and absent building block.

[mattmoor]

/unhold

[n3wscott]

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor, n3wscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [n3wscott]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment