Don't expose Envoy using hostPorts

Exposing services using hostPorts is discouraged by the official k8s
docs and has similar problems as using host networking.

On Packet, we expose Envoy as a service of type LoadBalancer so using
hostPorts isn't necessary. On AWS, we want to expose Envoy as a
service of type NodePort rather than using hostPorts. Therefore, we
can remove the 'hostPort' fields from the Envoy manifest.

assets/components/contour/templates/02-service-envoy.yaml

@@ -19,12 +19,17 @@ spec:
   externalTrafficPolicy: Local
   ports:
   - port: 80
+    {{- if eq .Values.envoy.serviceType "NodePort" }}
+    nodePort: 30080
+    {{- end }}
     name: http
     protocol: TCP
   - port: 443
+    {{- if eq .Values.envoy.serviceType "NodePort" }}
+    nodePort: 30443
+    {{- end }}
     name: https
     protocol: TCP
   selector:
     app: envoy
-  type: LoadBalancer
-
+  type: {{ .Values.envoy.serviceType }}

assets/components/contour/templates/03-envoy.yaml

@@ -101,11 +101,9 @@ spec:
               fieldPath: metadata.name
         ports:
         - containerPort: 80
-          hostPort: 80
           name: http
           protocol: TCP
         - containerPort: 443
-          hostPort: 443
           name: https
           protocol: TCP
         readinessProbe:

assets/components/contour/values.yaml

@@ -10,6 +10,7 @@ contour:
 envoy:
   image: docker.io/envoyproxy/envoy
   tag: v1.13.1
+  serviceType:
 
 nodeAffinity: {}
 # nodeAffinity:

ci/aws/aws-cluster.lokocfg.envsubst

