diff --git a/internal/controller/topology_reconciler.go b/internal/controller/topology_reconciler.go index 54395ca2a..4f2e432f0 100644 --- a/internal/controller/topology_reconciler.go +++ b/internal/controller/topology_reconciler.go @@ -2,11 +2,14 @@ package controllers import ( "context" + "fmt" "strings" "sync" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,14 +20,19 @@ import ( const ( TopologyConfigMapName = "topology" + // TODO: This size cap is a temporary workaround. The topology can outgrow a single ConfigMap, + // and other resources that consume it (e.g. the console-plugin) will need coordinated changes + // to support a different storage or serialization strategy. + maxTopologyBytes = 900 * 1024 // ~900KB, safely under the 1MB ConfigMap limit + oversizedPlaceholder = `digraph { "error" [label="Topology exceeds ConfigMap 1MB limit"] }` ) type TopologyReconciler struct { - Client *dynamic.DynamicClient + Client dynamic.Interface Namespace string } -func NewTopologyReconciler(client *dynamic.DynamicClient, namespace string) *TopologyReconciler { +func NewTopologyReconciler(client dynamic.Interface, namespace string) *TopologyReconciler { if namespace == "" { panic("namespace must be specified and can not be a blank string") } @@ -33,6 +41,27 @@ func NewTopologyReconciler(client *dynamic.DynamicClient, namespace string) *Top func (r *TopologyReconciler) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, _ *sync.Map) error { logger := controller.LoggerFromContext(ctx).WithName("topology file").WithValues("context", ctx) + tracer := controller.TracerFromContext(ctx) + ctx, span := tracer.Start(ctx, "TopologyReconciler.Reconcile") + defer span.End() + + span.SetAttributes( + attribute.String("configmap.name", TopologyConfigMapName), + attribute.String("configmap.namespace", r.Namespace), + ) + + topologyData := topology.ToDot() + topologySize := len(topologyData) + span.SetAttributes(attribute.Int("topology.size_bytes", topologySize)) + + if topologySize > maxTopologyBytes { + logger.Info("topology data exceeds ConfigMap size limit, using placeholder", + "size_bytes", topologySize, "max_bytes", maxTopologyBytes) + span.RecordError(fmt.Errorf("topology data is %d bytes, exceeds %d byte limit", topologySize, maxTopologyBytes)) + span.SetStatus(codes.Error, "topology exceeds ConfigMap size limit") + span.AddEvent("topology data replaced with placeholder") + topologyData = oversizedPlaceholder + } cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -41,12 +70,14 @@ func (r *TopologyReconciler) Reconcile(ctx context.Context, _ []controller.Resou Labels: map[string]string{kuadrant.TopologyLabel: "true"}, }, Data: map[string]string{ - "topology": topology.ToDot(), + "topology": topologyData, }, } unstructuredCM, err := controller.Destruct(cm) if err != nil { logger.Error(err, "failed to destruct topology configmap") + span.RecordError(err) + span.SetStatus(codes.Error, "failed to destruct topology configmap") return err } @@ -57,10 +88,17 @@ func (r *TopologyReconciler) Reconcile(ctx context.Context, _ []controller.Resou if len(existingTopologyConfigMaps) == 0 { _, err := r.Client.Resource(controller.ConfigMapsResource).Namespace(cm.Namespace).Create(ctx, unstructuredCM, metav1.CreateOptions{}) if errors.IsAlreadyExists(err) { - // This error can happen when the operator is starting, and the create event for the topology has not being processed. logger.Info("already created topology configmap, must not be in topology yet") + span.AddEvent("configmap already exists but not in topology yet") return err } + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "failed to create topology configmap") + } else { + span.AddEvent("topology configmap created") + span.SetStatus(codes.Ok, "") + } return err } @@ -74,9 +112,16 @@ func (r *TopologyReconciler) Reconcile(ctx context.Context, _ []controller.Resou _, err := r.Client.Resource(controller.ConfigMapsResource).Namespace(cm.Namespace).Update(ctx, unstructuredCM, metav1.UpdateOptions{}) if err != nil { logger.Error(err, "failed to update topology configmap") + span.RecordError(err) + span.SetStatus(codes.Error, "failed to update topology configmap") + } else { + span.AddEvent("topology configmap updated") + span.SetStatus(codes.Ok, "") } return err } + span.AddEvent("topology configmap unchanged") + span.SetStatus(codes.Ok, "") return nil } diff --git a/internal/controller/topology_reconciler_test.go b/internal/controller/topology_reconciler_test.go new file mode 100644 index 000000000..cab7d12cb --- /dev/null +++ b/internal/controller/topology_reconciler_test.go @@ -0,0 +1,180 @@ +//go:build unit + +package controllers + +import ( + "context" + "strings" + "sync" + "testing" + + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + "gotest.tools/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + dfake "k8s.io/client-go/dynamic/fake" + + "github.com/kuadrant/kuadrant-operator/internal/kuadrant" +) + +func TestTopologyReconciler_OversizedPlaceholder(t *testing.T) { + assert.Assert(t, len(oversizedPlaceholder) < maxTopologyBytes, + "placeholder (%d bytes) must be smaller than the limit (%d bytes)", len(oversizedPlaceholder), maxTopologyBytes) + assert.Assert(t, strings.Contains(oversizedPlaceholder, "digraph"), + "placeholder must be valid DOT syntax") +} + +func TestTopologyReconciler_Create(t *testing.T) { + namespace := "test-ns" + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + fakeClient := dfake.NewSimpleDynamicClient(scheme) + reconciler := NewTopologyReconciler(fakeClient, namespace) + + topology, err := machinery.NewTopology( + machinery.WithObjects( + &controller.RuntimeObject{ + Object: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-other-configmap", + Namespace: namespace, + }, + }, + }, + ), + ) + assert.NilError(t, err) + + err = reconciler.Reconcile(context.Background(), nil, topology, nil, &sync.Map{}) + assert.NilError(t, err) + + created, err := fakeClient.Resource(controller.ConfigMapsResource).Namespace(namespace).Get(context.Background(), TopologyConfigMapName, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Assert(t, created != nil) + assert.Equal(t, created.GetName(), TopologyConfigMapName) + assert.Equal(t, created.GetLabels()[kuadrant.TopologyLabel], "true") + + data, found, err := unstructuredNestedString(created.Object, "data", "topology") + assert.NilError(t, err) + assert.Assert(t, found, "topology data key should exist") + assert.Assert(t, strings.Contains(data, "digraph"), "topology data should be DOT format") +} + +func TestTopologyReconciler_Update(t *testing.T) { + namespace := "test-ns" + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + existingCM := &controller.RuntimeObject{ + Object: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: TopologyConfigMapName, + Namespace: namespace, + Labels: map[string]string{kuadrant.TopologyLabel: "true"}, + }, + Data: map[string]string{ + "topology": "old-data", + }, + }, + } + + topology, err := machinery.NewTopology( + machinery.WithObjects(existingCM), + ) + assert.NilError(t, err) + + fakeClient := dfake.NewSimpleDynamicClient(scheme) + // Pre-create the configmap so the update path is hit + unstructuredCM, err := controller.Destruct(existingCM.Object.(*corev1.ConfigMap)) + assert.NilError(t, err) + _, err = fakeClient.Resource(controller.ConfigMapsResource).Namespace(namespace).Create(context.Background(), unstructuredCM, metav1.CreateOptions{}) + assert.NilError(t, err) + + reconciler := NewTopologyReconciler(fakeClient, namespace) + err = reconciler.Reconcile(context.Background(), nil, topology, nil, &sync.Map{}) + assert.NilError(t, err) + + updated, err := fakeClient.Resource(controller.ConfigMapsResource).Namespace(namespace).Get(context.Background(), TopologyConfigMapName, metav1.GetOptions{}) + assert.NilError(t, err) + + data, found, err := unstructuredNestedString(updated.Object, "data", "topology") + assert.NilError(t, err) + assert.Assert(t, found, "topology data key should exist") + assert.Assert(t, data != "old-data", "topology data should have been updated") +} + +func TestTopologyReconciler_NoUpdateWhenUnchanged(t *testing.T) { + namespace := "test-ns" + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + // Build a topology first to get the expected DOT output + tempTopology, err := machinery.NewTopology() + assert.NilError(t, err) + expectedDot := tempTopology.ToDot() + + existingCM := &controller.RuntimeObject{ + Object: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: TopologyConfigMapName, + Namespace: namespace, + Labels: map[string]string{kuadrant.TopologyLabel: "true"}, + }, + Data: map[string]string{ + "topology": expectedDot, + }, + }, + } + + topology, err := machinery.NewTopology( + machinery.WithObjects(existingCM), + ) + assert.NilError(t, err) + + fakeClient := dfake.NewSimpleDynamicClient(scheme) + unstructuredCM, err := controller.Destruct(existingCM.Object.(*corev1.ConfigMap)) + assert.NilError(t, err) + _, err = fakeClient.Resource(controller.ConfigMapsResource).Namespace(namespace).Create(context.Background(), unstructuredCM, metav1.CreateOptions{}) + assert.NilError(t, err) + + reconciler := NewTopologyReconciler(fakeClient, namespace) + err = reconciler.Reconcile(context.Background(), nil, topology, nil, &sync.Map{}) + assert.NilError(t, err) +} + +func TestNewTopologyReconciler_PanicsOnEmptyNamespace(t *testing.T) { + defer func() { + r := recover() + assert.Assert(t, r != nil, "expected panic for empty namespace") + }() + NewTopologyReconciler(nil, "") +} + +func unstructuredNestedString(obj map[string]any, fields ...string) (string, bool, error) { + current := obj + for i, field := range fields { + if i == len(fields)-1 { + val, ok := current[field] + if !ok { + return "", false, nil + } + s, ok := val.(string) + return s, ok, nil + } + next, ok := current[field] + if !ok { + return "", false, nil + } + current, ok = next.(map[string]any) + if !ok { + return "", false, nil + } + } + return "", false, nil +}