Support k8s service bindings #555

tylerphelan · 2020-11-20T21:36:06Z

Issue: #536

Add v1alpha2 for Builds and Images
Add conversion webhook for crds
Continue to support v1alpha1 bindings in controller
Small breaking change in v1alpha1 api code (lastSource)
Conversion strategy for v1alpha1 -> v1alpha2 is via annotation
Do not remove any v1alpha1 api code

Open questions:

Does this adequately support ProvisionedServices?
~~- Is there a race condition between ProvisionedServices and Builds reconciling? (if a ProvisionedService is not Ready yet will the Build fail?)~~
- This uses meta.UnsafeGuessKindToResource, is that alright?
- Would love to hear thoughts.
Went with v1alpha2 but we could do v1beta1
Should we update the docs now? Or wait for the release so that master branch does not have inaccurate docs?
I added validation in the generator.go to check that service binding secrets include the required type and data.type based on the spec. Would love thoughts on that. Is it necessary?

Notes:

In order to integrate a ProvisionedService with kpack users will need to create rbac that allows the kpack-controller to get the resource via the spec
- We should document this

- Add v1alpha2 for Builds and Images - Add conversion webhook for crds - Continue to support v1alpha1 bindings in controller - Small breaking change in v1alpha1 api code (lastSource) - Conversion strategy for v1alpha1 -> v1alpha2 is via annotation - Do not remove any v1alpha1 api code #536

#536

scothis · 2020-12-08T16:14:07Z

if a ProvisionedService is not Ready yet will the Build fail?

ProvisionedService has no notion of "Ready" either there is a Secret that can be read, or there isn't

scothis

Perhaps I missed it, but I didn't see where the "application projection" is occurring. The Service Binding for K8s Spec is not the CNB Binding Spec minus a ConfigMap. Check out https://github.com/k8s-service-bindings/spec#application-projection

RBAC can be defined generically, take a look at https://github.com/k8s-service-bindings/spec#for-service-binding-implementors

pkg/buildpod/generator.go

Co-authored-by: Scott Andrews <sandrews@pivotal.io>

- Secret .stringData.type must be set and reflect .type as service.binding/{type} - Add RBAC aggregationRule for kpack controller to access ProvisionedServices - Default APIVersion to v1 for Secret service bindings - validate services .kind is set #536

tylerphelan · 2020-12-08T21:08:37Z

Many thanks @scothis for looking at this! Made some changes from the feedback.

Perhaps I missed it, but I didn't see where the "application projection" is occurring

I was missing the type and data.type requirements previously but the apis/build/v1alpha2/build_pod.go should implement the mounting as I understand it.

config/controllerrole.yaml

scothis · 2020-12-09T15:40:27Z

pkg/apis/build/v1alpha2/build_pod.go

+		// If this is a v1alpha1 binding
+		if s.V1Alpha1MetadataRef != nil {
+			metadataVolume := fmt.Sprintf("service-binding-metadata-%s", s.Name)
+			volumes = append(volumes,
+				corev1.Volume{
+					Name: metadataVolume,
+					VolumeSource: corev1.VolumeSource{
+						ConfigMap: &corev1.ConfigMapVolumeSource{
+							LocalObjectReference: *s.V1Alpha1MetadataRef,
+						},
+					},
+				},
+			)
+			volumeMounts = append(volumeMounts,
+				corev1.VolumeMount{
+					Name:      metadataVolume,
+					MountPath: filepath.Join(platformVolume.MountPath, "bindings", s.Name, "metadata"),
+					ReadOnly:  true,
+				},
+			)
+		}
+	}


There is no "metadata" directory in the k8s service binding spec, and the secret is in the wrong place for the cnb service binding spec.

rm unless this is doing something meaningful that I don't understand.

yea I did this wrong, fixing

this should be fixed now

pkg/apis/build/v1alpha2/build_pod.go

scothis · 2020-12-09T15:44:08Z

pkg/apis/build/v1alpha2/build_validation.go

