fix: prevent memoization accessing wrong config-maps (#11225)

Signed-off-by: Alan Clucas <alan@clucas.org>

docs/memoization.md

@@ -11,7 +11,8 @@ it stores the outputs of a template into a specified cache with a variable key.
 ## Cache Method
 
 Currently, caching can only be performed with config-maps.
-This allows you to easily manipulate cache entries manually through `kubectl` and the Kubernetes API without having to go through Argo.  
+This allows you to easily manipulate cache entries manually through `kubectl` and the Kubernetes API without having to go through Argo.
+All cache config-maps must have the label `workflows.argoproj.io/configmap-type: Cache` to be used as a cache. This prevents accidental access to other important config-maps in the system
 
 ## Using Memoization
 

workflow/controller/cache/configmap_cache.go

@@ -42,6 +42,22 @@ func (c *configMapCache) logInfo(fields log.Fields, message string) {
 	log.WithFields(log.Fields{"namespace": c.namespace, "name": c.name}).WithFields(fields).Info(message)
 }
 
+func (c *configMapCache) validateConfigmap(cm *apiv1.ConfigMap) error {
+	label, foundLabel := cm.GetLabels()[common.LabelKeyConfigMapType]
+	errString := ""
+	if !foundLabel {
+		errString = fmt.Sprintf("memoization configmap doesn't have %s label, refusing to use it", common.LabelKeyConfigMapType)
+	} else if label != common.LabelValueTypeConfigMapCache {
+		errString = fmt.Sprintf("memoization configmap doesn't have label %s = %s, refusing to use it", common.LabelKeyConfigMapType, common.LabelValueTypeConfigMapCache)
+	}
+	if errString != "" {
+		err := errors.New(errString)
+		c.logError(err, log.Fields{}, errString)
+		return err
+	}
+	return nil
+}
+
 func (c *configMapCache) Load(ctx context.Context, key string) (*Entry, error) {
 	if !cacheKeyRegex.MatchString(key) {
 		return nil, fmt.Errorf("invalid cache key: %s", key)
@@ -58,6 +74,11 @@ func (c *configMapCache) Load(ctx context.Context, key string) (*Entry, error) {
 		}
 		c.logError(err, log.Fields{}, "Error loading config map cache")
 		return nil, fmt.Errorf("could not load config map cache: %w", err)
+	} else {
+		err := c.validateConfigmap(cm)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	c.logInfo(log.Fields{}, "config map cache loaded")
@@ -115,6 +136,11 @@ func (c *configMapCache) Save(ctx context.Context, key string, nodeId string, va
 			c.logError(err, log.Fields{"key": key, "nodeId": nodeId}, "Error saving to ConfigMap cache")
 			return fmt.Errorf("could not save to config map cache: %w", err)
 		}
+	} else {
+		err := c.validateConfigmap(cache)
+		if err != nil {
+			return err
+		}
 	}
 
 	creationTime := time.Now()

workflow/controller/cache_test.go

@@ -9,6 +9,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/workflow/common"
 	"github.com/argoproj/argo-workflows/v3/workflow/controller/cache"
 )
 
@@ -23,6 +24,9 @@ var sampleConfigMapCacheEntry = apiv1.ConfigMap{
 	ObjectMeta: metav1.ObjectMeta{
 		Name:            "whalesay-cache",
 		ResourceVersion: "1630732",
+		Labels: map[string]string{
+			common.LabelKeyConfigMapType: common.LabelValueTypeConfigMapCache,
+		},
 	},
 }
 
@@ -34,6 +38,9 @@ var sampleConfigMapEmptyCacheEntry = apiv1.ConfigMap{
 	ObjectMeta: metav1.ObjectMeta{
 		Name:            "whalesay-cache",
 		ResourceVersion: "1630732",
+		Labels: map[string]string{
+			common.LabelKeyConfigMapType: common.LabelValueTypeConfigMapCache,
+		},
 	},
 }
 
@@ -46,9 +53,8 @@ func TestConfigMapCacheLoadHit(t *testing.T) {
 	assert.NoError(t, err)
 	c := cache.NewConfigMapCache("default", controller.kubeclientset, "whalesay-cache")
 
-	cm, err := controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, sampleConfigMapCacheEntry.Name, metav1.GetOptions{})
+	_, err = controller.kubeclientset.CoreV1().ConfigMaps("default").Get(ctx, sampleConfigMapCacheEntry.Name, metav1.GetOptions{})
 	assert.NoError(t, err)
-	assert.Nil(t, cm.Labels)
 
 	entry, err := c.Load(ctx, "hi-there-world")
 	assert.NoError(t, err)

workflow/controller/operator_test.go

@@ -5100,6 +5100,9 @@ func TestConfigMapCacheLoadOperate(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            "whalesay-cache",
 			ResourceVersion: "1630732",
+			Labels: map[string]string{
+				common.LabelKeyConfigMapType: common.LabelValueTypeConfigMapCache,
+			},
 		},
 	}
 	wf := wfv1.MustUnmarshalWorkflow(workflowCached)
