From f7ac2fb02dbc73bd087e46522ae17f3c95639967 Mon Sep 17 00:00:00 2001 From: Hasan Turken Date: Thu, 31 Aug 2023 14:34:58 +0300 Subject: [PATCH] Handle if neither reference nor selector provided Also: - Resolve comments in Usage - Remove composite check during deletion Signed-off-by: Hasan Turken --- .github/workflows/ci.yml | 2 +- .../apiextensions/usage/reconciler.go | 15 ++- .../apiextensions/usage/reconciler_test.go | 97 ++++------------ .../apiextensions/usage/resource/resource.go | 109 ------------------ .../apiextensions/usage/selector.go | 18 ++- .../apiextensions/usage/selector_test.go | 10 +- internal/usage/handler.go | 8 +- internal/usage/handler_test.go | 8 +- test/e2e/funcs/feature.go | 6 +- test/e2e/usage_test.go | 15 +-- 10 files changed, 66 insertions(+), 222 deletions(-) delete mode 100644 internal/controller/apiextensions/usage/resource/resource.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1a4e5fa5..03d0cb617 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -244,7 +244,7 @@ jobs: test-suite: - base - composition-webhook-schema-validation - - environment-config + - environment-configs - usage steps: diff --git a/internal/controller/apiextensions/usage/reconciler.go b/internal/controller/apiextensions/usage/reconciler.go index 1fc81ca79..e18758d8c 100644 --- a/internal/controller/apiextensions/usage/reconciler.go +++ b/internal/controller/apiextensions/usage/reconciler.go @@ -37,12 +37,11 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/ratelimiter" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" apiextensionscontroller "github.com/crossplane/crossplane/internal/controller/apiextensions/controller" - "github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource" "github.com/crossplane/crossplane/internal/usage" - "github.com/crossplane/crossplane/internal/xcrd" ) const ( @@ -215,8 +214,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco of := u.Spec.Of by := u.Spec.By - // Identify used xp resource as an unstructured object. - used := resource.New(resource.FromReference(v1.ObjectReference{ + // Identify used xp composed as an unstructured object. + used := composed.New(composed.FromReference(v1.ObjectReference{ Kind: of.Kind, Name: of.ResourceRef.Name, APIVersion: of.APIVersion, @@ -225,7 +224,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco if meta.WasDeleted(u) { if by != nil { // Identify using resource as an unstructured object. - using := resource.New(resource.FromReference(v1.ObjectReference{ + using := composed.New(composed.FromReference(v1.ObjectReference{ Kind: by.Kind, Name: by.ResourceRef.Name, APIVersion: by.APIVersion, @@ -239,8 +238,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, err } - if l := u.GetLabels()[xcrd.LabelKeyNamePrefixForComposed]; len(l) > 0 && l == using.GetLabels()[xcrd.LabelKeyNamePrefixForComposed] && err == nil { - // If the usage and using resource are part of the same composite resource, we need to wait for the using resource to be deleted + if err == nil { + // Using resource is still there, so we need to wait for it to be deleted. msg := "Waiting for using resource to be deleted." log.Debug(msg) r.record.Event(u, event.Normal(reasonWaitUsing, msg)) @@ -341,7 +340,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco if by != nil { // Identify using resource as an unstructured object. - using := resource.New(resource.FromReference(v1.ObjectReference{ + using := composed.New(composed.FromReference(v1.ObjectReference{ Kind: by.Kind, Name: by.ResourceRef.Name, APIVersion: by.APIVersion, diff --git a/internal/controller/apiextensions/usage/reconciler_test.go b/internal/controller/apiextensions/usage/reconciler_test.go index 666576156..81bddf71c 100644 --- a/internal/controller/apiextensions/usage/reconciler_test.go +++ b/internal/controller/apiextensions/usage/reconciler_test.go @@ -18,10 +18,10 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/errors" xpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/crossplane/crossplane-runtime/pkg/resource/fake" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed" "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" - "github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource" "github.com/crossplane/crossplane/internal/xcrd" ) @@ -159,7 +159,7 @@ func TestReconcile(t *testing.T) { switch o := obj.(type) { case *v1alpha1.Usage: o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} - case *resource.Unstructured: + case *composed.Unstructured: return errBoom } return nil @@ -194,13 +194,13 @@ func TestReconcile(t *testing.T) { switch o := obj.(type) { case *v1alpha1.Usage: o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} - case *resource.Unstructured: + case *composed.Unstructured: return nil } return nil }), MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error { - if _, ok := obj.(*resource.Unstructured); ok { + if _, ok := obj.(*composed.Unstructured); ok { return errBoom } return nil @@ -235,7 +235,7 @@ func TestReconcile(t *testing.T) { o.Spec.By = &v1alpha1.Resource{ ResourceRef: &v1alpha1.ResourceRef{Name: "using"}, } - case *resource.Unstructured: + case *composed.Unstructured: if o.GetName() == "using" { return errBoom } @@ -274,7 +274,7 @@ func TestReconcile(t *testing.T) { } return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetName() == "using" { o.SetAPIVersion("v1") o.SetKind("AnotherKind") @@ -323,7 +323,7 @@ func TestReconcile(t *testing.T) { } return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetName() == "using" { o.SetAPIVersion("v1") o.SetKind("AnotherKind") @@ -380,13 +380,13 @@ func TestReconcile(t *testing.T) { o.Spec.Reason = &reason return nil } - if _, ok := obj.(*resource.Unstructured); ok { + if _, ok := obj.(*composed.Unstructured); ok { return nil } return errors.New("unexpected object type") }), MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error { - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetLabels()[inUseLabelKey] != "true" { t.Fatalf("expected %s label to be true", inUseLabelKey) } @@ -429,7 +429,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if _, ok := obj.(*resource.Unstructured); ok { + if _, ok := obj.(*composed.Unstructured); ok { return kerrors.NewNotFound(schema.GroupResource{}, "") } return errors.New("unexpected object type") @@ -463,7 +463,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if _, ok := obj.(*resource.Unstructured); ok { + if _, ok := obj.(*composed.Unstructured); ok { return errBoom } return errors.New("unexpected object type") @@ -502,7 +502,7 @@ func TestReconcile(t *testing.T) { } return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetName() == "used" { o.SetLabels(map[string]string{inUseLabelKey: "true"}) } @@ -539,7 +539,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { o.SetLabels(map[string]string{inUseLabelKey: "true"}) return nil } @@ -577,7 +577,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { o.SetLabels(map[string]string{inUseLabelKey: "true"}) return nil } @@ -618,7 +618,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if _, ok := obj.(*resource.Unstructured); ok { + if _, ok := obj.(*composed.Unstructured); ok { return kerrors.NewNotFound(schema.GroupResource{}, "") } return errors.New("unexpected object type") @@ -652,7 +652,7 @@ func TestReconcile(t *testing.T) { o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"} return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { o.SetLabels(map[string]string{inUseLabelKey: "true"}) return nil } @@ -662,7 +662,7 @@ func TestReconcile(t *testing.T) { return nil }), MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error { - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetLabels()[inUseLabelKey] != "" { t.Errorf("expected in use label to be removed") } @@ -686,63 +686,8 @@ func TestReconcile(t *testing.T) { r: reconcile.Result{}, }, }, - "SuccessfulDeleteWithUsedAndUsing": { - reason: "We should return no error once we have successfully deleted the usage resource with using resource defined.", - args: args{ - mgr: &fake.Manager{}, - opts: []ReconcilerOption{ - WithClientApplicator(xpresource.ClientApplicator{ - Client: &test.MockClient{ - MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { - if o, ok := obj.(*v1alpha1.Usage); ok { - o.SetDeletionTimestamp(&now) - o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "used"} - o.Spec.By = &v1alpha1.Resource{ - APIVersion: "v1", - Kind: "AnotherKind", - ResourceRef: &v1alpha1.ResourceRef{Name: "using"}, - } - return nil - } - if o, ok := obj.(*resource.Unstructured); ok { - if o.GetName() == "used" { - o.SetLabels(map[string]string{inUseLabelKey: "true"}) - return nil - } - return nil - } - return errors.New("unexpected object type") - }), - MockList: test.NewMockListFn(nil, func(obj client.ObjectList) error { - return nil - }), - MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error { - if o, ok := obj.(*resource.Unstructured); ok { - if o.GetLabels()[inUseLabelKey] != "" { - t.Errorf("expected in use label to be removed") - } - return nil - } - return errors.New("unexpected object type") - }), - }, - }), - WithSelectorResolver(fakeSelectorResolver{ - resourceSelectorFn: func(ctx context.Context, u *v1alpha1.Usage) error { - return nil - }, - }), - WithFinalizer(xpresource.FinalizerFns{RemoveFinalizerFn: func(_ context.Context, _ xpresource.Object) error { - return nil - }}), - }, - }, - want: want{ - r: reconcile.Result{}, - }, - }, - "SuccessfulWaitWhenUsageAndUsingPartOfSameComposite": { - reason: "We should wait until the using resource is deleted when usage and using resources are part of same composite.", + "SuccessfulWaitWhenUsingStillThere": { + reason: "We should wait until the using resource is deleted.", args: args{ mgr: &fake.Manager{}, opts: []ReconcilerOption{ @@ -760,7 +705,7 @@ func TestReconcile(t *testing.T) { } return nil } - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetName() == "used" { o.SetLabels(map[string]string{inUseLabelKey: "true"}) } @@ -775,7 +720,7 @@ func TestReconcile(t *testing.T) { return nil }), MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error { - if o, ok := obj.(*resource.Unstructured); ok { + if o, ok := obj.(*composed.Unstructured); ok { if o.GetLabels()[inUseLabelKey] != "" { t.Errorf("expected in use label to be removed") } diff --git a/internal/controller/apiextensions/usage/resource/resource.go b/internal/controller/apiextensions/usage/resource/resource.go deleted file mode 100644 index ab8192b87..000000000 --- a/internal/controller/apiextensions/usage/resource/resource.go +++ /dev/null @@ -1,109 +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 resource contains an unstructured resource. -package resource - -import ( - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/types" -) - -// An Option modifies an unstructured composed resource. -type Option func(*Unstructured) - -// FromReference returns an Option that propagates the metadata in the supplied -// reference to an unstructured composed resource. -func FromReference(ref corev1.ObjectReference) Option { - return func(cr *Unstructured) { - cr.SetGroupVersionKind(ref.GroupVersionKind()) - cr.SetName(ref.Name) - cr.SetNamespace(ref.Namespace) - cr.SetUID(ref.UID) - } -} - -// New returns a new unstructured composed resource. -func New(opts ...Option) *Unstructured { - cr := &Unstructured{unstructured.Unstructured{Object: make(map[string]any)}} - for _, f := range opts { - f(cr) - } - return cr -} - -// An Unstructured composed resource. -type Unstructured struct { - unstructured.Unstructured -} - -// GetUnstructured returns the underlying *unstructured.Unstructured. -func (cr *Unstructured) GetUnstructured() *unstructured.Unstructured { - return &cr.Unstructured -} - -// OwnedBy returns true if the supplied UID is an owner of the composed -func (cr *Unstructured) OwnedBy(u types.UID) bool { - for _, owner := range cr.GetOwnerReferences() { - if owner.UID == u { - return true - } - } - return false -} - -// RemoveOwnerRef removes the supplied UID from the composed resource's owner -func (cr *Unstructured) RemoveOwnerRef(u types.UID) { - refs := cr.GetOwnerReferences() - for i := range refs { - if refs[i].UID == u { - cr.SetOwnerReferences(append(refs[:i], refs[i+1:]...)) - return - } - } -} - -// An ListOption modifies an unstructured list of composed resource. -type ListOption func(*UnstructuredList) - -// FromReferenceToList returns a ListOption that propagates the metadata in the -// supplied reference to an unstructured list composed resource. -func FromReferenceToList(ref corev1.ObjectReference) ListOption { - return func(list *UnstructuredList) { - list.SetAPIVersion(ref.APIVersion) - list.SetKind(ref.Kind + "List") - } -} - -// NewList returns a new unstructured list of composed resources. -func NewList(opts ...ListOption) *UnstructuredList { - cr := &UnstructuredList{unstructured.UnstructuredList{Object: make(map[string]any)}} - for _, f := range opts { - f(cr) - } - return cr -} - -// An UnstructuredList of composed resources. -type UnstructuredList struct { - unstructured.UnstructuredList -} - -// GetUnstructuredList returns the underlying *unstructured.Unstructured. -func (cr *UnstructuredList) GetUnstructuredList() *unstructured.UnstructuredList { - return &cr.UnstructuredList -} diff --git a/internal/controller/apiextensions/usage/selector.go b/internal/controller/apiextensions/usage/selector.go index 5c89f4577..2ae95bbd4 100644 --- a/internal/controller/apiextensions/usage/selector.go +++ b/internal/controller/apiextensions/usage/selector.go @@ -8,18 +8,20 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/meta" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" - "github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource" ) const ( errUpdateAfterResolveSelector = "cannot update usage after resolving selector" - errResolveSelectorForUsingResource = "cannot resolve selector for using resource" - errResolveSelectorForUsedResource = "cannot resolve selector for used resource" + errResolveSelectorForUsingResource = "cannot resolve selector at \"spec.by.resourceSelector\"" + errResolveSelectorForUsedResource = "cannot resolve selector at \"spec.of.resourceSelector\"" errListResourceMatchingLabels = "cannot list resources matching labels" errFmtResourcesNotFound = "no %q found matching labels: %q" errFmtResourcesNotFoundWithControllerRef = "no %q found matching labels: %q and with same controller reference" + errIdentifyUsedResource = "cannot identify used resource, neither \"spec.of.resourceRef\" nor \"spec.of.resourceSelector\" is set" + errIdentifyUsingResource = "cannot identify using resource, neither \"spec.by.resourceRef\" nor \"spec.by.resourceSelector\" is set" ) type apiSelectorResolver struct { @@ -30,11 +32,14 @@ func newAPISelectorResolver(c client.Client) *apiSelectorResolver { return &apiSelectorResolver{client: c} } -func (r *apiSelectorResolver) resolveSelectors(ctx context.Context, u *v1alpha1.Usage) error { +func (r *apiSelectorResolver) resolveSelectors(ctx context.Context, u *v1alpha1.Usage) error { //nolint:gocyclo // we need to resolve both selectors so no real complexity rather a duplication of := u.Spec.Of by := u.Spec.By if of.ResourceRef == nil || len(of.ResourceRef.Name) == 0 { + if of.ResourceSelector == nil { + return errors.New(errIdentifyUsedResource) + } if err := r.resolveSelector(ctx, u, &of); err != nil { return errors.Wrap(err, errResolveSelectorForUsedResource) } @@ -49,6 +54,9 @@ func (r *apiSelectorResolver) resolveSelectors(ctx context.Context, u *v1alpha1. } if by.ResourceRef == nil || len(by.ResourceRef.Name) == 0 { + if by.ResourceSelector == nil { + return errors.New(errIdentifyUsingResource) + } if err := r.resolveSelector(ctx, u, by); err != nil { return errors.Wrap(err, errResolveSelectorForUsingResource) } @@ -62,7 +70,7 @@ func (r *apiSelectorResolver) resolveSelectors(ctx context.Context, u *v1alpha1. } func (r *apiSelectorResolver) resolveSelector(ctx context.Context, u *v1alpha1.Usage, rs *v1alpha1.Resource) error { - l := resource.NewList(resource.FromReferenceToList(v1.ObjectReference{ + l := composed.NewList(composed.FromReferenceToList(v1.ObjectReference{ APIVersion: rs.APIVersion, Kind: rs.Kind, })) diff --git a/internal/controller/apiextensions/usage/selector_test.go b/internal/controller/apiextensions/usage/selector_test.go index 2958b451a..bb0c9920c 100644 --- a/internal/controller/apiextensions/usage/selector_test.go +++ b/internal/controller/apiextensions/usage/selector_test.go @@ -10,10 +10,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed" "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/crossplane/crossplane/apis/apiextensions/v1alpha1" - "github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource" ) var errBoom = errors.New("boom") @@ -199,7 +199,7 @@ func TestResolveSelectors(t *testing.T) { args: args{ client: &test.MockClient{ MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - l := list.(*resource.UnstructuredList) + l := list.(*composed.UnstructuredList) switch l.GetKind() { case "SomeKindList": l.Items = []unstructured.Unstructured{ @@ -245,7 +245,7 @@ func TestResolveSelectors(t *testing.T) { args: args{ client: &test.MockClient{ MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - l := list.(*resource.UnstructuredList) + l := list.(*composed.UnstructuredList) switch l.GetKind() { case "AnotherKindList": l.Items = []unstructured.Unstructured{ @@ -325,7 +325,7 @@ func TestResolveSelectors(t *testing.T) { args: args{ client: &test.MockClient{ MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - l := list.(*resource.UnstructuredList) + l := list.(*composed.UnstructuredList) switch l.GetKind() { case "SomeKindList": l.Items = []unstructured.Unstructured{ @@ -372,7 +372,7 @@ func TestResolveSelectors(t *testing.T) { args: args{ client: &test.MockClient{ MockList: test.NewMockListFn(nil, func(list client.ObjectList) error { - l := list.(*resource.UnstructuredList) + l := list.(*composed.UnstructuredList) switch l.GetKind() { case "SomeKindList": l.Items = []unstructured.Unstructured{ diff --git a/internal/usage/handler.go b/internal/usage/handler.go index 426d6be28..e4989d884 100644 --- a/internal/usage/handler.go +++ b/internal/usage/handler.go @@ -19,7 +19,6 @@ package usage import ( "context" - "errors" "fmt" "net/http" @@ -32,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/crossplane/crossplane-runtime/pkg/controller" + "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/logging" xpunstructured "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured" @@ -45,7 +45,7 @@ const ( InUseIndexKey = "inuse.apiversion.kind.name" // Error strings. - errUnexpectedOp = "unexpected operation" + errFmtUnexpectedOp = "unexpected operation %q, expected \"DELETE\"" ) // IndexValueForObject returns the index value for the given object. @@ -112,7 +112,7 @@ func NewHandler(reader client.Reader, opts ...HandlerOption) *Handler { func (h *Handler) Handle(ctx context.Context, request admission.Request) admission.Response { switch request.Operation { case admissionv1.Create, admissionv1.Update, admissionv1.Connect: - return admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)) + return admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, request.Operation)) case admissionv1.Delete: u := &unstructured.Unstructured{} if err := u.UnmarshalJSON(request.OldObject.Raw); err != nil { @@ -120,7 +120,7 @@ func (h *Handler) Handle(ctx context.Context, request admission.Request) admissi } return h.validateNoUsages(ctx, u) default: - return admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)) + return admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, request.Operation)) } } diff --git a/internal/usage/handler_test.go b/internal/usage/handler_test.go index bad337fbb..51151a42e 100644 --- a/internal/usage/handler_test.go +++ b/internal/usage/handler_test.go @@ -47,7 +47,7 @@ func TestHandle(t *testing.T) { }, }, want: want{ - resp: admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)), + resp: admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, admissionv1.Create)), }, }, "UnexpectedConnect": { @@ -60,7 +60,7 @@ func TestHandle(t *testing.T) { }, }, want: want{ - resp: admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)), + resp: admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, admissionv1.Connect)), }, }, "UnexpectedUpdate": { @@ -73,7 +73,7 @@ func TestHandle(t *testing.T) { }, }, want: want{ - resp: admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)), + resp: admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, admissionv1.Update)), }, }, "UnexpectedOperation": { @@ -86,7 +86,7 @@ func TestHandle(t *testing.T) { }, }, want: want{ - resp: admission.Errored(http.StatusBadRequest, errors.New(errUnexpectedOp)), + resp: admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, admissionv1.Operation("unknown"))), }, }, "DeleteWithoutOldObj": { diff --git a/test/e2e/funcs/feature.go b/test/e2e/funcs/feature.go index 8b42e1cd2..8f02f1282 100644 --- a/test/e2e/funcs/feature.go +++ b/test/e2e/funcs/feature.go @@ -544,7 +544,7 @@ func ListedResourcesValidatedWithin(d time.Duration, list k8s.ObjectList, min in // is not deleted within the supplied duration. func ListedResourcesDeletedWithin(d time.Duration, list k8s.ObjectList, listOptions ...resources.ListOption) features.Func { return func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { - if err := c.Client().Resources().List(context.TODO(), list, listOptions...); err != nil { + if err := c.Client().Resources().List(ctx, list, listOptions...); err != nil { return ctx } if err := wait.For(conditions.New(c.Client().Resources()).ResourcesDeleted(list), wait.WithTimeout(d)); err != nil { @@ -563,7 +563,7 @@ func ListedResourcesDeletedWithin(d time.Duration, list k8s.ObjectList, listOpti // not modified within the supplied duration. func ListedResourcesModifiedWith(list k8s.ObjectList, min int, modify func(object k8s.Object), listOptions ...resources.ListOption) features.Func { return func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { - if err := c.Client().Resources().List(context.TODO(), list, listOptions...); err != nil { + if err := c.Client().Resources().List(ctx, list, listOptions...); err != nil { return ctx } var found int @@ -574,7 +574,7 @@ func ListedResourcesModifiedWith(list k8s.ObjectList, min int, modify func(objec for _, obj := range metaList { if o, ok := obj.(k8s.Object); ok { modify(o) - if err = c.Client().Resources().Update(context.Background(), o); err != nil { + if err = c.Client().Resources().Update(ctx, o); err != nil { t.Errorf("failed to update resource %s/%s: %v", o.GetNamespace(), o.GetName(), err) return ctx } diff --git a/test/e2e/usage_test.go b/test/e2e/usage_test.go index ea262bb0b..652bb6180 100644 --- a/test/e2e/usage_test.go +++ b/test/e2e/usage_test.go @@ -1,18 +1,19 @@ package e2e import ( - xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - "github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource" + "testing" + "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/e2e-framework/klient/k8s" "sigs.k8s.io/e2e-framework/klient/k8s/resources" - "testing" - "time" - "sigs.k8s.io/e2e-framework/pkg/features" "sigs.k8s.io/e2e-framework/third_party/helm" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed" + apiextensionsv1 "github.com/crossplane/crossplane/apis/apiextensions/v1" pkgv1 "github.com/crossplane/crossplane/apis/pkg/v1" "github.com/crossplane/crossplane/test/e2e/config" @@ -122,12 +123,12 @@ func TestUsageStandalone(t *testing.T) { func TestUsageComposition(t *testing.T) { manifests := "test/e2e/manifests/apiextensions/usage/composition" - nopList := resource.NewList(resource.FromReferenceToList(corev1.ObjectReference{ + nopList := composed.NewList(composed.FromReferenceToList(corev1.ObjectReference{ APIVersion: "nop.crossplane.io/v1alpha1", Kind: "NopResource", })) - usageList := resource.NewList(resource.FromReferenceToList(corev1.ObjectReference{ + usageList := composed.NewList(composed.FromReferenceToList(corev1.ObjectReference{ APIVersion: "apiextensions.crossplane.io/v1alpha1", Kind: "Usage", }))