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.

Sign up for GitHub

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

Add CNB Service Bindings to Image and Build resources #371

Merged
merged 8 commits into from
May 11, 2020
Merged

Add CNB Service Bindings to Image and Build resources #371

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c01f7fc
Add CNB Service Bindings to Image and Build resources
scothis Apr 30, 2020
4ed73ac
Mount bindings under platform-dir
scothis May 5, 2020
b01248a
Add sample Image resource with bindings
scothis May 6, 2020
de67384
Merge branch 'master' into cnb-bindings
scothis May 6, 2020
990504f
Disable secret binding
scothis May 7, 2020
1b1064f
Revert "Disable secret binding"
scothis May 7, 2020
d196f92
Reject a build if a binding references a secret attached to a service…
scothis May 7, 2020
2e79287
Move binding volume setup into a helper
scothis May 11, 2020
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
4 changes: 4 additions & 0 deletions pkg/apis/build/v1alpha1/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ func (b *Build) BuilderSpec() BuildBuilderSpec {
return b.Spec.Builder
}

func (b *Build) Bindings() []Binding {
return b.Spec.Bindings
}

func (b *Build) IsRunning() bool {
if b == nil {
return false
Expand Down
57 changes: 51 additions & 6 deletions pkg/apis/build/v1alpha1/build_pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,51 @@ func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc Buil
SubPath: b.Spec.Source.SubPath, // empty string is a nop
}

bindingVolumes := []corev1.Volume{}
Copy link

Choose a reason for hiding this comment

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

can this be moved into a private method setupBindings()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bindingVolumeMounts := []corev1.VolumeMount{}
for _, binding := range b.Spec.Bindings {
metadataVolume := fmt.Sprintf("binding-metadata-%s", binding.Name)
bindingVolumes = append(bindingVolumes,
corev1.Volume{
Name: metadataVolume,
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: binding.MetadataRef.Name,
},
},
},
},
)
bindingVolumeMounts = append(bindingVolumeMounts,
corev1.VolumeMount{
Name: metadataVolume,
MountPath: fmt.Sprintf("%s/bindings/%s/metadata", platformVolume.MountPath, binding.Name),
ReadOnly: true,
},
)
if binding.SecretRef != nil {
secretVolume := fmt.Sprintf("binding-secret-%s", binding.Name)
bindingVolumes = append(bindingVolumes,
corev1.Volume{
Name: secretVolume,
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: binding.SecretRef.Name,
},
},
},
)
bindingVolumeMounts = append(bindingVolumeMounts,
corev1.VolumeMount{
Name: secretVolume,
MountPath: fmt.Sprintf("%s/bindings/%s/secret", platformVolume.MountPath, binding.Name),
ReadOnly: true,
},
)
}
}

return &corev1.Pod{
ObjectMeta: v1.ObjectMeta{
Name: b.PodName(),
Expand Down Expand Up @@ -188,11 +233,11 @@ func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc Buil
"-group=/layers/group.toml",
"-plan=/layers/plan.toml",
},
VolumeMounts: []corev1.VolumeMount{
VolumeMounts: append([]corev1.VolumeMount{
layersVolume,
platformVolume,
workspaceVolume,
},
}, bindingVolumeMounts...),
ImagePullPolicy: corev1.PullIfNotPresent,
},
)
Expand Down Expand Up @@ -253,11 +298,11 @@ func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc Buil
"-group=/layers/group.toml",
"-plan=/layers/plan.toml",
},
VolumeMounts: []corev1.VolumeMount{
VolumeMounts: append([]corev1.VolumeMount{
layersVolume,
platformVolume,
workspaceVolume,
},
}, bindingVolumeMounts...),
ImagePullPolicy: corev1.PullIfNotPresent,
},
)
Expand Down Expand Up @@ -315,7 +360,7 @@ func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc Buil
}
}),
ServiceAccountName: b.Spec.ServiceAccount,
Volumes: append(
Volumes: append(append(
secretVolumes,
corev1.Volume{
Name: cacheDirName,
Expand Down Expand Up @@ -347,7 +392,7 @@ func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc Buil
},
b.Spec.Source.Source().ImagePullSecretsVolume(),
builderSecretVolume(b.Spec.Builder),
),
), bindingVolumes...),
ImagePullSecrets: b.Spec.Builder.ImagePullSecrets,
},
}, nil
Expand Down
93 changes: 91 additions & 2 deletions pkg/apis/build/v1alpha1/build_pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,23 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
},
},
CacheName: "some-cache-name",
Bindings: []v1alpha1.Binding{
{
Name: "database",
MetadataRef: &corev1.LocalObjectReference{
Name: "database-configmap",
},
},
{
Name: "apm",
MetadataRef: &corev1.LocalObjectReference{
Name: "apm-configmap",
},
SecretRef: &corev1.LocalObjectReference{
Name: "apm-secret",
},
},
},
Env: []corev1.EnvVar{
{Name: "keyA", Value: "valueA"},
{Name: "keyB", Value: "valueB"},
Expand Down Expand Up @@ -222,6 +239,64 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
}
})

it("configures the bindings", func() {
pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig)
require.NoError(t, err)

assert.Contains(t,
pod.Spec.Volumes,
corev1.Volume{
Name: "binding-metadata-database",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "database-configmap",
},
},
},
},
corev1.Volume{
Name: "binding-metadata-apm",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "apm-configmap",
},
},
},
},
corev1.Volume{
Name: "binding-secret-apm",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: "apm-secret",
},
},
},
)

