Repository: kinvolk/lokomotive
Pull request #964 (2020-09-14 14:24:59)

prometheus-operator: Add external_url
This commit adds the feature removed sometime back called `external_url`
this allows the user to choose where(URL) the prometheus is exposed.


Fixes https://github.com/kinvolk/lokomotive/issues/826

TODOS:

- [x] Add test to verify the conversion.

surajssd: @invidian PTAL at the latest changes, I have tried to make the test generic using some reflect magic. 
surajssd: @johananl @invidian PTAL
johananl: Forgot to say that I wasn't sure how to test the `external_url` knob. I can see that its value gets rendered in the YAML, however I wasn't sure how to check the final result. For the future, I could definitely use some "how to test" instructions in the description :slightly_smiling_face: 

Patch:
diff --git a/docs/configuration-reference/components/prometheus-operator.md b/docs/configuration-reference/components/prometheus-operator.md
index 9ab46d0b..62d3c1b6 100644
--- a/docs/configuration-reference/components/prometheus-operator.md
+++ b/docs/configuration-reference/components/prometheus-operator.md
@@ -147,6 +147,7 @@ Example:
 | `prometheus.ingress.host`                       | Ingress URL host to expose Prometheus over the internet. **NOTE:** When running on Packet, a DNS entry pointing at the ingress controller needs to be created.                                                                                   |                                                                                                                          -                                                                                                                          |   string    |   true   |
 | `prometheus.ingress.class`                      | Ingress class to use for Prometheus ingress.                                                                                                                                                                                                     |                                                                                                                      `contour`                                                                                                                      |   string    |  false   |
 | `prometheus.ingress.certmanager_cluster_issuer` | `ClusterIssuer` to be used by cert-manager while issuing TLS certificates. Supported values: `letsencrypt-production`, `letsencrypt-staging`.                                                                                                    |                                                                                                              `letsencrypt-production`                                                                                                               |   string    |  false   |
