Skip to content

Commit cdb9db6

Browse files
scothissadlerap
authored andcommitted
Unproject using the same mapping used to project the binding
From the spec: > When a service binding projection is removed, the controller MUST use > the same mappings from the projection creation. After a > ClusterWorkloadResourceMapping resource is modified, each binding > targeting the mapped workload type MUST be removed, then reattempted > with the latest mapping. We now stash the mapping used to project the binding on the workload as an annotation. When unprojecting that same binding, we use the stashed mapping to unproject the binding. If updating an existing binding, the stashed mapping is used to cleanup existing state before the updated mapping is used to re-project the binding into the workload. Signed-off-by: Scott Andrews <andrewssc@vmware.com>
1 parent 1f525ba commit cdb9db6

11 files changed

Lines changed: 1254 additions & 307 deletions

controllers/servicebinding_controller_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ func TestServiceBindingReconciler(t *testing.T) {
5353
secretName := "my-secret"
5454
key := types.NamespacedName{Namespace: namespace, Name: name}
5555

56+
podSpecableMapping := `{"versions":[{"version":"*","annotations":".spec.template.metadata.annotations","containers":[{"path":".spec.template.spec.initContainers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"},{"path":".spec.template.spec.containers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"}],"volumes":".spec.template.spec.volumes"}]}`
57+
5658
scheme := runtime.NewScheme()
5759
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
5860
utilruntime.Must(servicebindingv1beta1.AddToScheme(scheme))
@@ -102,6 +104,9 @@ func TestServiceBindingReconciler(t *testing.T) {
102104
})
103105
})
104106
projectedWorkload := workload.
107+
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
108+
d.AddAnnotation(fmt.Sprintf("projector.servicebinding.io/mapping-%s", uid), podSpecableMapping)
109+
}).
105110
SpecDie(func(d *dieappsv1.DeploymentSpecDie) {
106111
d.TemplateDie(func(d *diecorev1.PodTemplateSpecDie) {
107112
d.MetadataDie(func(d *diemetav1.ObjectMetaDie) {
@@ -143,6 +148,7 @@ func TestServiceBindingReconciler(t *testing.T) {
143148
})
144149
})
145150
}).DieReleaseUnstructured()
151+
unstructured.SetNestedMap(unprojectedWorkload.UnstructuredContent(), map[string]interface{}{}, "metadata", "annotations")
146152
unstructured.SetNestedMap(unprojectedWorkload.UnstructuredContent(), map[string]interface{}{}, "spec", "template", "metadata", "annotations")
147153
containers, _, _ := unstructured.NestedSlice(unprojectedWorkload.UnstructuredContent(), "spec", "template", "spec", "containers")
148154
unstructured.SetNestedSlice(containers[0].(map[string]interface{}), []interface{}{}, "volumeMounts")
@@ -731,6 +737,8 @@ func TestProjectBinding(t *testing.T) {
731737
uid := types.UID("dde10100-d7b3-4cba-9430-51d60a8612a6")
732738
secretName := "my-secret"
733739

740+
podSpecableMapping := `{"versions":[{"version":"*","annotations":".spec.template.metadata.annotations","containers":[{"path":".spec.template.spec.initContainers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"},{"path":".spec.template.spec.containers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"}],"volumes":".spec.template.spec.volumes"}]}`
741+
734742
scheme := runtime.NewScheme()
735743
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
736744
utilruntime.Must(servicebindingv1beta1.AddToScheme(scheme))
@@ -781,6 +789,9 @@ func TestProjectBinding(t *testing.T) {
781789
})
782790
})
783791
projectedWorkload := workload.
792+
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
793+
d.AddAnnotation(fmt.Sprintf("projector.servicebinding.io/mapping-%s", uid), podSpecableMapping)
794+
}).
784795
SpecDie(func(d *dieappsv1.DeploymentSpecDie) {
785796
d.TemplateDie(func(d *diecorev1.PodTemplateSpecDie) {
786797
d.MetadataDie(func(d *diemetav1.ObjectMetaDie) {
@@ -822,6 +833,7 @@ func TestProjectBinding(t *testing.T) {
822833
})
823834
})
824835
}).DieReleaseUnstructured()
836+
unstructured.SetNestedMap(unprojectedWorkload.UnstructuredContent(), map[string]interface{}{}, "metadata", "annotations")
825837
unstructured.SetNestedMap(unprojectedWorkload.UnstructuredContent(), map[string]interface{}{}, "spec", "template", "metadata", "annotations")
826838
containers, _, _ := unstructured.NestedSlice(unprojectedWorkload.UnstructuredContent(), "spec", "template", "spec", "containers")
827839
unstructured.SetNestedSlice(containers[0].(map[string]interface{}), []interface{}{}, "volumeMounts")

controllers/webhook_controller_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@ func TestAdmissionProjectorWebhook(t *testing.T) {
188188
requestUID := types.UID("9deefaa1-2c90-4f40-9c7b-3f5c1fd75dde")
189189
bindingUID := types.UID("89deaf20-7bab-4610-81db-6f8c3f7fa51d")
190190

191+
podSpecableMapping := `{"versions":[{"version":"*","annotations":".spec.template.metadata.annotations","containers":[{"path":".spec.template.spec.initContainers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"},{"path":".spec.template.spec.containers[*]","name":".name","env":".env","volumeMounts":".volumeMounts"}],"volumes":".spec.template.spec.volumes"}]}`
192+
191193
workload := dieappsv1.DeploymentBlank.
192194
APIVersion("apps/v1").
193195
Kind("Deployment").
@@ -275,6 +277,9 @@ func TestAdmissionProjectorWebhook(t *testing.T) {
275277
AdmissionRequest: request.
276278
Object(
277279
workload.
280+
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
281+
d.AddAnnotation(fmt.Sprintf("projector.servicebinding.io/mapping-%s", bindingUID), podSpecableMapping)
282+
}).
278283
SpecDie(func(d *dieappsv1.DeploymentSpecDie) {
279284
d.TemplateDie(func(d *diecorev1.PodTemplateSpecDie) {
280285
d.MetadataDie(func(d *diemetav1.ObjectMetaDie) {
@@ -331,6 +336,13 @@ func TestAdmissionProjectorWebhook(t *testing.T) {
331336
ExpectedResponse: admission.Response{
332337
AdmissionResponse: response.DieRelease(),
333338
Patches: []jsonpatch.Operation{
339+
{
340+
Operation: "add",
341+
Path: "/metadata/annotations",
342+
Value: map[string]interface{}{
343+
fmt.Sprintf("projector.servicebinding.io/mapping-%s", bindingUID): podSpecableMapping,
344+
},
345+
},
334346
{
335347
Operation: "add",
336348
Path: "/spec/template/metadata/annotations",
@@ -399,6 +411,13 @@ func TestAdmissionProjectorWebhook(t *testing.T) {
399411
ExpectedResponse: admission.Response{
400412
AdmissionResponse: response.DieRelease(),
401413
Patches: []jsonpatch.Operation{
414+
{
415+
Operation: "add",
416+
Path: "/metadata/annotations",
417+
Value: map[string]interface{}{
418+
fmt.Sprintf("projector.servicebinding.io/mapping-%s", bindingUID): podSpecableMapping,
419+
},
420+
},
402421
{
403422
Operation: "add",
404423
Path: "/spec/template/metadata/annotations",

projector/binding.go

Lines changed: 114 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@ package projector
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"fmt"
2223
"path"
2324
"sort"
2425
"strings"
2526

2627
corev1 "k8s.io/api/core/v1"
28+
"k8s.io/apimachinery/pkg/api/meta"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2730
"k8s.io/apimachinery/pkg/runtime"
2831
"k8s.io/apimachinery/pkg/util/sets"
2932

@@ -37,6 +40,7 @@ const (
3740
SecretAnnotationPrefix = Group + "/secret-"
3841
TypeAnnotationPrefix = Group + "/type-"
3942
ProviderAnnotationPrefix = Group + "/provider-"
43+
MappingAnnotationPrefix = Group + "/mapping-"
4044
)
4145

4246
var _ ServiceBindingProjector = (*serviceBindingProjector)(nil)
@@ -54,35 +58,95 @@ func New(mappingSource MappingSource) ServiceBindingProjector {
5458
}
5559

5660
func (p *serviceBindingProjector) Project(ctx context.Context, binding *servicebindingv1beta1.ServiceBinding, workload runtime.Object) error {
57-
mapping, err := p.mappingSource.LookupMapping(ctx, workload)
61+
ctx, resourceMapping, version, err := p.lookupClusterMapping(ctx, workload)
5862
if err != nil {
5963
return err
6064
}
61-
mpt, err := NewMetaPodTemplate(ctx, workload, mapping)
65+
66+
// rather than attempt to merge an existing binding, unproject it
67+
if err := p.Unproject(ctx, binding, workload); err != nil {
68+
return err
69+
}
70+
71+
versionMapping := MappingVersion(version, resourceMapping)
72+
mpt, err := NewMetaPodTemplate(ctx, workload, versionMapping)
6273
if err != nil {
6374
return err
6475
}
6576
p.project(binding, mpt)
66-
return mpt.WriteToWorkload(ctx)
77+
78+
if p.secretName(binding) != "" {
79+
if err := p.stashLocalMapping(binding, mpt, resourceMapping); err != nil {
80+
return err
81+
}
82+
}
83+
if err := mpt.WriteToWorkload(ctx); err != nil {
84+
return err
85+
}
86+
87+
return nil
6788
}
6889

6990
func (p *serviceBindingProjector) Unproject(ctx context.Context, binding *servicebindingv1beta1.ServiceBinding, workload runtime.Object) error {
70-
mapping, err := p.mappingSource.LookupMapping(ctx, workload)
91+
resourceMapping, err := p.retrieveLocalMapping(binding, workload)
92+
if err != nil {
93+
return err
94+
}
95+
ctx, m, version, err := p.lookupClusterMapping(ctx, workload)
7196
if err != nil {
7297
return err
7398
}
74-
mpt, err := NewMetaPodTemplate(ctx, workload, mapping)
99+
if resourceMapping == nil {
100+
// fall back to using the remote mappings, this isn't ideal as the mapping may have changed after the binding was originally projected
101+
resourceMapping = m
102+
}
103+
versionMapping := MappingVersion(version, resourceMapping)
104+
mpt, err := NewMetaPodTemplate(ctx, workload, versionMapping)
75105
if err != nil {
76106
return err
77107
}
78108
p.unproject(binding, mpt)
79-
return mpt.WriteToWorkload(ctx)
109+
110+
if err := p.stashLocalMapping(binding, mpt, nil); err != nil {
111+
return err
112+
}
113+
if err := mpt.WriteToWorkload(ctx); err != nil {
114+
return err
115+
}
116+
117+
return nil
80118
}
81119

82-
func (p *serviceBindingProjector) project(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate) {
83-
// rather than attempt to merge an existing binding, unproject it
84-
p.unproject(binding, mpt)
120+
type mappingValue struct {
121+
WorkloadMapping *servicebindingv1beta1.ClusterWorkloadResourceMappingSpec
122+
RESTMapping *meta.RESTMapping
123+
}
124+
125+
// lookupClusterMapping resolves the mapping from the context or from the cluster. This
126+
// avoids redundant calls to the mappingSource for the same workload call when Unproject
127+
// is called from Project. When the lookup is from the cluster, the value is stashed into
128+
// the context for future lookups in this turn.
129+
func (p *serviceBindingProjector) lookupClusterMapping(ctx context.Context, workload runtime.Object) (context.Context, *servicebindingv1beta1.ClusterWorkloadResourceMappingSpec, string, error) {
130+
raw := ctx.Value(mappingValue{})
131+
if value, ok := raw.(mappingValue); ok {
132+
return ctx, value.WorkloadMapping, value.RESTMapping.Resource.Version, nil
133+
}
134+
rm, err := p.mappingSource.LookupRESTMapping(ctx, workload)
135+
if err != nil {
136+
return ctx, nil, "", err
137+
}
138+
wm, err := p.mappingSource.LookupWorkloadMapping(ctx, rm.Resource)
139+
if err != nil {
140+
return ctx, nil, "", err
141+
}
142+
ctx = context.WithValue(ctx, mappingValue{}, mappingValue{
143+
WorkloadMapping: wm,
144+
RESTMapping: rm,
145+
})
146+
return ctx, wm, rm.Resource.Version, nil
147+
}
85148

149+
func (p *serviceBindingProjector) project(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate) {
86150
if p.secretName(binding) == "" {
87151
// no secret to bind
88152
return
@@ -100,9 +164,9 @@ func (p *serviceBindingProjector) unproject(binding *servicebindingv1beta1.Servi
100164
}
101165

102166
// cleanup annotations
103-
delete(mpt.Annotations, p.secretAnnotationName(binding))
104-
delete(mpt.Annotations, p.typeAnnotationName(binding))
105-
delete(mpt.Annotations, p.providerAnnotationName(binding))
167+
delete(mpt.PodTemplateAnnotations, p.secretAnnotationName(binding))
168+
delete(mpt.PodTemplateAnnotations, p.typeAnnotationName(binding))
169+
delete(mpt.PodTemplateAnnotations, p.providerAnnotationName(binding))
106170
}
107171

108172
func (p *serviceBindingProjector) projectVolume(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate) {
@@ -296,7 +360,7 @@ func (p *serviceBindingProjector) projectEnv(binding *servicebindingv1beta1.Serv
296360

297361
func (p *serviceBindingProjector) unprojectEnv(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate, mc *metaContainer) {
298362
env := []corev1.EnvVar{}
299-
secret := mpt.Annotations[p.secretAnnotationName(binding)]
363+
secret := mpt.PodTemplateAnnotations[p.secretAnnotationName(binding)]
300364
typeFieldPath := fmt.Sprintf("metadata.annotations['%s']", p.typeAnnotationName(binding))
301365
providerFieldPath := fmt.Sprintf("metadata.annotations['%s']", p.providerAnnotationName(binding))
302366
for _, e := range mc.Env {
@@ -364,7 +428,7 @@ func (p *serviceBindingProjector) isProjectedEnv(e corev1.EnvVar, secrets sets.S
364428

365429
func (p *serviceBindingProjector) knownProjectedSecrets(mpt *metaPodTemplate) sets.String {
366430
secrets := sets.NewString()
367-
for k, v := range mpt.Annotations {
431+
for k, v := range mpt.PodTemplateAnnotations {
368432
if strings.HasPrefix(k, SecretAnnotationPrefix) {
369433
secrets.Insert(v)
370434
}
@@ -385,7 +449,7 @@ func (p *serviceBindingProjector) secretAnnotation(binding *servicebindingv1beta
385449
if secret == "" {
386450
return ""
387451
}
388-
mpt.Annotations[key] = secret
452+
mpt.PodTemplateAnnotations[key] = secret
389453
return secret
390454
}
391455

@@ -399,7 +463,7 @@ func (p *serviceBindingProjector) volumeName(binding *servicebindingv1beta1.Serv
399463

400464
func (p *serviceBindingProjector) typeAnnotation(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate) string {
401465
key := p.typeAnnotationName(binding)
402-
mpt.Annotations[key] = binding.Spec.Type
466+
mpt.PodTemplateAnnotations[key] = binding.Spec.Type
403467
return key
404468
}
405469

@@ -409,10 +473,43 @@ func (p *serviceBindingProjector) typeAnnotationName(binding *servicebindingv1be
409473

410474
func (p *serviceBindingProjector) providerAnnotation(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate) string {
411475
key := p.providerAnnotationName(binding)
412-
mpt.Annotations[key] = binding.Spec.Provider
476+
mpt.PodTemplateAnnotations[key] = binding.Spec.Provider
413477
return key
414478
}
415479

416480
func (p *serviceBindingProjector) providerAnnotationName(binding *servicebindingv1beta1.ServiceBinding) string {
417481
return fmt.Sprintf("%s%s", ProviderAnnotationPrefix, binding.UID)
418482
}
483+
484+
func (p *serviceBindingProjector) retrieveLocalMapping(binding *servicebindingv1beta1.ServiceBinding, workload runtime.Object) (*servicebindingv1beta1.ClusterWorkloadResourceMappingSpec, error) {
485+
annoations := workload.(metav1.Object).GetAnnotations()
486+
if annoations == nil {
487+
return nil, nil
488+
}
489+
data, ok := annoations[p.mappingAnnotationName(binding)]
490+
if !ok {
491+
return nil, nil
492+
}
493+
var mapping servicebindingv1beta1.ClusterWorkloadResourceMappingSpec
494+
if err := json.Unmarshal([]byte(data), &mapping); err != nil {
495+
return nil, err
496+
}
497+
return &mapping, nil
498+
}
499+
500+
func (p *serviceBindingProjector) stashLocalMapping(binding *servicebindingv1beta1.ServiceBinding, mpt *metaPodTemplate, mapping *servicebindingv1beta1.ClusterWorkloadResourceMappingSpec) error {
501+
if mapping == nil {
502+
delete(mpt.WorkloadAnnotations, p.mappingAnnotationName(binding))
503+
return nil
504+
}
505+
data, err := json.Marshal(mapping)
506+
if err != nil {
507+
return err
508+
}
509+
mpt.WorkloadAnnotations[p.mappingAnnotationName(binding)] = string(data)
510+
return nil
511+
}
512+
513+
func (p *serviceBindingProjector) mappingAnnotationName(binding *servicebindingv1beta1.ServiceBinding) string {
514+
return fmt.Sprintf("%s%s", MappingAnnotationPrefix, binding.UID)
515+
}

0 commit comments

Comments
 (0)