pkg/configmap: sort configmap data by key when loading bundle (#1630)

This functionality is used by OLMv0 to construct an `api.Bundle` object
that is needed to create install plan steps, which are eventually put
into the installplan.status.plan field. It is important to have
deterministic ordering of slice fields in Kubernetes APIs to avoid
constant re-reconciliation and/or false positive change detection.

Specifically in the case of InstallPlan steps, the order of the steps
does have _some_ semantic requirements (CSV, then CRDs, then everything
else). The problem is that if "everything else" is in a random order,
the order of the resulting steps will still have non-determinism.

If we have deterministic order of steps, OLM may be able to resolve a
class of bugs related to unnecessary creation of multiple install plans
that would otherwise be identical.

Signed-off-by: Joe Lanford <[email protected]>
Upstream-repository: operator-registry
Upstream-commit: e80f8925c8da9161270ad0389c088a29146eb81e

Author: joelanford

staging/operator-registry/pkg/configmap/configmap.go

@@ -3,6 +3,8 @@ package configmap
 import (
 	"errors"
 	"fmt"
+	"maps"
+	"slices"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -68,7 +70,9 @@ func loadBundle(entry *logrus.Entry, cm *corev1.ConfigMap) (*api.Bundle, map[str
 	}
 
 	// Add kube resources to the bundle.
-	for name, content := range data {
+	// Sort keys by name to guarantee consistent ordering of bundle objects.
+	for _, name := range slices.Sorted(maps.Keys(data)) {
+		content := data[name]
 		reader := strings.NewReader(content)
 		logger := entry.WithFields(logrus.Fields{
 			"key": name,

staging/operator-registry/pkg/configmap/configmap_test.go

@@ -62,7 +62,9 @@ func TestLoad(t *testing.T) {
 
 				unst, err := unstructuredlib.FromString(csvGot)
 				require.NoError(t, err)
-				assert.True(t, unst.GetName() == "first" || unst.GetName() == "second")
+
+				// The last CSV (by lexicographical sort of configmap data keys) always wins.
+				assert.Equal(t, "second", unst.GetName())
 			},
 		},
 		{
@@ -167,7 +169,9 @@ func TestLoadWriteRead(t *testing.T) {
 			bundleObjects, err := unstructuredlib.FromBundle(bundle)
 			require.NoError(t, err)
 
-			assert.ElementsMatch(t, expectedObjects, bundleObjects)
+			// Assert that the order of manifests from the original manifests
+			// directory is preserved (by lexicographical sorting by filename)
+			assert.Equal(t, expectedObjects, bundleObjects)
 		})
 	}
 }