Convert package metadata to v1 once

We need to handle package metadata that might be v1alpha1 or v1beta1 by
converting it to v1. We were doing this twice when resolving
dependencies in the lock - one on the caller side and once inside the
implementation. Now we only do it on the caller side.

Signed-off-by: Nic Cope <[email protected]>

Author: negz

apis/pkg/meta/v1/interfaces.go

@@ -16,6 +16,11 @@ limitations under the License.
 
 package v1
 
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	runtime "k8s.io/apimachinery/pkg/runtime"
+)
+
 var (
 	_ Pkg = &Configuration{}
 	_ Pkg = &Provider{}
@@ -25,6 +30,9 @@ var (
 // Pkg is a description of a Crossplane package.
 // +k8s:deepcopy-gen=false
 type Pkg interface {
+	runtime.Object
+	metav1.Object
+
 	GetCrossplaneConstraints() *CrossplaneConstraints
 	GetDependencies() []Dependency
 }

apis/pkg/meta/v1/zz_generated.deepcopy.go

@@ -25,7 +25,6 @@ import (
 	"github.com/google/go-containerregistry/pkg/name"
 	conregv1 "github.com/google/go-containerregistry/pkg/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -54,7 +53,7 @@ const (
 
 // DependencyManager is a lock on packages.
 type DependencyManager interface {
-	Resolve(ctx context.Context, pkg runtime.Object, pr v1.PackageRevision) (found, installed, invalid int, err error)
+	Resolve(ctx context.Context, meta pkgmetav1.Pkg, pr v1.PackageRevision) (found, installed, invalid int, err error)
 	RemoveSelf(ctx context.Context, pr v1.PackageRevision) error
 }
 
@@ -75,20 +74,15 @@ func NewPackageDependencyManager(c client.Client, nd dag.NewDAGFn, pkgType schem
 }
 
 // Resolve resolves package dependencies.
-func (m *PackageDependencyManager) Resolve(ctx context.Context, meta runtime.Object, pr v1.PackageRevision) (found, installed, invalid int, err error) { //nolint:gocognit // TODO(negz): Can this be refactored for less complexity?
+func (m *PackageDependencyManager) Resolve(ctx context.Context, meta pkgmetav1.Pkg, pr v1.PackageRevision) (found, installed, invalid int, err error) { //nolint:gocognit // TODO(negz): Can this be refactored for less complexity?
 	// If we are inactive, we don't need to resolve dependencies.
 	if pr.GetDesiredState() == v1.PackageRevisionInactive {
 		return 0, 0, 0, nil
 	}
 
-	pack, ok := xpkg.TryConvertToPkg(meta, &pkgmetav1.Provider{}, &pkgmetav1.Function{}, &pkgmetav1.Configuration{})
-	if !ok {
-		return found, installed, invalid, errors.New(errNotMeta)
-	}
-
 	// Copy package dependencies into Lock Dependencies.
-	sources := make([]v1beta1.Dependency, len(pack.GetDependencies()))
-	for i, dep := range pack.GetDependencies() {
+	sources := make([]v1beta1.Dependency, len(meta.GetDependencies()))
+	for i, dep := range meta.GetDependencies() {
 		pdep := v1beta1.Dependency{}
 		switch {
 		// If the GVK and package are specified explicitly they take precedence.

internal/controller/pkg/revision/dependency.go

@@ -23,7 +23,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	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/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -46,7 +45,7 @@ func TestResolve(t *testing.T) {
 
 	type args struct {
 		dep  *PackageDependencyManager
-		meta runtime.Object
+		meta pkgmetav1.Pkg
 		pr   v1.PackageRevision
 	}
 
@@ -75,21 +74,6 @@ func TestResolve(t *testing.T) {
 			},
 			want: want{},
 		},
-		"ErrNotMeta": {
-			reason: "Should return error if not a valid package meta type.",
-			args: args{
-				dep:  &PackageDependencyManager{},
-				meta: &v1.Configuration{},
-				pr: &v1.ConfigurationRevision{
-					Spec: v1.PackageRevisionSpec{
-						DesiredState: v1.PackageRevisionActive,
-					},
-				},
-			},
-			want: want{
-				err: errors.New(errNotMeta),
-			},
-		},
 		"ErrGetLock": {
 			reason: "Should return error if we cannot get lock.",
 			args: args{

internal/controller/pkg/revision/dependency_test.go

@@ -123,7 +123,7 @@ func FuzzRevisionControllerPackageHandling(f *testing.F) {
 		if err := linter.Lint(pkg); err != nil {
 			return
 		}
-		pkgMeta, _ := xpkg.TryConvert(pkg.GetMeta()[0], &pkgmetav1.Provider{}, &pkgmetav1.Configuration{})
+		pkgMeta, _ := xpkg.TryConvertToPkg(pkg.GetMeta()[0], &pkgmetav1.Provider{}, &pkgmetav1.Configuration{})
 		c, err := getFuzzMockClient(ff)
 		if err != nil {
 			return

internal/controller/pkg/revision/fuzz_test.go

@@ -28,7 +28,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	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/types"
 	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -804,11 +803,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		return reconcile.Result{}, err
 	}
 
-	pkgMeta, _ := xpkg.TryConvert(pkg.GetMeta()[0], &pkgmetav1.Provider{}, &pkgmetav1.Configuration{}, &pkgmetav1.Function{})
+	pkgMeta, _ := xpkg.TryConvertToPkg(pkg.GetMeta()[0], &pkgmetav1.Provider{}, &pkgmetav1.Configuration{}, &pkgmetav1.Function{})
 
-	pmo := pkgMeta.(metav1.Object) //nolint:forcetypeassert // Will always be metav1.Object.
-	meta.AddLabels(pr, pmo.GetLabels())
-	meta.AddAnnotations(pr, pmo.GetAnnotations())
+	meta.AddLabels(pr, pkgMeta.GetLabels())
+	meta.AddAnnotations(pr, pkgMeta.GetAnnotations())
 	if err := r.client.Update(ctx, pr); err != nil {
 		if kerrors.IsConflict(err) {
 			return reconcile.Result{Requeue: true}, nil

internal/controller/pkg/revision/reconciler.go

@@ -44,6 +44,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 
+	pkgmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1"
 	v1 "github.com/crossplane/crossplane/apis/pkg/v1"
 	"github.com/crossplane/crossplane/internal/features"
 	verfake "github.com/crossplane/crossplane/internal/version/fake"
@@ -154,7 +155,7 @@ func NewMockRemoveSelfFn(err error) func() error {
 	return func() error { return err }
 }
 
-func (m *MockDependencyManager) Resolve(_ context.Context, _ runtime.Object, _ v1.PackageRevision) (int, int, int, error) {
+func (m *MockDependencyManager) Resolve(_ context.Context, _ pkgmetav1.Pkg, _ v1.PackageRevision) (int, int, int, error) {
 	return m.MockResolve()
 }