@@ -49,6 +49,7 @@ component "prometheus-operator" {
 component "contour" {
   ingress_hosts     = ["dex.$CLUSTER_ID.$AWS_DNS_ZONE", "gangway.$CLUSTER_ID.$AWS_DNS_ZONE"]
   enable_monitoring = true
+  service_type      = "NodePort"
 }
 
 component "metrics-server" {}

docs/configuration-reference/components/contour.md

@@ -35,6 +35,7 @@ component "contour" {
   # Optional arguments
   enable_monitoring = false
   ingress_hosts     = ["*.example.lokomotive.org"]
+  service_type      = "NodePort"
 
   node_affinity {
     key      = "node-role.kubernetes.io/node"
@@ -66,12 +67,13 @@ Table of all the arguments accepted by the component.
 
 Example:
 
-| Argument            | Description                                                                                             | Default | Required |
-|---------------------|---------------------------------------------------------------------------------------------------------|:-------:|:--------:|
-| `enable_monitoring` | Create Prometheus Operator configs to scrape Contour and Envoy metrics. Also deploys Grafana Dashboard. | false   | false    |
-| `ingress_hosts`     | [ExternalDNS component](external-dns.md) creates DNS entries from the values provided.                  | ""      | false    |
-| `node_affinity`     | Node affinity for deploying the operator pod and envoy daemonset.                                       | -       | false    |
-| `toleration`        | Tolerations that the operator and envoy pods will tolerate.                                             | -       | false    |
+| Argument            | Description                                                                                             | Default        | Required |
+|---------------------|---------------------------------------------------------------------------------------------------------|:--------------:|:--------:|
+| `enable_monitoring` | Create Prometheus Operator configs to scrape Contour and Envoy metrics. Also deploys Grafana Dashboard. | false          | false    |
+| `ingress_hosts`     | [ExternalDNS component](external-dns.md) creates DNS entries from the values provided.                  | ""             | false    |
+| `node_affinity`     | Node affinity for deploying the operator pod and envoy daemonset.                                       | -              | false    |
+| `service_type`      | The type of Kubernetes service used to expose Envoy.                                                    | "LoadBalancer" | false    |
+| `toleration`        | Tolerations that the operator and envoy pods will tolerate.                                             | -              | false    |
 
 ## Applying
 

pkg/components/contour/component.go

@@ -24,7 +24,11 @@ import (
 	"github.com/kinvolk/lokomotive/pkg/components"
 )
 
-const name = "contour"
+const (
+	name                    = "contour"
+	serviceTypeNodePort     = "NodePort"
+	serviceTypeLoadBalancer = "LoadBalancer"
+)
 
 func init() {
 	components.Register(name, newComponent())
@@ -41,30 +45,44 @@ type component struct {
 	// This solution is a workaround for projectcontour/contour#403
 	// More details regarding this workaround and other solutions is captured in
 	// https://github.com/kinvolk/PROJECT-Lokomotive-Kubernetes/issues/474
-	IngressHosts []string `hcl:"ingress_hosts,optional"`
-
+	IngressHosts    []string            `hcl:"ingress_hosts,optional"`
 	NodeAffinity    []util.NodeAffinity `hcl:"node_affinity,block"`
 	NodeAffinityRaw string
-
-	Tolerations    []util.Toleration `hcl:"toleration,block"`
-	TolerationsRaw string
+	ServiceType     string            `hcl:"service_type,optional"`
+	Tolerations     []util.Toleration `hcl:"toleration,block"`
+	TolerationsRaw  string
 }
 
 func newComponent() *component {
-	return &component{}
+	return &component{
+		ServiceType: serviceTypeLoadBalancer,
+	}
 }
 
 func (c *component) LoadConfig(configBody *hcl.Body, evalContext *hcl.EvalContext) hcl.Diagnostics {
+	diagnostics := hcl.Diagnostics{}
+
 	if configBody == nil {
 		return hcl.Diagnostics{
 			components.HCLDiagConfigBodyNil,
 		}
 	}
-	if err := gohcl.DecodeBody(*configBody, evalContext, c); err != nil {
-		return err
+
+	d := gohcl.DecodeBody(*configBody, evalContext, c)
+	if d.HasErrors() {
+		diagnostics = append(diagnostics, d...)
+		return diagnostics
+	}
+
+	// Validate service type.
+	if c.ServiceType != serviceTypeNodePort && c.ServiceType != serviceTypeLoadBalancer {
+		diagnostics = append(diagnostics, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("Unknown service type %q", c.ServiceType),
+		})
 	}
 
-	return nil
+	return diagnostics
 }
 
 func (c *component) RenderManifests() (map[string]string, error) {

pkg/components/contour/component_test.go

@@ -17,55 +17,92 @@ package contour
 import (
 	"testing"
 
-	"github.com/hashicorp/hcl/v2"
-
 	"github.com/kinvolk/lokomotive/pkg/components/util"
 )
 
-func testRenderManifest(t *testing.T, configHCL string) {
-	body, diagnostics := util.GetComponentBody(configHCL, name)
-	if diagnostics != nil {
-		t.Fatalf("Error getting component body: %v", diagnostics)
-	}
-
-	component := newComponent()
-	diagnostics = component.LoadConfig(body, &hcl.EvalContext{})
-	if diagnostics.HasErrors() {
-		t.Fatalf("Valid config should not return error, got: %s", diagnostics)
-	}
-
-	m, err := component.RenderManifests()
-	if err != nil {
-		t.Fatalf("Rendering manifests with valid config should succeed, got: %s", err)
-	}
-	if len(m) <= 0 {
-		t.Fatalf("Rendered manifests shouldn't be empty")
-	}
-}
-
-func TestRenderManifestWithIngressHosts(t *testing.T) {
-	configHCL := `
+//nolint:funlen
+func TestRenderManifest(t *testing.T) {
+	tests := []struct {
+		desc    string
+		hcl     string
+		wantErr bool
+	}{
+		{
+			desc: "With ingress hosts",
+			hcl: `
 component "contour" {
   ingress_hosts = ["test.domain.com"]
 }
-`
-	testRenderManifest(t, configHCL)
-}
-
-func TestRenderManifestWithIngressHostsWildcard(t *testing.T) {
-	configHCL := `
+			`,
+		},
+		{
+			desc: "With ingress hosts wildcard",
+			hcl: `
 component "contour" {
   ingress_hosts = ["*.domain.com"]
 }
-`
-	testRenderManifest(t, configHCL)
-}
-
-func TestRenderManifestWithServiceMonitor(t *testing.T) {
-	configHCL := `
+		`,
+		},
+		{
+			desc: "With monitoring",
+			hcl: `
 component "contour" {
   enable_monitoring = true
 }
-`
-	testRenderManifest(t, configHCL)
+			`,
+		},
+		{
+			desc: "With service type",
+			hcl: `
+component "contour" {
+  service_type = "NodePort"
+}
+			`,
+		},
+		{
+			desc: "Wrong service type",
+			hcl: `
+component "contour" {
+  service_type = "stuff"
+}
+			`,
+			wantErr: true,
+		},
+		{
+			desc: "Non-existent field",
+			hcl: `
+component "contour" {
+  stuff = 3
+}
+			`,
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range tests {
+		b, d := util.GetComponentBody(tc.hcl, name)
+		if d != nil {
+			t.Errorf("%s - Error getting component body: %v", tc.desc, d)
+		}
+
+		c := newComponent()
+		d = c.LoadConfig(b, nil)
+
+		if !tc.wantErr && d.HasErrors() {
+			t.Errorf("%s - Valid config should not return error, got: %s", tc.desc, d)
+		}
+
+		if tc.wantErr && !d.HasErrors() {
+			t.Errorf("%s - Wrong config should have returned an error", tc.desc)
+		}
+
+		m, err := c.RenderManifests()
+		if err != nil {
+			t.Errorf("%s - Rendering manifests with valid config should succeed, got: %s", tc.desc, err)
+		}
+
+		if len(m) == 0 {
+			t.Errorf("%s - Rendered manifests shouldn't be empty", tc.desc)
+		}
+	}
 }

pkg/components/contour/manifest.go

@@ -28,6 +28,9 @@ ingressHosts:
   {{- end }}
 {{- end }}
 
+envoy:
+  serviceType: {{ .ServiceType }}
+
 {{- if .NodeAffinity }}
 nodeAffinity:
   requiredDuringSchedulingIgnoredDuringExecution: