diff --git a/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_lifecycle.go b/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_lifecycle.go index dae82a44..3ee2074c 100644 --- a/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_lifecycle.go +++ b/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_lifecycle.go @@ -9,6 +9,7 @@ import ( "context" "crypto/sha1" "fmt" + "sort" "strings" corev1 "k8s.io/api/core/v1" @@ -25,6 +26,7 @@ const ( ServiceBindingProjectionConditionApplicationAvailable = "ApplicationAvailable" ServiceBindingRootEnv = "SERVICE_BINDING_ROOT" + bindingVolumePrefix = "binding-" ) var sbpCondSet = apis.NewLivingConditionSet( @@ -68,45 +70,48 @@ func (b *ServiceBindingProjection) Do(ctx context.Context, ps *duckv1.WithPod) { return } - existingVolumes := sets.NewString() - for _, v := range ps.Spec.Template.Spec.Volumes { - existingVolumes.Insert(v.Name) - } + injectedSecrets, injectedVolumes := b.injectedValues(ps) - newVolumes := sets.NewString() sb := b.Spec.Binding - bindingVolume := truncateAt63("binding-%x", sha1.Sum([]byte(sb.Name))) - if !existingVolumes.Has(bindingVolume) { - ps.Spec.Template.Spec.Volumes = append(ps.Spec.Template.Spec.Volumes, corev1.Volume{ - Name: bindingVolume, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: sb.Name, - }, + volume := corev1.Volume{ + Name: truncateAt63("%s%x", bindingVolumePrefix, sha1.Sum([]byte(sb.Name))), + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: sb.Name, }, - }) - existingVolumes.Insert(bindingVolume) - newVolumes.Insert(bindingVolume) + }, } + ps.Spec.Template.Spec.Volumes = append(ps.Spec.Template.Spec.Volumes, volume) + injectedSecrets.Insert(volume.Secret.SecretName) + injectedVolumes.Insert(volume.Name) + sort.SliceStable(ps.Spec.Template.Spec.Volumes, func(i, j int) bool { + iname := ps.Spec.Template.Spec.Volumes[i].Name + jname := ps.Spec.Template.Spec.Volumes[j].Name + // only sort injected volumes + if !injectedVolumes.HasAll(iname, jname) { + return false + } + return iname < jname + }) + // track which secret is injected, so it can be removed when no longer used + ps.Annotations[b.annotationKey()] = volume.Secret.SecretName + for i := range ps.Spec.Template.Spec.InitContainers { c := &ps.Spec.Template.Spec.InitContainers[i] if b.isTargetContainer(-1, c) { - b.doContainer(ctx, ps, c, bindingVolume, sb.Name) + b.doContainer(ctx, ps, c, volume.Name, sb.Name, injectedVolumes, injectedSecrets) } } for i := range ps.Spec.Template.Spec.Containers { c := &ps.Spec.Template.Spec.Containers[i] if b.isTargetContainer(i, c) { - b.doContainer(ctx, ps, c, bindingVolume, sb.Name) + b.doContainer(ctx, ps, c, volume.Name, sb.Name, injectedVolumes, injectedSecrets) } } - - // track which volumes are injected, so they can be removed when no longer used - ps.Annotations[b.annotationKey()] = strings.Join(newVolumes.List(), ",") } -func (b *ServiceBindingProjection) doContainer(ctx context.Context, ps *duckv1.WithPod, c *corev1.Container, bindingVolume, secretName string) { +func (b *ServiceBindingProjection) doContainer(ctx context.Context, ps *duckv1.WithPod, c *corev1.Container, bindingVolume, secretName string, allInjectedVolumes, allInjectedSecrets sets.String) { mountPath := "" // lookup predefined mount path for _, e := range c.Env { @@ -124,31 +129,46 @@ func (b *ServiceBindingProjection) doContainer(ctx context.Context, ps *duckv1.W }) } - containerVolumes := sets.NewString() - for _, vm := range c.VolumeMounts { - containerVolumes.Insert(vm.Name) - } - - if !containerVolumes.Has(bindingVolume) { - // inject metadata - c.VolumeMounts = append(c.VolumeMounts, corev1.VolumeMount{ - Name: bindingVolume, - MountPath: fmt.Sprintf("%s/%s", mountPath, b.Spec.Name), - ReadOnly: true, - }) - } - - for _, e := range b.Spec.Env { - c.Env = append(c.Env, corev1.EnvVar{ - Name: e.Name, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: secretName, + // inject metadata + c.VolumeMounts = append(c.VolumeMounts, corev1.VolumeMount{ + Name: bindingVolume, + MountPath: fmt.Sprintf("%s/%s", mountPath, b.Spec.Name), + ReadOnly: true, + }) + sort.SliceStable(c.VolumeMounts, func(i, j int) bool { + iname := c.VolumeMounts[i].Name + jname := c.VolumeMounts[j].Name + // only sort injected volume mounts + if !allInjectedVolumes.HasAll(iname, jname) { + return false + } + return iname < jname + }) + + if len(b.Spec.Env) != 0 { + for _, e := range b.Spec.Env { + c.Env = append(c.Env, corev1.EnvVar{ + Name: e.Name, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secretName, + }, + Key: e.Key, }, - Key: e.Key, }, - }, + }) + } + sort.SliceStable(c.Env, func(i, j int) bool { + iv := c.Env[i] + jv := c.Env[j] + // only sort injected env + if iv.ValueFrom == nil || iv.ValueFrom.SecretKeyRef == nil || + jv.ValueFrom == nil || jv.ValueFrom.SecretKeyRef == nil || + !allInjectedSecrets.HasAll(iv.ValueFrom.SecretKeyRef.Name, jv.ValueFrom.SecretKeyRef.Name) { + return false + } + return iv.Name < jv.Name }) } } @@ -179,36 +199,32 @@ func (b *ServiceBindingProjection) Undo(ctx context.Context, ps *duckv1.WithPod) } key := b.annotationKey() - - boundVolumes := sets.NewString(strings.Split(ps.Annotations[key], ",")...) - boundSecrets := sets.NewString() + removeSecret := ps.Annotations[key] + removeVolume := "" + delete(ps.Annotations, key) preservedVolumes := []corev1.Volume{} for _, v := range ps.Spec.Template.Spec.Volumes { - if !boundVolumes.Has(v.Name) { - preservedVolumes = append(preservedVolumes, v) + if v.Secret != nil && v.Secret.SecretName == removeSecret { + removeVolume = v.Name continue } - if v.Secret != nil { - boundSecrets = boundSecrets.Insert(v.Secret.SecretName) - } + preservedVolumes = append(preservedVolumes, v) } ps.Spec.Template.Spec.Volumes = preservedVolumes for i := range ps.Spec.Template.Spec.InitContainers { - b.undoContainer(ctx, ps, &ps.Spec.Template.Spec.InitContainers[i], boundVolumes, boundSecrets) + b.undoContainer(ctx, ps, &ps.Spec.Template.Spec.InitContainers[i], removeSecret, removeVolume) } for i := range ps.Spec.Template.Spec.Containers { - b.undoContainer(ctx, ps, &ps.Spec.Template.Spec.Containers[i], boundVolumes, boundSecrets) + b.undoContainer(ctx, ps, &ps.Spec.Template.Spec.Containers[i], removeSecret, removeVolume) } - - delete(ps.Annotations, key) } -func (b *ServiceBindingProjection) undoContainer(ctx context.Context, ps *duckv1.WithPod, c *corev1.Container, boundVolumes, boundSecrets sets.String) { +func (b *ServiceBindingProjection) undoContainer(ctx context.Context, ps *duckv1.WithPod, c *corev1.Container, removeSecret, removeVolume string) { preservedMounts := []corev1.VolumeMount{} for _, vm := range c.VolumeMounts { - if !boundVolumes.Has(vm.Name) { + if removeVolume != vm.Name { preservedMounts = append(preservedMounts, vm) } } @@ -216,7 +232,7 @@ func (b *ServiceBindingProjection) undoContainer(ctx context.Context, ps *duckv1 preservedEnv := []corev1.EnvVar{} for _, e := range c.Env { - if e.ValueFrom == nil || e.ValueFrom.SecretKeyRef == nil || !boundSecrets.Has(e.ValueFrom.SecretKeyRef.Name) { + if e.ValueFrom == nil || e.ValueFrom.SecretKeyRef == nil || e.ValueFrom.SecretKeyRef.Name != removeSecret { preservedEnv = append(preservedEnv, e) } } @@ -227,6 +243,22 @@ func (b *ServiceBindingProjection) annotationKey() string { return truncateAt63("%s-%x", ServiceBindingProjectionAnnotationKey, sha1.Sum([]byte(b.Name))) } +func (b *ServiceBindingProjection) injectedValues(ps *duckv1.WithPod) (sets.String, sets.String) { + secrets := sets.NewString() + volumes := sets.NewString() + for k, v := range ps.Annotations { + if strings.HasPrefix(k, ServiceBindingProjectionAnnotationKey) { + secrets.Insert(v) + } + } + for _, v := range ps.Spec.Template.Spec.Volumes { + if v.Secret != nil && secrets.Has(v.Secret.SecretName) { + volumes.Insert(v.Name) + } + } + return secrets, volumes +} + func (bs *ServiceBindingProjectionStatus) InitializeConditions() { sbpCondSet.Manage(bs).InitializeConditions() } diff --git a/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_test.go b/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_test.go index b2265e71..fc5ae22b 100644 --- a/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_test.go +++ b/pkg/apis/servicebindinginternal/v1alpha2/servicebindingprojection_test.go @@ -496,7 +496,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-a,injected-b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -504,8 +504,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ {Name: "preserve"}, - {Name: "injected-a"}, - {Name: "injected-b"}, + {Name: "injected", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "injected-secret"}}}, }, }, }, @@ -533,7 +532,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -556,7 +555,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { }, }, Volumes: []corev1.Volume{ - {Name: "injected"}, + {Name: "injected", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "injected-secret"}}}, }, }, }, @@ -595,7 +594,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -640,14 +639,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { }, }, Volumes: []corev1.Volume{ - { - Name: "injected", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "injected-secret", - }, - }, - }, + {Name: "injected", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "injected-secret"}}}, }, }, }, @@ -689,7 +681,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-a,injected-b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -697,8 +689,7 @@ func TestServiceBindingProjection_Undo(t *testing.T) { Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ {Name: "preserve"}, - {Name: "injected-a"}, - {Name: "injected-b"}, + {Name: "injected", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "injected-secret"}}}, }, }, }, @@ -769,7 +760,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -861,7 +852,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -957,7 +948,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1034,7 +1025,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1108,7 +1099,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1185,7 +1176,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1253,7 +1244,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "injected", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1261,7 +1252,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ {Name: "preserve"}, - {Name: "injected"}, + {Name: "injected", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "my-secret"}}}, }, }, }, @@ -1270,7 +1261,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1308,7 +1299,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { seed: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1365,7 +1356,7 @@ func TestServiceBindingProjection_Do(t *testing.T) { expected: &duckv1.WithPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "internal.service.binding/projection-16384e6a11df69776193b6a877b": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", }, }, Spec: duckv1.WithPodSpec{ @@ -1465,6 +1456,159 @@ func TestServiceBindingProjection_Do(t *testing.T) { }, }, }, + { + name: "deterministic sorting of injected values with multiple bindings", + binding: &ServiceBindingProjection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-binding", + }, + Spec: ServiceBindingProjectionSpec{ + Name: "my-binding-name", + Binding: corev1.LocalObjectReference{ + Name: "my-secret", + }, + Env: []EnvVar{ + { + Name: "MY_VAR", + Key: "my-key", + }, + }, + }, + }, + seed: &duckv1.WithPod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "internal.service.binding/projection-4b2c350fb984fc36b6cf39515a2": "other-secret", + }, + }, + Spec: duckv1.WithPodSpec{ + Template: duckv1.PodSpecable{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: "PRESERVE", + }, + { + Name: "SERVICE_BINDING_ROOT", + Value: "/bindings", + }, + { + Name: "OTHER_VAR", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "other-secret", + }, + Key: "my-key", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "binding-a9a23274b0590d5057aae1ae621be723716c4dd5", + MountPath: "/bindings/other-binding-name", + ReadOnly: true, + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "binding-a9a23274b0590d5057aae1ae621be723716c4dd5", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "other-secret", + }, + }, + }, + }, + }, + }, + }, + }, + expected: &duckv1.WithPod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "internal.service.binding/projection-4b2c350fb984fc36b6cf39515a2": "other-secret", + "internal.service.binding/projection-16384e6a11df69776193b6a877b": "my-secret", + }, + }, + Spec: duckv1.WithPodSpec{ + Template: duckv1.PodSpecable{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: "PRESERVE", + }, + { + Name: "SERVICE_BINDING_ROOT", + Value: "/bindings", + }, + { + Name: "MY_VAR", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "my-secret", + }, + Key: "my-key", + }, + }, + }, + { + Name: "OTHER_VAR", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "other-secret", + }, + Key: "my-key", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + MountPath: "/bindings/my-binding-name", + ReadOnly: true, + }, + { + Name: "binding-a9a23274b0590d5057aae1ae621be723716c4dd5", + MountPath: "/bindings/other-binding-name", + ReadOnly: true, + }, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "my-secret", + }, + }, + }, + { + Name: "binding-a9a23274b0590d5057aae1ae621be723716c4dd5", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "other-secret", + }, + }, + }, + }, + }, + }, + }, + }, + }, } for _, c := range tests { t.Run(c.name, func(t *testing.T) { diff --git a/pkg/reconciler/servicebindingprojection/servicebindingprojection_test.go b/pkg/reconciler/servicebindingprojection/servicebindingprojection_test.go index dd85c850..3aaa8130 100644 --- a/pkg/reconciler/servicebindingprojection/servicebindingprojection_test.go +++ b/pkg/reconciler/servicebindingprojection/servicebindingprojection_test.go @@ -113,7 +113,7 @@ func TestReconcile(t *testing.T) { Namespace: namespace, Name: "my-application", Annotations: map[string]string{ - "internal.service.binding/projection-e9ead9b18f311f72f9c7a54af76": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-e9ead9b18f311f72f9c7a54af76": "my-secret", }, }, Spec: appsv1.DeploymentSpec{ @@ -220,7 +220,7 @@ func TestReconcile(t *testing.T) { Namespace: namespace, Name: "my-application", Annotations: map[string]string{ - "internal.service.binding/projection-e9ead9b18f311f72f9c7a54af76": "binding-5c5a15a8b0b3e154d77746945e563ba40100681b", + "internal.service.binding/projection-e9ead9b18f311f72f9c7a54af76": "my-secret", }, }, // will also remove injected PodTemplateSpec items, but the