add new test for projection, fix ARS version projection

On-behalf-of: @SAP [email protected]

Author: xrstf

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/evanphx/json-patch/v5 v5.9.0
 	github.com/go-logr/logr v1.4.2
 	github.com/go-logr/zapr v1.3.0
+	github.com/google/go-cmp v0.6.0
 	github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20240817110845-a9eb9752bfeb
 	github.com/kcp-dev/client-go v0.0.0-20240912145314-f5949d81732a
 	github.com/kcp-dev/code-generator/v2 v2.3.1
@@ -53,7 +54,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.1-0.20210504230335-f78f29fc09ea // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect

internal/controller/apiresourceschema/controller.go

@@ -133,7 +133,10 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR
 	}
 
 	// project the CRD
-	projectedCRD := r.projectResourceNames(r.apiExportName, crd, pubResource.Spec.Projection)
+	projectedCRD, err := r.applyProjection(r.apiExportName, crd, pubResource)
+	if err != nil {
+		return nil, fmt.Errorf("failed to apply projection rules: %w", err)
+	}
 
 	// to prevent changing the source GVK e.g. from "apps/v1 Daemonset" to "core/v1 Pod",
 	// we include the source GVK in hashed form in the final APIResourceSchema name.
@@ -147,7 +150,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR
 	err = r.kcpClient.Get(wsCtx, types.NamespacedName{Name: arsName}, ars, &ctrlruntimeclient.GetOptions{})
 
 	if apierrors.IsNotFound(err) {
-		if err := r.createAPIResourceSchema(wsCtx, log, r.apiExportName, projectedCRD, arsName, pubResource.Spec.Resource.Version); err != nil {
+		if err := r.createAPIResourceSchema(wsCtx, log, r.apiExportName, projectedCRD, arsName); err != nil {
 			return nil, fmt.Errorf("failed to create APIResourceSchema: %w", err)
 		}
 	} else if err != nil {
@@ -170,25 +173,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR
 	return nil, nil
 }
 
-func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.SugaredLogger, apigroup string, projectedCRD *apiextensionsv1.CustomResourceDefinition, arsName string, selectedVersion string) error {
-	// At this moment we ignore every non-selected version in the CRD, as we have not fully
-	// decided on how to support the API version lifecycle yet. Having multiple versions in
-	// the CRD will make kcp require a `conversion` to also be configured. Since we cannot
-	// enforce that and want to instead work with existing CRDs as best as we can, we chose
-	// this option (instead of error'ing out if a conversion is missing).
-	projectedCRD.Spec.Conversion = nil
-	projectedCRD.Spec.Versions = slices.DeleteFunc(projectedCRD.Spec.Versions, func(v apiextensionsv1.CustomResourceDefinitionVersion) bool {
-		return v.Name != selectedVersion
-	})
-
-	if len(projectedCRD.Spec.Versions) != 1 {
-		// This should never happen because of checks earlier in the reconciler.
-		return fmt.Errorf("invalid CRD: cannot find selected version %q", selectedVersion)
-	}
-
-	projectedCRD.Spec.Versions[0].Served = true
-	projectedCRD.Spec.Versions[0].Storage = true
-
+func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.SugaredLogger, apigroup string, projectedCRD *apiextensionsv1.CustomResourceDefinition, arsName string) error {
 	// prefix is irrelevant as the reconciling framework will use arsName anyway
 	converted, err := kcpdevv1alpha1.CRDToAPIResourceSchema(projectedCRD, "irrelevant")
 	if err != nil {
@@ -214,12 +199,35 @@ func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.Sugar
 	return r.kcpClient.Create(ctx, ars)
 }
 
-func (r *Reconciler) projectResourceNames(apiGroup string, crd *apiextensionsv1.CustomResourceDefinition, projection *syncagentv1alpha1.ResourceProjection) *apiextensionsv1.CustomResourceDefinition {
+func (r *Reconciler) applyProjection(apiGroup string, crd *apiextensionsv1.CustomResourceDefinition, pr *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) {
 	result := crd.DeepCopy()
 	result.Spec.Group = apiGroup
 
+	// At this moment we ignore every non-selected version in the CRD, as we have not fully
+	// decided on how to support the API version lifecycle yet. Having multiple versions in
+	// the CRD will make kcp require a `conversion` to also be configured. Since we cannot
+	// enforce that and want to instead work with existing CRDs as best as we can, we chose
+	// this option (instead of error'ing out if a conversion is missing).
+	result.Spec.Conversion = nil
+	result.Spec.Versions = slices.DeleteFunc(result.Spec.Versions, func(v apiextensionsv1.CustomResourceDefinitionVersion) bool {
+		return v.Name != pr.Spec.Resource.Version
+	})
+
+	if len(result.Spec.Versions) != 1 {
+		// This should never happen because of checks earlier in the reconciler.
+		return nil, fmt.Errorf("invalid CRD: cannot find selected version %q", pr.Spec.Resource.Version)
+	}
+
+	result.Spec.Versions[0].Served = true
+	result.Spec.Versions[0].Storage = true
+
+	projection := pr.Spec.Projection
 	if projection == nil {
-		return result
+		return result, nil
+	}
+
+	if projection.Version != "" {
+		result.Spec.Versions[0].Name = projection.Version
 	}
 
 	if projection.Kind != "" {
@@ -246,7 +254,7 @@ func (r *Reconciler) projectResourceNames(apiGroup string, crd *apiextensionsv1.
 		result.Spec.Names.ShortNames = projection.ShortNames
 	}
 
-	return result
+	return result, nil
 }
 
 // getAPIResourceSchemaName generates the name for the ARS in kcp. Note that

test/e2e/apiresourceschema/apiresourceschema_test.go

@@ -24,12 +24,14 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	"github.com/google/go-cmp/cmp"
 
 	syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1"
 	"github.com/kcp-dev/api-syncagent/test/utils"
 
 	kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
 
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -272,3 +274,106 @@ func TestARSDropsAllVersionsExceptTheSelectedOne(t *testing.T) {
 		t.Fatalf("Expected ARS to contain %q, but contains %q.", theVersion, name)
 	}
 }
+
+func TestProjection(t *testing.T) {
+	const (
+		apiExportName   = "example.com"
+		originalVersion = "v1"
+	)
+
+	ctx := context.Background()
+	ctrlruntime.SetLogger(logr.Discard())
+
+	// setup a test environment in kcp
+	orgKubconfig := utils.CreateOrganization(t, ctx, "ars-projections", apiExportName)
+
+	// start a service cluster
+	envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{
+		"test/crds/crontab.yaml",
+	})
+
+	// publish Crontabs
+	t.Logf("Publishing CronTabs…")
+	pr := &syncagentv1alpha1.PublishedResource{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "publish-crontabs",
+		},
+		Spec: syncagentv1alpha1.PublishedResourceSpec{
+			Resource: syncagentv1alpha1.SourceResourceDescriptor{
+				APIGroup: "example.com",
+				Version:  originalVersion,
+				Kind:     "CronTab",
+			},
+			Projection: &syncagentv1alpha1.ResourceProjection{
+				Version:    "v6",
+				Scope:      syncagentv1alpha1.ClusterScoped,
+				Kind:       "CronusTabulatus",
+				Plural:     "cronustabulati",
+				ShortNames: []string{"cront"},
+			},
+		},
+	}
+
+	if err := envtestClient.Create(ctx, pr); err != nil {
+		t.Fatalf("Failed to create PublishedResource: %v", err)
+	}
+
+	// let the agent do its thing
+	utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName)
+
+	// wait for the APIExport to be updated
+	t.Logf("Waiting for APIExport to be updated…")
+	orgClient := utils.GetClient(t, orgKubconfig)
+	apiExportKey := types.NamespacedName{Name: apiExportName}
+
+	var arsName string
+	err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) {
+		apiExport := &kcpapisv1alpha1.APIExport{}
+		err = orgClient.Get(ctx, apiExportKey, apiExport)
+		if err != nil {
+			return false, err
+		}
+
+		if len(apiExport.Spec.LatestResourceSchemas) == 0 {
+			return false, nil
+		}
+
+		arsName = apiExport.Spec.LatestResourceSchemas[0]
+
+		return true, nil
+	})
+	if err != nil {
+		t.Fatalf("Failed to wait for APIExport to be updated: %v", err)
+	}
+
+	// check the APIResourceSchema
+	ars := &kcpapisv1alpha1.APIResourceSchema{}
+	err = orgClient.Get(ctx, types.NamespacedName{Name: arsName}, ars)
+	if err != nil {
+		t.Fatalf("APIResourceSchema does not exist: %v", err)
+	}
+
+	if len(ars.Spec.Versions) != 1 {
+		t.Fatalf("Expected only one version to remain in ARS, but found %d.", len(ars.Spec.Versions))
+	}
+
+	if name := ars.Spec.Versions[0].Name; name != pr.Spec.Projection.Version {
+		t.Errorf("Expected ARS to contain version %q, but contains %q.", pr.Spec.Projection.Version, name)
+	}
+
+	if ars.Spec.Scope != apiextensionsv1.ResourceScope(pr.Spec.Projection.Scope) {
+		t.Errorf("Expected ARS to be of scope %q, but is %q.", pr.Spec.Projection.Scope, ars.Spec.Scope)
+	}
+
+	if ars.Spec.Names.Kind != pr.Spec.Projection.Kind {
+		t.Errorf("Expected ARS to be kind %q, but is %q.", pr.Spec.Projection.Kind, ars.Spec.Names.Kind)
+	}
+
+	if ars.Spec.Names.Plural != pr.Spec.Projection.Plural {
+		t.Errorf("Expected ARS to have plural name %q, but has %q.", pr.Spec.Projection.Plural, ars.Spec.Names.Plural)
+	}
+
+	if !cmp.Equal(ars.Spec.Names.ShortNames, pr.Spec.Projection.ShortNames) {
+		t.Errorf("Expected ARS to have short names %v, but has %v.", pr.Spec.Projection.ShortNames, ars.Spec.Names.ShortNames)
+	}
+}