Add flag to disable cross namespace refs to AlertProviders and MetricTemplates

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

cmd/flagger/main.go

@@ -84,6 +84,7 @@ var (
 	ver                      bool
 	kubeconfigServiceMesh    string
 	clusterName              string
+	noCrossNamespaceRefs     bool
 )
 
 func init() {
@@ -117,6 +118,7 @@ func init() {
 	flag.BoolVar(&ver, "version", false, "Print version")
 	flag.StringVar(&kubeconfigServiceMesh, "kubeconfig-service-mesh", "", "Path to a kubeconfig for the service mesh control plane cluster.")
 	flag.StringVar(&clusterName, "cluster-name", "", "Cluster name to be included in alert msgs.")
+	flag.BoolVar(&noCrossNamespaceRefs, "no-cross-namespace-refs", false, "When set to true, Flagger can only refer to resources in the same namespace.")
 }
 
 func main() {
@@ -241,6 +243,7 @@ func main() {
 		version.VERSION,
 		fromEnv("EVENT_WEBHOOK_URL", eventWebhook),
 		clusterName,
+		noCrossNamespaceRefs,
 	)
 
 	// leader election context

pkg/apis/flagger/v1beta1/canary.go

@@ -70,15 +70,15 @@ type CanarySpec struct {
 	MetricsServer string `json:"metricsServer,omitempty"`
 
 	// TargetRef references a target resource
-	TargetRef CrossNamespaceObjectReference `json:"targetRef"`
+	TargetRef LocalObjectReference `json:"targetRef"`
 
 	// AutoscalerRef references an autoscaling resource
 	// +optional
-	AutoscalerRef *CrossNamespaceObjectReference `json:"autoscalerRef,omitempty"`
+	AutoscalerRef *LocalObjectReference `json:"autoscalerRef,omitempty"`
 
 	// Reference to NGINX ingress resource
 	// +optional
-	IngressRef *CrossNamespaceObjectReference `json:"ingressRef,omitempty"`
+	IngressRef *LocalObjectReference `json:"ingressRef,omitempty"`
 
 	// Reference to Gloo Upstream resource. Upstream config is copied from
 	// the referenced upstream to the upstreams generated by flagger.
@@ -393,6 +393,21 @@ type CrossNamespaceObjectReference struct {
 	Namespace string `json:"namespace,omitempty"`
 }
 
+// LocalObjectReference contains enough information to let you locate the typed
+// referenced object in the same namespace.
+type LocalObjectReference struct {
+	// API version of the referent
+	// +optional
+	APIVersion string `json:"apiVersion,omitempty"`
+
+	// Kind of the referent
+	// +optional
+	Kind string `json:"kind,omitempty"`
+
+	// Name of the referent
+	Name string `json:"name"`
+}
+
 // CustomMetadata holds labels and annotations to set on generated objects.
 type CustomMetadata struct {
 	Labels      map[string]string `json:"labels,omitempty"`

pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go

@@ -422,12 +422,12 @@ func (in *CanarySpec) DeepCopyInto(out *CanarySpec) {
 	out.TargetRef = in.TargetRef
 	if in.AutoscalerRef != nil {
 		in, out := &in.AutoscalerRef, &out.AutoscalerRef
-		*out = new(CrossNamespaceObjectReference)
+		*out = new(LocalObjectReference)
 		**out = **in
 	}
 	if in.IngressRef != nil {
 		in, out := &in.IngressRef, &out.IngressRef
-		*out = new(CrossNamespaceObjectReference)
+		*out = new(LocalObjectReference)
 		**out = **in
 	}
 	if in.UpstreamRef != nil {
@@ -621,6 +621,22 @@ func (in *CustomMetadata) DeepCopy() *CustomMetadata {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *LocalObjectReference) DeepCopyInto(out *LocalObjectReference) {
+	*out = *in
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalObjectReference.
+func (in *LocalObjectReference) DeepCopy() *LocalObjectReference {
+	if in == nil {
+		return nil
+	}
+	out := new(LocalObjectReference)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *MetricTemplate) DeepCopyInto(out *MetricTemplate) {
 	*out = *in

pkg/canary/daemonset_fixture_test.go

@@ -352,7 +352,7 @@ func newDaemonSetControllerTestCanary(dc daemonsetConfigs) *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       dc.name,
 				APIVersion: "apps/v1",
 				Kind:       "DaemonSet",

pkg/canary/deployment_fixture_test.go

@@ -392,12 +392,12 @@ func newDeploymentControllerTestCanary(cc canaryConfigs) *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       cc.targetName,
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
 			},
-			AutoscalerRef: &flaggerv1.CrossNamespaceObjectReference{
+			AutoscalerRef: &flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "autoscaling/v2beta2",
 				Kind:       "HorizontalPodAutoscaler",

pkg/controller/controller.go

@@ -49,24 +49,25 @@ const controllerAgentName = "flagger"
 
 // Controller is managing the canary objects and schedules canary deployments
 type Controller struct {
-	kubeClient       kubernetes.Interface
-	flaggerClient    clientset.Interface
-	flaggerInformers Informers
-	flaggerSynced    cache.InformerSynced
-	flaggerWindow    time.Duration
-	workqueue        workqueue.RateLimitingInterface
-	eventRecorder    record.EventRecorder
-	logger           *zap.SugaredLogger
-	canaries         *sync.Map
-	jobs             map[string]CanaryJob
-	recorder         metrics.Recorder
-	notifier         notifier.Interface
-	canaryFactory    *canary.Factory
-	routerFactory    *router.Factory
-	observerFactory  *observers.Factory
-	meshProvider     string
-	eventWebhook     string
-	clusterName      string
+	kubeClient           kubernetes.Interface
+	flaggerClient        clientset.Interface
+	flaggerInformers     Informers
+	flaggerSynced        cache.InformerSynced
+	flaggerWindow        time.Duration
+	workqueue            workqueue.RateLimitingInterface
+	eventRecorder        record.EventRecorder
+	logger               *zap.SugaredLogger
+	canaries             *sync.Map
+	jobs                 map[string]CanaryJob
+	recorder             metrics.Recorder
+	notifier             notifier.Interface
+	canaryFactory        *canary.Factory
+	routerFactory        *router.Factory
+	observerFactory      *observers.Factory
+	meshProvider         string
+	eventWebhook         string
+	clusterName          string
+	noCrossNamespaceRefs bool
 }
 
 type Informers struct {
@@ -89,6 +90,7 @@ func NewController(
 	version string,
 	eventWebhook string,
 	clusterName string,
+	noCrossNamespaceRefs bool,
 ) *Controller {
 	logger.Debug("Creating event broadcaster")
 	flaggerscheme.AddToScheme(scheme.Scheme)
@@ -103,24 +105,25 @@ func NewController(
 	recorder.SetInfo(version, meshProvider)
 
 	ctrl := &Controller{
-		kubeClient:       kubeClient,
-		flaggerClient:    flaggerClient,
-		flaggerInformers: flaggerInformers,
-		flaggerSynced:    flaggerInformers.CanaryInformer.Informer().HasSynced,
-		workqueue:        workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerAgentName),
-		eventRecorder:    eventRecorder,
-		logger:           logger,
-		canaries:         new(sync.Map),
-		jobs:             map[string]CanaryJob{},
-		flaggerWindow:    flaggerWindow,
-		observerFactory:  observerFactory,
-		recorder:         recorder,
-		notifier:         notifier,
-		canaryFactory:    canaryFactory,
-		routerFactory:    routerFactory,
-		meshProvider:     meshProvider,
-		eventWebhook:     eventWebhook,
-		clusterName:      clusterName,
+		kubeClient:           kubeClient,
+		flaggerClient:        flaggerClient,
+		flaggerInformers:     flaggerInformers,
+		flaggerSynced:        flaggerInformers.CanaryInformer.Informer().HasSynced,
+		workqueue:            workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerAgentName),
+		eventRecorder:        eventRecorder,
+		logger:               logger,
+		canaries:             new(sync.Map),
+		jobs:                 map[string]CanaryJob{},
+		flaggerWindow:        flaggerWindow,
+		observerFactory:      observerFactory,
+		recorder:             recorder,
+		notifier:             notifier,
+		canaryFactory:        canaryFactory,
+		routerFactory:        routerFactory,
+		meshProvider:         meshProvider,
+		eventWebhook:         eventWebhook,
+		clusterName:          clusterName,
+		noCrossNamespaceRefs: noCrossNamespaceRefs,
 	}
 
 	flaggerInformers.CanaryInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{

pkg/controller/events.go

@@ -108,7 +108,12 @@ func (c *Controller) alert(canary *flaggerv1.Canary, message string, metadata bo
 
 		// determine alert provider namespace
 		providerNamespace := canary.GetNamespace()
-		if alert.ProviderRef.Namespace != "" {
+		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_daemonset_fixture_test.go

@@ -261,7 +261,7 @@ func newDaemonSetTestCanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "DaemonSet",
@@ -318,7 +318,7 @@ func newDaemonSetTestCanaryAB() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "DaemonSet",

pkg/controller/scheduler_deployment_fixture_test.go

@@ -289,12 +289,12 @@ func newDeploymentTestCanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
 			},
-			AutoscalerRef: &flaggerv1.CrossNamespaceObjectReference{
+			AutoscalerRef: &flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "autoscaling/v2beta2",
 				Kind:       "HorizontalPodAutoscaler",
@@ -351,12 +351,12 @@ func newDeploymentTestCanaryAB() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
 			},
-			AutoscalerRef: &flaggerv1.CrossNamespaceObjectReference{
+			AutoscalerRef: &flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "autoscaling/v2beta2",
 				Kind:       "HorizontalPodAutoscaler",

pkg/controller/scheduler_metrics.go

@@ -54,7 +54,10 @@ func (c *Controller) checkMetricProviderAvailability(canary *flaggerv1.Canary) e
 
 		if metric.TemplateRef != nil {
 			namespace := canary.Namespace
-			if metric.TemplateRef.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
 			}
 
@@ -238,7 +241,11 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool {
 	for _, metric := range canary.GetAnalysis().Metrics {
 		if metric.TemplateRef != nil {
 			namespace := canary.Namespace
-			if metric.TemplateRef.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,5 +65,9 @@ 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))
 	})
 }

pkg/controller/scheduler_svc_test.go

@@ -123,7 +123,7 @@ func newTestServiceCanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "core/v1",
 				Kind:       "Service",
@@ -161,7 +161,7 @@ func newTestServiceCanaryMaxWeight() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "core/v1",
 				Kind:       "Service",
@@ -199,7 +199,7 @@ func newTestServiceCanaryWithWeightsHappyCase() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "core/v1",
 				Kind:       "Service",
@@ -236,7 +236,7 @@ func newTestServiceCanaryWithWeightsOverflow() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "core/v1",
 				Kind:       "Service",

pkg/router/gateway_api.go

@@ -64,9 +64,6 @@ func (gwr *GatewayAPIRouter) Reconcile(canary *flaggerv1.Canary) error {
 	apexSvcName, primarySvcName, canarySvcName := canary.GetServiceNames()
 
 	hrNamespace := canary.Namespace
-	if canary.Spec.TargetRef.Namespace != "" {
-		hrNamespace = canary.Spec.TargetRef.Namespace
-	}
 
 	hostNames := []v1alpha2.Hostname{}
 	for _, host := range canary.Spec.Service.Hosts {
@@ -193,9 +190,6 @@ func (gwr *GatewayAPIRouter) GetRoutes(canary *flaggerv1.Canary) (
 ) {
 	apexSvcName, primarySvcName, canarySvcName := canary.GetServiceNames()
 	hrNamespace := canary.Namespace
-	if canary.Spec.TargetRef.Namespace != "" {
-		hrNamespace = canary.Spec.TargetRef.Namespace
-	}
 	httpRoute, err := gwr.gatewayAPIClient.GatewayapiV1alpha2().HTTPRoutes(hrNamespace).Get(context.TODO(), apexSvcName, metav1.GetOptions{})
 	if err != nil {
 		err = fmt.Errorf("HTTPRoute %s.%s get error: %w", apexSvcName, hrNamespace, err)
@@ -228,9 +222,6 @@ func (gwr *GatewayAPIRouter) SetRoutes(
 	cWeight := int32(canaryWeight)
 	apexSvcName, primarySvcName, canarySvcName := canary.GetServiceNames()
 	hrNamespace := canary.Namespace
-	if canary.Spec.TargetRef.Namespace != "" {
-		hrNamespace = canary.Spec.TargetRef.Namespace
-	}
 	httpRoute, err := gwr.gatewayAPIClient.GatewayapiV1alpha2().HTTPRoutes(hrNamespace).Get(context.TODO(), apexSvcName, metav1.GetOptions{})
 	if err != nil {
 		return fmt.Errorf("HTTPRoute %s.%s get error: %w", apexSvcName, hrNamespace, err)

pkg/router/router_test.go

@@ -92,7 +92,7 @@ func newTestCanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -171,7 +171,7 @@ func newTestCanaryAppMesh() *flaggerv1.Canary {
 			},
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -217,7 +217,7 @@ func newTestSMICanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -268,7 +268,7 @@ func newTestABTest() *flaggerv1.Canary {
 			Name:      "abtest",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "abtest",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -417,12 +417,12 @@ func newTestCanaryIngress() *flaggerv1.Canary {
 			},
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
 			},
-			IngressRef: &flaggerv1.CrossNamespaceObjectReference{
+			IngressRef: &flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "networking.k8s.io/v1",
 				Kind:       "Ingress",
@@ -492,7 +492,7 @@ func newTestGatewayAPICanary() *flaggerv1.Canary {
 			Name:      "podinfo",
 		},
 		Spec: flaggerv1.CanarySpec{
-			TargetRef: flaggerv1.CrossNamespaceObjectReference{
+			TargetRef: flaggerv1.LocalObjectReference{
 				Name:       "podinfo",
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",