@@ -5172,6 +5175,9 @@ func TestConfigMapCacheLoadOperateMaxAge(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:            "whalesay-cache",
 				ResourceVersion: "1630732",
+				Labels: map[string]string{
+					common.LabelKeyConfigMapType: common.LabelValueTypeConfigMapCache,
+				},
 			},
 		}
 	}
@@ -5217,6 +5223,45 @@ func TestConfigMapCacheLoadOperateMaxAge(t *testing.T) {
 	}
 }
 
+func TestConfigMapCacheLoadNoLabels(t *testing.T) {
+	sampleConfigMapCacheEntry := apiv1.ConfigMap{
+		Data: map[string]string{
+			"hi-there-world": `{"ExpiresAt":"2020-06-18T17:11:05Z","NodeID":"memoize-abx4124-123129321123","Outputs":{}}`,
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ConfigMap",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            "whalesay-cache",
+			ResourceVersion: "1630732",
+		},
+	}
+	wf := wfv1.MustUnmarshalWorkflow(workflowCached)
+	cancel, controller := newController()
+	defer cancel()
+
+	ctx := context.Background()
+	_, err := controller.wfclientset.ArgoprojV1alpha1().Workflows(wf.ObjectMeta.Namespace).Create(ctx, wf, metav1.CreateOptions{})
+	assert.NoError(t, err)
+	_, err = controller.kubeclientset.CoreV1().ConfigMaps("default").Create(ctx, &sampleConfigMapCacheEntry, metav1.CreateOptions{})
+	assert.NoError(t, err)
+
+	woc := newWorkflowOperationCtx(wf, controller)
+	fn := func() {
+		woc.operate(ctx)
+	}
+	assert.NotPanics(t, fn)
+	assert.Equal(t, wfv1.WorkflowError, woc.wf.Status.Phase)
+
+	if assert.Len(t, woc.wf.Status.Nodes, 1) {
+		for _, node := range woc.wf.Status.Nodes {
+			assert.Nil(t, node.Outputs)
+			assert.Equal(t, wfv1.NodeError, node.Phase)
+		}
+	}
+}
+
 func TestConfigMapCacheLoadNilOutputs(t *testing.T) {
 	sampleConfigMapCacheEntry := apiv1.ConfigMap{
 		Data: map[string]string{
@@ -5229,6 +5274,9 @@ func TestConfigMapCacheLoadNilOutputs(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            "whalesay-cache",
 			ResourceVersion: "1630732",
+			Labels: map[string]string{
+				common.LabelKeyConfigMapType: common.LabelValueTypeConfigMapCache,
+			},
 		},
 	}
 	wf := wfv1.MustUnmarshalWorkflow(workflowCached)