for _, containerIdx := range []int{1 /* detect */, 4 /* build */} {
assert.Contains(t,
pod.Spec.InitContainers[containerIdx].VolumeMounts,
corev1.VolumeMount{
Name: "binding-metadata-database",
MountPath: "/platform/bindings/database/metadata",
ReadOnly: true,
},
corev1.VolumeMount{
Name: "binding-metadata-apm",
MountPath: "/platform/bindings/apm/metadata",
ReadOnly: true,
},
corev1.VolumeMount{
Name: "binding-secret-apm",
MountPath: "/platform/bindings/apm/secret",
ReadOnly: true,
},
)
}
})

it("configures prepare with docker and git credentials", func() {
pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig)
require.NoError(t, err)
Expand Down Expand Up @@ -420,6 +495,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
"layers-dir",
"platform-dir",
"workspace-dir",
"binding-metadata-database",
"binding-metadata-apm",
"binding-secret-apm",
}, names(pod.Spec.InitContainers[1].VolumeMounts))
})

Expand Down Expand Up @@ -504,6 +582,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
"layers-dir",
"platform-dir",
"workspace-dir",
"binding-metadata-database",
"binding-metadata-apm",
"binding-secret-apm",
}))
})

Expand Down Expand Up @@ -554,7 +635,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
pod, err := build.BuildPod(config, nil, buildPodBuilderConfig)
require.NoError(t, err)

require.Len(t, pod.Spec.Volumes, 7)
require.Len(t, pod.Spec.Volumes, 10)
assert.Equal(t, corev1.Volume{
Name: "cache-dir",
VolumeSource: corev1.VolumeSource{
Expand All @@ -568,7 +649,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
pod, err := build.BuildPod(config, nil, buildPodBuilderConfig)
require.NoError(t, err)

require.Len(t, pod.Spec.Volumes, 7)
require.Len(t, pod.Spec.Volumes, 10)
assert.Equal(t, corev1.Volume{
Name: "cache-dir",
VolumeSource: corev1.VolumeSource{
Expand Down Expand Up @@ -596,6 +677,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
require.Len(t, pod.Spec.ImagePullSecrets, 1)
assert.Equal(t, corev1.LocalObjectReference{Name: "some-image-secret"}, pod.Spec.ImagePullSecrets[0])
})

it("mounts volumes for bindings", func() {
pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig)
require.NoError(t, err)

require.Len(t, pod.Spec.ImagePullSecrets, 1)
assert.Equal(t, corev1.LocalObjectReference{Name: "some-image-secret"}, pod.Spec.ImagePullSecrets[0])
})
})

when("0.3 platform api", func() {
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/build/v1alpha1/build_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,23 @@ type BuildSpec struct {
Source SourceConfig `json:"source"`
CacheName string `json:"cacheName,omitempty"`
// +listType
Bindings Bindings `json:"bindings,omitempty"`
// +listType
Env []corev1.EnvVar `json:"env,omitempty"`
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
LastBuild *LastBuild `json:"lastBuild,omitempty"`
}

// +k8s:openapi-gen=true
type Bindings []Binding

// +k8s:openapi-gen=true
type Binding struct {
Name string `json:"name",omitempty"`
MetadataRef *corev1.LocalObjectReference `json:"metadataRef,omitempty"`
SecretRef *corev1.LocalObjectReference `json:"secretRed,omitempty"`
}

// +k8s:openapi-gen=true
type LastBuild struct {
Image string `json:"image,omitempty"`
Expand Down
49 changes: 49 additions & 0 deletions pkg/apis/build/v1alpha1/build_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package v1alpha1

import (
"context"
"fmt"
"regexp"

"knative.dev/pkg/apis"
"knative.dev/pkg/kmp"
Expand All @@ -24,6 +26,7 @@ func (bs *BuildSpec) Validate(ctx context.Context) *apis.FieldError {
Also(validate.Tags(bs.Tags)).
Also(bs.Builder.Validate(ctx).ViaField("builder")).
Also(bs.Source.Validate(ctx).ViaField("source")).
Also(bs.Bindings.Validate(ctx).ViaField("bindings")).
Also(bs.LastBuild.Validate(ctx).ViaField("lastBuild")).
Also(bs.validateImmutableFields(ctx))
}
Expand Down Expand Up @@ -54,6 +57,52 @@ func (bbs *BuildBuilderSpec) Validate(ctx context.Context) *apis.FieldError {
return validate.Image(bbs.Image)
}

func (bs Bindings) Validate(ctx context.Context) *apis.FieldError {
var errs *apis.FieldError
names := map[string]int{}
for i, b := range bs {
// check name uniqueness
if n, ok := names[b.Name]; ok {
errs = errs.Also(
apis.ErrGeneric(
fmt.Sprintf("duplicate binding name %q", b.Name),
fmt.Sprintf("[%d].name", n),
fmt.Sprintf("[%d].name", i),
),
)
}
names[b.Name] = i
errs = errs.Also(b.Validate(ctx).ViaIndex(i))
}
return errs
}

var bindingNameRE = regexp.MustCompile(`^[a-z0-9\-\.]{1,253}$`)

func (b *Binding) Validate(context context.Context) *apis.FieldError {
var errs *apis.FieldError

if b.Name == "" {
errs = errs.Also(apis.ErrMissingField("name"))
} else if !bindingNameRE.MatchString(b.Name) {
errs = errs.Also(apis.ErrInvalidValue(b.Name, "name"))
}

if b.MetadataRef == nil {
// metadataRef is required
errs = errs.Also(apis.ErrMissingField("metadataRef"))
} else if b.MetadataRef.Name == "" {
errs = errs.Also(apis.ErrMissingField("metadataRef.name"))
}

if b.SecretRef != nil && b.SecretRef.Name == "" {
// secretRef is optional
errs = errs.Also(apis.ErrMissingField("secretRef.name"))
}

return errs
}

func (lb *LastBuild) Validate(context context.Context) *apis.FieldError {
if lb == nil || lb.Image == "" {
return nil
Expand Down
Loading