🌱 Add integration tests for the apiexport controller

cmd/api-syncagent/main.go

@@ -53,6 +53,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
 	ctrlruntimelog "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 )
 
 func main() {
@@ -168,9 +169,11 @@ func setupLocalManager(ctx context.Context, opts *Options) (manager.Manager, err
 		BaseContext: func() context.Context {
 			return ctx
 		},
+		Metrics:                 metricsserver.Options{BindAddress: opts.MetricsAddr},
 		LeaderElection:          opts.EnableLeaderElection,
 		LeaderElectionID:        "syncagent." + opts.AgentName,
 		LeaderElectionNamespace: opts.Namespace,
+		HealthProbeBindAddress:  opts.HealthAddr,
 	})
 	if err != nil {
 		return nil, err

cmd/api-syncagent/options.go

@@ -65,12 +65,16 @@ type Options struct {
 	KubeconfigCAFileOverride string
 
 	LogOptions log.Options
+
+	MetricsAddr string
+	HealthAddr  string
 }
 
 func NewOptions() *Options {
 	return &Options{
 		LogOptions:                log.NewDefaultOptions(),
 		PublishedResourceSelector: labels.Everything(),
+		MetricsAddr:               "127.0.0.1:8085",
 	}
 }
 
@@ -83,8 +87,10 @@ func (o *Options) AddFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.APIExportRef, "apiexport-ref", o.APIExportRef, "name of the APIExport in kcp that this Sync Agent is powering")
 	flags.StringVar(&o.PublishedResourceSelectorString, "published-resource-selector", o.PublishedResourceSelectorString, "restrict this Sync Agent to only process PublishedResources matching this label selector (optional)")
 	flags.BoolVar(&o.EnableLeaderElection, "enable-leader-election", o.EnableLeaderElection, "whether to perform leader election")
-	flags.StringVar(&o.KubeconfigHostOverride, "kubeconfig-host-override", o.KubeconfigHostOverride, "Override the host configured in the local kubeconfig")
-	flags.StringVar(&o.KubeconfigCAFileOverride, "kubeconfig-ca-file-override", o.KubeconfigCAFileOverride, "Override the server CA file configured in the local kubeconfig")
+	flags.StringVar(&o.KubeconfigHostOverride, "kubeconfig-host-override", o.KubeconfigHostOverride, "override the host configured in the local kubeconfig")
+	flags.StringVar(&o.KubeconfigCAFileOverride, "kubeconfig-ca-file-override", o.KubeconfigCAFileOverride, "override the server CA file configured in the local kubeconfig")
+	flags.StringVar(&o.MetricsAddr, "metrics-address", o.MetricsAddr, "host and port to serve Prometheus metrics via /metrics (HTTP)")
+	flags.StringVar(&o.HealthAddr, "health-address", o.HealthAddr, "host and port to serve probes via /readyz and /healthz (HTTP)")
 }
 
 func (o *Options) Validate() error {

docs/faq.md

@@ -36,3 +36,19 @@ Only those required for its own operation. If you configure a namespaced resourc
 automatically add a claim for `namespaces` in kcp, plus it will add either `configmaps` or `secrets`
 if related resources are configured in a `PublishedResource`. But you cannot specify additional
 permissions claims.
+
+## I am seeing errors in the agent logs, what's going on?
+
+Errors like
+
+> reflector.go:561] k8s.io/[email protected]/tools/cache/reflector.go:243: failed to list
+> example.com/v1, Kind=Dummy: the server could not find the requested resource
+
+or
+
+> reflector.go:158] "Unhandled Error" err="k8s.io/[email protected]/tools/cache/reflector.go:243:
+> Failed to watch kcp.example.com/v1, Kind=Dummy: failed to list kcp.example.com/v1, Kind=Dummy:
+> the server could not find the requested resource" logger="UnhandledError"
+
+are typical when bootstrapping new APIExports in kcp. They are only cause for concern if they
+persist after configuring all PublishedResources.

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,17 +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
-	})
-
+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 {
@@ -206,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 != "" {
@@ -238,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/crds/backup.yaml

@@ -0,0 +1,27 @@
+# sourced from https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: backups.eksempel.no
+spec:
+  group: eksempel.no
+  scope: Namespaced
+  names:
+    plural: backups
+    singular: backup
+    kind: Backup
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                source:
+                  type: string
+                destination:
+                  type: string

test/crds/crontab-improved.yaml

@@ -0,0 +1,32 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: crontabs.example.com
+spec:
+  group: example.com
+  scope: Namespaced
+  names:
+    plural: crontabs
+    singular: crontab
+    kind: CronTab
+    shortNames:
+      - ct
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                cronSpec:
+                  type: string
+                image:
+                  type: string
+                command:
+                  type: string
+                replicas:
+                  type: integer

test/crds/crontab-multi-versions.yaml

@@ -0,0 +1,47 @@
+# sourced from https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: crontabs.example.com
+spec:
+  group: example.com
+  scope: Namespaced
+  names:
+    plural: crontabs
+    singular: crontab
+    kind: CronTab
+    shortNames:
+      - ct
+  versions:
+    - name: v1
+      served: true
+      storage: false
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                cronSpec:
+                  type: string
+                image:
+                  type: string
+                replicas:
+                  type: integer
+    - name: v2
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                cronSpec:
+                  type: string
+                image:
+                  type: string
+                replicas:
+                  type: integer