refactor: extract tls config from urlfetchers to ensure the same conf…

…ig is use for all the workflow

controllers/client/grafana_client.go

@@ -2,8 +2,6 @@ package client
 
 import (
 	"context"
-	"crypto/tls"
-	"crypto/x509"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -126,57 +124,6 @@ func getAdminCredentials(ctx context.Context, c client.Client, grafana *v1beta1.
 	return credentials, nil
 }
 
-// build the tls.Config object based on the content of the Grafana CR object
-func buildTLSConfiguration(ctx context.Context, c client.Client, grafana *v1beta1.Grafana) (*tls.Config, error) {
-	if grafana.IsInternal() || grafana.Spec.External.TLS == nil {
-		return nil, nil
-	}
-	tlsConfigBlock := grafana.Spec.External.TLS
-
-	if tlsConfigBlock.InsecureSkipVerify {
-		return BuildInsecureTLSConfiguration(), nil
-	}
-
-	tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12}
-
-	secret := &v1.Secret{}
-	selector := client.ObjectKey{
-		Name:      tlsConfigBlock.CertSecretRef.Name,
-		Namespace: grafana.Namespace,
-	}
-	err := c.Get(ctx, selector, secret)
-	if err != nil {
-		return nil, err
-	}
-
-	if secret.Data == nil {
-		return nil, fmt.Errorf("empty credential secret: %v/%v", grafana.Namespace, tlsConfigBlock.CertSecretRef.Name)
-	}
-
-	crt, crtPresent := secret.Data["tls.crt"]
-	key, keyPresent := secret.Data["tls.key"]
-
-	if (crtPresent && !keyPresent) || (keyPresent && !crtPresent) {
-		return nil, fmt.Errorf("invalid secret %v/%v. tls.crt and tls.key needs to be present together when one of them is declared", tlsConfigBlock.CertSecretRef.Namespace, tlsConfigBlock.CertSecretRef.Name)
-	} else if crtPresent && keyPresent {
-		loadedCrt, err := tls.X509KeyPair(crt, key)
-		if err != nil {
-			return nil, fmt.Errorf("certificate from secret %v/%v cannot be parsed : %w", grafana.Namespace, tlsConfigBlock.CertSecretRef.Name, err)
-		}
-		tlsConfig.Certificates = []tls.Certificate{loadedCrt}
-	}
-
-	if ca, ok := secret.Data["ca.crt"]; ok {
-		caCertPool := x509.NewCertPool()
-		if !caCertPool.AppendCertsFromPEM(ca) {
-			return nil, fmt.Errorf("failed to add ca.crt from the secret %s/%s", tlsConfigBlock.CertSecretRef.Namespace, tlsConfigBlock.CertSecretRef.Name)
-		}
-		tlsConfig.RootCAs = caCertPool
-	}
-
-	return tlsConfig, nil
-}
-
 func InjectAuthHeaders(ctx context.Context, c client.Client, grafana *v1beta1.Grafana, req *http.Request) error {
 	creds, err := getAdminCredentials(ctx, c, grafana)
 	if err != nil {

controllers/client/http_client.go

@@ -2,7 +2,6 @@ package client
 
 import (
 	"context"
-	"crypto/tls"
 	"net/http"
 	"time"
 
@@ -32,7 +31,3 @@ func NewHTTPClient(ctx context.Context, c client.Client, grafana *v1beta1.Grafan
 		Timeout:   time.Second * timeout,
 	}, nil
 }
-
-func BuildInsecureTLSConfiguration() *tls.Config {
-	return &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true} // #nosec G402 - Linter disabled because InsecureSkipVerify is the wanted behavior for this usecase
-}

controllers/client/tls.go

@@ -0,0 +1,68 @@
+package client
+
+import (
+	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+
+	"github.com/grafana/grafana-operator/v5/api/v1beta1"
+	v1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var (
+	DefaultTLSConfiguration  = &tls.Config{MinVersion: tls.VersionTLS12}
+	InsecureTLSConfiguration = &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true} // #nosec G402 - Linter disabled because InsecureSkipVerify is the wanted behavior for this variable
+)
+
+// build the tls.Config object based on the content of the Grafana CR object
+func buildTLSConfiguration(ctx context.Context, c client.Client, grafana *v1beta1.Grafana) (*tls.Config, error) {
+	if grafana.IsInternal() || grafana.Spec.External.TLS == nil {
+		return nil, nil
+	}
+	tlsConfigBlock := grafana.Spec.External.TLS
+
+	if tlsConfigBlock.InsecureSkipVerify {
+		return InsecureTLSConfiguration, nil
+	}
+
+	tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12}
+
+	secret := &v1.Secret{}
+	selector := client.ObjectKey{
+		Name:      tlsConfigBlock.CertSecretRef.Name,
+		Namespace: grafana.Namespace,
+	}
+	err := c.Get(ctx, selector, secret)
+	if err != nil {
+		return nil, err
+	}
+
+	if secret.Data == nil {
+		return nil, fmt.Errorf("empty credential secret: %v/%v", grafana.Namespace, tlsConfigBlock.CertSecretRef.Name)
+	}
+
+	crt, crtPresent := secret.Data["tls.crt"]
+	key, keyPresent := secret.Data["tls.key"]
+
+	if (crtPresent && !keyPresent) || (keyPresent && !crtPresent) {
+		return nil, fmt.Errorf("invalid secret %v/%v. tls.crt and tls.key needs to be present together when one of them is declared", tlsConfigBlock.CertSecretRef.Namespace, tlsConfigBlock.CertSecretRef.Name)
+	} else if crtPresent && keyPresent {
+		loadedCrt, err := tls.X509KeyPair(crt, key)
+		if err != nil {
+			return nil, fmt.Errorf("certificate from secret %v/%v cannot be parsed : %w", grafana.Namespace, tlsConfigBlock.CertSecretRef.Name, err)
+		}
+		tlsConfig.Certificates = []tls.Certificate{loadedCrt}
+	}
+
+	if ca, ok := secret.Data["ca.crt"]; ok {
+		caCertPool := x509.NewCertPool()
+		if !caCertPool.AppendCertsFromPEM(ca) {
+			return nil, fmt.Errorf("failed to add ca.crt from the secret %s/%s", tlsConfigBlock.CertSecretRef.Namespace, tlsConfigBlock.CertSecretRef.Name)
+		}
+		tlsConfig.RootCAs = caCertPool
+	}
+
+	return tlsConfig, nil
+}

