diff --git a/cmd/flagger/main.go b/cmd/flagger/main.go index 62f442f7..83105a97 100644 --- a/cmd/flagger/main.go +++ b/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 diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 627d288b..13a45a10 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/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"` diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index b1183b12..b9cdca9b 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/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 diff --git a/pkg/canary/daemonset_fixture_test.go b/pkg/canary/daemonset_fixture_test.go index 01c21be8..3fb4059f 100644 --- a/pkg/canary/daemonset_fixture_test.go +++ b/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", diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index ebe3951d..cf08695f 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/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", diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 87390ec5..d4c97579 100644 --- a/pkg/controller/controller.go +++ b/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{ diff --git a/pkg/controller/events.go b/pkg/controller/events.go index 0ff3eb70..a09011c7 100644 --- a/pkg/controller/events.go +++ b/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 } diff --git a/pkg/controller/scheduler_daemonset_fixture_test.go b/pkg/controller/scheduler_daemonset_fixture_test.go index e0ac8ba8..8ff046a8 100644 --- a/pkg/controller/scheduler_daemonset_fixture_test.go +++ b/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", diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/pkg/controller/scheduler_deployment_fixture_test.go index 6fb5c89e..ef3eebda 100644 --- a/pkg/controller/scheduler_deployment_fixture_test.go +++ b/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", diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index f15945e2..705e88a5 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/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 } diff --git a/pkg/controller/scheduler_metrics_test.go b/pkg/controller/scheduler_metrics_test.go index 81ec50cc..163f5509 100644 --- a/pkg/controller/scheduler_metrics_test.go +++ b/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)) }) } diff --git a/pkg/controller/scheduler_svc_test.go b/pkg/controller/scheduler_svc_test.go index 4cad2a22..639c9f34 100644 --- a/pkg/controller/scheduler_svc_test.go +++ b/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", diff --git a/pkg/router/gateway_api.go b/pkg/router/gateway_api.go index 16be700d..c9b0888d 100644 --- a/pkg/router/gateway_api.go +++ b/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) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index e077e008..9f52cbc1 100644 --- a/pkg/router/router_test.go +++ b/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",