Merge pull request #5354 from crossplane/backport-5353-to-release-1.15

internal/controller/apiextensions/usage/reconciler.go

@@ -28,6 +28,7 @@ import (
 	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"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -70,6 +71,7 @@ const (
 	errAddFinalizer         = "cannot add finalizer"
 	errRemoveFinalizer      = "cannot remove finalizer"
 	errUpdateStatus         = "cannot update status of usage"
+	errParseAPIVersion      = "cannot parse APIVersion"
 )
 
 // Event reasons.
@@ -216,6 +218,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		log.Debug(errGetUsage, "error", err)
 		return reconcile.Result{}, errors.Wrap(xpresource.IgnoreNotFound(err), errGetUsage)
 	}
+
+	// Validate APIVersion of used object provided as input.
+	// We parse this value while indexing the objects, and we need to make sure it is valid.
+	_, err := schema.ParseGroupVersion(u.Spec.Of.APIVersion)
+	if err != nil {
+		return reconcile.Result{}, errors.Wrap(err, errParseAPIVersion)
+	}
+
 	orig := u.DeepCopy()
 
 	if err := r.usage.resolveSelectors(ctx, u); err != nil {

internal/controller/apiextensions/usage/reconciler_test.go

@@ -82,6 +82,27 @@ func TestReconcile(t *testing.T) {
 				r: reconcile.Result{},
 			},
 		},
+		"CannotParseApiVersion": {
+			reason: "We should return an error if we cannot parse APIVersion of used resource.",
+			args: args{
+				mgr: &fake.Manager{},
+				opts: []ReconcilerOption{
+					WithClientApplicator(xpresource.ClientApplicator{
+						Client: &test.MockClient{
+							MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
+								o := obj.(*v1alpha1.Usage)
+								o.Spec.Of.APIVersion = "/invalid/"
+								o.Spec.Of.ResourceSelector = &v1alpha1.ResourceSelector{MatchLabels: map[string]string{"foo": "bar"}}
+								return nil
+							}),
+						},
+					}),
+				},
+			},
+			want: want{
+				err: errors.Wrap(errors.New("unexpected GroupVersion string: /invalid/"), errParseAPIVersion),
+			},
+		},
 		"CannotResolveSelectors": {
 			reason: "We should return an error if we cannot resolve selectors.",
 			args: args{

internal/usage/handler.go

@@ -25,6 +25,7 @@ import (
 	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -53,7 +54,14 @@ func IndexValueForObject(u *unstructured.Unstructured) string {
 }
 
 func indexValue(apiVersion, kind, name string) string {
-	return fmt.Sprintf("%s.%s.%s", apiVersion, kind, name)
+	// There are two sources for "apiVersion" input, one is from the unstructured objects fetched from k8s and the other
+	// is from the Usage spec. The one from the objects from k8s is already validated by the k8s API server, so we don't
+	// need to validate it again. The one from the Usage spec is validated by the Usage controller, so we don't need to
+	// validate it as well. So we can safely ignore the error here.
+	// Another reason to ignore the error is that the IndexerFunc using this value to index the objects does not return
+	// an error, so we cannot bubble up the error here.
+	gr, _ := schema.ParseGroupVersion(apiVersion)
+	return fmt.Sprintf("%s.%s.%s", gr.Group, kind, name)
 }
 
 // SetupWebhookWithManager sets up the webhook with the manager.