+
+	for i, s := range b.Spec.Services {
+		if s.Kind == "Secret" && s.APIVersion == "" {
+			b.Spec.Services[i].APIVersion = "v1"


is there precedent for defaulting the APIVersion in an object reference?

This was just me speculating that this might help usability

scothis · 2020-12-09T15:46:47Z

pkg/apis/build/v1alpha2/build_pod.go

+	}
+	serviceBindingRootEnv = corev1.EnvVar{
+		Name:  serviceBindingRootEnvVar,
+		Value: filepath.Join(platformVolume.MountPath, "bindings"),


K8s service bindings and cnb service bindings should not share the same directory.

As I understand it, buildpacks will only read from one location for bindings based on this and a quick convo with @nebhale

Given that, I made build pods only project CNB bindings OR K8s bindings in which case why not use the same directory?

Curious to peoples' thoughts

@tylerphelan that makes sense to me. I always assumed it would be either/or and not both.

- aggregationRule will overwrite existing rules #536

- refactor

matthewmcnew · 2021-01-05T19:56:31Z

@tylerphelan If we are holding off on this for a bit could we move it draft?

c0d1ngm0nk3y · 2021-04-22T15:02:21Z

pkg/buildpod/generator.go


 	"github.com/pivotal/kpack/pkg/apis/build/v1alpha1"
+	"github.com/pivotal/kpack/pkg/apis/build/v1alpha2"


Is it really the best approach to mix build/v1alpha1 and build/v1alpha2? It can get very tricky which struct to take from which import and the errors can be very subtle.

Another approach would bump the api as a whole and only use build/v1alpha2. Maybe even use a import like

buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2"

So the code didn't have to be changed all over if a version is bumped.

matthewmcnew · 2021-05-11T21:14:05Z

Tagging this as 0.4.0 to target releasing alongside the api changes in our 0.4.0 release.

tomkennedy513 · 2021-09-28T21:12:01Z

closed in favor of #823

tylerphelan requested review from matthewmcnew, djoyahoy, tomkennedy513, elenorebastian, sukhil-suresh and aemengo November 20, 2020 21:36

Tyler Phelan added 3 commits December 2, 2020 15:18

Support ProvisionedService duck types for service bindings

2bbd6af

#536

Update openapi codegen

9112ee1

#536

goimports

5ae39e4

tylerphelan requested review from scothis and removed request for elenorebastian December 2, 2020 20:29

scothis reviewed Dec 8, 2020

View reviewed changes

pkg/buildpod/generator.go Outdated Show resolved Hide resolved

Tyler Phelan and others added 2 commits December 8, 2020 14:20

check secret apiVersion

3d40e96

Co-authored-by: Scott Andrews <sandrews@pivotal.io>

tylerphelan mentioned this pull request Dec 8, 2020

document kpack service bindings #570

Closed

scothis reviewed Dec 9, 2020

View reviewed changes

tylerphelan linked an issue Dec 9, 2020 that may be closed by this pull request

Support k8s Service Bindings #536

Closed

Tyler Phelan added 3 commits December 9, 2020 12:07

Remove leftover unused functions

fa1e562

Use new role + binding for controller service binding RBAC

6780e96

- aggregationRule will overwrite existing rules #536

Fix v1alpha1 cnb app projection

0e0cebc

- refactor

tylerphelan mentioned this pull request Dec 10, 2020

Migrating to a new build api version #574

Closed

tylerphelan marked this pull request as draft January 5, 2021 19:57

matthewmcnew mentioned this pull request Apr 20, 2021

Image registry cache #669

Merged

c0d1ngm0nk3y reviewed Apr 22, 2021

View reviewed changes

c0d1ngm0nk3y mentioned this pull request Apr 28, 2021

Bump build api from v1alpha1 v1alpha2 #676

Merged

matthewmcnew added the 0.4.0 label May 11, 2021

c0d1ngm0nk3y mentioned this pull request May 12, 2021

Introducing stable alias for buildapi to ease version bumping #675

Merged

tomkennedy513 removed a link to an issue Sep 13, 2021

Support k8s Service Bindings #536

Closed

matthewmcnew mentioned this pull request Sep 14, 2021

add support for kubernetes service bindings #823

Merged

tomkennedy513 closed this Sep 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support k8s service bindings #555

Support k8s service bindings #555

tylerphelan commented Nov 20, 2020 •

edited

Loading

scothis commented Dec 8, 2020

scothis left a comment

tylerphelan commented Dec 8, 2020

scothis Dec 9, 2020

tylerphelan Dec 9, 2020

tylerphelan Dec 10, 2020

scothis Dec 9, 2020

tylerphelan Dec 9, 2020

scothis Dec 9, 2020

tylerphelan Dec 9, 2020

djoyahoy Dec 14, 2020

matthewmcnew commented Jan 5, 2021

c0d1ngm0nk3y Apr 22, 2021

matthewmcnew commented May 11, 2021

tomkennedy513 commented Sep 28, 2021


		"github.com/pivotal/kpack/pkg/apis/build/v1alpha1"
		"github.com/pivotal/kpack/pkg/apis/build/v1alpha2"

Support k8s service bindings #555

Support k8s service bindings #555

Conversation

tylerphelan commented Nov 20, 2020 • edited Loading

scothis commented Dec 8, 2020

scothis left a comment

Choose a reason for hiding this comment

tylerphelan commented Dec 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewmcnew commented Jan 5, 2021

Choose a reason for hiding this comment

matthewmcnew commented May 11, 2021

tomkennedy513 commented Sep 28, 2021

tylerphelan commented Nov 20, 2020 •

edited

Loading