From defa32285298facf15da5d5b7c5d1fd5727a84a3 Mon Sep 17 00:00:00 2001 From: Nic Cope Date: Fri, 16 Oct 2020 19:54:18 -0700 Subject: [PATCH] Remove reconcilers and utilities pertaining to workloads https://github.com/crossplane/crossplane/issues/1755 This removes support for the secret propagation and target controllers that each Kubernetes Cluster managed resource implemented before workloads were deprecated. See the above issue for context. Signed-off-by: Nic Cope --- pkg/reconciler/secret/doc.go | 18 -- pkg/reconciler/secret/reconciler.go | 160 ---------- pkg/reconciler/secret/reconciler_test.go | 262 ---------------- pkg/reconciler/target/doc.go | 20 -- pkg/reconciler/target/reconciler.go | 229 -------------- pkg/reconciler/target/reconciler_test.go | 384 ----------------------- pkg/resource/api.go | 7 - pkg/resource/enqueue_handlers.go | 45 --- pkg/resource/enqueue_handlers_test.go | 44 +-- pkg/resource/fake/mocks.go | 24 -- pkg/resource/interfaces.go | 11 - pkg/resource/resource.go | 3 - 12 files changed, 1 insertion(+), 1206 deletions(-) delete mode 100644 pkg/reconciler/secret/doc.go delete mode 100644 pkg/reconciler/secret/reconciler.go delete mode 100644 pkg/reconciler/secret/reconciler_test.go delete mode 100644 pkg/reconciler/target/doc.go delete mode 100644 pkg/reconciler/target/reconciler.go delete mode 100644 pkg/reconciler/target/reconciler_test.go diff --git a/pkg/reconciler/secret/doc.go b/pkg/reconciler/secret/doc.go deleted file mode 100644 index aff02f0..0000000 --- a/pkg/reconciler/secret/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright 2020 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 secret provides a reconciler that propagates Kubernetes secrets. -package secret diff --git a/pkg/reconciler/secret/reconciler.go b/pkg/reconciler/secret/reconciler.go deleted file mode 100644 index 0d98527..0000000 --- a/pkg/reconciler/secret/reconciler.go +++ /dev/null @@ -1,160 +0,0 @@ -/* -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 secret - -import ( - "context" - "strings" - "time" - - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/crossplane/crossplane-runtime/pkg/event" - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/meta" - "github.com/crossplane/crossplane-runtime/pkg/resource" -) - -const secretReconcileTimeout = 1 * time.Minute - -// Error messages. -const ( - errGetSecret = "cannot get connection secret" - errUpdateSecret = "cannot update connection secret" - errPropagationNotAllowed = "the propagating connection secret does not allow propagation to the propagated connection secret" -) - -// Event reasons -const ( - reasonPropagatedFrom event.Reason = "PropagatedDataFrom" - reasonPropagatedTo event.Reason = "PropagatedDataTo" -) - -// ControllerName returns the recommended name for controllers that use this -// package to reconcile a particular kind of resource claim. -func ControllerName(kind string) string { - return "secretpropagating/" + strings.ToLower(kind) -} - -// Reconciler reconciles secrets by propagating their data from 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. -type Reconciler struct { - client client.Client - - log logging.Logger - record event.Recorder -} - -// A ReconcilerOption configures a Reconciler. -type ReconcilerOption func(*Reconciler) - -// WithLogger specifies how the Reconciler should log messages. -func WithLogger(l logging.Logger) ReconcilerOption { - return func(r *Reconciler) { - r.log = l - } -} - -// WithRecorder specifies how the Reconciler should record events. -func WithRecorder(er event.Recorder) ReconcilerOption { - return func(r *Reconciler) { - r.record = er - } -} - -// NewReconciler returns a Reconciler that reconciles secrets by propagating -// their data from 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 NewReconciler(m manager.Manager, o ...ReconcilerOption) *Reconciler { - r := &Reconciler{ - client: m.GetClient(), - log: logging.NewNopLogger(), - record: event.NewNopRecorder(), - } - - for _, ro := range o { - ro(r) - } - - return r -} - -// Reconcile a secret by propagating its data from another secret. -func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) { - log := r.log.WithValues("request", req) - log.Debug("Reconciling") - - ctx, cancel := context.WithTimeout(context.Background(), secretReconcileTimeout) - defer cancel() - - // 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{} - if err := r.client.Get(ctx, req.NamespacedName, 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. - log.Debug("Cannot get propagated secret", "error", err) - return reconcile.Result{}, errors.Wrap(resource.IgnoreNotFound(err), errGetSecret) - } - - record := r.record.WithAnnotations("to-namespace", to.GetNamespace(), "to-name", to.GetName()) - log = log.WithValues("to-namespace", to.GetNamespace(), "to-name", to.GetName()) - - // The 'from' secret is also know as the 'propagating' secret. - from := &corev1.Secret{} - if err := r.client.Get(ctx, meta.AllowsPropagationFrom(to), 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. - log.Debug("Cannot get propagating secret", "error", err) - return reconcile.Result{}, errors.Wrap(resource.IgnoreNotFound(err), errGetSecret) - } - - record = record.WithAnnotations("from-namespace", from.GetNamespace(), "from-name", from.GetName()) - log = log.WithValues("from-namespace", from.GetNamespace(), "from-name", from.GetName()) - - if allowed := meta.AllowsPropagationTo(from); !allowed[req.NamespacedName] { - // The propagating secret did not expect this propagated secret. We - // assume we have a watch on both secrets, and will be requeued if and - // when this situation is remedied. - log.Debug("Propagation not allowed") - return reconcile.Result{}, errors.New(errPropagationNotAllowed) - - } - - to.Data = from.Data - - // If our update was unsuccessful. Keep trying to update - // additional secrets but implicitly requeue when finished. - log.Debug("Propagated secret data") - record.Event(to, event.Normal(reasonPropagatedFrom, "Data propagated from secret")) - record.Event(from, event.Normal(reasonPropagatedTo, "Data propagated to secret")) - return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Update(ctx, to), errUpdateSecret) -} diff --git a/pkg/reconciler/secret/reconciler_test.go b/pkg/reconciler/secret/reconciler_test.go deleted file mode 100644 index d8edd7b..0000000 --- a/pkg/reconciler/secret/reconciler_test.go +++ /dev/null @@ -1,262 +0,0 @@ -/* -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 secret - -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/crossplane/crossplane-runtime/apis/core/v1alpha1" - "github.com/crossplane/crossplane-runtime/pkg/meta" - "github.com/crossplane/crossplane-runtime/pkg/resource" - "github.com/crossplane/crossplane-runtime/pkg/resource/fake" - "github.com/crossplane/crossplane-runtime/pkg/test" -) - -func TestReconciler(t *testing.T) { - type args struct { - m manager.Manager - } - - type want struct { - result reconcile.Result - err error - } - - mg := &fake.Managed{ - ObjectMeta: metav1.ObjectMeta{Name: "coolmanaged"}, - ConnectionSecretWriterTo: fake.ConnectionSecretWriterTo{Ref: &v1alpha1.SecretReference{ - Namespace: "coolns", - Name: "coolmanagedsecret", - }}, - } - tg := &fake.Target{ - ObjectMeta: metav1.ObjectMeta{Namespace: "coolns", Name: "cooltarget"}, - LocalConnectionSecretWriterTo: fake.LocalConnectionSecretWriterTo{Ref: &v1alpha1.LocalSecretReference{ - Name: "cooltargetsecret", - }}, - } - from := resource.ConnectionSecretFor(mg, fake.GVK(mg)) - to := resource.LocalConnectionSecretFor(tg, fake.GVK(tg)) - - fromData := map[string][]byte{"cool": {1}} - - errBoom := errors.New("boom") - - cases := map[string]struct { - args args - want want - }{ - "ToNotFound": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - return kerrors.NewNotFound(schema.GroupResource{}, "") - default: - return errors.New("unexpected secret name") - } - }, - }, - }, - }, - want: want{ - result: reconcile.Result{}, - }, - }, - "GetToError": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - return errBoom - default: - return errors.New("unexpected secret name") - } - }, - }, - }, - }, - want: want{ - err: errors.Wrap(errBoom, errGetSecret), - }, - }, - "FromNotFound": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - meta.AllowPropagation(from.DeepCopy(), o.(metav1.Object)) - case from.GetName(): - return kerrors.NewNotFound(schema.GroupResource{}, from.GetName()) - default: - return errors.New("unexpected secret name") - } - return nil - }, - }, - }, - }, - want: want{ - result: reconcile.Result{}, - }, - }, - "GetFromError": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - meta.AllowPropagation(from.DeepCopy(), o.(metav1.Object)) - case from.GetName(): - return errBoom - default: - return errors.New("unexpected secret name") - } - return nil - }, - }, - }, - }, - want: want{ - err: errors.Wrap(errBoom, errGetSecret), - }, - }, - "PropagationNotAllowedError": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - meta.AllowPropagation(from.DeepCopy(), o.(metav1.Object)) - case from.GetName(): - *o.(*corev1.Secret) = *from - 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{}, - err: errors.New(errPropagationNotAllowed), - }, - }, - "UpdateToError": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - s := to.DeepCopy() - meta.AllowPropagation(from.DeepCopy(), s) - *o.(*corev1.Secret) = *s - case from.GetName(): - meta.AllowPropagation(o.(metav1.Object), to.DeepCopy()) - 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), - }, - }, - "SuccessfulSingle": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch n.Name { - case to.GetName(): - s := to.DeepCopy() - meta.AllowPropagation(from.DeepCopy(), s) - *o.(*corev1.Secret) = *s - case from.GetName(): - s := from.DeepCopy() - s.Data = fromData - meta.AllowPropagation(s, to.DeepCopy()) - *o.(*corev1.Secret) = *s - default: - return errors.New("unexpected secret name") - } - return nil - }, - MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { - want := to.DeepCopy() - want.Data = fromData - meta.AllowPropagation(from.DeepCopy(), want) - 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 := NewReconciler(tc.args.m) - got, err := r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: to.GetNamespace(), Name: to.GetName()}}) - - 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) - } - }) - } -} diff --git a/pkg/reconciler/target/doc.go b/pkg/reconciler/target/doc.go deleted file mode 100644 index f155988..0000000 --- a/pkg/reconciler/target/doc.go +++ /dev/null @@ -1,20 +0,0 @@ -/* -Copyright 2020 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 target provides a reconciler that propagates a Kubernetes cluster -// managed resource's connection secret to a Kubernetes target. -// Deprecated: See https://github.com/crossplane/crossplane/issues/1595 -package target diff --git a/pkg/reconciler/target/reconciler.go b/pkg/reconciler/target/reconciler.go deleted file mode 100644 index 7f0b6bd..0000000 --- a/pkg/reconciler/target/reconciler.go +++ /dev/null @@ -1,229 +0,0 @@ -/* -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 target - -import ( - "context" - "strings" - "time" - - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" - "github.com/crossplane/crossplane-runtime/pkg/event" - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/crossplane/crossplane-runtime/pkg/meta" - "github.com/crossplane/crossplane-runtime/pkg/resource" -) - -const ( - targetReconcileTimeout = 1 * time.Minute - aShortWait = 30 * time.Second -) - -// Error strings -const ( - errGetTarget = "unable to get Target" - errUpdateTarget = "unable to update Target" -) - -// Event reasons. -const ( - reasonSetSecretRef event.Reason = "SetSecretRef" - reasonCannotGetManaged event.Reason = "CannotGetManaged" - reasonCannotPropagateSecret event.Reason = "CannotPropagateSecret" - reasonPropagatedSecret event.Reason = "PropagatedSecret" -) - -// TypeSecretPropagated resources have had connection information -// propagated to their secret reference. -const TypeSecretPropagated v1alpha1.ConditionType = "ConnectionSecretPropagated" - -// NOTE(negz): This reconciler exists to service KubernetesApplications, which -// are deprecated per https://github.com/crossplane/crossplane/issues/1595. It -// had a PropagationSuccess condition but did not use it. I elected not to fix -// this due to its impending removal. - -// ReasonSecretPropagationError indicates that secret propagation failed. -const ReasonSecretPropagationError v1alpha1.ConditionReason = "PropagationError" - -// ControllerName returns the recommended name for controllers that use this -// package to reconcile a particular kind of target. -func ControllerName(kind string) string { - return "target/" + strings.ToLower(kind) -} - -// A Reconciler reconciles targets by propagating the secret of the -// referenced managed resource. -type Reconciler struct { - client client.Client - newTarget func() resource.Target - newManaged func() resource.Managed - - propagator resource.ManagedConnectionPropagator - - log logging.Logger - record event.Recorder -} - -// A ReconcilerOption configures a Reconciler. -type ReconcilerOption func(*Reconciler) - -// WithManagedConnectionPropagator specifies which ManagedConnectionPropagator -// should be used to propagate resource connection details to their target. -func WithManagedConnectionPropagator(p resource.ManagedConnectionPropagator) ReconcilerOption { - return func(r *Reconciler) { - r.propagator = p - } -} - -// WithLogger specifies how the Reconciler should log messages. -func WithLogger(l logging.Logger) ReconcilerOption { - return func(r *Reconciler) { - r.log = l - } -} - -// WithRecorder specifies how the Reconciler should record events. -func WithRecorder(er event.Recorder) ReconcilerOption { - return func(r *Reconciler) { - r.record = er - } -} - -// NewReconciler returns a Reconciler that reconciles KubernetesTargets by -// propagating the referenced Kubernetes cluster's connection Secret to the -// namespace of the KubernetesTarget. -func NewReconciler(m manager.Manager, of resource.TargetKind, with resource.ManagedKind, o ...ReconcilerOption) *Reconciler { - nt := func() resource.Target { - return resource.MustCreateObject(schema.GroupVersionKind(of), m.GetScheme()).(resource.Target) - } - nr := func() resource.Managed { - return resource.MustCreateObject(schema.GroupVersionKind(with), m.GetScheme()).(resource.Managed) - } - - // Panic early if we've been asked to reconcile a target or resource kind - // that has not been registered with our controller manager's scheme. - _, _ = nt(), nr() - - r := &Reconciler{ - client: m.GetClient(), - newTarget: nt, - newManaged: nr, - - // TODO(negz): Switch to resource.ConnectionPropagator after this has - // been deprecated for a release or two. - //nolint:staticcheck - propagator: resource.NewAPIManagedConnectionPropagator(m.GetClient(), m.GetScheme()), - - log: logging.NewNopLogger(), - record: event.NewNopRecorder(), - } - - for _, ro := range o { - ro(r) - } - - return r -} - -// TODO(negz): Could we use a regular ReconcileError instead of a bespoke -// SecretPropagationError for this controller? - -// Reconcile a target with a concrete managed resource. -func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) { - log := r.log.WithValues("request", req) - log.Debug("Reconciling") - - ctx, cancel := context.WithTimeout(context.Background(), targetReconcileTimeout) - defer cancel() - - target := r.newTarget() - if err := r.client.Get(ctx, req.NamespacedName, target); err != nil { - // There's no need to requeue if we no longer exist. Otherwise we'll be - // requeued implicitly because we return an error. - log.Debug("Cannot get target", "error", err) - return reconcile.Result{}, errors.Wrap(resource.IgnoreNotFound(err), errGetTarget) - } - - // Our watch predicates ensure we only reconcile targets with a non-nil - // resource reference. - record := r.record.WithAnnotations("managed-name", target.GetResourceReference().Name) - log = log.WithValues( - "uid", target.GetUID(), - "version", target.GetResourceVersion(), - "managed-name", target.GetResourceReference().Name, - ) - - if target.GetWriteConnectionSecretToReference() == nil { - // If the ConnectionSecretRef is not set on this Target, we generate a - // Secret name that matches the UID of the Target. We are implicitly - // requeued because of the Target update. - log.Debug("Set secret reference to target UID") - record.Event(target, event.Normal(reasonSetSecretRef, "Set secret reference to target UID")) - target.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{Name: string(target.GetUID())}) - return reconcile.Result{}, errors.Wrap(r.client.Update(ctx, target), errUpdateTarget) - } - - // TODO(negz): Move this above the secret ref check. Is this needed at all? - if meta.WasDeleted(target) { - // If the Target was deleted, there is nothing left for us to do. - return reconcile.Result{Requeue: false}, nil - } - - managed := r.newManaged() - if err := r.client.Get(ctx, meta.NamespacedNameOf(target.GetResourceReference()), managed); err != nil { - log.Debug("Cannot get referenced managed resource", "error", err, "requeue-after", time.Now().Add(aShortWait)) - record.Event(target, event.Warning(reasonCannotGetManaged, err)) - target.SetConditions(SecretPropagationError(err)) - return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, target), errUpdateTarget) - } - - if err := r.propagator.PropagateConnection(ctx, target, managed); err != nil { - // If we fail to propagate the connection secret of a bound managed - // resource, we try again after a short wait. - log.Debug("Cannot propagate connection secret", "error", err, "requeue-after", time.Now().Add(aShortWait)) - record.Event(target, event.Warning(reasonCannotPropagateSecret, err)) - target.SetConditions(SecretPropagationError(err)) - return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, target), errUpdateTarget) - } - - // No need to requeue. - log.Debug("Successfully propagated connection secret") - record.Event(target, event.Normal(reasonPropagatedSecret, "Successfully propagated connection secret")) - return reconcile.Result{Requeue: false}, nil -} - -// SecretPropagationError returns a condition indicating that Crossplane was -// unable to propagate connection data to the referenced secret. This could be -// because it was unable to find the managed resource that owns the secret to be -// propagated. -func SecretPropagationError(err error) v1alpha1.Condition { - return v1alpha1.Condition{ - Type: TypeSecretPropagated, - Status: corev1.ConditionFalse, - LastTransitionTime: metav1.Now(), - Reason: ReasonSecretPropagationError, - Message: err.Error(), - } -} diff --git a/pkg/reconciler/target/reconciler_test.go b/pkg/reconciler/target/reconciler_test.go deleted file mode 100644 index 7e68d48..0000000 --- a/pkg/reconciler/target/reconciler_test.go +++ /dev/null @@ -1,384 +0,0 @@ -/* -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 target - -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/crossplane/crossplane-runtime/apis/core/v1alpha1" - "github.com/crossplane/crossplane-runtime/pkg/resource" - "github.com/crossplane/crossplane-runtime/pkg/resource/fake" - "github.com/crossplane/crossplane-runtime/pkg/test" -) - -func TestReconciler(t *testing.T) { - type args struct { - m manager.Manager - of resource.TargetKind - with resource.ManagedKind - o []ReconcilerOption - } - - type want struct { - result reconcile.Result - err error - } - - now := metav1.Now() - ns := "namespace" - tgname := "cooltarget" - mgname := "coolmanaged" - tguid := types.UID("tg-uuid") - mguid := types.UID("mg-uuid") - tgcsname := "cooltargetsecret" - mgcsname := "coolmanagedsecret" - mgcsnamespace := "coolns" - - errBoom := errors.New("boom") - errUnexpected := errors.New("unexpected object type") - - cases := map[string]struct { - args args - want want - }{ - "ErrorGetTarget": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - *o = fake.Target{} - return errBoom - default: - return errUnexpected - } - }, - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - }, - want: want{ - result: reconcile.Result{}, - err: errors.Wrap(errBoom, errGetTarget), - }, - }, - "SuccessTargetHasNoSecretRef": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - tg := &fake.Target{ObjectMeta: metav1.ObjectMeta{ - UID: tguid, - Name: tgname, - Namespace: ns, - }} - tg.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - *o = *tg - return nil - default: - return errUnexpected - } - }, - MockUpdate: test.NewMockUpdateFn(nil, func(got runtime.Object) error { - want := &fake.Target{} - want.SetName(tgname) - want.SetNamespace(ns) - want.SetUID(tguid) - want.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - want.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{ - Name: string(tguid), - }) - if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" { - t.Errorf("-want, +got:\n%s", diff) - } - return nil - }), - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - }, - want: want{ - result: reconcile.Result{}, - }, - }, - "TargetWasDeleted": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - tg := &fake.Target{ObjectMeta: metav1.ObjectMeta{ - UID: tguid, - Name: tgname, - Namespace: ns, - }} - tg.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - tg.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{ - Name: tgcsname, - }) - tg.SetDeletionTimestamp(&now) - *o = *tg - return nil - default: - return errUnexpected - } - }, - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - }, - want: want{ - result: reconcile.Result{Requeue: false}, - }, - }, - "TargetNotFound": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - *o = fake.Target{} - return kerrors.NewNotFound(schema.GroupResource{}, "") - default: - return errUnexpected - } - }, - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - }, - want: want{ - result: reconcile.Result{}, - }, - }, - "ErrorGetManaged": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - tg := &fake.Target{ObjectMeta: metav1.ObjectMeta{ - UID: tguid, - Name: tgname, - Namespace: ns, - }} - tg.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - tg.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{ - Name: tgcsname, - }) - *o = *tg - return nil - case *fake.Managed: - *o = fake.Managed{} - return errBoom - default: - return errUnexpected - } - }, - MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error { - want := &fake.Target{} - want.SetName(tgname) - want.SetNamespace(ns) - want.SetUID(tguid) - want.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - want.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{Name: tgcsname}) - want.SetConditions(SecretPropagationError(errBoom)) - if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" { - t.Errorf("-want, +got:\n%s", diff) - } - return nil - }), - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - }, - want: want{ - result: reconcile.Result{RequeueAfter: aShortWait}, - }, - }, - "ErrorSecretPropagationFailed": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - tg := &fake.Target{ObjectMeta: metav1.ObjectMeta{ - UID: tguid, - Name: tgname, - Namespace: ns, - }} - tg.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - tg.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{ - Name: tgcsname, - }) - *o = *tg - return nil - case *fake.Managed: - mg := &fake.Managed{ObjectMeta: metav1.ObjectMeta{ - UID: mguid, - Name: mgname, - }} - mg.SetWriteConnectionSecretToReference(&v1alpha1.SecretReference{ - Name: mgcsname, - Namespace: mgcsnamespace, - }) - *o = *mg - return nil - default: - return errUnexpected - } - }, - MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error { - want := &fake.Target{} - want.SetName(tgname) - want.SetNamespace(ns) - want.SetUID(tguid) - want.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - want.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{Name: tgcsname}) - want.SetConditions(SecretPropagationError(errBoom)) - if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" { - t.Errorf("-want, +got:\n%s", diff) - } - return nil - }), - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - o: []ReconcilerOption{ - WithManagedConnectionPropagator(resource.ManagedConnectionPropagatorFn( - func(_ context.Context, _ resource.LocalConnectionSecretOwner, _ resource.Managed) error { - return errBoom - }, - )), - }, - }, - want: want{ - result: reconcile.Result{RequeueAfter: aShortWait}, - }, - }, - "Successful": { - args: args{ - m: &fake.Manager{ - Client: &test.MockClient{ - MockGet: func(_ context.Context, n types.NamespacedName, o runtime.Object) error { - switch o := o.(type) { - case *fake.Target: - tg := &fake.Target{ObjectMeta: metav1.ObjectMeta{ - UID: tguid, - Name: tgname, - Namespace: ns, - }} - tg.SetResourceReference(&corev1.ObjectReference{ - Name: mgname, - }) - tg.SetWriteConnectionSecretToReference(&v1alpha1.LocalSecretReference{ - Name: tgcsname, - }) - *o = *tg - return nil - case *fake.Managed: - mg := &fake.Managed{ObjectMeta: metav1.ObjectMeta{ - UID: mguid, - Name: mgname, - }} - mg.SetWriteConnectionSecretToReference(&v1alpha1.SecretReference{ - Name: mgcsname, - Namespace: mgcsnamespace, - }) - *o = *mg - return nil - default: - return errUnexpected - } - }, - }, - Scheme: fake.SchemeWith(&fake.Target{}, &fake.Managed{}), - }, - of: resource.TargetKind(fake.GVK(&fake.Target{})), - with: resource.ManagedKind(fake.GVK(&fake.Managed{})), - o: []ReconcilerOption{ - WithManagedConnectionPropagator(resource.ManagedConnectionPropagatorFn( - func(_ context.Context, _ resource.LocalConnectionSecretOwner, _ resource.Managed) error { return nil }, - )), - }, - }, - want: want{ - result: reconcile.Result{Requeue: false}, - }, - }, - } - - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - r := NewReconciler(tc.args.m, tc.args.of, tc.args.with, tc.args.o...) - got, err := r.Reconcile(reconcile.Request{}) - - 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) - } - }) - } -} diff --git a/pkg/resource/api.go b/pkg/resource/api.go index 42ecc78..102731a 100644 --- a/pkg/resource/api.go +++ b/pkg/resource/api.go @@ -47,13 +47,6 @@ type APIManagedConnectionPropagator struct { Propagator ConnectionPropagator } -// NewAPIManagedConnectionPropagator returns a new APIConnectionPropagator. -// -// Deprecated: Use NewAPIConnectionPropagator. -func NewAPIManagedConnectionPropagator(c client.Client, t runtime.ObjectTyper) *APIManagedConnectionPropagator { - return &APIManagedConnectionPropagator{Propagator: NewAPIConnectionPropagator(c, t)} -} - // PropagateConnection details from the supplied resource. func (a *APIManagedConnectionPropagator) PropagateConnection(ctx context.Context, to LocalConnectionSecretOwner, mg Managed) error { return a.Propagator.PropagateConnection(ctx, to, mg) diff --git a/pkg/resource/enqueue_handlers.go b/pkg/resource/enqueue_handlers.go index e5f3edc..1bafa3a 100644 --- a/pkg/resource/enqueue_handlers.go +++ b/pkg/resource/enqueue_handlers.go @@ -17,62 +17,17 @@ limitations under the License. package resource import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "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" - - "github.com/crossplane/crossplane-runtime/pkg/meta" ) type adder interface { Add(item interface{}) } -// EnqueueRequestForPropagated enqueues a reconcile.Request for the -// NamespacedName of a propagated object, i.e. an object with propagation -// metadata annotations. -type EnqueueRequestForPropagated struct{} - -// Create adds a NamespacedName for the supplied CreateEvent if its Object is -// propagated. -func (e *EnqueueRequestForPropagated) Create(evt event.CreateEvent, q workqueue.RateLimitingInterface) { - addPropagated(evt.Object, q) -} - -// Update adds a NamespacedName for the supplied UpdateEvent if its Objects are -// propagated. -func (e *EnqueueRequestForPropagated) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) { - addPropagated(evt.ObjectOld, q) - addPropagated(evt.ObjectNew, q) -} - -// Delete adds a NamespacedName for the supplied DeleteEvent if its Object is -// propagated. -func (e *EnqueueRequestForPropagated) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInterface) { - addPropagated(evt.Object, q) -} - -// Generic adds a NamespacedName for the supplied GenericEvent if its Object is -// propagated. -func (e *EnqueueRequestForPropagated) Generic(evt event.GenericEvent, q workqueue.RateLimitingInterface) { - addPropagated(evt.Object, q) -} - -func addPropagated(obj runtime.Object, queue adder) { - o, ok := obj.(metav1.Object) - if !ok { - return - } - - // Otherwise we should enqueue a request for the objects it propagates to. - for nn := range meta.AllowsPropagationTo(o) { - queue.Add(reconcile.Request{NamespacedName: nn}) - } -} - // EnqueueRequestForProviderConfig enqueues a reconcile.Request for a referenced // ProviderConfig. type EnqueueRequestForProviderConfig struct{} diff --git a/pkg/resource/enqueue_handlers_test.go b/pkg/resource/enqueue_handlers_test.go index d169ddd..556daa6 100644 --- a/pkg/resource/enqueue_handlers_test.go +++ b/pkg/resource/enqueue_handlers_test.go @@ -20,19 +20,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" - 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" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" - "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/resource/fake" ) var ( - _ handler.EventHandler = &EnqueueRequestForPropagated{} + _ handler.EventHandler = &EnqueueRequestForProviderConfig{} ) type addFn func(item interface{}) @@ -41,46 +39,6 @@ func (fn addFn) Add(item interface{}) { fn(item) } -func TestAddPropagated(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") }), - }, - "ObjectMissingAnnotation": { - obj: &fake.Managed{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ - "some.annotation": "some-value", - }}}, - queue: addFn(func(_ interface{}) { t.Errorf("queue.Add() called unexpectedly") }), - }, - "IsPropagator": { - obj: func() runtime.Object { - tg := &fake.Target{} - tg.SetNamespace(ns) - tg.SetName(name) - mg := &fake.Managed{} - meta.AllowPropagation(mg, tg) - return mg - }(), - 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 { - addPropagated(tc.obj, tc.queue) - } -} - func TestAddProviderConfig(t *testing.T) { name := "coolname" diff --git a/pkg/resource/fake/mocks.go b/pkg/resource/fake/mocks.go index 61acdb5..e853fb0 100644 --- a/pkg/resource/fake/mocks.go +++ b/pkg/resource/fake/mocks.go @@ -243,30 +243,6 @@ func (m *Managed) DeepCopyObject() runtime.Object { return out } -// Target is a mock that implements Target interface. -type Target struct { - metav1.ObjectMeta - ManagedResourceReferencer - LocalConnectionSecretWriterTo - v1alpha1.ConditionedStatus -} - -// GetObjectKind returns schema.ObjectKind. -func (m *Target) GetObjectKind() schema.ObjectKind { - return schema.EmptyObjectKind -} - -// DeepCopyObject returns a deep copy of Target as runtime.Object. -func (m *Target) DeepCopyObject() runtime.Object { - out := &Target{} - j, err := json.Marshal(m) - if err != nil { - panic(err) - } - _ = json.Unmarshal(j, out) - return out -} - // Composite is a mock that implements Composite interface. type Composite struct { metav1.ObjectMeta diff --git a/pkg/resource/interfaces.go b/pkg/resource/interfaces.go index 4575914..71846ad 100644 --- a/pkg/resource/interfaces.go +++ b/pkg/resource/interfaces.go @@ -177,17 +177,6 @@ type ProviderConfigUsageList interface { GetItems() []ProviderConfigUsage } -// A Target is a Kubernetes object that refers to credentials to connect -// to a deployment target. Target is a subset of the Claim interface. -type Target interface { - Object - - LocalConnectionSecretWriterTo - ManagedResourceReferencer - - Conditioned -} - // A Composite resource composes one or more Composed resources. type Composite interface { Object diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index 87b6a16..c59c4b9 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -47,9 +47,6 @@ const ( // A ManagedKind contains the type metadata for a kind of managed resource. type ManagedKind schema.GroupVersionKind -// A TargetKind contains the type metadata for a kind of target resource. -type TargetKind schema.GroupVersionKind - // A CompositeKind contains the type metadata for a kind of composite resource. type CompositeKind schema.GroupVersionKind