verify canary spec before syncing

Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>

Author: Sanskar Jaiswal

docs/gitbook/faq.md

@@ -74,8 +74,8 @@ to finish, and disable it afterward (`skipAnalysis: false`).
 
 #### How to disable cross namespace references?
 
-Flagger by default can access resources across namespaces (`AlertProivder` and `MetricProvider`). If you're in a multi-tenant enviornemnt
-and wish to disable this, you can do so through the `no-cross-namespace-refs` flag.
+Flagger by default can access resources across namespaces (`AlertProivder`, `MetricProvider` and Gloo `Upsteream`).
+If you're in a multi-tenant environment and wish to disable this, you can do so through the `no-cross-namespace-refs` flag.
 
 ```
 flagger \

pkg/controller/controller.go

@@ -253,6 +253,10 @@ func (c *Controller) syncHandler(key string) error {
 		return nil
 	}
 
+	if err := c.verifyCanary(cd); err != nil {
+		return fmt.Errorf("invalid canary spec: %s", err)
+	}
+
 	// Finalize if canary has been marked for deletion and revert is desired
 	if cd.Spec.RevertOnDeletion && cd.ObjectMeta.DeletionTimestamp != nil {
 		// If finalizers have been previously removed proceed
@@ -315,6 +319,34 @@ func (c *Controller) enqueue(obj interface{}) {
 	c.workqueue.AddRateLimited(key)
 }
 
+func (c *Controller) verifyCanary(canary *flaggerv1.Canary) error {
+	if c.noCrossNamespaceRefs {
+		if err := verifyNoCrossNamespaceRefs(canary); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func verifyNoCrossNamespaceRefs(canary *flaggerv1.Canary) error {
+	if canary.Spec.UpstreamRef != nil && canary.Spec.UpstreamRef.Namespace != canary.Namespace {
+		return fmt.Errorf("can't access gloo upstream %s.%s, cross-namespace references are blocked", canary.Spec.UpstreamRef.Name, canary.Spec.UpstreamRef.Namespace)
+	}
+	if canary.Spec.Analysis != nil {
+		for _, metric := range canary.Spec.Analysis.Metrics {
+			if metric.TemplateRef != nil && metric.TemplateRef.Namespace != canary.Namespace {
+				return fmt.Errorf("can't access metric template %s.%s, cross-namespace references are blocked", metric.TemplateRef.Name, metric.TemplateRef.Namespace)
+			}
+		}
+		for _, alert := range canary.Spec.Analysis.Alerts {
+			if alert.ProviderRef.Namespace != canary.Namespace {
+				return fmt.Errorf("can't access alert provider %s.%s, cross-namespace references are blocked", alert.ProviderRef.Name, alert.ProviderRef.Namespace)
+			}
+		}
+	}
+	return nil
+}
+
 func checkCustomResourceType(obj interface{}, logger *zap.SugaredLogger) (flaggerv1.Canary, bool) {
 	var roll *flaggerv1.Canary
 	var ok bool

pkg/controller/controller_test.go

@@ -0,0 +1,107 @@
+package controller
+
+import (
+	"testing"
+
+	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestController_verifyCanary(t *testing.T) {
+	tests := []struct {
+		name    string
+		canary  flaggerv1.Canary
+		wantErr bool
+	}{
+		{
+			name: "Gloo upstream in a different namespace should return an error",
+			canary: flaggerv1.Canary{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cd-1",
+					Namespace: "default",
+				},
+				Spec: flaggerv1.CanarySpec{
+					UpstreamRef: &flaggerv1.CrossNamespaceObjectReference{
+						Name:      "upstream",
+						Namespace: "test",
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Gloo upstream in the same namespace is allowed",
+			canary: flaggerv1.Canary{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cd-1",
+					Namespace: "default",
+				},
+				Spec: flaggerv1.CanarySpec{
+					UpstreamRef: &flaggerv1.CrossNamespaceObjectReference{
+						Name:      "upstream",
+						Namespace: "default",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "MetricTemplate in a different namespace should return an error",
+			canary: flaggerv1.Canary{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cd-1",
+					Namespace: "default",
+				},
+				Spec: flaggerv1.CanarySpec{
+					Analysis: &flaggerv1.CanaryAnalysis{
+						Metrics: []flaggerv1.CanaryMetric{
+							{
+								TemplateRef: &flaggerv1.CrossNamespaceObjectReference{
+									Name:      "mt-1",
+									Namespace: "test",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "AlertProvider in a different namespace should return an error",
+			canary: flaggerv1.Canary{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cd-1",
+					Namespace: "default",
+				},
+				Spec: flaggerv1.CanarySpec{
+					Analysis: &flaggerv1.CanaryAnalysis{
+						Alerts: []flaggerv1.CanaryAlert{
+							{
+								ProviderRef: flaggerv1.CrossNamespaceObjectReference{
+									Name:      "ap-1",
+									Namespace: "test",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+	}
+
+	ctrl := &Controller{
+		noCrossNamespaceRefs: true,
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := ctrl.verifyCanary(&test.canary)
+			if test.wantErr {
+				require.Error(t, err)
+			}
+		})
+	}
+}

pkg/controller/events.go

@@ -109,11 +109,6 @@ func (c *Controller) alert(canary *flaggerv1.Canary, message string, metadata bo
 		// determine alert provider namespace
 		providerNamespace := canary.GetNamespace()
 		if alert.ProviderRef.Namespace != canary.Namespace {
-			if c.noCrossNamespaceRefs {
-				c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)).
-					Errorf("can't access alert provider ref %s.%s, cross-namespace references are blocked", alert.ProviderRef.Name, alert.ProviderRef.Namespace)
-				return
-			}
 			providerNamespace = alert.ProviderRef.Namespace
 		}
 

pkg/controller/scheduler_metrics.go

@@ -55,9 +55,6 @@ func (c *Controller) checkMetricProviderAvailability(canary *flaggerv1.Canary) e
 		if metric.TemplateRef != nil {
 			namespace := canary.Namespace
 			if metric.TemplateRef.Namespace != canary.Namespace {
-				if c.noCrossNamespaceRefs {
-					return fmt.Errorf("can't access metric template ref %s.%s, cross-namespace references are blocked", metric.TemplateRef.Name, metric.TemplateRef.Namespace)
-				}
 				namespace = metric.TemplateRef.Namespace
 			}
 
@@ -242,10 +239,6 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool {
 		if metric.TemplateRef != nil {
 			namespace := canary.Namespace
 			if metric.TemplateRef.Namespace != canary.Namespace {
-				if c.noCrossNamespaceRefs {
-					c.recordEventErrorf(canary, "Metric template %s.%s error: cross-namespace references are blocked", metric.TemplateRef.Name, metric.TemplateRef.Namespace)
-					return false
-				}
 				namespace = metric.TemplateRef.Namespace
 			}
 

pkg/controller/scheduler_metrics_test.go

@@ -65,9 +65,5 @@ func TestController_checkMetricProviderAvailability(t *testing.T) {
 			Namespace: "default",
 		}
 		require.NoError(t, ctrl.checkMetricProviderAvailability(canary))
-
-		ctrl.noCrossNamespaceRefs = true
-		canary.Namespace = "test"
-		require.Error(t, ctrl.checkMetricProviderAvailability(canary))
 	})
 }