+| `prometheus.external_url`                    | The URL on which Prometheus will be accessible. If not provided, the URL is taken from `prometheus.ingress.host` with `https` as a scheme.                                                                                                          |                                                                                                                          -                                                                                                                          |   string    |  false   |
 | `alertmanager_retention`                     | Time duration Alertmanager shall retain data for. Must match the regular expression `[0-9]+(ms\|s\|m\|h)` (milliseconds, seconds, minutes and hours).                                                                                               |                                                                                                                       `120h`                                                                                                                        |   string    |  false   |
 | `alertmanager_external_url`                  | The external URL the Alertmanager instances will be available under. This is necessary to generate correct URLs. This is necessary if Alertmanager is not served from root of a DNS name.                                                           |                                                                                                                         ""                                                                                                                          |   string    |  false   |
 | `alertmanager_config`                        | Provide YAML file path to configure Alertmanager. See [https://prometheus.io/docs/alerting/configuration/#configuration-file](https://prometheus.io/docs/alerting/configuration/#configuration-file).                                               | `{"global":{"resolve_timeout":"5m"},"route":{"group_by":["job"],"group_wait":"30s","group_interval":"5m","repeat_interval":"12h","receiver":"null","routes":[{"match":{"alertname":"Watchdog"},"receiver":"null"}]},"receivers":[{"name":"null"}]}` |   string    |  false   |
diff --git a/pkg/components/prometheus-operator/component.go b/pkg/components/prometheus-operator/component.go
index 1fb5731c..2989a16f 100644
--- a/pkg/components/prometheus-operator/component.go
+++ b/pkg/components/prometheus-operator/component.go
@@ -16,6 +16,7 @@ package prometheus
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/gohcl"
@@ -63,6 +64,7 @@ type Prometheus struct {
 	WatchLabeledPrometheusRules bool              `hcl:"watch_labeled_prometheus_rules,optional"`
 	Ingress                     *types.Ingress    `hcl:"ingress,block"`
 	ExternalLabels              map[string]string `hcl:"external_labels,optional"`
+	ExternalURL                 string            `hcl:"external_url,optional"`
 }
 
 type component struct {
@@ -162,6 +164,30 @@ func (c *component) LoadConfig(configBody *hcl.Body, evalContext *hcl.EvalContex
 		c.Prometheus.Ingress.SetDefaults()
 	}
 
+	// If user has provided both `prometheus.ingress.host` and `prometheus.external_url`, the
+	// hostnames should be the same.
+	if c.Prometheus != nil && c.Prometheus.ExternalURL != "" && c.Prometheus.Ingress != nil {
+		exu, err := url.Parse(c.Prometheus.ExternalURL)
+		if err != nil {
+			return hcl.Diagnostics{
+				{
+					Severity: hcl.DiagError,
+					Summary: fmt.Sprintf("parsing 'prometheus.external_url' with value %q as URL: %v",
+						c.Prometheus.ExternalURL, err),
+				},
+			}
+		}
+
+		if exu.Host != c.Prometheus.Ingress.Host {
+			return hcl.Diagnostics{
+				{
+					Severity: hcl.DiagError,
+					Summary:  "'prometheus.external_url' and 'prometheus.ingress.host' do not match",
+				},
+			}
+		}
+	}
+
 	return nil
 }
 
diff --git a/pkg/components/prometheus-operator/template.go b/pkg/components/prometheus-operator/template.go
index bdf57023..f45661cd 100644
--- a/pkg/components/prometheus-operator/template.go
+++ b/pkg/components/prometheus-operator/template.go
@@ -118,7 +118,9 @@ prometheus:
       secretName: {{ .Prometheus.Ingress.Host }}-tls
   {{ end }}
   prometheusSpec:
-    {{ if .Prometheus.Ingress }}
+    {{ if .Prometheus.ExternalURL }}
+    externalUrl: {{ .Prometheus.ExternalURL }}
+    {{ else if .Prometheus.Ingress }}
     externalUrl: https://{{.Prometheus.Ingress.Host}}
     {{ end }}
     {{ if .Prometheus.NodeSelector }}
diff --git a/vendor/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml.go b/vendor/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml.go
new file mode 100644
index 00000000..2fdd1d43
--- /dev/null
+++ b/vendor/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml.go
@@ -0,0 +1,46 @@
+/*
+Copyright 2014 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package yaml
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/yaml"
+)
+
+// yamlSerializer converts YAML passed to the Decoder methods to JSON.
+type yamlSerializer struct {
+	// the nested serializer
+	runtime.Serializer
+}
+
+// yamlSerializer implements Serializer
+var _ runtime.Serializer = yamlSerializer{}
+
+// NewDecodingSerializer adds YAML decoding support to a serializer that supports JSON.
+func NewDecodingSerializer(jsonSerializer runtime.Serializer) runtime.Serializer {
+	return &yamlSerializer{jsonSerializer}
+}
+
+func (c yamlSerializer) Decode(data []byte, gvk *schema.GroupVersionKind, into runtime.Object) (runtime.Object, *schema.GroupVersionKind, error) {
+	out, err := yaml.ToJSON(data)
+	if err != nil {
+		return nil, nil, err
+	}
+	data = out
+	return c.Serializer.Decode(data, gvk, into)
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 67b66a5a..c218ad50 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -575,6 +575,7 @@ k8s.io/apimachinery/pkg/runtime/serializer/protobuf
 k8s.io/apimachinery/pkg/runtime/serializer/recognizer
 k8s.io/apimachinery/pkg/runtime/serializer/streaming
 k8s.io/apimachinery/pkg/runtime/serializer/versioning
+k8s.io/apimachinery/pkg/runtime/serializer/yaml
 k8s.io/apimachinery/pkg/selection
 k8s.io/apimachinery/pkg/types
 k8s.io/apimachinery/pkg/util/cache


Test patch:
diff --git a/pkg/components/internal/testutil/common.go b/pkg/components/internal/testutil/common.go
new file mode 100644
index 00000000..1cb0b3d8
--- /dev/null
+++ b/pkg/components/internal/testutil/common.go
@@ -0,0 +1,61 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package testutil
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+
+	"github.com/kinvolk/lokomotive/pkg/components"
+	"github.com/kinvolk/lokomotive/pkg/components/util"
+)
+
+// ConfigFromMap takes a map and a key and returns the value associated with the key. If the key
+// does not exist in that map, the function fails.
+func ConfigFromMap(t *testing.T, m map[string]string, k string) string {
+	ret, ok := m[k]
+	if !ok {
+		t.Fatalf("Config not found with filename: %q", k)
+	}
+
+	return ret
+}
+
+// RenderManifests converts a component into YAML manifests.
+func RenderManifests(
+	t *testing.T, component components.Component, componentName string, hclConfig string,
+) map[string]string {
+	body, diagnostics := util.GetComponentBody(hclConfig, componentName)
+	if diagnostics.HasErrors() {
+		t.Fatalf("Getting component body: %v", diagnostics.Errs())
+	}
+
+	diagnostics = component.LoadConfig(body, &hcl.EvalContext{})
+	if diagnostics.HasErrors() {
+		t.Fatalf("Loading configuration: %v", diagnostics)
+	}
+
+	ret, err := component.RenderManifests()
+	if err != nil {
+		t.Fatalf("Rendering manifests: %v", err)
+	}
+
+	if len(ret) == 0 {
+		t.Fatalf("Rendered manifests shouldn't be empty")
+	}
+
+	return ret
+}
diff --git a/pkg/components/internal/testutil/doc.go b/pkg/components/internal/testutil/doc.go
new file mode 100644
index 00000000..8dfd9f8f
--- /dev/null
+++ b/pkg/components/internal/testutil/doc.go
@@ -0,0 +1,16 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package testutil contains helper functions for component unit tests.
+package testutil
diff --git a/pkg/components/internal/testutil/jsonpath.go b/pkg/components/internal/testutil/jsonpath.go
new file mode 100644
index 00000000..f72c4eb6
--- /dev/null
+++ b/pkg/components/internal/testutil/jsonpath.go
@@ -0,0 +1,91 @@
+// Copyright 2020 The Lokomotive Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package testutil
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	yamlserializer "k8s.io/apimachinery/pkg/runtime/serializer/yaml"
+	"k8s.io/client-go/util/jsonpath"
+)
+
+// unstructredObj accepts a Kubernetes manifest in YAML format and returns an object of type
+// `unstructured.Unstructured`. This object has many methods that can be used by the consumer to
+// extract metadata from the Kubernetes manifest.
+func unstructredObj(t *testing.T, yamlObj string) *unstructured.Unstructured {
+	u := &unstructured.Unstructured{}
+
+	// Decode YAML into `unstructured.Unstructured`.
+	dec := yamlserializer.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
+	if _, _, err := dec.Decode([]byte(yamlObj), nil, u); err != nil {
+		t.Fatalf("Converting config to unstructured.Unstructured: %v", err)
+	}
+
+	return u
+}
+
+// valFromObject takes a JSON path as a string and an object of type `unstructured.Unstructured`.
+// This function returns an object of type `reflect.Value` at that JSON path.
+func valFromObject(t *testing.T, jp string, obj *unstructured.Unstructured) reflect.Value {
+	jPath := jsonpath.New("parse")
+	if err := jPath.Parse(jp); err != nil {
+		t.Fatalf("Parsing JSONPath: %v", err)
+	}
+
+	v, err := jPath.FindResults(obj.Object)
+	if err != nil {
+		t.Fatalf("Finding results using JSONPath in the YAML file: %v", err)
+	}
+
+	if len(v) == 0 || len(v[0]) == 0 {
+		t.Fatalf("No result found")
+	}
+
+	return v[0][0]
+}
+
+// jsonPathValue extracts an object at a JSON path from a YAML config, and returns an interface
+// object.
+func jsonPathValue(t *testing.T, yamlConfig string, jsonPath string) interface{} {
+	u := unstructredObj(t, yamlConfig)
+	got := valFromObject(t, jsonPath, u)
+
+	switch got.Kind() { //nolint:exhaustive
+	case reflect.Interface:
+		// TODO: Add type switch here for concrete types.
+		return got.Interface()
+	default:
+		t.Fatalf("Extracted object has an unknown type: %v", got.Kind())
+	}
+
+	return nil
+}
+
+// MatchJSONPathStringValue is a helper function for component unit tests. It compares the string at
+// a JSON path in a YAML config to the expected string.
+func MatchJSONPathStringValue(t *testing.T, yamlConfig string, jsonPath string, expected string) {
+	obj := jsonPathValue(t, yamlConfig, jsonPath)
+
+	got, ok := obj.(string)
+	if !ok {
+		t.Fatalf("Value is not string: %#v", obj)
+	}
+
+	if got != expected {
+		t.Fatalf("Expected: %s, Got: %s", expected, got)
+	}
+}
diff --git a/pkg/components/prometheus-operator/component_test.go b/pkg/components/prometheus-operator/component_test.go
index 8de99225..03c17782 100644
--- a/pkg/components/prometheus-operator/component_test.go
+++ b/pkg/components/prometheus-operator/component_test.go
@@ -17,6 +17,7 @@ package prometheus
 import (
 	"testing"
 
+	"github.com/kinvolk/lokomotive/pkg/components/internal/testutil"
 	"github.com/kinvolk/lokomotive/pkg/components/util"
 )
 
@@ -62,6 +63,34 @@ component "prometheus-operator" {
 }`,
 			wantErr: true,
 		},
+		{
+			desc: "prometheus ingress and external_url given and are different",
+			hcl: `
+component "prometheus-operator" {
+  prometheus {
+	external_url = "https://prometheus.notmydomain.net"
+    ingress {
+      host = "prometheus.mydomain.net"
+    }
+  }
+}
+`,
+			wantErr: true,
+		},
+		{
+			desc: "prometheus ingress and external_url given and are same",
+			hcl: `
+component "prometheus-operator" {
+  prometheus {
+	external_url = "https://prometheus.mydomain.net"
+    ingress {
+      host = "prometheus.mydomain.net"
+    }
+  }
+}
+`,
+			wantErr: false,
+		},
 	}
 
 	for _, tc := range tests {
@@ -98,3 +127,75 @@ component "prometheus-operator" {
 		})
 	}
 }
+
+//nolint:funlen
+func TestConversion(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		inputConfig          string
+		expectedManifestName string
+		expected             string
+		jsonPath             string
+	}{
+		{
+			name: "use external_url param",
+			inputConfig: `
+component "prometheus-operator" {
+  prometheus {
+    external_url = "https://prometheus.externalurl.net"
+  }
+}
+`,
+			expectedManifestName: "prometheus-operator/templates/prometheus/prometheus.yaml",
+			expected:             "https://prometheus.externalurl.net",
+			jsonPath:             "{.spec.externalUrl}",
+		},
+		{
+			name: "no external_url param",
+			inputConfig: `
+		component "prometheus-operator" {
+		  prometheus {
+		    ingress {
+		      host                       = "prometheus.mydomain.net"
+		      class                      = "contour"
+		      certmanager_cluster_issuer = "letsencrypt-production"
+		    }
+		  }
+		}
+		`,
+			expectedManifestName: "prometheus-operator/templates/prometheus/prometheus.yaml",
+			expected:             "https://prometheus.mydomain.net",
+			jsonPath:             "{.spec.externalUrl}",
+		},
+		{
+			name: "ingress creation for prometheus",
+			inputConfig: `
+		component "prometheus-operator" {
+		  prometheus {
+		    ingress {
+		      host                       = "prometheus.mydomain.net"
+		      class                      = "contour"
+		      certmanager_cluster_issuer = "letsencrypt-production"
+		    }
+		  }
+		}
+		`,
+			expectedManifestName: "prometheus-operator/templates/prometheus/ingress.yaml",
+			expected:             "prometheus.mydomain.net",
+			jsonPath:             "{.spec.rules[0].host}",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			component := newComponent()
+			m := testutil.RenderManifests(t, component, name, tc.inputConfig)
+			gotConfig := testutil.ConfigFromMap(t, m, tc.expectedManifestName)
+
+			testutil.MatchJSONPathStringValue(t, gotConfig, tc.jsonPath, tc.expected)
+		})
+	}
+}