FIX: Prevent operator crash when topology ConfigMap exceeds 1MB limit

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
 }

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
+}