From 06d32049c843b990bdd43842e4abeb0cee90e82a Mon Sep 17 00:00:00 2001 From: Joakim Ahrlin Date: Tue, 15 Jun 2021 13:48:46 +0200 Subject: [PATCH 1/5] add option for insecureSkipVerify for Prometheus and Graphite Signed-off-by: Joakim Ahrlin --- cmd/flagger/main.go | 4 +++- pkg/apis/flagger/v1beta1/canary.go | 4 ++++ pkg/apis/flagger/v1beta1/metric.go | 4 ++++ pkg/controller/scheduler_daemonset_fixture_test.go | 2 +- pkg/controller/scheduler_deployment_fixture_test.go | 2 +- pkg/controller/scheduler_metrics.go | 4 ++-- pkg/controller/scheduler_metrics_test.go | 4 ++-- pkg/metrics/observers/factory.go | 9 +++++---- pkg/metrics/providers/graphite.go | 11 ++++++++++- pkg/metrics/providers/prometheus.go | 11 ++++++++++- 10 files changed, 42 insertions(+), 13 deletions(-) diff --git a/cmd/flagger/main.go b/cmd/flagger/main.go index 1bab4ec2e..7d88cf9dd 100644 --- a/cmd/flagger/main.go +++ b/cmd/flagger/main.go @@ -59,6 +59,7 @@ var ( kubeconfigQPS int kubeconfigBurst int metricsServer string + insecureSkipVerify bool controlLoopInterval time.Duration logLevel string port string @@ -91,6 +92,7 @@ func init() { flag.IntVar(&kubeconfigBurst, "kubeconfig-burst", 250, "Set Burst for kubeconfig.") flag.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") flag.StringVar(&metricsServer, "metrics-server", "http://prometheus:9090", "Prometheus URL.") + flag.BoolVar(&insecureSkipVerify, "insecure-skip-verify", false, "disable verification of certificate for metrics server, this is insecure") flag.DurationVar(&controlLoopInterval, "control-loop-interval", 10*time.Second, "Kubernetes API sync interval.") flag.StringVar(&logLevel, "log-level", "debug", "Log level can be: debug, info, warning, error.") flag.StringVar(&port, "port", "8080", "Port to listen on.") @@ -190,7 +192,7 @@ func main() { logger.Infof("Watching namespace %s", namespace) } - observerFactory, err := observers.NewFactory(metricsServer) + observerFactory, err := observers.NewFactory(metricsServer, insecureSkipVerify) if err != nil { logger.Fatalf("Error building prometheus client: %s", err.Error()) } diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 0e6c533c3..7e29bfb28 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -66,6 +66,10 @@ type CanarySpec struct { // +optional MetricsServer string `json:"metricsServer,omitempty"` + // InsecureSkipVerify disables certificate verification for the provider + // +optional + InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` + // TargetRef references a target resource TargetRef CrossNamespaceObjectReference `json:"targetRef"` diff --git a/pkg/apis/flagger/v1beta1/metric.go b/pkg/apis/flagger/v1beta1/metric.go index 549fcfb3e..3d151c54a 100644 --- a/pkg/apis/flagger/v1beta1/metric.go +++ b/pkg/apis/flagger/v1beta1/metric.go @@ -74,6 +74,10 @@ type MetricTemplateProvider struct { // Region of the provider // +optional Region string `json:"region,omitempty"` + + // InsecureSkipVerify disables certificate verification for the provider + // +optional + InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` } // MetricTemplateModel is the query template model diff --git a/pkg/controller/scheduler_daemonset_fixture_test.go b/pkg/controller/scheduler_daemonset_fixture_test.go index e0ac8ba85..1ee2010f5 100644 --- a/pkg/controller/scheduler_daemonset_fixture_test.go +++ b/pkg/controller/scheduler_daemonset_fixture_test.go @@ -95,7 +95,7 @@ func newDaemonSetFixture(c *flaggerv1.Canary) daemonSetFixture { rf := router.NewFactory(nil, kubeClient, flaggerClient, "annotationsPrefix", "", logger, flaggerClient) // init observer - observerFactory, _ := observers.NewFactory(testMetricsServerURL) + observerFactory, _ := observers.NewFactory(testMetricsServerURL, false) // init canary factory configTracker := &canary.ConfigTracker{ diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/pkg/controller/scheduler_deployment_fixture_test.go index 6fb5c89e5..9d3558b48 100644 --- a/pkg/controller/scheduler_deployment_fixture_test.go +++ b/pkg/controller/scheduler_deployment_fixture_test.go @@ -123,7 +123,7 @@ func newDeploymentFixture(c *flaggerv1.Canary) fixture { rf := router.NewFactory(nil, kubeClient, flaggerClient, "annotationsPrefix", "", logger, flaggerClient) // init observer - observerFactory, _ := observers.NewFactory(testMetricsServerURL) + observerFactory, _ := observers.NewFactory(testMetricsServerURL, false) // init canary factory configTracker := &canary.ConfigTracker{ diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index 9e94f770f..00772da33 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -41,7 +41,7 @@ func (c *Controller) checkMetricProviderAvailability(canary *flaggerv1.Canary) e observerFactory := c.observerFactory if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, canary.Spec.InsecureSkipVerify) if err != nil { return fmt.Errorf("error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) } @@ -120,7 +120,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool { // override the global metrics server if one is specified in the canary spec if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, canary.Spec.InsecureSkipVerify) if err != nil { c.recordEventErrorf(canary, "Error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) return false diff --git a/pkg/controller/scheduler_metrics_test.go b/pkg/controller/scheduler_metrics_test.go index 81ec50ccb..d8e87ad13 100644 --- a/pkg/controller/scheduler_metrics_test.go +++ b/pkg/controller/scheduler_metrics_test.go @@ -32,13 +32,13 @@ func TestController_checkMetricProviderAvailability(t *testing.T) { // ok analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{Name: "request-success-rate"}}} canary := &flaggerv1.Canary{Spec: flaggerv1.CanarySpec{Analysis: analysis}} - obs, err := observers.NewFactory(testMetricsServerURL) + obs, err := observers.NewFactory(testMetricsServerURL, false) require.NoError(t, err) ctrl := Controller{observerFactory: obs, logger: zap.S(), eventRecorder: &record.FakeRecorder{}} require.NoError(t, ctrl.checkMetricProviderAvailability(canary)) // error - ctrl.observerFactory, err = observers.NewFactory("http://non-exist") + ctrl.observerFactory, err = observers.NewFactory("http://non-exist", false) require.NoError(t, err) require.Error(t, ctrl.checkMetricProviderAvailability(canary)) diff --git a/pkg/metrics/observers/factory.go b/pkg/metrics/observers/factory.go index 901d58575..5abaa4490 100644 --- a/pkg/metrics/observers/factory.go +++ b/pkg/metrics/observers/factory.go @@ -27,11 +27,12 @@ type Factory struct { Client providers.Interface } -func NewFactory(metricsServer string) (*Factory, error) { +func NewFactory(metricsServer string, insecureSkipVerify bool) (*Factory, error) { client, err := providers.NewPrometheusProvider(flaggerv1.MetricTemplateProvider{ - Type: "prometheus", - Address: metricsServer, - SecretRef: nil, + Type: "prometheus", + Address: metricsServer, + SecretRef: nil, + InsecureSkipVerify: insecureSkipVerify, }, nil) if err != nil { return nil, err diff --git a/pkg/metrics/providers/graphite.go b/pkg/metrics/providers/graphite.go index a19501821..a89646811 100644 --- a/pkg/metrics/providers/graphite.go +++ b/pkg/metrics/providers/graphite.go @@ -18,6 +18,7 @@ package providers import ( "context" + "crypto/tls" "encoding/json" "fmt" "io/ioutil" @@ -104,6 +105,7 @@ type GraphiteProvider struct { username string password string timeout time.Duration + client *http.Client } // NewGraphiteProvider takes a provider spec and credentials map, @@ -119,6 +121,13 @@ func NewGraphiteProvider(provider flaggerv1.MetricTemplateProvider, credentials graph := GraphiteProvider{ url: *graphiteURL, timeout: 5 * time.Second, + client: http.DefaultClient, + } + + if provider.InsecureSkipVerify { + t := http.DefaultTransport.(*http.Transport).Clone() + t.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + graph.client = &http.Client{Transport: t} } if provider.SecretRef == nil { @@ -168,7 +177,7 @@ func (g *GraphiteProvider) RunQuery(query string) (float64, error) { ctx, cancel := context.WithTimeout(req.Context(), g.timeout) defer cancel() - r, err := http.DefaultClient.Do(req.WithContext(ctx)) + r, err := g.client.Do(req.WithContext(ctx)) if err != nil { return 0, fmt.Errorf("request failed: %w", err) } diff --git a/pkg/metrics/providers/prometheus.go b/pkg/metrics/providers/prometheus.go index 427190838..834baa456 100644 --- a/pkg/metrics/providers/prometheus.go +++ b/pkg/metrics/providers/prometheus.go @@ -18,6 +18,7 @@ package providers import ( "context" + "crypto/tls" "encoding/json" "fmt" "io/ioutil" @@ -39,6 +40,7 @@ type PrometheusProvider struct { url url.URL username string password string + client *http.Client } type prometheusResponse struct { @@ -64,6 +66,13 @@ func NewPrometheusProvider(provider flaggerv1.MetricTemplateProvider, credential prom := PrometheusProvider{ timeout: 5 * time.Second, url: *promURL, + client: http.DefaultClient, + } + + if provider.InsecureSkipVerify { + t := http.DefaultTransport.(*http.Transport).Clone() + t.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + prom.client = &http.Client{Transport: t} } if provider.SecretRef != nil { @@ -106,7 +115,7 @@ func (p *PrometheusProvider) RunQuery(query string) (float64, error) { ctx, cancel := context.WithTimeout(req.Context(), p.timeout) defer cancel() - r, err := http.DefaultClient.Do(req.WithContext(ctx)) + r, err := p.client.Do(req.WithContext(ctx)) if err != nil { return 0, fmt.Errorf("request failed: %w", err) } From 428cac9284ff21a01c4b0a76e762a5a5770fbecb Mon Sep 17 00:00:00 2001 From: Joakim Ahrlin Date: Tue, 15 Jun 2021 16:32:24 +0200 Subject: [PATCH 2/5] remove insecureSkipVerify arg option and update CRDs Signed-off-by: Joakim Ahrlin --- artifacts/flagger/crd.yaml | 3 +++ charts/flagger/crds/crd.yaml | 3 +++ cmd/flagger/main.go | 4 +--- kustomize/base/flagger/crd.yaml | 3 +++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 724e0ec93..e4a1c3a94 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -1122,6 +1122,9 @@ spec: region: description: Region of the provider type: string + insecureSkipVerify: + description: Disable SSL certificate validation for the provider address + type: boolean query: description: Query of this metric template type: string diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 724e0ec93..e4a1c3a94 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -1122,6 +1122,9 @@ spec: region: description: Region of the provider type: string + insecureSkipVerify: + description: Disable SSL certificate validation for the provider address + type: boolean query: description: Query of this metric template type: string diff --git a/cmd/flagger/main.go b/cmd/flagger/main.go index 7d88cf9dd..803420261 100644 --- a/cmd/flagger/main.go +++ b/cmd/flagger/main.go @@ -59,7 +59,6 @@ var ( kubeconfigQPS int kubeconfigBurst int metricsServer string - insecureSkipVerify bool controlLoopInterval time.Duration logLevel string port string @@ -92,7 +91,6 @@ func init() { flag.IntVar(&kubeconfigBurst, "kubeconfig-burst", 250, "Set Burst for kubeconfig.") flag.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") flag.StringVar(&metricsServer, "metrics-server", "http://prometheus:9090", "Prometheus URL.") - flag.BoolVar(&insecureSkipVerify, "insecure-skip-verify", false, "disable verification of certificate for metrics server, this is insecure") flag.DurationVar(&controlLoopInterval, "control-loop-interval", 10*time.Second, "Kubernetes API sync interval.") flag.StringVar(&logLevel, "log-level", "debug", "Log level can be: debug, info, warning, error.") flag.StringVar(&port, "port", "8080", "Port to listen on.") @@ -192,7 +190,7 @@ func main() { logger.Infof("Watching namespace %s", namespace) } - observerFactory, err := observers.NewFactory(metricsServer, insecureSkipVerify) + observerFactory, err := observers.NewFactory(metricsServer, false) if err != nil { logger.Fatalf("Error building prometheus client: %s", err.Error()) } diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 724e0ec93..e4a1c3a94 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -1122,6 +1122,9 @@ spec: region: description: Region of the provider type: string + insecureSkipVerify: + description: Disable SSL certificate validation for the provider address + type: boolean query: description: Query of this metric template type: string From ba0e2aa1c10614d26e7cdedab30e26ea67b7b63b Mon Sep 17 00:00:00 2001 From: Joakim Ahrlin Date: Tue, 15 Jun 2021 17:21:31 +0200 Subject: [PATCH 3/5] remove insecureSkipVerify Signed-off-by: Joakim Ahrlin --- pkg/apis/flagger/v1beta1/canary.go | 4 ---- pkg/controller/scheduler_metrics.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 7e29bfb28..0e6c533c3 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -66,10 +66,6 @@ type CanarySpec struct { // +optional MetricsServer string `json:"metricsServer,omitempty"` - // InsecureSkipVerify disables certificate verification for the provider - // +optional - InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` - // TargetRef references a target resource TargetRef CrossNamespaceObjectReference `json:"targetRef"` diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index 00772da33..3fa51d41e 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -41,7 +41,7 @@ func (c *Controller) checkMetricProviderAvailability(canary *flaggerv1.Canary) e observerFactory := c.observerFactory if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, canary.Spec.InsecureSkipVerify) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, false) if err != nil { return fmt.Errorf("error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) } From 469395077b1f3b5d9fa87a2fb78d3a22c7c33565 Mon Sep 17 00:00:00 2001 From: Joakim Ahrlin Date: Tue, 15 Jun 2021 17:24:36 +0200 Subject: [PATCH 4/5] remove insecureSkipVerify from canary Signed-off-by: Joakim Ahrlin --- pkg/controller/scheduler_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index 3fa51d41e..9290b5165 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -120,7 +120,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool { // override the global metrics server if one is specified in the canary spec if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, canary.Spec.InsecureSkipVerify) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, false) if err != nil { c.recordEventErrorf(canary, "Error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) return false From f1569b65cd3dcfbe07fd8ef7514b6900f6c1e7cc Mon Sep 17 00:00:00 2001 From: Joakim Ahrlin Date: Tue, 15 Jun 2021 17:46:41 +0200 Subject: [PATCH 5/5] remove insecureSkipVerify from NewFactory Signed-off-by: Joakim Ahrlin --- cmd/flagger/main.go | 2 +- pkg/controller/scheduler_daemonset_fixture_test.go | 2 +- pkg/controller/scheduler_deployment_fixture_test.go | 2 +- pkg/controller/scheduler_metrics.go | 4 ++-- pkg/controller/scheduler_metrics_test.go | 4 ++-- pkg/metrics/observers/factory.go | 9 ++++----- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/cmd/flagger/main.go b/cmd/flagger/main.go index 803420261..1bab4ec2e 100644 --- a/cmd/flagger/main.go +++ b/cmd/flagger/main.go @@ -190,7 +190,7 @@ func main() { logger.Infof("Watching namespace %s", namespace) } - observerFactory, err := observers.NewFactory(metricsServer, false) + observerFactory, err := observers.NewFactory(metricsServer) if err != nil { logger.Fatalf("Error building prometheus client: %s", err.Error()) } diff --git a/pkg/controller/scheduler_daemonset_fixture_test.go b/pkg/controller/scheduler_daemonset_fixture_test.go index 1ee2010f5..e0ac8ba85 100644 --- a/pkg/controller/scheduler_daemonset_fixture_test.go +++ b/pkg/controller/scheduler_daemonset_fixture_test.go @@ -95,7 +95,7 @@ func newDaemonSetFixture(c *flaggerv1.Canary) daemonSetFixture { rf := router.NewFactory(nil, kubeClient, flaggerClient, "annotationsPrefix", "", logger, flaggerClient) // init observer - observerFactory, _ := observers.NewFactory(testMetricsServerURL, false) + observerFactory, _ := observers.NewFactory(testMetricsServerURL) // init canary factory configTracker := &canary.ConfigTracker{ diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/pkg/controller/scheduler_deployment_fixture_test.go index 9d3558b48..6fb5c89e5 100644 --- a/pkg/controller/scheduler_deployment_fixture_test.go +++ b/pkg/controller/scheduler_deployment_fixture_test.go @@ -123,7 +123,7 @@ func newDeploymentFixture(c *flaggerv1.Canary) fixture { rf := router.NewFactory(nil, kubeClient, flaggerClient, "annotationsPrefix", "", logger, flaggerClient) // init observer - observerFactory, _ := observers.NewFactory(testMetricsServerURL, false) + observerFactory, _ := observers.NewFactory(testMetricsServerURL) // init canary factory configTracker := &canary.ConfigTracker{ diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index 9290b5165..9e94f770f 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -41,7 +41,7 @@ func (c *Controller) checkMetricProviderAvailability(canary *flaggerv1.Canary) e observerFactory := c.observerFactory if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, false) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer) if err != nil { return fmt.Errorf("error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) } @@ -120,7 +120,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool { // override the global metrics server if one is specified in the canary spec if canary.Spec.MetricsServer != "" { var err error - observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer, false) + observerFactory, err = observers.NewFactory(canary.Spec.MetricsServer) if err != nil { c.recordEventErrorf(canary, "Error building Prometheus client for %s %v", canary.Spec.MetricsServer, err) return false diff --git a/pkg/controller/scheduler_metrics_test.go b/pkg/controller/scheduler_metrics_test.go index d8e87ad13..81ec50ccb 100644 --- a/pkg/controller/scheduler_metrics_test.go +++ b/pkg/controller/scheduler_metrics_test.go @@ -32,13 +32,13 @@ func TestController_checkMetricProviderAvailability(t *testing.T) { // ok analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{Name: "request-success-rate"}}} canary := &flaggerv1.Canary{Spec: flaggerv1.CanarySpec{Analysis: analysis}} - obs, err := observers.NewFactory(testMetricsServerURL, false) + obs, err := observers.NewFactory(testMetricsServerURL) require.NoError(t, err) ctrl := Controller{observerFactory: obs, logger: zap.S(), eventRecorder: &record.FakeRecorder{}} require.NoError(t, ctrl.checkMetricProviderAvailability(canary)) // error - ctrl.observerFactory, err = observers.NewFactory("http://non-exist", false) + ctrl.observerFactory, err = observers.NewFactory("http://non-exist") require.NoError(t, err) require.Error(t, ctrl.checkMetricProviderAvailability(canary)) diff --git a/pkg/metrics/observers/factory.go b/pkg/metrics/observers/factory.go index 5abaa4490..901d58575 100644 --- a/pkg/metrics/observers/factory.go +++ b/pkg/metrics/observers/factory.go @@ -27,12 +27,11 @@ type Factory struct { Client providers.Interface } -func NewFactory(metricsServer string, insecureSkipVerify bool) (*Factory, error) { +func NewFactory(metricsServer string) (*Factory, error) { client, err := providers.NewPrometheusProvider(flaggerv1.MetricTemplateProvider{ - Type: "prometheus", - Address: metricsServer, - SecretRef: nil, - InsecureSkipVerify: insecureSkipVerify, + Type: "prometheus", + Address: metricsServer, + SecretRef: nil, }, nil) if err != nil { return nil, err