From e10b14deeb42866d693b04e2e2c66e969f7a09e5 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 2 Oct 2024 05:29:33 +0530 Subject: [PATCH 01/22] feat: Integrating ELK provider Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/elk/elk.go | 121 ++++++++++++++++++ metrics-operator/go.mod | 4 + metrics-operator/go.sum | 6 + 3 files changed, 131 insertions(+) create mode 100644 metrics-operator/controllers/common/providers/elk/elk.go diff --git a/metrics-operator/controllers/common/providers/elk/elk.go b/metrics-operator/controllers/common/providers/elk/elk.go new file mode 100644 index 0000000000..636f520c49 --- /dev/null +++ b/metrics-operator/controllers/common/providers/elk/elk.go @@ -0,0 +1,121 @@ +package elasticsearch + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/go-logr/logr" + metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" + elastic "github.com/elastic/go-elasticsearch/v8" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + warningLogStringElastic = "%s API returned warnings: %s" +) + +type KeptnElasticProvider struct { + Log logr.Logger + K8sClient client.Client + Elastic *elastic.Client +} + +func NewElasticProvider(log logr.Logger, k8sClient client.Client, elasticURL string) (*KeptnElasticProvider, error) { + cfg := elastic.Config{ + Addresses: []string{ + elasticURL, + }, + } + es, err := elastic.NewClient(cfg) + if err != nil { + return nil, fmt.Errorf("failed to create elasticsearch client: %w", err) + } + + return &KeptnElasticProvider{ + Log: log, + K8sClient: k8sClient, + Elastic: es, + }, nil +} + +func (r *KeptnElasticProvider) FetchAnalysisValue(ctx context.Context, query string, analysis metricsapi.Analysis, provider *metricsapi.KeptnMetricsProvider) (string, error) { + ctx, cancel := context.WithTimeout(ctx, 20*time.Second) + defer cancel() + + result, err := r.runElasticQuery(ctx, query, analysis.GetFrom(), analysis.GetTo()) + if err != nil { + return "", err + } + + r.Log.Info(fmt.Sprintf("Elasticsearch query result: %v", result)) + return r.extractMetric(result) +} + +func (r *KeptnElasticProvider) EvaluateQuery(ctx context.Context, metric metricsapi.KeptnMetric, provider metricsapi.KeptnMetricsProvider) (string, []byte, error) { + ctx, cancel := context.WithTimeout(ctx, 20*time.Second) + defer cancel() + + result, err := r.runElasticQuery(ctx, metric.Spec.Query, time.Now().Add(-30*time.Minute), time.Now()) + if err != nil { + return "", nil, err + } + + metricValue, err := r.extractMetric(result) + if err != nil { + return "", nil, err + } + + return metricValue, []byte{}, nil +} + +func (r *KeptnElasticProvider) runElasticQuery(ctx context.Context, query string, from, to time.Time) (map[string]interface{}, error) { + queryBody := fmt.Sprintf(` + { + "query": { + "range": { + "@timestamp": { + "gte": "%s", + "lte": "%s" + } + } + } + }`, from.Format(time.RFC3339), to.Format(time.RFC3339)) + + res, err := r.Elastic.Search( + r.Elastic.Search.WithContext(ctx), + r.Elastic.Search.WithBody(strings.NewReader(queryBody)), + ) + if err != nil { + return nil, fmt.Errorf("failed to execute Elasticsearch query: %w", err) + } + defer res.Body.Close() + + if warnings, ok := res.Header["Warning"]; ok { + r.Log.Info(fmt.Sprintf(warningLogStringElastic, "Elasticsearch", warnings)) + } + + var result map[string]interface{} + if err := json.NewDecoder(res.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("failed to parse Elasticsearch response: %w", err) + } + return result, nil +} + +func (r *KeptnElasticProvider) extractMetric(result map[string]interface{}) (string, error) { + hits, ok := result["hits"].(map[string]interface{}) + if !ok { + return "", fmt.Errorf("invalid result format: missing 'hits' field") + } + + totalHits, ok := hits["total"].(map[string]interface{}) + if !ok { + return "", fmt.Errorf("invalid result format: missing 'total' field in 'hits'") + } + + value := fmt.Sprintf("%v", totalHits["value"]) + + return value, nil +} diff --git a/metrics-operator/go.mod b/metrics-operator/go.mod index 2f46c4cbaa..d0e882330b 100644 --- a/metrics-operator/go.mod +++ b/metrics-operator/go.mod @@ -31,6 +31,8 @@ require ( sigs.k8s.io/yaml v1.4.0 ) +require github.com/elastic/elastic-transport-go/v8 v8.6.0 // indirect + require ( github.com/DataDog/zstd v1.5.2 // indirect github.com/NYTimes/gziphandler v1.1.1 // indirect @@ -43,6 +45,8 @@ require ( github.com/coreos/go-semver v0.3.1 // indirect github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/elastic/go-elasticsearch/v7 v7.17.10 + github.com/elastic/go-elasticsearch/v8 v8.15.0 github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch v5.7.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect diff --git a/metrics-operator/go.sum b/metrics-operator/go.sum index a36a64200d..bc99a9c842 100644 --- a/metrics-operator/go.sum +++ b/metrics-operator/go.sum @@ -29,6 +29,12 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/elastic/elastic-transport-go/v8 v8.6.0 h1:Y2S/FBjx1LlCv5m6pWAF2kDJAHoSjSRSJCApolgfthA= +github.com/elastic/elastic-transport-go/v8 v8.6.0/go.mod h1:YLHer5cj0csTzNFXoNQ8qhtGY1GTvSqPnKWKaqQE3Hk= +github.com/elastic/go-elasticsearch/v7 v7.17.10 h1:TCQ8i4PmIJuBunvBS6bwT2ybzVFxxUhhltAs3Gyu1yo= +github.com/elastic/go-elasticsearch/v7 v7.17.10/go.mod h1:OJ4wdbtDNk5g503kvlHLyErCgQwwzmDtaFC4XyOxXA4= +github.com/elastic/go-elasticsearch/v8 v8.15.0 h1:IZyJhe7t7WI3NEFdcHnf6IJXqpRf+8S8QWLtZYYyBYk= +github.com/elastic/go-elasticsearch/v8 v8.15.0/go.mod h1:HCON3zj4btpqs2N1jjsAy4a/fiAul+YBP00mBH4xik8= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch v5.7.0+incompatible h1:vgGkfT/9f8zE6tvSCe74nfpAVDQ2tG6yudJd8LBksgI= From ba03ab3a5bf31013a539fd500bc458f9b9ca16e4 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Sat, 5 Oct 2024 05:53:01 +0530 Subject: [PATCH 02/22] feat: Implemented TLS round tripper Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 128 ++++++++++++++++-- 1 file changed, 119 insertions(+), 9 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index a2330f9ac9..f4d390476c 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -2,8 +2,10 @@ package prometheus import ( "context" + "crypto/tls" "errors" "net/http" + "sync" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" promapi "github.com/prometheus/client_golang/api" @@ -13,23 +15,112 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const secretKeyUserName = "user" -const secretKeyPassword = "password" +const ( + secretKeyUserName = "user" + secretKeyPassword = "password" + secretKeyCAFile = "caFile" + secretKeyCertFile = "certFile" + secretKeyKeyFile = "keyFile" +) -var ErrSecretKeyRefNotDefined = errors.New("the SecretKeyRef property with the Prometheus API Key is missing") -var ErrInvalidSecretFormat = errors.New("secret key does not contain user and password") +var ( + ErrSecretKeyRefNotDefined = errors.New("the SecretKeyRef property with the Prometheus API Key is missing") + ErrInvalidSecretFormat = errors.New("secret key does not contain required fields") +) type SecretData struct { User string `json:"user"` Password config.Secret `json:"password"` + CAFile string `json:"caFile"` + CertFile string `json:"certFile"` + KeyFile string `json:"keyFile"` } -//go:generate moq -pkg fake -skip-ensure -out ./fake/roundtripper_mock.go . IRoundTripper type IRoundTripper interface { GetRoundTripper(context.Context, metricsapi.KeptnMetricsProvider, client.Client) (http.RoundTripper, error) } -type RoundTripperRetriever struct { +type RoundTripperRetriever struct{} + +type TLSRoundTripperSettings struct { + CAFile string + CertFile string + KeyFile string + ServerName string + InsecureSkipVerify bool +} + +type tlsRoundTripper struct { + settings TLSRoundTripperSettings + newRT func(*tls.Config) (http.RoundTripper, error) + mtx sync.RWMutex + rt http.RoundTripper + tlsConfig *tls.Config +} + +func NewTLSRoundTripper(settings TLSRoundTripperSettings) (*tlsRoundTripper, error) { + return &tlsRoundTripper{ + settings: settings, + newRT: func(config *tls.Config) (http.RoundTripper, error) { + return &http.Transport{ + TLSClientConfig: config, + }, nil + }, + }, nil +} + +func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + t.mtx.RLock() + rt := t.rt + t.mtx.RUnlock() + + if rt == nil { + t.mtx.Lock() + defer t.mtx.Unlock() + if t.rt == nil { + var err error + t.tlsConfig, err = newTLSConfig(&t.settings) + if err != nil { + return nil, err + } + rt, err = t.newRT(t.tlsConfig) + if err != nil { + return nil, err + } + t.rt = rt + } + rt = t.rt + } + + return rt.RoundTrip(req) +} + +func newTLSConfig(settings *TLSRoundTripperSettings) (*tls.Config, error) { + tlsConfig := &tls.Config{ + InsecureSkipVerify: settings.InsecureSkipVerify, + ServerName: settings.ServerName, + } + + if settings.CAFile != "" { + caConfig := &config.TLSConfig{ + CAFile: settings.CAFile, + } + ca, err := config.NewTLSConfig(caConfig) + if err != nil { + return nil, err + } + tlsConfig.RootCAs = ca.RootCAs + } + + if settings.CertFile != "" && settings.KeyFile != "" { + cert, err := tls.LoadX509KeyPair(settings.CertFile, settings.KeyFile) + if err != nil { + return nil, err + } + tlsConfig.Certificates = []tls.Certificate{cert} + } + + return tlsConfig, nil } func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (http.RoundTripper, error) { @@ -40,13 +131,27 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met } return nil, err } - return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", promapi.DefaultRoundTripper), nil + + settings := TLSRoundTripperSettings{ + CAFile: secret.CAFile, + CertFile: secret.CertFile, + KeyFile: secret.KeyFile, + ServerName: provider.Spec.TargetServer, + } + + tlsRT, err := NewTLSRoundTripper(settings) + if err != nil { + return nil, err + } + + return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", tlsRT), nil } func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { if !provider.HasSecretDefined() { return nil, ErrSecretKeyRefNotDefined } + secret := &corev1.Secret{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: provider.Spec.SecretKeyRef.Name, Namespace: provider.Namespace}, secret); err != nil { return nil, err @@ -54,11 +159,16 @@ func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsPr var secretData SecretData user, ok := secret.Data[secretKeyUserName] - pw, yes := secret.Data[secretKeyPassword] - if !ok || !yes { + pw, pwOk := secret.Data[secretKeyPassword] + if !ok || !pwOk { return nil, ErrInvalidSecretFormat } + secretData.User = string(user) secretData.Password = config.Secret(pw) + secretData.CAFile = string(secret.Data[secretKeyCAFile]) + secretData.CertFile = string(secret.Data[secretKeyCertFile]) + secretData.KeyFile = string(secret.Data[secretKeyKeyFile]) + return &secretData, nil } From 68f2271e7fb4d480f0f103e006aaf504cccf42eb Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Sat, 5 Oct 2024 05:59:20 +0530 Subject: [PATCH 03/22] Removed unwanted folders Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/elk/elk.go | 121 ------------------ 1 file changed, 121 deletions(-) delete mode 100644 metrics-operator/controllers/common/providers/elk/elk.go diff --git a/metrics-operator/controllers/common/providers/elk/elk.go b/metrics-operator/controllers/common/providers/elk/elk.go deleted file mode 100644 index 636f520c49..0000000000 --- a/metrics-operator/controllers/common/providers/elk/elk.go +++ /dev/null @@ -1,121 +0,0 @@ -package elasticsearch - -import ( - "context" - "encoding/json" - "fmt" - "strings" - "time" - - "github.com/go-logr/logr" - metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" - elastic "github.com/elastic/go-elasticsearch/v8" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - warningLogStringElastic = "%s API returned warnings: %s" -) - -type KeptnElasticProvider struct { - Log logr.Logger - K8sClient client.Client - Elastic *elastic.Client -} - -func NewElasticProvider(log logr.Logger, k8sClient client.Client, elasticURL string) (*KeptnElasticProvider, error) { - cfg := elastic.Config{ - Addresses: []string{ - elasticURL, - }, - } - es, err := elastic.NewClient(cfg) - if err != nil { - return nil, fmt.Errorf("failed to create elasticsearch client: %w", err) - } - - return &KeptnElasticProvider{ - Log: log, - K8sClient: k8sClient, - Elastic: es, - }, nil -} - -func (r *KeptnElasticProvider) FetchAnalysisValue(ctx context.Context, query string, analysis metricsapi.Analysis, provider *metricsapi.KeptnMetricsProvider) (string, error) { - ctx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() - - result, err := r.runElasticQuery(ctx, query, analysis.GetFrom(), analysis.GetTo()) - if err != nil { - return "", err - } - - r.Log.Info(fmt.Sprintf("Elasticsearch query result: %v", result)) - return r.extractMetric(result) -} - -func (r *KeptnElasticProvider) EvaluateQuery(ctx context.Context, metric metricsapi.KeptnMetric, provider metricsapi.KeptnMetricsProvider) (string, []byte, error) { - ctx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() - - result, err := r.runElasticQuery(ctx, metric.Spec.Query, time.Now().Add(-30*time.Minute), time.Now()) - if err != nil { - return "", nil, err - } - - metricValue, err := r.extractMetric(result) - if err != nil { - return "", nil, err - } - - return metricValue, []byte{}, nil -} - -func (r *KeptnElasticProvider) runElasticQuery(ctx context.Context, query string, from, to time.Time) (map[string]interface{}, error) { - queryBody := fmt.Sprintf(` - { - "query": { - "range": { - "@timestamp": { - "gte": "%s", - "lte": "%s" - } - } - } - }`, from.Format(time.RFC3339), to.Format(time.RFC3339)) - - res, err := r.Elastic.Search( - r.Elastic.Search.WithContext(ctx), - r.Elastic.Search.WithBody(strings.NewReader(queryBody)), - ) - if err != nil { - return nil, fmt.Errorf("failed to execute Elasticsearch query: %w", err) - } - defer res.Body.Close() - - if warnings, ok := res.Header["Warning"]; ok { - r.Log.Info(fmt.Sprintf(warningLogStringElastic, "Elasticsearch", warnings)) - } - - var result map[string]interface{} - if err := json.NewDecoder(res.Body).Decode(&result); err != nil { - return nil, fmt.Errorf("failed to parse Elasticsearch response: %w", err) - } - return result, nil -} - -func (r *KeptnElasticProvider) extractMetric(result map[string]interface{}) (string, error) { - hits, ok := result["hits"].(map[string]interface{}) - if !ok { - return "", fmt.Errorf("invalid result format: missing 'hits' field") - } - - totalHits, ok := hits["total"].(map[string]interface{}) - if !ok { - return "", fmt.Errorf("invalid result format: missing 'total' field in 'hits'") - } - - value := fmt.Sprintf("%v", totalHits["value"]) - - return value, nil -} From a3d0d35b6b7464585ec3a079b78d781429ffe5b4 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Sat, 5 Oct 2024 06:03:10 +0530 Subject: [PATCH 04/22] fix: removed unwanted lines form go.mod and go.sum files Signed-off-by: Bharadwajshivam28 --- metrics-operator/go.mod | 4 ---- metrics-operator/go.sum | 6 ------ 2 files changed, 10 deletions(-) diff --git a/metrics-operator/go.mod b/metrics-operator/go.mod index d0e882330b..2f46c4cbaa 100644 --- a/metrics-operator/go.mod +++ b/metrics-operator/go.mod @@ -31,8 +31,6 @@ require ( sigs.k8s.io/yaml v1.4.0 ) -require github.com/elastic/elastic-transport-go/v8 v8.6.0 // indirect - require ( github.com/DataDog/zstd v1.5.2 // indirect github.com/NYTimes/gziphandler v1.1.1 // indirect @@ -45,8 +43,6 @@ require ( github.com/coreos/go-semver v0.3.1 // indirect github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/elastic/go-elasticsearch/v7 v7.17.10 - github.com/elastic/go-elasticsearch/v8 v8.15.0 github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch v5.7.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect diff --git a/metrics-operator/go.sum b/metrics-operator/go.sum index bc99a9c842..a36a64200d 100644 --- a/metrics-operator/go.sum +++ b/metrics-operator/go.sum @@ -29,12 +29,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= -github.com/elastic/elastic-transport-go/v8 v8.6.0 h1:Y2S/FBjx1LlCv5m6pWAF2kDJAHoSjSRSJCApolgfthA= -github.com/elastic/elastic-transport-go/v8 v8.6.0/go.mod h1:YLHer5cj0csTzNFXoNQ8qhtGY1GTvSqPnKWKaqQE3Hk= -github.com/elastic/go-elasticsearch/v7 v7.17.10 h1:TCQ8i4PmIJuBunvBS6bwT2ybzVFxxUhhltAs3Gyu1yo= -github.com/elastic/go-elasticsearch/v7 v7.17.10/go.mod h1:OJ4wdbtDNk5g503kvlHLyErCgQwwzmDtaFC4XyOxXA4= -github.com/elastic/go-elasticsearch/v8 v8.15.0 h1:IZyJhe7t7WI3NEFdcHnf6IJXqpRf+8S8QWLtZYYyBYk= -github.com/elastic/go-elasticsearch/v8 v8.15.0/go.mod h1:HCON3zj4btpqs2N1jjsAy4a/fiAul+YBP00mBH4xik8= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch v5.7.0+incompatible h1:vgGkfT/9f8zE6tvSCe74nfpAVDQ2tG6yudJd8LBksgI= From 4f019d39f5b3ccec15634253abb30ff59ca1c2d0 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 9 Oct 2024 17:10:47 +0530 Subject: [PATCH 05/22] modified GetRoundTripper function Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 127 ++---------------- 1 file changed, 14 insertions(+), 113 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index f4d390476c..2569f16bf7 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "errors" "net/http" - "sync" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" promapi "github.com/prometheus/client_golang/api" @@ -15,112 +14,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ( - secretKeyUserName = "user" - secretKeyPassword = "password" - secretKeyCAFile = "caFile" - secretKeyCertFile = "certFile" - secretKeyKeyFile = "keyFile" -) +const secretKeyUserName = "user" +const secretKeyPassword = "password" -var ( - ErrSecretKeyRefNotDefined = errors.New("the SecretKeyRef property with the Prometheus API Key is missing") - ErrInvalidSecretFormat = errors.New("secret key does not contain required fields") -) +var ErrSecretKeyRefNotDefined = errors.New("the SecretKeyRef property with the Prometheus API Key is missing") +var ErrInvalidSecretFormat = errors.New("secret key does not contain user and password") type SecretData struct { User string `json:"user"` Password config.Secret `json:"password"` - CAFile string `json:"caFile"` - CertFile string `json:"certFile"` - KeyFile string `json:"keyFile"` } +// IRoundTripper interface defines the method to get the RoundTripper type IRoundTripper interface { GetRoundTripper(context.Context, metricsapi.KeptnMetricsProvider, client.Client) (http.RoundTripper, error) } -type RoundTripperRetriever struct{} - -type TLSRoundTripperSettings struct { - CAFile string - CertFile string - KeyFile string - ServerName string - InsecureSkipVerify bool -} - -type tlsRoundTripper struct { - settings TLSRoundTripperSettings - newRT func(*tls.Config) (http.RoundTripper, error) - mtx sync.RWMutex - rt http.RoundTripper - tlsConfig *tls.Config -} - -func NewTLSRoundTripper(settings TLSRoundTripperSettings) (*tlsRoundTripper, error) { - return &tlsRoundTripper{ - settings: settings, - newRT: func(config *tls.Config) (http.RoundTripper, error) { - return &http.Transport{ - TLSClientConfig: config, - }, nil - }, - }, nil -} - -func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - t.mtx.RLock() - rt := t.rt - t.mtx.RUnlock() - - if rt == nil { - t.mtx.Lock() - defer t.mtx.Unlock() - if t.rt == nil { - var err error - t.tlsConfig, err = newTLSConfig(&t.settings) - if err != nil { - return nil, err - } - rt, err = t.newRT(t.tlsConfig) - if err != nil { - return nil, err - } - t.rt = rt - } - rt = t.rt - } - - return rt.RoundTrip(req) -} - -func newTLSConfig(settings *TLSRoundTripperSettings) (*tls.Config, error) { - tlsConfig := &tls.Config{ - InsecureSkipVerify: settings.InsecureSkipVerify, - ServerName: settings.ServerName, - } - - if settings.CAFile != "" { - caConfig := &config.TLSConfig{ - CAFile: settings.CAFile, - } - ca, err := config.NewTLSConfig(caConfig) - if err != nil { - return nil, err - } - tlsConfig.RootCAs = ca.RootCAs - } - - if settings.CertFile != "" && settings.KeyFile != "" { - cert, err := tls.LoadX509KeyPair(settings.CertFile, settings.KeyFile) - if err != nil { - return nil, err - } - tlsConfig.Certificates = []tls.Certificate{cert} - } - - return tlsConfig, nil +// RoundTripperRetriever implements the IRoundTripper interface +type RoundTripperRetriever struct { } func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (http.RoundTripper, error) { @@ -132,19 +43,13 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return nil, err } - settings := TLSRoundTripperSettings{ - CAFile: secret.CAFile, - CertFile: secret.CertFile, - KeyFile: secret.KeyFile, - ServerName: provider.Spec.TargetServer, - } - - tlsRT, err := NewTLSRoundTripper(settings) - if err != nil { - return nil, err + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, + }, } - return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", tlsRT), nil + return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil } func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { @@ -159,16 +64,12 @@ func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsPr var secretData SecretData user, ok := secret.Data[secretKeyUserName] - pw, pwOk := secret.Data[secretKeyPassword] - if !ok || !pwOk { + pw, yes := secret.Data[secretKeyPassword] + if !ok || !yes { return nil, ErrInvalidSecretFormat } secretData.User = string(user) secretData.Password = config.Secret(pw) - secretData.CAFile = string(secret.Data[secretKeyCAFile]) - secretData.CertFile = string(secret.Data[secretKeyCertFile]) - secretData.KeyFile = string(secret.Data[secretKeyKeyFile]) - return &secretData, nil } From f6b8797fa26e146fc6cc9d0e2cc318c43ca85d16 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 9 Oct 2024 17:12:06 +0530 Subject: [PATCH 06/22] modified the common.go file Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/prometheus/common.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index 2569f16bf7..b27c54e489 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -25,12 +25,11 @@ type SecretData struct { Password config.Secret `json:"password"` } -// IRoundTripper interface defines the method to get the RoundTripper +//go:generate moq -pkg fake -skip-ensure -out ./fake/roundtripper_mock.go . IRoundTripper type IRoundTripper interface { GetRoundTripper(context.Context, metricsapi.KeptnMetricsProvider, client.Client) (http.RoundTripper, error) } -// RoundTripperRetriever implements the IRoundTripper interface type RoundTripperRetriever struct { } From c42cf6fc36741af822471675931d81ccef42fa4b Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Thu, 10 Oct 2024 09:43:55 +0530 Subject: [PATCH 07/22] feat: modified tls method to use exisiting behavior and allow tls skip Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/prometheus/common.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index b27c54e489..2a06de1cc3 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -42,10 +42,9 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return nil, err } - transport := &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, - }, + transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, } return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil From 98405120734feba3e1f6b946352ba8c2fa76ec18 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 14:35:38 +0530 Subject: [PATCH 08/22] maintaining original DefaultRoundTripper behavior Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index 2a06de1cc3..566959c557 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -4,13 +4,12 @@ import ( "context" "crypto/tls" "errors" - "net/http" - metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "net/http" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -42,31 +41,35 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return nil, err } - transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() - transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, + baseTransport := promapi.DefaultRoundTripper + + if provider.Spec.InsecureSkipTlsVerify { + if httpTransport, ok := baseTransport.(*http.Transport); ok { + newTransport := httpTransport.Clone() + newTransport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + baseTransport = newTransport + } } - return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil + return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", baseTransport), nil } func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { if !provider.HasSecretDefined() { return nil, ErrSecretKeyRefNotDefined } - secret := &corev1.Secret{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: provider.Spec.SecretKeyRef.Name, Namespace: provider.Namespace}, secret); err != nil { return nil, err } - var secretData SecretData user, ok := secret.Data[secretKeyUserName] pw, yes := secret.Data[secretKeyPassword] if !ok || !yes { return nil, ErrInvalidSecretFormat } - secretData.User = string(user) secretData.Password = config.Secret(pw) return &secretData, nil From bf7a3790076cc71bf6c917f01593377480c3aed3 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 15:07:04 +0530 Subject: [PATCH 09/22] modification in the logic and alignment of test according to the current method Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 15 ++++----------- .../common/providers/prometheus/common_test.go | 12 ++++++++++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index 566959c557..c68472dc3c 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -41,19 +41,12 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return nil, err } - baseTransport := promapi.DefaultRoundTripper - - if provider.Spec.InsecureSkipTlsVerify { - if httpTransport, ok := baseTransport.(*http.Transport); ok { - newTransport := httpTransport.Clone() - newTransport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } - baseTransport = newTransport - } + transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, } - return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", baseTransport), nil + return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil } func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 046a99fc05..e65a9edc3c 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -2,6 +2,7 @@ package prometheus import ( "context" + "crypto/tls" "net/http" "net/http/httptest" "reflect" @@ -137,11 +138,18 @@ func Test_GetRoundtripper(t *testing.T) { Key: "", Optional: nil, }, + InsecureSkipTlsVerify: true, }, }, k8sClient: fake.NewClient(goodsecret), - want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), - wantErr: false, + want: func() http.RoundTripper { + transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + return config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", transport) + }(), + wantErr: false, }, { name: "TestSecretNotDefined", From 61501397b42418eeb37aafb9e8a7ed3054952618 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 16:23:14 +0530 Subject: [PATCH 10/22] fix: fixed failure of test_case Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 68 ++++++++++++++----- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index e65a9edc3c..e2e1c6d9ee 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -2,16 +2,15 @@ package prometheus import ( "context" - "crypto/tls" + "encoding/base64" "net/http" "net/http/httptest" - "reflect" "strings" "testing" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" - promapi "github.com/prometheus/client_golang/api" + "github.com/prometheus/common/config" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -120,7 +119,8 @@ func Test_GetRoundtripper(t *testing.T) { name string provider metricsapi.KeptnMetricsProvider k8sClient client.Client - want http.RoundTripper + wantUser string + wantPass string wantErr bool errorStr string }{ @@ -138,24 +138,19 @@ func Test_GetRoundtripper(t *testing.T) { Key: "", Optional: nil, }, - InsecureSkipTlsVerify: true, }, }, k8sClient: fake.NewClient(goodsecret), - want: func() http.RoundTripper { - transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() - transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } - return config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", transport) - }(), - wantErr: false, + wantUser: "myuser", + wantPass: "mytoken", + wantErr: false, }, { name: "TestSecretNotDefined", provider: metricsapi.KeptnMetricsProvider{}, k8sClient: fake.NewClient(), - want: promapi.DefaultRoundTripper, + wantUser: "", + wantPass: "", wantErr: false, }, { @@ -175,7 +170,8 @@ func Test_GetRoundtripper(t *testing.T) { }, }, k8sClient: fake.NewClient(), - want: nil, + wantUser: "", + wantPass: "", wantErr: true, errorStr: "not found", }, @@ -193,8 +189,46 @@ func Test_GetRoundtripper(t *testing.T) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) + + if tt.wantErr { + return + } + + if _, ok := got.(*http.Transport); ok { + if tt.wantUser != "" || tt.wantPass != "" { + t.Errorf("getRoundtripper() got default RoundTripper, want BasicAuth") + } + return + } + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader := r.Header.Get("Authorization") + if authHeader == "" { + t.Errorf("Authorization header not set") + return + } + + auth := strings.SplitN(authHeader, " ", 2) + if len(auth) != 2 || auth[0] != "Basic" { + t.Errorf("Invalid Authorization header format") + return + } + payload, _ := base64.StdEncoding.DecodeString(auth[1]) + pair := strings.SplitN(string(payload), ":", 2) + + if pair[0] != tt.wantUser { + t.Errorf("got user = %v, want %v", pair[0], tt.wantUser) + } + if pair[1] != tt.wantPass { + t.Errorf("got password = %v, want %v", pair[1], tt.wantPass) + } + })) + defer testServer.Close() + + req, _ := http.NewRequest("GET", testServer.URL, nil) + _, err = got.RoundTrip(req) + if err != nil { + t.Errorf("RoundTrip error: %v", err) } }) } From f9df3af18364343acc9288e47a3256414379e1ca Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 17:20:38 +0530 Subject: [PATCH 11/22] modifing test file Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 64 ++++--------------- 1 file changed, 12 insertions(+), 52 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index e2e1c6d9ee..661045af9b 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -2,15 +2,16 @@ package prometheus import ( "context" - "encoding/base64" "net/http" "net/http/httptest" + // "reflect" "strings" "testing" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" - + + // promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -120,7 +121,7 @@ func Test_GetRoundtripper(t *testing.T) { provider metricsapi.KeptnMetricsProvider k8sClient client.Client wantUser string - wantPass string + wantPass config.Secret wantErr bool errorStr string }{ @@ -129,20 +130,16 @@ func Test_GetRoundtripper(t *testing.T) { provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ - Type: "", - TargetServer: "", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", }, - Key: "", - Optional: nil, }, }, }, k8sClient: fake.NewClient(goodsecret), wantUser: "myuser", - wantPass: "mytoken", + wantPass: config.Secret("mytoken"), wantErr: false, }, { @@ -158,14 +155,10 @@ func Test_GetRoundtripper(t *testing.T) { provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ - Type: "", - TargetServer: "", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", }, - Key: "", - Optional: nil, }, }, }, @@ -185,50 +178,17 @@ func Test_GetRoundtripper(t *testing.T) { t.Errorf("getRoundtripper() error = %v, wantErr %v", err, tt.wantErr) return } - if tt.errorStr != "" && !strings.Contains(err.Error(), tt.errorStr) { + if tt.errorStr != "" && err != nil && !strings.Contains(err.Error(), tt.errorStr) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - if tt.wantErr { - return - } - - if _, ok := got.(*http.Transport); ok { - if tt.wantUser != "" || tt.wantPass != "" { - t.Errorf("getRoundtripper() got default RoundTripper, want BasicAuth") - } - return - } - - testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - authHeader := r.Header.Get("Authorization") - if authHeader == "" { - t.Errorf("Authorization header not set") - return - } - - auth := strings.SplitN(authHeader, " ", 2) - if len(auth) != 2 || auth[0] != "Basic" { - t.Errorf("Invalid Authorization header format") - return - } - payload, _ := base64.StdEncoding.DecodeString(auth[1]) - pair := strings.SplitN(string(payload), ":", 2) - - if pair[0] != tt.wantUser { - t.Errorf("got user = %v, want %v", pair[0], tt.wantUser) - } - if pair[1] != tt.wantPass { - t.Errorf("got password = %v, want %v", pair[1], tt.wantPass) - } - })) - defer testServer.Close() - - req, _ := http.NewRequest("GET", testServer.URL, nil) - _, err = got.RoundTrip(req) - if err != nil { - t.Errorf("RoundTrip error: %v", err) + // Check user and password, instead of comparing the RoundTripper objects directly + if gotTransport, ok := got.(*config.BasicAuthRoundTripper); ok { + require.Equal(t, tt.wantUser, gotTransport.Username) + require.Equal(t, tt.wantPass, gotTransport.Password) + } else if !tt.wantErr { + t.Errorf("getRoundtripper() expected BasicAuthRoundTripper, but got = %T", got) } }) } From c3e396281065d5cc9ec08c097d9c1dded1370277 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 17:54:38 +0530 Subject: [PATCH 12/22] changes in test file Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 661045af9b..44707cb78b 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -10,7 +10,6 @@ import ( metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" - // promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" "github.com/stretchr/testify/require" @@ -116,12 +115,13 @@ func Test_GetRoundtripper(t *testing.T) { "password": []byte("mytoken"), }, } + tests := []struct { name string provider metricsapi.KeptnMetricsProvider k8sClient client.Client wantUser string - wantPass config.Secret + wantPass string wantErr bool errorStr string }{ @@ -130,16 +130,20 @@ func Test_GetRoundtripper(t *testing.T) { provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ + Type: "", + TargetServer: "", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", }, + Key: "", + Optional: nil, }, }, }, k8sClient: fake.NewClient(goodsecret), wantUser: "myuser", - wantPass: config.Secret("mytoken"), + wantPass: "mytoken", wantErr: false, }, { @@ -155,10 +159,14 @@ func Test_GetRoundtripper(t *testing.T) { provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ + Type: "", + TargetServer: "", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", }, + Key: "", + Optional: nil, }, }, }, @@ -173,22 +181,30 @@ func Test_GetRoundtripper(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := RoundTripperRetriever{}.GetRoundTripper(context.TODO(), tt.provider, tt.k8sClient) - t.Log(err) + + // Check if an error was expected and if the error occurred. if (err != nil) != tt.wantErr { t.Errorf("getRoundtripper() error = %v, wantErr %v", err, tt.wantErr) return } + + // If an error string is expected, ensure the error message contains it. if tt.errorStr != "" && err != nil && !strings.Contains(err.Error(), tt.errorStr) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - // Check user and password, instead of comparing the RoundTripper objects directly - if gotTransport, ok := got.(*config.BasicAuthRoundTripper); ok { - require.Equal(t, tt.wantUser, gotTransport.Username) - require.Equal(t, tt.wantPass, gotTransport.Password) - } else if !tt.wantErr { - t.Errorf("getRoundtripper() expected BasicAuthRoundTripper, but got = %T", got) + // If no error is expected, check the credentials inside the round tripper + if !tt.wantErr { + basicAuthRT, ok := got.(*config.BasicAuthRoundTripper) + if !ok { + t.Errorf("Expected BasicAuthRoundTripper, got %T", got) + return + } + if basicAuthRT.User != tt.wantUser || basicAuthRT.Password != tt.wantPass { + t.Errorf("getRoundtripper() credentials = (%v, %v), want (%v, %v)", + basicAuthRT.User, basicAuthRT.Password, tt.wantUser, tt.wantPass) + } } }) } From c4ecd7e7e4b4048775558554bebfa721dc21913a Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Mon, 14 Oct 2024 21:29:43 +0530 Subject: [PATCH 13/22] feat: modifing unit test Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 97 +++++++++++-------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 44707cb78b..26b86479cd 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -2,14 +2,17 @@ package prometheus import ( "context" + "encoding/base64" "net/http" "net/http/httptest" + // "reflect" "strings" "testing" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" + // promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" "github.com/stretchr/testify/require" @@ -115,15 +118,14 @@ func Test_GetRoundtripper(t *testing.T) { "password": []byte("mytoken"), }, } - tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - wantUser string - wantPass string - wantErr bool - errorStr string + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + wantUser string + wantPassword string + wantErr bool + errorStr string }{ { name: "TestSuccess", @@ -141,18 +143,18 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(goodsecret), - wantUser: "myuser", - wantPass: "mytoken", - wantErr: false, + k8sClient: fake.NewClient(goodsecret), + wantUser: "myuser", + wantPassword: "mytoken", + wantErr: false, }, { - name: "TestSecretNotDefined", - provider: metricsapi.KeptnMetricsProvider{}, - k8sClient: fake.NewClient(), - wantUser: "", - wantPass: "", - wantErr: false, + name: "TestSecretNotDefined", + provider: metricsapi.KeptnMetricsProvider{}, + k8sClient: fake.NewClient(), + wantUser: "", + wantPassword: "", + wantErr: false, }, { name: "TestErrorFromGetPrometheusSecretNotExists", @@ -170,42 +172,59 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(), - wantUser: "", - wantPass: "", - wantErr: true, - errorStr: "not found", + k8sClient: fake.NewClient(), + wantUser: "", + wantPassword: "", + wantErr: true, + errorStr: "not found", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := RoundTripperRetriever{}.GetRoundTripper(context.TODO(), tt.provider, tt.k8sClient) - - // Check if an error was expected and if the error occurred. + t.Log(err) if (err != nil) != tt.wantErr { t.Errorf("getRoundtripper() error = %v, wantErr %v", err, tt.wantErr) return } - - // If an error string is expected, ensure the error message contains it. - if tt.errorStr != "" && err != nil && !strings.Contains(err.Error(), tt.errorStr) { + if tt.errorStr != "" && !strings.Contains(err.Error(), tt.errorStr) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - // If no error is expected, check the credentials inside the round tripper - if !tt.wantErr { - basicAuthRT, ok := got.(*config.BasicAuthRoundTripper) - if !ok { - t.Errorf("Expected BasicAuthRoundTripper, got %T", got) - return - } - if basicAuthRT.User != tt.wantUser || basicAuthRT.Password != tt.wantPass { - t.Errorf("getRoundtripper() credentials = (%v, %v), want (%v, %v)", - basicAuthRT.User, basicAuthRT.Password, tt.wantUser, tt.wantPass) + if tt.wantErr { + return + } + + // Create a mock server to capture the request + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth := r.Header.Get("Authorization") + if tt.wantUser != "" || tt.wantPassword != "" { + expected := "Basic " + base64.StdEncoding.EncodeToString([]byte(tt.wantUser+":"+tt.wantPassword)) + if auth != expected { + t.Errorf("getRoundtripper() got Authorization = %v, want %v", auth, expected) + } + } else if auth != "" { + t.Errorf("getRoundtripper() got unexpected Authorization header: %v", auth) } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Create a test request to the mock server + req, err := http.NewRequest("GET", server.URL, nil) + require.NoError(t, err) + + // Use the RoundTripper to send the request + resp, err := got.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + + // Check if the request was successful + if resp.StatusCode != http.StatusOK { + t.Errorf("getRoundtripper() got status code %v, want %v", resp.StatusCode, http.StatusOK) } }) } -} +} \ No newline at end of file From 4672e23d5a5248bec55384352eaac6efe6399009 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Tue, 15 Oct 2024 16:15:12 +0530 Subject: [PATCH 14/22] feat: code Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 1 + .../providers/prometheus/common_test.go | 81 +++++-------------- 2 files changed, 23 insertions(+), 59 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index c68472dc3c..1496cdfab2 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -49,6 +49,7 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil } + func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { if !provider.HasSecretDefined() { return nil, ErrSecretKeyRefNotDefined diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 26b86479cd..42336e5eea 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -2,18 +2,15 @@ package prometheus import ( "context" - "encoding/base64" "net/http" "net/http/httptest" - - // "reflect" + "reflect" "strings" "testing" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" - - // promapi "github.com/prometheus/client_golang/api" + promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -119,13 +116,12 @@ func Test_GetRoundtripper(t *testing.T) { }, } tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - wantUser string - wantPassword string - wantErr bool - errorStr string + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + want http.RoundTripper + wantErr bool + errorStr string }{ { name: "TestSuccess", @@ -143,18 +139,16 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(goodsecret), - wantUser: "myuser", - wantPassword: "mytoken", - wantErr: false, + k8sClient: fake.NewClient(goodsecret), + want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), + wantErr: false, }, { - name: "TestSecretNotDefined", - provider: metricsapi.KeptnMetricsProvider{}, - k8sClient: fake.NewClient(), - wantUser: "", - wantPassword: "", - wantErr: false, + name: "TestSecretNotDefined", + provider: metricsapi.KeptnMetricsProvider{}, + k8sClient: fake.NewClient(), + want: promapi.DefaultRoundTripper, + wantErr: false, }, { name: "TestErrorFromGetPrometheusSecretNotExists", @@ -172,11 +166,10 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(), - wantUser: "", - wantPassword: "", - wantErr: true, - errorStr: "not found", + k8sClient: fake.NewClient(), + want: nil, + wantErr: true, + errorStr: "not found", }, } @@ -192,38 +185,8 @@ func Test_GetRoundtripper(t *testing.T) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - - if tt.wantErr { - return - } - - // Create a mock server to capture the request - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - auth := r.Header.Get("Authorization") - if tt.wantUser != "" || tt.wantPassword != "" { - expected := "Basic " + base64.StdEncoding.EncodeToString([]byte(tt.wantUser+":"+tt.wantPassword)) - if auth != expected { - t.Errorf("getRoundtripper() got Authorization = %v, want %v", auth, expected) - } - } else if auth != "" { - t.Errorf("getRoundtripper() got unexpected Authorization header: %v", auth) - } - w.WriteHeader(http.StatusOK) - })) - defer server.Close() - - // Create a test request to the mock server - req, err := http.NewRequest("GET", server.URL, nil) - require.NoError(t, err) - - // Use the RoundTripper to send the request - resp, err := got.RoundTrip(req) - require.NoError(t, err) - defer resp.Body.Close() - - // Check if the request was successful - if resp.StatusCode != http.StatusOK { - t.Errorf("getRoundtripper() got status code %v, want %v", resp.StatusCode, http.StatusOK) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) } }) } From 1dd3f1d9b9716b3eda51a127e904d7caf158cbec Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Tue, 15 Oct 2024 17:36:22 +0530 Subject: [PATCH 15/22] fix: removed clone() from the logic Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/prometheus/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index 1496cdfab2..9d3a39ca0c 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -41,7 +41,7 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return nil, err } - transport := promapi.DefaultRoundTripper.(*http.Transport).Clone() + transport := promapi.DefaultRoundTripper.(*http.Transport) transport.TLSClientConfig = &tls.Config{ InsecureSkipVerify: provider.Spec.InsecureSkipTlsVerify, } From aef152ed943ba32774c2d13fabd530601c04676f Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Tue, 15 Oct 2024 17:51:00 +0530 Subject: [PATCH 16/22] feat: modifying unit test Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 1 - .../providers/prometheus/common_test.go | 43 ++++++++++++++++--- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index 9d3a39ca0c..f3f8e8fa7a 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -49,7 +49,6 @@ func (r RoundTripperRetriever) GetRoundTripper(ctx context.Context, provider met return config.NewBasicAuthRoundTripper(secret.User, secret.Password, "", "", transport), nil } - func getPrometheusSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider, k8sClient client.Client) (*SecretData, error) { if !provider.HasSecretDefined() { return nil, ErrSecretKeyRefNotDefined diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 42336e5eea..b7bf55c1a1 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -116,12 +116,13 @@ func Test_GetRoundtripper(t *testing.T) { }, } tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - want http.RoundTripper - wantErr bool - errorStr string + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + want http.RoundTripper + wantErr bool + errorStr string + insecureSkipTlsVerify bool }{ { name: "TestSuccess", @@ -137,6 +138,7 @@ func Test_GetRoundtripper(t *testing.T) { Key: "", Optional: nil, }, + InsecureSkipTlsVerify: false, }, }, k8sClient: fake.NewClient(goodsecret), @@ -171,6 +173,25 @@ func Test_GetRoundtripper(t *testing.T) { wantErr: true, errorStr: "not found", }, + { + name: "TestInsecureSkipTlsVerifyEnabled", + provider: metricsapi.KeptnMetricsProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Spec: metricsapi.KeptnMetricsProviderSpec{ + SecretKeyRef: v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "test", + }, + Key: "", + Optional: nil, + }, + InsecureSkipTlsVerify: true, + }, + }, + k8sClient: fake.NewClient(goodsecret), + want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), + wantErr: false, + }, } for _, tt := range tests { @@ -188,6 +209,14 @@ func Test_GetRoundtripper(t *testing.T) { if !reflect.DeepEqual(got, tt.want) { t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) } + + if transport, ok := got.(*config.BasicAuthRoundTripper); ok { + if transport.Transport != nil { + if httpTransport, ok := transport.Transport.(*http.Transport); ok { + require.Equal(t, tt.provider.Spec.InsecureSkipTlsVerify, httpTransport.TLSClientConfig.InsecureSkipVerify, "Expected InsecureSkipTlsVerify to be set") + } + } + } }) } -} \ No newline at end of file +} From f46b6a2acd302e0c5bdb5a4f8c6bdb0020d4e85a Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Tue, 15 Oct 2024 18:40:24 +0530 Subject: [PATCH 17/22] feat: Added InsecureSkipTlsVerify flag Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 52 +++---------------- 1 file changed, 7 insertions(+), 45 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index b7bf55c1a1..92b27bd11a 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -116,21 +116,19 @@ func Test_GetRoundtripper(t *testing.T) { }, } tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - want http.RoundTripper - wantErr bool - errorStr string - insecureSkipTlsVerify bool + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + want http.RoundTripper + wantErr bool + errorStr string }{ { name: "TestSuccess", provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ - Type: "", - TargetServer: "", + TargetServer: "someTargetServer", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", @@ -145,34 +143,6 @@ func Test_GetRoundtripper(t *testing.T) { want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: false, }, - { - name: "TestSecretNotDefined", - provider: metricsapi.KeptnMetricsProvider{}, - k8sClient: fake.NewClient(), - want: promapi.DefaultRoundTripper, - wantErr: false, - }, - { - name: "TestErrorFromGetPrometheusSecretNotExists", - provider: metricsapi.KeptnMetricsProvider{ - ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, - Spec: metricsapi.KeptnMetricsProviderSpec{ - Type: "", - TargetServer: "", - SecretKeyRef: v1.SecretKeySelector{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "test", - }, - Key: "", - Optional: nil, - }, - }, - }, - k8sClient: fake.NewClient(), - want: nil, - wantErr: true, - errorStr: "not found", - }, { name: "TestInsecureSkipTlsVerifyEnabled", provider: metricsapi.KeptnMetricsProvider{ @@ -209,14 +179,6 @@ func Test_GetRoundtripper(t *testing.T) { if !reflect.DeepEqual(got, tt.want) { t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) } - - if transport, ok := got.(*config.BasicAuthRoundTripper); ok { - if transport.Transport != nil { - if httpTransport, ok := transport.Transport.(*http.Transport); ok { - require.Equal(t, tt.provider.Spec.InsecureSkipTlsVerify, httpTransport.TLSClientConfig.InsecureSkipVerify, "Expected InsecureSkipTlsVerify to be set") - } - } - } }) } } From 0024f6f2699f40b4acda87fabcda3d738b2c83e8 Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 16 Oct 2024 14:54:22 +0530 Subject: [PATCH 18/22] adding tests in the common_test.go Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common.go | 3 +- .../providers/prometheus/common_test.go | 32 +++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common.go b/metrics-operator/controllers/common/providers/prometheus/common.go index f3f8e8fa7a..ce7bd1e28c 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common.go +++ b/metrics-operator/controllers/common/providers/prometheus/common.go @@ -4,12 +4,13 @@ import ( "context" "crypto/tls" "errors" + "net/http" + metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1" promapi "github.com/prometheus/client_golang/api" "github.com/prometheus/common/config" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "net/http" "sigs.k8s.io/controller-runtime/pkg/client" ) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 92b27bd11a..e92b23cc80 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -128,7 +128,8 @@ func Test_GetRoundtripper(t *testing.T) { provider: metricsapi.KeptnMetricsProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: metricsapi.KeptnMetricsProviderSpec{ - TargetServer: "someTargetServer", + Type: "", + TargetServer: "", SecretKeyRef: v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ Name: "test", @@ -136,13 +137,40 @@ func Test_GetRoundtripper(t *testing.T) { Key: "", Optional: nil, }, - InsecureSkipTlsVerify: false, }, }, k8sClient: fake.NewClient(goodsecret), want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: false, }, + { + name: "TestSecretNotDefined", + provider: metricsapi.KeptnMetricsProvider{}, + k8sClient: fake.NewClient(), + want: promapi.DefaultRoundTripper, + wantErr: false, + }, + { + name: "TestErrorFromGetPrometheusSecretNotExists", + provider: metricsapi.KeptnMetricsProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Spec: metricsapi.KeptnMetricsProviderSpec{ + Type: "", + TargetServer: "", + SecretKeyRef: v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "test", + }, + Key: "", + Optional: nil, + }, + }, + }, + k8sClient: fake.NewClient(), + want: nil, + wantErr: true, + errorStr: "not found", + }, { name: "TestInsecureSkipTlsVerifyEnabled", provider: metricsapi.KeptnMetricsProvider{ From eea2d1429bcfbb2682bd443225908d3c4a72c72f Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 16 Oct 2024 16:35:37 +0530 Subject: [PATCH 19/22] formatted files Signed-off-by: Bharadwajshivam28 --- .../controllers/common/providers/prometheus/common_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index e92b23cc80..643db17ce7 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -197,7 +197,7 @@ func Test_GetRoundtripper(t *testing.T) { got, err := RoundTripperRetriever{}.GetRoundTripper(context.TODO(), tt.provider, tt.k8sClient) t.Log(err) if (err != nil) != tt.wantErr { - t.Errorf("getRoundtripper() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("getRoundtripper() error. = %v, wantErr %v", err, tt.wantErr) return } if tt.errorStr != "" && !strings.Contains(err.Error(), tt.errorStr) { From 6cc8224cac292161e7567fc1c9d4c8117be8296a Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Tue, 22 Oct 2024 15:48:22 +0530 Subject: [PATCH 20/22] Improvde test case Signed-off-by: Bharadwajshivam28 --- .../common/providers/prometheus/common_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 643db17ce7..57312925ed 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -4,7 +4,7 @@ import ( "context" "net/http" "net/http/httptest" - "reflect" + // "reflect" "strings" "testing" @@ -187,7 +187,6 @@ func Test_GetRoundtripper(t *testing.T) { }, }, k8sClient: fake.NewClient(goodsecret), - want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: false, }, } @@ -197,15 +196,24 @@ func Test_GetRoundtripper(t *testing.T) { got, err := RoundTripperRetriever{}.GetRoundTripper(context.TODO(), tt.provider, tt.k8sClient) t.Log(err) if (err != nil) != tt.wantErr { - t.Errorf("getRoundtripper() error. = %v, wantErr %v", err, tt.wantErr) + t.Errorf("getRoundtripper() error = %v, wantErr %v", err, tt.wantErr) return } if tt.errorStr != "" && !strings.Contains(err.Error(), tt.errorStr) { t.Errorf("getRoundtripper() error = %s, wantErr %s", err.Error(), tt.errorStr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) + if !tt.wantErr && got == nil { + t.Errorf("getRoundtripper() returned nil, expected a RoundTripper") + } + // if !reflect.DeepEqual(got, tt.want) { + // t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) + // } + if tr, ok := got.(*http.Transport); ok { + if tr.TLSClientConfig.InsecureSkipVerify != tt.provider.Spec.InsecureSkipTlsVerify { + t.Errorf("RoundTripper TLSClientConfig.InsecureSkipVerify = %v, expected %v", + tr.TLSClientConfig.InsecureSkipVerify, tt.provider.Spec.InsecureSkipTlsVerify) + } } }) } From 0013e90665b63a6971530b6ba32ee9822e156ffc Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 23 Oct 2024 19:05:46 +0530 Subject: [PATCH 21/22] Added username and password parameter Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index 57312925ed..f28b8ed9e3 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -4,7 +4,6 @@ import ( "context" "net/http" "net/http/httptest" - // "reflect" "strings" "testing" @@ -116,12 +115,14 @@ func Test_GetRoundtripper(t *testing.T) { }, } tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - want http.RoundTripper - wantErr bool - errorStr string + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + wantUser string + wantPassword string + wantRoundTripper http.RoundTripper + wantErr bool + errorStr string }{ { name: "TestSuccess", @@ -140,14 +141,18 @@ func Test_GetRoundtripper(t *testing.T) { }, }, k8sClient: fake.NewClient(goodsecret), - want: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), + wantUser: "myuser", + wantPassword: "mytoken", + wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: false, }, { name: "TestSecretNotDefined", provider: metricsapi.KeptnMetricsProvider{}, k8sClient: fake.NewClient(), - want: promapi.DefaultRoundTripper, + wantUser: "myuser", + wantPassword: "mytoken", + wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: false, }, { @@ -167,7 +172,9 @@ func Test_GetRoundtripper(t *testing.T) { }, }, k8sClient: fake.NewClient(), - want: nil, + wantUser: "myuser", + wantPassword: "mytoken", + wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), wantErr: true, errorStr: "not found", }, @@ -206,9 +213,6 @@ func Test_GetRoundtripper(t *testing.T) { if !tt.wantErr && got == nil { t.Errorf("getRoundtripper() returned nil, expected a RoundTripper") } - // if !reflect.DeepEqual(got, tt.want) { - // t.Errorf("getRoundtripper() got = %v, want %v", got, tt.want) - // } if tr, ok := got.(*http.Transport); ok { if tr.TLSClientConfig.InsecureSkipVerify != tt.provider.Spec.InsecureSkipTlsVerify { t.Errorf("RoundTripper TLSClientConfig.InsecureSkipVerify = %v, expected %v", From 6d639d16061ddb17eb9cb553beb032831ded258d Mon Sep 17 00:00:00 2001 From: Bharadwajshivam28 Date: Wed, 23 Oct 2024 19:06:40 +0530 Subject: [PATCH 22/22] formatted file Signed-off-by: Bharadwajshivam28 --- .../providers/prometheus/common_test.go | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/metrics-operator/controllers/common/providers/prometheus/common_test.go b/metrics-operator/controllers/common/providers/prometheus/common_test.go index f28b8ed9e3..9b08acd41a 100644 --- a/metrics-operator/controllers/common/providers/prometheus/common_test.go +++ b/metrics-operator/controllers/common/providers/prometheus/common_test.go @@ -115,14 +115,14 @@ func Test_GetRoundtripper(t *testing.T) { }, } tests := []struct { - name string - provider metricsapi.KeptnMetricsProvider - k8sClient client.Client - wantUser string - wantPassword string + name string + provider metricsapi.KeptnMetricsProvider + k8sClient client.Client + wantUser string + wantPassword string wantRoundTripper http.RoundTripper - wantErr bool - errorStr string + wantErr bool + errorStr string }{ { name: "TestSuccess", @@ -140,20 +140,20 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(goodsecret), + k8sClient: fake.NewClient(goodsecret), wantUser: "myuser", wantPassword: "mytoken", wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), - wantErr: false, + wantErr: false, }, { - name: "TestSecretNotDefined", - provider: metricsapi.KeptnMetricsProvider{}, - k8sClient: fake.NewClient(), + name: "TestSecretNotDefined", + provider: metricsapi.KeptnMetricsProvider{}, + k8sClient: fake.NewClient(), wantUser: "myuser", wantPassword: "mytoken", wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), - wantErr: false, + wantErr: false, }, { name: "TestErrorFromGetPrometheusSecretNotExists", @@ -171,12 +171,12 @@ func Test_GetRoundtripper(t *testing.T) { }, }, }, - k8sClient: fake.NewClient(), + k8sClient: fake.NewClient(), wantUser: "myuser", wantPassword: "mytoken", wantRoundTripper: config.NewBasicAuthRoundTripper("myuser", "mytoken", "", "", promapi.DefaultRoundTripper), - wantErr: true, - errorStr: "not found", + wantErr: true, + errorStr: "not found", }, { name: "TestInsecureSkipTlsVerifyEnabled",