From 5414d37376b8adde82b851818a3a747f0a6de4b3 Mon Sep 17 00:00:00 2001 From: Timofei Larkin Date: Thu, 13 Nov 2025 13:19:59 +0300 Subject: [PATCH] [api] Fix representation of dynamic list kinds ## What this PR does This patch fixes an issue with the Cozystack API server that causes it to respond with the first registered dynamic list kind when listing dynamic resources. E.g., when running `k get postgreses`, the raw JSON response from the cozystack API would be ```json { "apiVersion":"apps.cozystack.io/v1alpha1", "kind":"BootBoxList", "items": [ { "apiVersion":"apps.cozystack.io/v1alpha1", "kind":"Postgres", ... }, ... ], ... } ``` The root cause is the way the `Typer` interface is implemented for the `runtime.Scheme` where the dynamic types are registered. Since the base type for all dynamic types is a `&cozyv1alpha1.Application{}`, all types are registered under the same key and the `Typer` defaults to the first `GroupVersionKind` that was registered. Only when a correctly formed `&unstructured.Unstructured{}` is returned by the API, is this resolving logic circumvented and the `GroupVersionKind` is instead inferred from the fields of the returned object. Even an `UnstructuredList` is not acceptable as a return type, instead the `items` key should be directly set on the underlying `map[string]interface{}`. This patch implements the changes detailed above. Additionally, the following features, fixes, and improvements are added: * Makefile targets to build and run the Cozystack API locally, against a Kubernetes server in the environment's KUBECONFIG. Debugging with Delve is also supported. * CI tests are added to verify the new changes. * A bug in the registration of the corev1alpha1 types is fixed. * Updated the `ConvertToTable` function to properly handle list kinds which might be of the `&unstructured.Unstructured{}` concrete type (not an `UnstructuredList`). * The scheme used by the API server's Client and WatchClient is separated from the scheme used to serve dynamic types. * The client config for reading the runtime configuration now uses the controller-runtime, which handles flags and environment variables properly, unlike `clientcmd`. ### Release note ```release-note [api] Fix incorrect list kind for list requests to the Cozystack API for dynamic resources. Add Makefile targets for local testing. Minor schema building improvements. ``` Signed-off-by: Timofei Larkin --- hack/e2e-test-openapi.bats | 23 ++++++ packages/system/cozystack-api/.gitignore | 1 + packages/system/cozystack-api/Makefile | 12 +++ .../cozystack-api/cozystack-api-openssl.cnf | 13 ++++ pkg/apiserver/apiserver.go | 22 ++++-- pkg/cmd/server/start.go | 4 +- pkg/registry/apps/application/rest.go | 78 +++++++++++++++---- 7 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 packages/system/cozystack-api/.gitignore create mode 100644 packages/system/cozystack-api/cozystack-api-openssl.cnf diff --git a/hack/e2e-test-openapi.bats b/hack/e2e-test-openapi.bats index aaddbe84..32128f07 100644 --- a/hack/e2e-test-openapi.bats +++ b/hack/e2e-test-openapi.bats @@ -19,3 +19,26 @@ curl -sS --fail 'http://localhost:21234/openapi/v2?timeout=32s' -H 'Accept: application/com.github.proto-openapi.spec.v2@v1.0+protobuf' > /dev/null ) } + +@test "Test kinds" { + val=$(kubectl get --raw /apis/apps.cozystack.io/v1alpha1/tenants | jq -r '.kind') + if [ "$val" != "TenantList" ]; then + echo "Expected kind to be TenantList, got $val" + exit 1 + fi + val=$(kubectl get --raw /apis/apps.cozystack.io/v1alpha1/tenants | jq -r '.items[0].kind') + if [ "$val" != "Tenant" ]; then + echo "Expected kind to be Tenant, got $val" + exit 1 + fi + val=$(kubectl get --raw /apis/apps.cozystack.io/v1alpha1/ingresses | jq -r '.kind') + if [ "$val" != "IngressList" ]; then + echo "Expected kind to be IngressList, got $val" + exit 1 + fi + val=$(kubectl get --raw /apis/apps.cozystack.io/v1alpha1/ingresses | jq -r '.items[0].kind') + if [ "$val" != "Ingress" ]; then + echo "Expected kind to be Ingress, got $val" + exit 1 + fi +} diff --git a/packages/system/cozystack-api/.gitignore b/packages/system/cozystack-api/.gitignore new file mode 100644 index 00000000..f907698c --- /dev/null +++ b/packages/system/cozystack-api/.gitignore @@ -0,0 +1 @@ +apiserver.local.config/ diff --git a/packages/system/cozystack-api/Makefile b/packages/system/cozystack-api/Makefile index 6ba3479b..a3e0d931 100644 --- a/packages/system/cozystack-api/Makefile +++ b/packages/system/cozystack-api/Makefile @@ -4,6 +4,18 @@ NAMESPACE=cozy-system include ../../../scripts/common-envs.mk include ../../../scripts/package.mk +run-local: + openssl req -nodes -new -x509 -keyout /tmp/ca.key -out /tmp/ca.crt -subj "/CN=kube-ca" + openssl req -out /tmp/client.csr -new -newkey rsa:2048 -nodes -keyout /tmp/client.key -subj "/C=US/ST=SomeState/L=L/OU=Dev/CN=development/O=system:masters" + openssl x509 -req -days 365 -in /tmp/client.csr -CA /tmp/ca.crt -CAkey /tmp/ca.key -set_serial 01 -sha256 -out /tmp/client.crt + openssl req -out /tmp/apiserver.csr -new -newkey rsa:2048 -nodes -keyout /tmp/apiserver.key -subj "/CN=cozystack-api" -config cozystack-api-openssl.cnf + openssl x509 -req -days 365 -in /tmp/apiserver.csr -CA /tmp/ca.crt -CAkey /tmp/ca.key -set_serial 01 -sha256 -out /tmp/apiserver.crt -extensions v3_req -extfile cozystack-api-openssl.cnf + CGO_ENABLED=0 go build -o /tmp/cozystack-api ../../../cmd/cozystack-api/main.go + /tmp/cozystack-api --client-ca-file /tmp/ca.crt --tls-cert-file /tmp/apiserver.crt --tls-private-key-file /tmp/apiserver.key --secure-port 6443 --kubeconfig $(KUBECONFIG) --authorization-kubeconfig $(KUBECONFIG) --authentication-kubeconfig $(KUBECONFIG) + +debug: + dlv debug ../../../cmd/cozystack-api/main.go -- --client-ca-file /tmp/ca.crt --tls-cert-file /tmp/apiserver.crt --tls-private-key-file /tmp/apiserver.key --secure-port 6443 --kubeconfig $(KUBECONFIG) --authorization-kubeconfig $(KUBECONFIG) --authentication-kubeconfig $(KUBECONFIG) + image: image-cozystack-api image-cozystack-api: diff --git a/packages/system/cozystack-api/cozystack-api-openssl.cnf b/packages/system/cozystack-api/cozystack-api-openssl.cnf new file mode 100644 index 00000000..5425fb00 --- /dev/null +++ b/packages/system/cozystack-api/cozystack-api-openssl.cnf @@ -0,0 +1,13 @@ +[ req ] +distinguished_name = req_distinguished_name +req_extensions = v3_req +prompt = no + +[ req_distinguished_name ] +CN = cozystack-api + +[ v3_req ] +subjectAltName = @alt_names + +[ alt_names ] +IP.1 = 127.0.0.1 diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index ba5cee5b..fccbf9a9 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -30,14 +30,16 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" + "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/cozystack/cozystack/pkg/apis/apps" appsinstall "github.com/cozystack/cozystack/pkg/apis/apps/install" - coreinstall "github.com/cozystack/cozystack/pkg/apis/apps/install" "github.com/cozystack/cozystack/pkg/apis/core" + coreinstall "github.com/cozystack/cozystack/pkg/apis/core/install" "github.com/cozystack/cozystack/pkg/config" cozyregistry "github.com/cozystack/cozystack/pkg/registry" applicationstorage "github.com/cozystack/cozystack/pkg/registry/apps/application" @@ -48,7 +50,8 @@ import ( var ( // Scheme defines methods for serializing and deserializing API objects. - Scheme = runtime.NewScheme() + Scheme = runtime.NewScheme() + mgrScheme = runtime.NewScheme() // Codecs provides methods for retrieving codecs and serializers for specific // versions and content types. Codecs = serializer.NewCodecFactory(Scheme) @@ -57,18 +60,23 @@ var ( ) func init() { + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{ + Development: true, + // any other zap.Options tweaks + }))) + klog.SetLogger(ctrl.Log.WithName("klog")) appsinstall.Install(Scheme) coreinstall.Install(Scheme) // Register HelmRelease types. - if err := helmv2.AddToScheme(Scheme); err != nil { + if err := helmv2.AddToScheme(mgrScheme); err != nil { panic(fmt.Errorf("Failed to add HelmRelease types to scheme: %w", err)) } - if err := corev1.AddToScheme(Scheme); err != nil { + if err := corev1.AddToScheme(mgrScheme); err != nil { panic(fmt.Errorf("Failed to add core types to scheme: %w", err)) } - if err := rbacv1.AddToScheme(Scheme); err != nil { + if err := rbacv1.AddToScheme(mgrScheme); err != nil { panic(fmt.Errorf("Failed to add RBAC types to scheme: %w", err)) } // Add unversioned types. @@ -134,7 +142,7 @@ func (c completedConfig) New() (*CozyServer, error) { } mgr, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: Scheme, + Scheme: mgrScheme, Cache: cache.Options{SyncPeriod: &syncPeriod}, }) if err != nil { @@ -164,7 +172,7 @@ func (c completedConfig) New() (*CozyServer, error) { } cli := mgr.GetClient() - watchCli, err := client.NewWithWatch(cfg, client.Options{Scheme: Scheme}) + watchCli, err := client.NewWithWatch(cfg, client.Options{Scheme: mgrScheme}) if err != nil { return nil, fmt.Errorf("failed to build watch client: %w", err) } diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index c6fd23a1..3427e61d 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -42,11 +42,11 @@ import ( genericoptions "k8s.io/apiserver/pkg/server/options" utilfeature "k8s.io/apiserver/pkg/util/feature" utilversionpkg "k8s.io/apiserver/pkg/util/version" - "k8s.io/client-go/tools/clientcmd" "k8s.io/component-base/featuregate" baseversion "k8s.io/component-base/version" netutils "k8s.io/utils/net" "sigs.k8s.io/controller-runtime/pkg/client" + k8sconfig "sigs.k8s.io/controller-runtime/pkg/client/config" ) // CozyServerOptions holds the state for the Cozy API server @@ -150,7 +150,7 @@ func (o *CozyServerOptions) Complete() error { return fmt.Errorf("failed to register types: %w", err) } - cfg, err := clientcmd.BuildConfigFromFlags("", "") + cfg, err := k8sconfig.GetConfig() if err != nil { return fmt.Errorf("failed to get kubeconfig: %w", err) } diff --git a/pkg/registry/apps/application/rest.go b/pkg/registry/apps/application/rest.go index 37a5d9a5..0c2dd3a9 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -142,9 +142,17 @@ func (r *REST) GetSingularName() string { // Create handles the creation of a new Application by converting it to a HelmRelease func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { // Assert the object is of type Application - app, ok := obj.(*appsv1alpha1.Application) + us, ok := obj.(*unstructured.Unstructured) if !ok { - return nil, fmt.Errorf("expected Application object, got %T", obj) + return nil, fmt.Errorf("expected unstructured.Unstructured object, got %T", obj) + } + + app := &appsv1alpha1.Application{} + + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(us.Object, app); err != nil { + errMsg := fmt.Sprintf("returned unstructured.Unstructured object was not an Application") + klog.Errorf(errMsg) + return nil, fmt.Errorf(errMsg) } // Convert Application to HelmRelease @@ -389,11 +397,9 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption } // Explicitly set apiVersion and kind in unstructured object - appList := &unstructured.UnstructuredList{} - appList.SetAPIVersion("apps.cozystack.io/v1alpha1") - appList.SetKind(r.kindName + "List") + appList := r.NewList().(*unstructured.Unstructured) appList.SetResourceVersion(hrList.GetResourceVersion()) - appList.Items = items + appList.Object["items"] = items klog.V(6).Infof("Successfully listed %d Application resources in namespace %s", len(items), namespace) return appList, nil @@ -441,9 +447,16 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje } // Assert the new object is of type Application - app, ok := newObj.(*appsv1alpha1.Application) + us, ok := newObj.(*unstructured.Unstructured) if !ok { - errMsg := fmt.Sprintf("expected Application object, got %T", newObj) + errMsg := fmt.Sprintf("expected unstructured.Unstructured object, got %T", newObj) + klog.Errorf(errMsg) + return nil, false, fmt.Errorf(errMsg) + } + app := &appsv1alpha1.Application{} + + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(us.Object, app); err != nil { + errMsg := fmt.Sprintf("returned unstructured.Unstructured object was not an Application") klog.Errorf(errMsg) return nil, false, fmt.Errorf(errMsg) } @@ -516,14 +529,12 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje klog.Errorf("Failed to convert Application to unstructured for resource %s: %v", convertedApp.GetName(), err) return nil, false, fmt.Errorf("failed to convert Application to unstructured: %v", err) } - - // Explicitly set apiVersion and kind in unstructured object - unstructuredApp["apiVersion"] = "apps.cozystack.io/v1alpha1" - unstructuredApp["kind"] = r.kindName + obj := &unstructured.Unstructured{Object: unstructuredApp} + obj.SetGroupVersionKind(r.gvk) klog.V(6).Infof("Returning patched Application object: %+v", unstructuredApp) - return &unstructured.Unstructured{Object: unstructuredApp}, false, nil + return obj, false, nil } // Delete removes an Application by deleting the corresponding HelmRelease @@ -723,11 +734,13 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio klog.Errorf("Failed to convert Application to unstructured: %v", err) continue } + obj := &unstructured.Unstructured{Object: unstructuredApp} + obj.SetGroupVersionKind(r.gvk) // Create watch event with Application object appEvent := watch.Event{ Type: event.Type, - Object: &unstructured.Unstructured{Object: unstructuredApp}, + Object: obj, } // Send event to custom watcher @@ -1060,6 +1073,34 @@ func (r *REST) ConvertToTable(ctx context.Context, object runtime.Object, tableO table = r.buildTableFromApplications(apps) table.ListMeta.ResourceVersion = obj.GetResourceVersion() case *unstructured.Unstructured: + var apps []appsv1alpha1.Application + for { + var items interface{} + var ok bool + var objects []unstructured.Unstructured + if items, ok = obj.Object["items"]; !ok { + break + } + if objects, ok = items.([]unstructured.Unstructured); !ok { + break + } + apps = make([]appsv1alpha1.Application, 0, len(objects)) + var a appsv1alpha1.Application + for i := range objects { + err := runtime.DefaultUnstructuredConverter.FromUnstructured(objects[i].Object, &a) + if err != nil { + klog.Errorf("Failed to convert Unstructured to Application: %v", err) + continue + } + apps = append(apps, a) + } + break + } + if apps != nil { + table = r.buildTableFromApplications(apps) + table.ListMeta.ResourceVersion = obj.GetResourceVersion() + break + } var app appsv1alpha1.Application err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &app) if err != nil { @@ -1196,12 +1237,17 @@ func (r *REST) Destroy() { // New creates a new instance of Application func (r *REST) New() runtime.Object { - return &appsv1alpha1.Application{} + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(r.gvk) + return obj } // NewList returns an empty list of Application objects func (r *REST) NewList() runtime.Object { - return &appsv1alpha1.ApplicationList{} + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(r.gvk.GroupVersion().WithKind(r.kindName + "List")) + obj.Object["items"] = make([]interface{}, 0) + return obj } // Kind returns the resource kind used for API discovery