From 60a817595997fedaa6dfcb6eef9e8341d3ee7269 Mon Sep 17 00:00:00 2001 From: Jonathan Yu Date: Mon, 6 Dec 2021 23:46:58 +0000 Subject: [PATCH 1/3] feat: promote ingress settings to public * Add ingress section to values.yaml and re-generate documentation * Add tests for ingress settings * Change comment syntax to {{/* */}} to prevent it from appearing in the generated Ingress spec --- README.md | 6 ++ templates/ingress.yaml | 21 ++--- tests/go.mod | 1 + tests/go.sum | 2 + tests/ingress_test.go | 170 +++++++++++++++++++++++++++++++++++++++++ tests/values.go | 16 +++- values.yaml | 17 +++++ 7 files changed, 223 insertions(+), 10 deletions(-) create mode 100644 tests/ingress_test.go diff --git a/README.md b/README.md index 2a70cac7..80f0145b 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,12 @@ View [our docs](https://coder.com/docs/setup/installation) for detailed installa | coderd.trustProxyIP | bool | Whether Coder should trust X-Real-IP and/or X-Forwarded-For headers from your reverse proxy. This should only be turned on if you're using a reverse proxy that sets both of these headers. This is always enabled if the Nginx ingress is deployed. | `false` | | envbox | object | Required for running Docker inside containers. See requirements: https://coder.com/docs/coder/v1.19/admin/workspace-management/cvms | `{"image":""}` | | envbox.image | string | Injected by Coder during release. | `""` | +| ingress | object | Configure an Ingress to route traffic to Coder services. | `{"annotations":{},"enable":false,"host":"coder.coderhost.com","tls":{"enable":false}}` | +| ingress.annotations | object | Additional annotations to add to the Ingress object. The behavior is typically dependent on the Ingress Controller implementation, and useful for managing features like TLS termination. | `{}` | +| ingress.enable | bool | A boolean controlling whether to create an Ingress. | `false` | +| ingress.host | string | The hostname to proxy to the Coder installation. The cluster Ingress Controller typically uses server name indication or the HTTP Host header to route traffic. | `"coder.coderhost.com"` | +| ingress.tls | object | Configures TLS settings for the Ingress. | `{"enable":false}` | +| ingress.tls.enable | bool | Determines whether the Ingress handles TLS. | `false` | | logging | object | Configures the logging format and output of Coder. | `{"human":"/dev/stderr","json":"","splunk":{"channel":"","token":"","url":""},"stackdriver":""}` | | logging.human | string | Location to send logs that are formatted for readability. Set to an empty string to disable. | `"/dev/stderr"` | | logging.json | string | Location to send logs that are formatted as JSON. Set to an empty string to disable. | `""` | diff --git a/templates/ingress.yaml b/templates/ingress.yaml index d1dddcbb..f16874a9 100644 --- a/templates/ingress.yaml +++ b/templates/ingress.yaml @@ -433,15 +433,18 @@ spec: name: {{ include "coder.serviceName" . }} port: name: tcp-{{ include "coder.serviceName" . }} - {{ $devURLHost := merge .Values dict | dig "coderd" "devurlsHost" "" }} - # Regex docs on '*-suffix.example.com'. This is required as the original input including the suffix is - # not a legal ingress host. We need to remove the suffic, and keep the wildcard '*'. - # - '\\*' Starts with '*' - # - '[^.]*' Suffix is 0 or more characters, '-suffix' - # - '(' Start domain capture grp - # - '\\.' The domain should be separated with a '.' from the subdomain - # - '.*' Rest of the domain. - # - ')' $1 is the ''.example.com' + {{- /* Regex docs on '*-suffix.example.com'. This is required as the original + * input including the suffix is not a legal ingress host. We need to + * remove the suffix, and keep the wildcard '*'. + * + * - '\\*' Starts with '*' + * - '[^.]*' Suffix is 0 or more characters, '-suffix' + * - '(' Start domain capture group + * - '\\.' The domain should be separated with a '.' from the subdomain + * - '.*' Rest of the domain. + * - ')' $1 is the ''.example.com' + */ -}} + {{- $devURLHost := merge .Values dict | dig "coderd" "devurlsHost" "" }} - host: {{ regexReplaceAll "\\*[^.]*(\\..*)" $devURLHost "*${1}" | quote }} http: paths: diff --git a/tests/go.mod b/tests/go.mod index dca97a30..142ca736 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -3,6 +3,7 @@ module cdr.dev/enterprise-helm/tests go 1.17 require ( + github.com/jinzhu/copier v0.3.4 github.com/stretchr/testify v1.7.0 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 helm.sh/helm/v3 v3.7.1 diff --git a/tests/go.sum b/tests/go.sum index 3d06a543..4c952167 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -502,6 +502,8 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU= github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6tORTn+6F6j+Jc8TOr5osrynvN6ivFWZ2GA= +github.com/jinzhu/copier v0.3.4 h1:mfU6jI9PtCeUjkjQ322dlff9ELjGDu975C2p/nrubVI= +github.com/jinzhu/copier v0.3.4/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg= github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20160803190731-bd40a432e4c7/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik= diff --git a/tests/ingress_test.go b/tests/ingress_test.go new file mode 100644 index 00000000..c1b1b9b1 --- /dev/null +++ b/tests/ingress_test.go @@ -0,0 +1,170 @@ +package tests + +import ( + "testing" + + netv1 "k8s.io/api/networking/v1" + "k8s.io/utils/pointer" + + "github.com/jinzhu/copier" + "github.com/stretchr/testify/require" +) + +func TestIngress(t *testing.T) { + chart := LoadChart(t) + + tests := []struct { + Name string + ValuesFunc func(v *CoderValues) + AssertFunc func(t *testing.T, ingress *netv1.Ingress) + }{ + { + Name: "simple-ingress", + ValuesFunc: func(v *CoderValues) { + v.Ingress.Enable = pointer.Bool(true) + v.Ingress.Host = pointer.String("install.coder.com") + v.Coderd.DevURLsHost = pointer.String("*.install.coder.app") + }, + AssertFunc: func(t *testing.T, ingress *netv1.Ingress) { + defaultAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/proxy-body-size": "0", + } + require.Equal(t, defaultAnnotations, ingress.Annotations) + pathTypePrefix := netv1.PathTypePrefix + expectedRules := []netv1.IngressRule{ + { + Host: "install.coder.com", + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: &pathTypePrefix, + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "coderd", + Port: netv1.ServiceBackendPort{ + Name: "tcp-coderd", + }, + }, + }, + }, + }, + }, + }, + }, + { + Host: "*.install.coder.app", + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: &pathTypePrefix, + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "coderd", + Port: netv1.ServiceBackendPort{ + Name: "tcp-coderd", + }, + }, + }, + }, + }, + }, + }, + }, + } + require.Equal(t, expectedRules, ingress.Spec.Rules, "expected ingress spec to match") + }, + }, + { + Name: "devurl-suffix", + ValuesFunc: func(v *CoderValues) { + v.Ingress.Enable = pointer.Bool(true) + v.Ingress.Host = pointer.String("install.coder.com") + v.Coderd.DevURLsHost = pointer.String("*-dev.install.coder.app") + }, + AssertFunc: func(t *testing.T, ingress *netv1.Ingress) { + defaultAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/proxy-body-size": "0", + } + require.Equal(t, defaultAnnotations, ingress.Annotations) + pathTypePrefix := netv1.PathTypePrefix + expectedRules := []netv1.IngressRule{ + { + Host: "install.coder.com", + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: &pathTypePrefix, + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "coderd", + Port: netv1.ServiceBackendPort{ + Name: "tcp-coderd", + }, + }, + }, + }, + }, + }, + }, + }, + { + Host: "*.install.coder.app", + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: &pathTypePrefix, + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "coderd", + Port: netv1.ServiceBackendPort{ + Name: "tcp-coderd", + }, + }, + }, + }, + }, + }, + }, + }, + } + require.Equal(t, expectedRules, ingress.Spec.Rules, "expected ingress spec to match") + }, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.Name, func(t *testing.T) { + // Clone the original values + values := &CoderValues{} + copier.Copy(values, chart.OriginalValues) + + // Run function to perform test-specific modifications of defaults + test.ValuesFunc(values) + + // Verify the results using AssertFunc + objs, err := chart.Render(values, nil, nil) + require.NoError(t, err, "chart render failed") + + var found bool + for _, obj := range objs { + ingress, ok := obj.(*netv1.Ingress) + if ok && ingress.Name == "coderd-ingress" { + found = true + test.AssertFunc(t, ingress) + break + } + } + require.True(t, found, "expected ingress in manifests") + }) + } +} diff --git a/tests/values.go b/tests/values.go index 139fd6ec..181a7777 100644 --- a/tests/values.go +++ b/tests/values.go @@ -60,9 +60,10 @@ type Chart struct { // // TODO: generate these structs from a values.schema.json type CoderValues struct { - Coderd *CoderdValues `json:"coderd" yaml:"coderd"` Certs *CertsValues `json:"certs" yaml:"certs"` + Coderd *CoderdValues `json:"coderd" yaml:"coderd"` Envbox *EnvboxValues `json:"envbox" yaml:"envbox"` + Ingress *IngressValues `json:"ingress" yaml:"ingress"` Logging *LoggingValues `json:"logging" yaml:"logging"` Metrics *MetricsValues `json:"metrics" yaml:"metrics"` Postgres *PostgresValues `json:"postgres" yaml:"postgres"` @@ -166,6 +167,19 @@ type EnvboxValues struct { Image *string `json:"image" yaml:"image"` } +// IngressValues reflect values from ingress. +type IngressValues struct { + Enable *bool `json:"enable" yaml:"enable"` + Host *string `json:"host" yaml:"host"` + Annotations map[string]string `json:"annotations" yaml:"annotations"` + TLS *IngressTLSValues `json:"tls" yaml:"tls"` +} + +// IngressTLSValues reflect values from ingress.tls. +type IngressTLSValues struct { + Enable *bool `json:"enable" yaml:"enable"` +} + // LoggingValues reflect values from logging. type LoggingValues struct { Human *string `json:"human" yaml:"human"` diff --git a/values.yaml b/values.yaml index c1a83393..cc2fe51a 100644 --- a/values.yaml +++ b/values.yaml @@ -172,6 +172,23 @@ coderd: # weight: 1 # ``` +# ingress -- Configure an Ingress to route traffic to Coder services. +ingress: + # ingress.enable -- A boolean controlling whether to create an Ingress. + enable: false + # ingress.host -- The hostname to proxy to the Coder installation. + # The cluster Ingress Controller typically uses server name indication + # or the HTTP Host header to route traffic. + host: "" + # ingress.annotations -- Additional annotations to add to the Ingress + # object. The behavior is typically dependent on the Ingress Controller + # implementation, and useful for managing features like TLS termination. + annotations: {} + # ingress.tls -- Configures TLS settings for the Ingress. + tls: + # ingress.tls.enable -- Determines whether the Ingress handles TLS. + enable: false + # envbox -- Required for running Docker inside containers. See requirements: # https://coder.com/docs/coder/v1.19/admin/workspace-management/cvms envbox: From e4920342d434ae2830d2a674e631b6df8188d44b Mon Sep 17 00:00:00 2001 From: Jonathan Yu Date: Tue, 7 Dec 2021 17:57:59 +0000 Subject: [PATCH 2/3] simplify rules --- tests/ingress_test.go | 115 +++++++++++++----------------------------- 1 file changed, 36 insertions(+), 79 deletions(-) diff --git a/tests/ingress_test.go b/tests/ingress_test.go index c1b1b9b1..cde897ac 100644 --- a/tests/ingress_test.go +++ b/tests/ingress_test.go @@ -13,9 +13,35 @@ import ( func TestIngress(t *testing.T) { chart := LoadChart(t) + pathTypePrefix := netv1.PathTypePrefix + coderdIngressRule := netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: &pathTypePrefix, + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "coderd", + Port: netv1.ServiceBackendPort{ + Name: "tcp-coderd", + }, + }, + }, + }, + }, + }, + } + tests := []struct { - Name string + Name string + // ValuesFunc is called to configure the values used in this test. + // The function should override the CoderValues as appropriate for + // the test in question. ValuesFunc func(v *CoderValues) + // AssertFunc is called after rendering the chart, with the resulting + // Ingress object. You can use it to assert properties about the + // Ingress object. AssertFunc func(t *testing.T, ingress *netv1.Ingress) }{ { @@ -30,49 +56,15 @@ func TestIngress(t *testing.T) { "nginx.ingress.kubernetes.io/proxy-body-size": "0", } require.Equal(t, defaultAnnotations, ingress.Annotations) - pathTypePrefix := netv1.PathTypePrefix + expectedRules := []netv1.IngressRule{ { - Host: "install.coder.com", - IngressRuleValue: netv1.IngressRuleValue{ - HTTP: &netv1.HTTPIngressRuleValue{ - Paths: []netv1.HTTPIngressPath{ - { - Path: "/", - PathType: &pathTypePrefix, - Backend: netv1.IngressBackend{ - Service: &netv1.IngressServiceBackend{ - Name: "coderd", - Port: netv1.ServiceBackendPort{ - Name: "tcp-coderd", - }, - }, - }, - }, - }, - }, - }, + Host: "install.coder.com", + IngressRuleValue: coderdIngressRule, }, { - Host: "*.install.coder.app", - IngressRuleValue: netv1.IngressRuleValue{ - HTTP: &netv1.HTTPIngressRuleValue{ - Paths: []netv1.HTTPIngressPath{ - { - Path: "/", - PathType: &pathTypePrefix, - Backend: netv1.IngressBackend{ - Service: &netv1.IngressServiceBackend{ - Name: "coderd", - Port: netv1.ServiceBackendPort{ - Name: "tcp-coderd", - }, - }, - }, - }, - }, - }, - }, + Host: "*.install.coder.app", + IngressRuleValue: coderdIngressRule, }, } require.Equal(t, expectedRules, ingress.Spec.Rules, "expected ingress spec to match") @@ -90,49 +82,14 @@ func TestIngress(t *testing.T) { "nginx.ingress.kubernetes.io/proxy-body-size": "0", } require.Equal(t, defaultAnnotations, ingress.Annotations) - pathTypePrefix := netv1.PathTypePrefix expectedRules := []netv1.IngressRule{ { - Host: "install.coder.com", - IngressRuleValue: netv1.IngressRuleValue{ - HTTP: &netv1.HTTPIngressRuleValue{ - Paths: []netv1.HTTPIngressPath{ - { - Path: "/", - PathType: &pathTypePrefix, - Backend: netv1.IngressBackend{ - Service: &netv1.IngressServiceBackend{ - Name: "coderd", - Port: netv1.ServiceBackendPort{ - Name: "tcp-coderd", - }, - }, - }, - }, - }, - }, - }, + Host: "install.coder.com", + IngressRuleValue: coderdIngressRule, }, { - Host: "*.install.coder.app", - IngressRuleValue: netv1.IngressRuleValue{ - HTTP: &netv1.HTTPIngressRuleValue{ - Paths: []netv1.HTTPIngressPath{ - { - Path: "/", - PathType: &pathTypePrefix, - Backend: netv1.IngressBackend{ - Service: &netv1.IngressServiceBackend{ - Name: "coderd", - Port: netv1.ServiceBackendPort{ - Name: "tcp-coderd", - }, - }, - }, - }, - }, - }, - }, + Host: "*.install.coder.app", + IngressRuleValue: coderdIngressRule, }, } require.Equal(t, expectedRules, ingress.Spec.Rules, "expected ingress spec to match") From 000395f71e0632c9fe82aa40952e480f69bc1732 Mon Sep 17 00:00:00 2001 From: Jonathan Yu Date: Tue, 7 Dec 2021 17:59:04 +0000 Subject: [PATCH 3/3] update README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 80f0145b..74c2120d 100644 --- a/README.md +++ b/README.md @@ -63,10 +63,10 @@ View [our docs](https://coder.com/docs/setup/installation) for detailed installa | coderd.trustProxyIP | bool | Whether Coder should trust X-Real-IP and/or X-Forwarded-For headers from your reverse proxy. This should only be turned on if you're using a reverse proxy that sets both of these headers. This is always enabled if the Nginx ingress is deployed. | `false` | | envbox | object | Required for running Docker inside containers. See requirements: https://coder.com/docs/coder/v1.19/admin/workspace-management/cvms | `{"image":""}` | | envbox.image | string | Injected by Coder during release. | `""` | -| ingress | object | Configure an Ingress to route traffic to Coder services. | `{"annotations":{},"enable":false,"host":"coder.coderhost.com","tls":{"enable":false}}` | +| ingress | object | Configure an Ingress to route traffic to Coder services. | `{"annotations":{},"enable":false,"host":"","tls":{"enable":false}}` | | ingress.annotations | object | Additional annotations to add to the Ingress object. The behavior is typically dependent on the Ingress Controller implementation, and useful for managing features like TLS termination. | `{}` | | ingress.enable | bool | A boolean controlling whether to create an Ingress. | `false` | -| ingress.host | string | The hostname to proxy to the Coder installation. The cluster Ingress Controller typically uses server name indication or the HTTP Host header to route traffic. | `"coder.coderhost.com"` | +| ingress.host | string | The hostname to proxy to the Coder installation. The cluster Ingress Controller typically uses server name indication or the HTTP Host header to route traffic. | `""` | | ingress.tls | object | Configures TLS settings for the Ingress. | `{"enable":false}` | | ingress.tls.enable | bool | Determines whether the Ingress handles TLS. | `false` | | logging | object | Configures the logging format and output of Coder. | `{"human":"/dev/stderr","json":"","splunk":{"channel":"","token":"","url":""},"stackdriver":""}` |