controllers/dashboard_controller.go

@@ -481,7 +481,7 @@ func (r *GrafanaDashboardReconciler) fetchDashboardJson(ctx context.Context, das
 	case v1beta1.DashboardSourceTypeGzipJson:
 		return v1beta1.Gunzip([]byte(dashboard.Spec.GzipJson))
 	case v1beta1.DashboardSourceTypeUrl:
-		return fetchers.FetchDashboardFromUrl(dashboard)
+		return fetchers.FetchDashboardFromUrl(dashboard, client2.InsecureTLSConfiguration)
 	case v1beta1.DashboardSourceTypeJsonnet:
 		envs, err := r.getDashboardEnvs(ctx, dashboard)
 		if err != nil {

controllers/fetchers/grafana_com_fetcher.go

@@ -1,6 +1,7 @@
 package fetchers
 
 import (
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -20,8 +21,10 @@ func FetchDashboardFromGrafanaCom(dashboard *v1beta1.GrafanaDashboard) ([]byte,
 
 	source := dashboard.Spec.GrafanaCom
 
+	tlsConfig := client2.DefaultTLSConfiguration
+
 	if source.Revision == nil {
-		rev, err := getLatestGrafanaComRevision(dashboard)
+		rev, err := getLatestGrafanaComRevision(dashboard, tlsConfig)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get latest revision for dashboard id %d: %w", source.Id, err)
 		}
@@ -30,10 +33,10 @@ func FetchDashboardFromGrafanaCom(dashboard *v1beta1.GrafanaDashboard) ([]byte,
 
 	dashboard.Spec.Url = fmt.Sprintf("%s/%d/revisions/%d/download", grafanaComDashboardApiUrlRoot, source.Id, *source.Revision)
 
-	return FetchDashboardFromUrl(dashboard)
+	return FetchDashboardFromUrl(dashboard, tlsConfig)
 }
 
-func getLatestGrafanaComRevision(dashboard *v1beta1.GrafanaDashboard) (int, error) {
+func getLatestGrafanaComRevision(dashboard *v1beta1.GrafanaDashboard, tlsConfig *tls.Config) (int, error) {
 	source := dashboard.Spec.GrafanaCom
 	url := fmt.Sprintf("%s/%d/revisions", grafanaComDashboardApiUrlRoot, source.Id)
 
@@ -42,8 +45,6 @@ func getLatestGrafanaComRevision(dashboard *v1beta1.GrafanaDashboard) (int, erro
 		return -1, err
 	}
 
-	// insecure to true because we don't know if the target URL is recognized by the default certificate
-	tlsConfig := client2.BuildInsecureTLSConfiguration()
 	client := client2.NewInstrumentedRoundTripper(fmt.Sprintf("%v/%v", dashboard.Namespace, dashboard.Name), metrics.GrafanaComApiRevisionRequests, true, tlsConfig)
 	response, err := client.RoundTrip(request)
 	if err != nil {

controllers/fetchers/url_fetcher.go

@@ -1,21 +1,21 @@
 package fetchers
 
 import (
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"time"
 
 	"github.com/grafana/grafana-operator/v5/api/v1beta1"
-	"github.com/grafana/grafana-operator/v5/controllers/client"
 	client2 "github.com/grafana/grafana-operator/v5/controllers/client"
 	"github.com/grafana/grafana-operator/v5/controllers/metrics"
 
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func FetchDashboardFromUrl(dashboard *v1beta1.GrafanaDashboard) ([]byte, error) {
+func FetchDashboardFromUrl(dashboard *v1beta1.GrafanaDashboard, tlsConfig *tls.Config) ([]byte, error) {
 	url, err := url.Parse(dashboard.Spec.Url)
 	if err != nil {
 		return nil, err
@@ -31,7 +31,6 @@ func FetchDashboardFromUrl(dashboard *v1beta1.GrafanaDashboard) ([]byte, error)
 		return nil, err
 	}
 
-	tlsConfig := client.BuildInsecureTLSConfiguration()
 	client := client2.NewInstrumentedRoundTripper(fmt.Sprintf("%v/%v", dashboard.Namespace, dashboard.Name), metrics.DashboardUrlRequests, true, tlsConfig)
 	response, err := client.RoundTrip(request)
 	if err != nil {

controllers/fetchers/url_fetcher_test.go

@@ -28,7 +28,7 @@ func TestFetchDashboardFromUrl(t *testing.T) {
 		Status: v1beta1.GrafanaDashboardStatus{},
 	}
 
-	fetchedDashboard, err := FetchDashboardFromUrl(dashboard)
+	fetchedDashboard, err := FetchDashboardFromUrl(dashboard, nil)
 	assert.Nil(t, err)
 	assert.Equal(t, dashboardJSON, fetchedDashboard, "Fetched dashboard doesn't match the original")