diff --git a/pkg/resource/api.go b/pkg/resource/api.go index 15d0c94..97a047e 100644 --- a/pkg/resource/api.go +++ b/pkg/resource/api.go @@ -37,6 +37,7 @@ const ( errUpdateClaim = "cannot update resource claim" errGetSecret = "cannot get managed resource's connection secret" errSecretConflict = "cannot establish control of existing connection secret" + errUpdateSecret = "cannot update connection secret" errCreateOrUpdateSecret = "cannot create or update connection secret" errUpdateManaged = "cannot update managed resource" errUpdateManagedStatus = "cannot update managed resource status" @@ -102,8 +103,16 @@ func (a *APIManagedConnectionPropagator) PropagateConnection(ctx context.Context return errors.Wrap(err, errGetSecret) } + // Make sure the managed resource is the controller of the connection secret + // it references before we propagate it. This ensures a managed resource + // cannot use Crossplane to circumvent RBAC by propagating a secret it does + // not own. + if c := metav1.GetControllerOf(mgcs); c == nil || c.UID != mg.GetUID() { + return errors.New(errSecretConflict) + } + cmcs := ConnectionSecretFor(cm, MustGetKind(cm, a.typer)) - _, err := util.CreateOrUpdate(ctx, a.client, cmcs, func() error { + if _, err := util.CreateOrUpdate(ctx, a.client, cmcs, func() error { // Inside this anonymous function cmcs could either be unchanged (if // it does not exist in the API server) or updated to reflect its // current state according to the API server. @@ -111,10 +120,23 @@ func (a *APIManagedConnectionPropagator) PropagateConnection(ctx context.Context return errors.New(errSecretConflict) } cmcs.Data = mgcs.Data + meta.AddAnnotations(cmcs, map[string]string{ + AnnotationKeyPropagateFromNamespace: mgcs.GetNamespace(), + AnnotationKeyPropagateFromName: mgcs.GetName(), + AnnotationKeyPropagateFromUID: string(mgcs.GetUID()), + }) return nil + }); err != nil { + return errors.Wrap(err, errCreateOrUpdateSecret) + } + + meta.AddAnnotations(mgcs, map[string]string{ + AnnotationKeyPropagateToNamespace: cmcs.GetNamespace(), + AnnotationKeyPropagateToName: cmcs.GetName(), + AnnotationKeyPropagateToUID: string(cmcs.GetUID()), }) - return errors.Wrap(err, errCreateOrUpdateSecret) + return errors.Wrap(a.client.Update(ctx, mgcs), errUpdateSecret) } // An APIManagedBinder binds resources to claims by updating them in a diff --git a/pkg/resource/api_test.go b/pkg/resource/api_test.go index 80b9799..c766fd8 100644 --- a/pkg/resource/api_test.go +++ b/pkg/resource/api_test.go @@ -169,6 +169,7 @@ func TestPropagateConnection(t *testing.T) { cmname := "coolclaim" mgname := "coolmanaged" + uid := types.UID("definitely-a-uuid") cmcsname := "coolclaimsecret" mgcsname := "coolmanagedsecret" mgcsdata := map[string][]byte{"cool": []byte("data")} @@ -219,12 +220,24 @@ func TestPropagateConnection(t *testing.T) { fields: fields{ client: &test.MockClient{ MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - s := &corev1.Secret{} - s.SetOwnerReferences([]metav1.OwnerReference{{ - UID: types.UID("some-other-uuid"), - Controller: &controller, - }}) - *o.(*corev1.Secret) = *s + switch n.Name { + case cmcsname: + s := &corev1.Secret{} + s.SetOwnerReferences([]metav1.OwnerReference{{ + UID: types.UID("some-other-uuid"), + Controller: &controller, + }}) + *o.(*corev1.Secret) = *s + case mgcsname: + s := &corev1.Secret{} + s.SetOwnerReferences([]metav1.OwnerReference{{ + UID: uid, + Controller: &controller, + }}) + *o.(*corev1.Secret) = *s + default: + return errors.New("unexpected secret name") + } return nil }, MockUpdate: test.NewMockUpdateFn(nil), @@ -238,7 +251,7 @@ func TestPropagateConnection(t *testing.T) { MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: cmcsname}}, }, mg: &MockManaged{ - ObjectMeta: metav1.ObjectMeta{Name: mgname}, + ObjectMeta: metav1.ObjectMeta{Name: mgname, UID: uid}, MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: mgcsname}}, }, }, @@ -248,8 +261,20 @@ func TestPropagateConnection(t *testing.T) { fields: fields{ client: &test.MockClient{ MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - s := &corev1.Secret{} - *o.(*corev1.Secret) = *s + switch n.Name { + case mgcsname: + s := &corev1.Secret{} + s.SetOwnerReferences([]metav1.OwnerReference{{ + UID: uid, + Controller: &controller, + }}) + *o.(*corev1.Secret) = *s + case cmcsname: + // A secret without any owner references. + *o.(*corev1.Secret) = corev1.Secret{} + default: + return errors.New("unexpected secret name") + } return nil }, MockUpdate: test.NewMockUpdateFn(nil), @@ -263,33 +288,39 @@ func TestPropagateConnection(t *testing.T) { MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: cmcsname}}, }, mg: &MockManaged{ - ObjectMeta: metav1.ObjectMeta{Name: mgname}, + ObjectMeta: metav1.ObjectMeta{Name: mgname, UID: uid}, MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: mgcsname}}, }, }, want: errors.Wrap(errors.New(errSecretConflict), errCreateOrUpdateSecret), }, - "Successful": { + "ManagedSecretUpdateError": { fields: fields{ client: &test.MockClient{ MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - if n.Name == mgcsname { - *o.(*corev1.Secret) = corev1.Secret{Data: mgcsdata} + + switch n.Name { + case mgcsname: + s := corev1.Secret{} + s.SetNamespace(namespace) + s.SetUID(uid) + s.SetOwnerReferences([]metav1.OwnerReference{{UID: uid, Controller: &controller}}) + s.SetName(mgcsname) + s.Data = mgcsdata + *o.(*corev1.Secret) = s + case cmcsname: + default: + return errors.New("unexpected secret name") } return nil }, MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { - want := &corev1.Secret{} - want.SetName(cmcsname) - want.SetOwnerReferences([]metav1.OwnerReference{{ - Name: cmname, - APIVersion: MockGVK(&MockClaim{}).GroupVersion().String(), - Kind: MockGVK(&MockClaim{}).Kind, - Controller: &controller, - }}) - want.Data = mgcsdata - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("-want, +got:\n%s", diff) + switch got.(metav1.Object).GetName() { + case cmcsname: + case mgcsname: + return errBoom + default: + return errors.New("unexpected secret name") } return nil }), @@ -303,7 +334,78 @@ func TestPropagateConnection(t *testing.T) { MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: cmcsname}}, }, mg: &MockManaged{ - ObjectMeta: metav1.ObjectMeta{Name: mgname}, + ObjectMeta: metav1.ObjectMeta{Name: mgname, UID: uid}, + MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: mgcsname}}, + }, + }, + want: errors.Wrap(errBoom, errUpdateSecret), + }, + "Successful": { + fields: fields{ + client: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := corev1.Secret{} + s.SetNamespace(namespace) + s.SetUID(uid) + s.SetOwnerReferences([]metav1.OwnerReference{{UID: uid, Controller: &controller}}) + + switch n.Name { + case mgcsname: + s.SetName(mgcsname) + s.Data = mgcsdata + *o.(*corev1.Secret) = s + case cmcsname: + s.SetName(cmcsname) + *o.(*corev1.Secret) = s + default: + return errors.New("unexpected secret name") + } + return nil + }, + MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { + want := &corev1.Secret{} + want.SetNamespace(namespace) + want.SetUID(uid) + want.SetOwnerReferences([]metav1.OwnerReference{{UID: uid, Controller: &controller}}) + want.Data = mgcsdata + + switch got.(metav1.Object).GetName() { + case cmcsname: + want.SetName(cmcsname) + want.SetAnnotations(map[string]string{ + AnnotationKeyPropagateFromNamespace: namespace, + AnnotationKeyPropagateFromName: mgcsname, + AnnotationKeyPropagateFromUID: string(uid), + }) + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("-want, +got:\n%s", diff) + } + case mgcsname: + want.SetName(mgcsname) + want.SetAnnotations(map[string]string{ + AnnotationKeyPropagateToNamespace: namespace, + AnnotationKeyPropagateToName: cmcsname, + AnnotationKeyPropagateToUID: string(uid), + }) + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("-want, +got:\n%s", diff) + } + default: + return errors.New("unexpected secret name") + } + return nil + }), + }, + typer: MockSchemeWith(&MockClaim{}, &MockManaged{}), + }, + args: args{ + ctx: context.Background(), + cm: &MockClaim{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: cmname, UID: uid}, + MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: cmcsname}}, + }, + mg: &MockManaged{ + ObjectMeta: metav1.ObjectMeta{Name: mgname, UID: uid}, MockConnectionSecretWriterTo: MockConnectionSecretWriterTo{Ref: corev1.LocalObjectReference{Name: mgcsname}}, }, }, diff --git a/pkg/resource/claim_reconciler.go b/pkg/resource/claim_reconciler.go index b37195c..81aaa35 100644 --- a/pkg/resource/claim_reconciler.go +++ b/pkg/resource/claim_reconciler.go @@ -76,7 +76,7 @@ type ManagedConfigurator interface { Configure(ctx context.Context, cm Claim, cs NonPortableClass, mg Managed) error } -// A ManagedConfiguratorFn is a function that sastisfies the +// A ManagedConfiguratorFn is a function that satisfies the // ManagedConfigurator interface. type ManagedConfiguratorFn func(ctx context.Context, cm Claim, cs NonPortableClass, mg Managed) error @@ -93,7 +93,7 @@ type ManagedCreator interface { Create(ctx context.Context, cm Claim, cs NonPortableClass, mg Managed) error } -// A ManagedCreatorFn is a function that sastisfies the ManagedCreator interface. +// A ManagedCreatorFn is a function that satisfies the ManagedCreator interface. type ManagedCreatorFn func(ctx context.Context, cm Claim, cs NonPortableClass, mg Managed) error // Create the supplied resource. @@ -108,7 +108,7 @@ type ManagedConnectionPropagator interface { PropagateConnection(ctx context.Context, cm Claim, mg Managed) error } -// A ManagedConnectionPropagatorFn is a function that sastisfies the +// A ManagedConnectionPropagatorFn is a function that satisfies the // ManagedConnectionPropagator interface. type ManagedConnectionPropagatorFn func(ctx context.Context, cm Claim, mg Managed) error @@ -123,7 +123,7 @@ type ManagedBinder interface { Bind(ctx context.Context, cm Claim, mg Managed) error } -// A ManagedBinderFn is a function that sastisfies the ManagedBinder interface. +// A ManagedBinderFn is a function that satisfies the ManagedBinder interface. type ManagedBinderFn func(ctx context.Context, cm Claim, mg Managed) error // Bind the supplied resource claim to the supplied managed resource. @@ -136,7 +136,7 @@ type ManagedFinalizer interface { Finalize(ctx context.Context, cm Managed) error } -// A ManagedFinalizerFn is a function that sastisfies the ManagedFinalizer interface. +// A ManagedFinalizerFn is a function that satisfies the ManagedFinalizer interface. type ManagedFinalizerFn func(ctx context.Context, cm Managed) error // Finalize the supplied managed resource. @@ -149,7 +149,7 @@ type ClaimFinalizer interface { Finalize(ctx context.Context, cm Claim) error } -// A ClaimFinalizerFn is a function that sastisfies the ClaimFinalizer interface. +// A ClaimFinalizerFn is a function that satisfies the ClaimFinalizer interface. type ClaimFinalizerFn func(ctx context.Context, cm Claim) error // Finalize the supplied managed resource. diff --git a/pkg/resource/enqueue_claim.go b/pkg/resource/enqueue_handlers.go similarity index 56% rename from pkg/resource/enqueue_claim.go rename to pkg/resource/enqueue_handlers.go index 489b131..e647e13 100644 --- a/pkg/resource/enqueue_claim.go +++ b/pkg/resource/enqueue_handlers.go @@ -18,6 +18,7 @@ package resource import ( "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -25,6 +26,10 @@ import ( "github.com/crossplaneio/crossplane-runtime/pkg/meta" ) +type adder interface { + Add(item interface{}) +} + // EnqueueRequestForClaim enqueues a reconcile.Request for the NamespacedName // of a ClaimReferencer's ClaimReference. type EnqueueRequestForClaim struct{} @@ -54,12 +59,58 @@ func (e *EnqueueRequestForClaim) Generic(evt event.GenericEvent, q workqueue.Rat addClaim(evt.Object, q) } -type adder interface { - Add(item interface{}) -} - func addClaim(obj runtime.Object, queue adder) { if cr, ok := obj.(ClaimReferencer); ok && cr.GetClaimReference() != nil { queue.Add(reconcile.Request{NamespacedName: meta.NamespacedNameOf(cr.GetClaimReference())}) } } + +// EnqueueRequestForPropagator enqueues a reconcile.Request for the +// NamespacedName of a propagated object, i.e. an object with propagation +// metadata annotations. +type EnqueueRequestForPropagator struct{} + +// Create adds a NamespacedName for the supplied CreateEvent if its Object is +// propagated. +func (e *EnqueueRequestForPropagator) Create(evt event.CreateEvent, q workqueue.RateLimitingInterface) { + addPropagator(evt.Object, q) +} + +// Update adds a NamespacedName for the supplied UpdateEvent if its Objects are +// propagated. +func (e *EnqueueRequestForPropagator) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) { + addPropagator(evt.ObjectOld, q) + addPropagator(evt.ObjectNew, q) +} + +// Delete adds a NamespacedName for the supplied DeleteEvent if its Object is +// propagated. +func (e *EnqueueRequestForPropagator) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInterface) { + addPropagator(evt.Object, q) +} + +// Generic adds a NamespacedName for the supplied GenericEvent if its Object is +// propagated. +func (e *EnqueueRequestForPropagator) Generic(evt event.GenericEvent, q workqueue.RateLimitingInterface) { + addPropagator(evt.Object, q) +} + +func addPropagator(obj runtime.Object, queue adder) { + ao, ok := obj.(annotated) + if !ok { + return + } + + a := ao.GetAnnotations() + switch { + case a[AnnotationKeyPropagateFromNamespace] == "": + return + case a[AnnotationKeyPropagateFromName] == "": + return + default: + queue.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: a[AnnotationKeyPropagateFromNamespace], + Name: a[AnnotationKeyPropagateFromName], + }}) + } +} diff --git a/pkg/resource/enqueue_claim_test.go b/pkg/resource/enqueue_handlers_test.go similarity index 57% rename from pkg/resource/enqueue_claim_test.go rename to pkg/resource/enqueue_handlers_test.go index 32a158a..c86f98c 100644 --- a/pkg/resource/enqueue_claim_test.go +++ b/pkg/resource/enqueue_handlers_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -46,7 +47,6 @@ func TestAddClaim(t *testing.T) { queue adder }{ "ObjectIsNotAClaimReferencer": { - obj: &corev1.Pod{}, queue: addFn(func(_ interface{}) { t.Errorf("queue.Add() called unexpectedly") }), }, "ObjectHasNilClaimReference": { @@ -63,7 +63,6 @@ func TestAddClaim(t *testing.T) { if diff := cmp.Diff(want, got); diff != "" { t.Errorf("-want, +got:\n%s", diff) } - }), }, } @@ -72,3 +71,45 @@ func TestAddClaim(t *testing.T) { addClaim(tc.obj, tc.queue) } } + +func TestAddPropagator(t *testing.T) { + ns := "coolns" + name := "coolname" + + cases := map[string]struct { + obj runtime.Object + queue adder + }{ + "ObjectIsNotAnnotated": { + queue: addFn(func(_ interface{}) { t.Errorf("queue.Add() called unexpectedly") }), + }, + "ObjectMissing" + AnnotationKeyPropagateFromNamespace: { + obj: &MockManaged{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromName: name, + }}}, + queue: addFn(func(_ interface{}) { t.Errorf("queue.Add() called unexpectedly") }), + }, + "ObjectMissing" + AnnotationKeyPropagateFromName: { + obj: &MockManaged{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + }}}, + queue: addFn(func(_ interface{}) { t.Errorf("queue.Add() called unexpectedly") }), + }, + "IsPropagatedObject": { + obj: &MockManaged{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: name, + }}}, + queue: addFn(func(got interface{}) { + want := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: ns, Name: name}} + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("-want, +got:\n%s", diff) + } + }), + }, + } + + for _, tc := range cases { + addPropagator(tc.obj, tc.queue) + } +} diff --git a/pkg/resource/predicates.go b/pkg/resource/predicates.go index fdd82e5..b8946a1 100644 --- a/pkg/resource/predicates.go +++ b/pkg/resource/predicates.go @@ -19,6 +19,7 @@ package resource import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -53,6 +54,18 @@ func AnyOf(fn ...PredicateFn) PredicateFn { } } +// AllOf accepts objects that pass all of the supplied predicate functions. +func AllOf(fn ...PredicateFn) PredicateFn { + return func(obj runtime.Object) bool { + for _, f := range fn { + if !f(obj) { + return false + } + } + return true + } +} + // HasManagedResourceReferenceKind accepts objects that reference the supplied // managed resource kind. func HasManagedResourceReferenceKind(k ManagedKind) PredicateFn { @@ -81,6 +94,70 @@ func IsManagedKind(k ManagedKind, ot runtime.ObjectTyper) PredicateFn { } } +// IsControlledByKind accepts objects that are controlled by a resource of the +// supplied kind. +func IsControlledByKind(k schema.GroupVersionKind) PredicateFn { + return func(obj runtime.Object) bool { + mo, ok := obj.(metav1.Object) + if !ok { + return false + } + + ref := metav1.GetControllerOf(mo) + if ref == nil { + return false + } + + return ref.APIVersion == k.GroupVersion().String() && ref.Kind == k.Kind + } +} + +// IsPropagator accepts objects that request to be partially or fully propagated +// to another object of the same kind. +func IsPropagator() PredicateFn { + return func(obj runtime.Object) bool { + ao, ok := obj.(annotated) + if !ok { + return false + } + + a := ao.GetAnnotations() + switch { + case a[AnnotationKeyPropagateToNamespace] == "": + return false + case a[AnnotationKeyPropagateToName] == "": + return false + case a[AnnotationKeyPropagateToUID] == "": + return false + default: + return true + } + } +} + +// IsPropagated accepts objects that consent to be partially or fully propagated +// from another object of the same kind. +func IsPropagated() PredicateFn { + return func(obj runtime.Object) bool { + ao, ok := obj.(annotated) + if !ok { + return false + } + + a := ao.GetAnnotations() + switch { + case a[AnnotationKeyPropagateFromNamespace] == "": + return false + case a[AnnotationKeyPropagateFromName] == "": + return false + case a[AnnotationKeyPropagateFromUID] == "": + return false + default: + return true + } + } +} + // HasIndirectClassReferenceKind accepts namespaced objects that reference the // supplied non-portable class kind via the supplied portable class kind. func HasIndirectClassReferenceKind(c client.Client, oc runtime.ObjectCreater, k ClassKinds) PredicateFn { diff --git a/pkg/resource/predicates_test.go b/pkg/resource/predicates_test.go index 5ad7f80..3c4cff2 100644 --- a/pkg/resource/predicates_test.go +++ b/pkg/resource/predicates_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -58,7 +59,45 @@ func TestAnyOf(t *testing.T) { } }) } +} +func TestAllOf(t *testing.T) { + cases := map[string]struct { + fns []PredicateFn + obj runtime.Object + want bool + }{ + "AllPredicatesPass": { + fns: []PredicateFn{ + func(obj runtime.Object) bool { return true }, + func(obj runtime.Object) bool { return true }, + }, + want: true, + }, + "NoPredicatesPass": { + fns: []PredicateFn{ + func(obj runtime.Object) bool { return false }, + func(obj runtime.Object) bool { return false }, + }, + want: false, + }, + "SomePredicatesPass": { + fns: []PredicateFn{ + func(obj runtime.Object) bool { return false }, + func(obj runtime.Object) bool { return true }, + }, + want: false, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := AllOf(tc.fns...)(tc.obj) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("AllOf(...): -want, +got:\n%s", diff) + } + }) + } } func TestHasManagedResourceReferenceKind(t *testing.T) { @@ -140,6 +179,162 @@ func TestIsManagedKind(t *testing.T) { } } +func TestIsControlledByKind(t *testing.T) { + controller := true + + cases := map[string]struct { + kind schema.GroupVersionKind + obj runtime.Object + want bool + }{ + "NoObjectMeta": { + want: false, + }, + "NoControllerRef": { + obj: &corev1.Secret{}, + want: false, + }, + "WrongAPIVersion": { + kind: MockGVK(&MockManaged{}), + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{OwnerReferences: []v1.OwnerReference{ + { + Kind: MockGVK(&MockManaged{}).Kind, + Controller: &controller, + }, + }}}, + want: false, + }, + "WrongKind": { + kind: MockGVK(&MockManaged{}), + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{OwnerReferences: []v1.OwnerReference{ + { + APIVersion: MockGVK(&MockManaged{}).GroupVersion().String(), + Controller: &controller, + }, + }}}, + want: false, + }, + "IsControlledByKind": { + kind: MockGVK(&MockManaged{}), + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{OwnerReferences: []v1.OwnerReference{ + { + APIVersion: MockGVK(&MockManaged{}).GroupVersion().String(), + Kind: MockGVK(&MockManaged{}).Kind, + Controller: &controller, + }, + }}}, + want: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := IsControlledByKind(tc.kind)(tc.obj) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("IsControlledByKind(...): -want, +got:\n%s", diff) + } + }) + } +} + +func TestIsPropagator(t *testing.T) { + cases := map[string]struct { + obj runtime.Object + want bool + }{ + "NotAnAnnotator": { + want: false, + }, + "Missing" + AnnotationKeyPropagateToNamespace: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateToName: name, + AnnotationKeyPropagateToUID: string(uid), + }}}, + want: false, + }, + "Missing" + AnnotationKeyPropagateToName: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: namespace, + AnnotationKeyPropagateToUID: string(uid), + }}}, + want: false, + }, + "Missing" + AnnotationKeyPropagateToUID: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: namespace, + AnnotationKeyPropagateToName: name, + }}}, + want: false, + }, + "IsPropagator": { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: namespace, + AnnotationKeyPropagateToName: name, + AnnotationKeyPropagateToUID: string(uid), + }}}, + want: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := IsPropagator()(tc.obj) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("IsPropagator(...): -want, +got:\n%s", diff) + } + }) + } +} + +func TestIsPropagated(t *testing.T) { + cases := map[string]struct { + obj runtime.Object + want bool + }{ + "NotAnAnnotator": { + want: false, + }, + "Missing" + AnnotationKeyPropagateFromNamespace: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromName: name, + AnnotationKeyPropagateFromUID: string(uid), + }}}, + want: false, + }, + "Missing" + AnnotationKeyPropagateFromName: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: namespace, + AnnotationKeyPropagateFromUID: string(uid), + }}}, + want: false, + }, + "Missing" + AnnotationKeyPropagateFromUID: { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: namespace, + AnnotationKeyPropagateFromName: name, + }}}, + want: false, + }, + "IsPropagated": { + obj: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: namespace, + AnnotationKeyPropagateFromName: name, + AnnotationKeyPropagateFromUID: string(uid), + }}}, + want: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := IsPropagated()(tc.obj) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("IsPropagated(...): -want, +got:\n%s", diff) + } + }) + } +} + func TestHasIndirectClassReferenceKind(t *testing.T) { errUnexpected := errors.New("unexpected object type") diff --git a/pkg/resource/secret_reconciler.go b/pkg/resource/secret_reconciler.go new file mode 100644 index 0000000..e56d7d3 --- /dev/null +++ b/pkg/resource/secret_reconciler.go @@ -0,0 +1,120 @@ +/* +Copyright 2019 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resource + +import ( + "context" + "time" + + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/crossplaneio/crossplane-runtime/pkg/logging" +) + +// Supported resources with all of these annotations will be fully or partially +// propagated to the named resource of the same kind, assuming it exists and +// consents to propagation. +const ( + AnnotationKeyPropagateToNamespace = "crossplane.io/propagate-to-namespace" + AnnotationKeyPropagateToName = "crossplane.io/propagate-to-name" + AnnotationKeyPropagateToUID = "crossplane.io/propagate-to-uid" +) + +// Supported resources with all of these annotations consent to be fully or +// partially propagated from the named resource of the same kind. +const ( + AnnotationKeyPropagateFromNamespace = "crossplane.io/propagate-from-namespace" + AnnotationKeyPropagateFromName = "crossplane.io/propagate-from-name" + AnnotationKeyPropagateFromUID = "crossplane.io/propagate-from-uid" +) + +type annotated interface { + GetAnnotations() map[string]string +} + +const ( + secretControllerName = "secretpropagator.crossplane.io" + secretReconcileTimeout = 1 * time.Minute +) + +// NewSecretPropagatingReconciler returns a Reconciler that reconciles secrets +// by propagating their data to another secret. Both secrets must consent to +// this process by including propagation annotations. The Reconciler assumes it +// has a watch on both propagating (from) and propagated (to) secrets. +func NewSecretPropagatingReconciler(m manager.Manager) reconcile.Reconciler { + client := m.GetClient() + + return reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { + log.V(logging.Debug).Info("Reconciling", "controller", secretControllerName, "request", req) + + ctx, cancel := context.WithTimeout(context.Background(), secretReconcileTimeout) + defer cancel() + + // The 'from' secret is also know as the 'propagating' secret. + from := &corev1.Secret{} + if err := client.Get(ctx, req.NamespacedName, from); err != nil { + // There's no propagation to be done if the secret we're propagating + // from does not exist. We assume we have a watch on that secret and + // will be queued if/when it is created. Otherwise we'll be requeued + // implicitly because we return an error. + return reconcile.Result{}, errors.Wrap(IgnoreNotFound(err), errGetSecret) + } + + // The 'to' secret is also known as the 'propagated' secret. We guard + // against abusers of the propagation process by requiring that both + // secrets consent to propagation by specifying each other's UID. We + // cannot know the UID of a secret that doesn't exist, so the propagated + // secret must be created outside of the propagation process. + to := &corev1.Secret{} + n := types.NamespacedName{ + Namespace: from.GetAnnotations()[AnnotationKeyPropagateToNamespace], + Name: from.GetAnnotations()[AnnotationKeyPropagateToName], + } + if err := client.Get(ctx, n, to); err != nil { + // There's no propagation to be done if the secret we propagate to + // does not exist. We assume we have a watch on that secret and will + // be queued if/when it is created. Otherwise we'll be requeued + // implicitly because we return an error. + return reconcile.Result{}, errors.Wrap(IgnoreNotFound(err), errGetSecret) + } + + if from.GetAnnotations()[AnnotationKeyPropagateToUID] != string(to.GetUID()) { + // The propagating secret expected a different propagated secret. We + // assume we have a watch on both secrets, and will be requeued if + // and when this situation is remedied. + return reconcile.Result{}, nil + } + + if to.GetAnnotations()[AnnotationKeyPropagateFromUID] != string(from.GetUID()) { + // The propagated secret expected a different propagating secret. We + // assume we have a watch on both secrets, and will be requeued if + // and when this situation is remedied. + return reconcile.Result{}, nil + } + + to.Data = from.Data + + // If our update was successful there's nothing else to do. We assume we + // have a watch on both secrets and will be queued if either changes. + // Otherwise we'll be requeued implicitly because we return an error. + return reconcile.Result{Requeue: false}, errors.Wrap(client.Update(ctx, to), errUpdateSecret) + }) +} diff --git a/pkg/resource/secret_reconciler_test.go b/pkg/resource/secret_reconciler_test.go new file mode 100644 index 0000000..a95d182 --- /dev/null +++ b/pkg/resource/secret_reconciler_test.go @@ -0,0 +1,402 @@ +/* +Copyright 2019 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resource + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/crossplaneio/crossplane-runtime/pkg/test" +) + +func TestSecretPropagatingReconciler(t *testing.T) { + type args struct { + m manager.Manager + } + + type want struct { + result reconcile.Result + err error + } + + ns := "namespace" + + fromName := "from" + fromUID := types.UID("from-uid") + fromData := map[string][]byte{"cool": []byte("data")} + + toName := "to" + toUID := types.UID("to-uid") + + errBoom := errors.New("boom") + + cases := map[string]struct { + args args + want want + }{ + "FromNotFound": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + switch n.Name { + case fromName: + return kerrors.NewNotFound(schema.GroupResource{}, "") + default: + return errors.New("unexpected secret name") + } + }, + }, + }, + }, + want: want{ + result: reconcile.Result{}, + }, + }, + "GetFromError": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + switch n.Name { + case fromName: + return errBoom + default: + return errors.New("unexpected secret name") + } + }, + }, + }, + }, + want: want{ + err: errors.Wrap(errBoom, errGetSecret), + }, + }, + "ToNotFound": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: string(toUID), + }, + }, + Data: fromData, + } + case toName: + return kerrors.NewNotFound(schema.GroupResource{}, "") + default: + return errors.New("unexpected secret name") + } + return nil + }, + }, + }, + }, + want: want{ + result: reconcile.Result{}, + }, + }, + "GetToError": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: string(toUID), + }, + }, + Data: fromData, + } + case toName: + return errBoom + default: + return errors.New("unexpected secret name") + } + return nil + }, + }, + }, + }, + want: want{ + err: errors.Wrap(errBoom, errGetSecret), + }, + }, + "UnexpectedToUID": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: "some-other-uuid", + }, + }, + Data: fromData, + } + case toName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: toName, + UID: toUID, + Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: fromName, + AnnotationKeyPropagateFromUID: string(fromUID), + }, + }, + } + default: + return errors.New("unexpected secret name") + } + return nil + }, + MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { + return errors.New("called unexpectedly") + }), + }, + }, + }, + want: want{ + result: reconcile.Result{}, + }, + }, + "UnexpectedFromUID": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: string(toUID), + }, + }, + Data: fromData, + } + case toName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: toName, + UID: toUID, + Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: fromName, + AnnotationKeyPropagateFromUID: "some-other-uuid", + }, + }, + } + default: + return errors.New("unexpected secret name") + } + return nil + }, + MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { + return errors.New("called unexpectedly") + }), + }, + }, + }, + want: want{ + result: reconcile.Result{}, + }, + }, + "UpdateToError": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: string(toUID), + }, + }, + Data: fromData, + } + case toName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: toName, + UID: toUID, + Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: fromName, + AnnotationKeyPropagateFromUID: string(fromUID), + }, + }, + } + default: + return errors.New("unexpected secret name") + } + return nil + }, + MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { + return errBoom + }), + }, + }, + }, + want: want{ + err: errors.Wrap(errBoom, errUpdateSecret), + }, + }, + "Successful": { + args: args{ + m: &MockManager{ + c: &test.MockClient{ + MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { + s := o.(*corev1.Secret) + + switch n.Name { + case fromName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: fromName, + UID: fromUID, + Annotations: map[string]string{ + AnnotationKeyPropagateToNamespace: ns, + AnnotationKeyPropagateToName: toName, + AnnotationKeyPropagateToUID: string(toUID), + }, + }, + Data: fromData, + } + case toName: + *s = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: toName, + UID: toUID, + Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: fromName, + AnnotationKeyPropagateFromUID: string(fromUID), + }, + }, + } + default: + return errors.New("unexpected secret name") + } + return nil + }, + MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { + want := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: toName, + UID: toUID, + Annotations: map[string]string{ + AnnotationKeyPropagateFromNamespace: ns, + AnnotationKeyPropagateFromName: fromName, + AnnotationKeyPropagateFromUID: string(fromUID), + }, + }, + Data: fromData, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("-want, +got:\n%s", diff) + } + return nil + }), + }, + }, + }, + want: want{ + result: reconcile.Result{}, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := NewSecretPropagatingReconciler(tc.args.m) + got, err := r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: ns, Name: fromName}}) + + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("r.Reconcile(...): -want error, +got error:\n%s", diff) + } + + if diff := cmp.Diff(tc.want.result, got); diff != "" { + t.Errorf("r.Reconcile(...): -want, +got:\n%s", diff) + } + }) + } +}