Skip to content

Commit 9d1afa8

Browse files
shashankramshashankram
authored
codegen: correctly render port names with hyphen (#489)
Currently, port names with hyphens do not render correctly in the Helm template due how Helm processes hypenated values. We would see an error as follows: Error: parse error at (gloo-platform/templates/deployment.yaml:392): bad character U+002D '-' The workaround is to use the Helm index function to correctly render service port names with hyphens. Signed-off-by: Shashank Ram <[email protected]>
1 parent 5b87daa commit 9d1afa8

File tree

9 files changed

+319
-5
lines changed

9 files changed

+319
-5
lines changed

‎changelog/v0.34.0/svc-port-name.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
changelog:
2+
- type: NEW_FEATURE
3+
issueLink: https://github.com/solo-io/skv2/issues/488
4+
description: >
5+
Allow service port names to contain hyphens.

‎codegen/cmd_test.go‎

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ var _ = Describe("Cmd", func() {
4242
"encoding/protobuf/cue/cue.proto",
4343
}
4444
It("install conditional sidecars", func() {
45-
var (
46-
agentConditional = "and ($.Values.glooAgent.enabled) ($.Values.glooAgent.runAsSidecar)"
47-
)
45+
agentConditional := "and ($.Values.glooAgent.enabled) ($.Values.glooAgent.runAsSidecar)"
4846

4947
cmd := &Command{
5048
Chart: &Chart{
@@ -1804,6 +1802,81 @@ roleRef:
18041802
[]v1.EnvVar{{Name: "FOO", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "bar"}, Key: "baz"}}}}),
18051803
)
18061804

1805+
DescribeTable("rendering service ports",
1806+
func(portName string) {
1807+
cmd := &Command{
1808+
Chart: &Chart{
1809+
Operators: []Operator{
1810+
{
1811+
Name: "painter",
1812+
Deployment: Deployment{
1813+
Container: Container{
1814+
Image: Image{
1815+
Tag: "v0.0.0",
1816+
Repository: "painter",
1817+
Registry: "quay.io/solo-io",
1818+
PullPolicy: "IfNotPresent",
1819+
},
1820+
},
1821+
},
1822+
Service: Service{
1823+
Ports: []ServicePort{{
1824+
Name: portName,
1825+
DefaultPort: 9900,
1826+
}},
1827+
},
1828+
},
1829+
},
1830+
1831+
Values: nil,
1832+
Data: Data{
1833+
ApiVersion: "v1",
1834+
Description: "",
1835+
Name: "Painting Operator",
1836+
Version: "v0.0.1",
1837+
Home: "https://docs.solo.io/skv2/latest",
1838+
Sources: []string{
1839+
"https://github.com/solo-io/skv2",
1840+
},
1841+
},
1842+
},
1843+
1844+
ManifestRoot: "codegen/test/chart-svcport",
1845+
}
1846+
1847+
err := cmd.Execute()
1848+
Expect(err).NotTo(HaveOccurred())
1849+
1850+
helmValues := map[string]interface{}{}
1851+
1852+
renderedManifests := helmTemplate("codegen/test/chart-svcport", helmValues)
1853+
1854+
var renderedSvc *v1.Service
1855+
decoder := kubeyaml.NewYAMLOrJSONDecoder(bytes.NewBuffer(renderedManifests), 4096)
1856+
for {
1857+
obj := &unstructured.Unstructured{}
1858+
err := decoder.Decode(obj)
1859+
if err != nil {
1860+
break
1861+
}
1862+
if obj.GetName() != "painter" || obj.GetKind() != "Service" {
1863+
continue
1864+
}
1865+
1866+
bytes, err := obj.MarshalJSON()
1867+
Expect(err).NotTo(HaveOccurred())
1868+
renderedSvc = &v1.Service{}
1869+
err = json.Unmarshal(bytes, renderedSvc)
1870+
Expect(err).NotTo(HaveOccurred())
1871+
break
1872+
}
1873+
Expect(renderedSvc).NotTo(BeNil())
1874+
Expect(renderedSvc.Spec.Ports[0].Name).To(Equal(portName))
1875+
},
1876+
Entry("port name without hyphen", "foo"),
1877+
Entry("port name with hyphen", "foo-bar"),
1878+
)
1879+
18071880
It("can configure cluster-scoped and namespace-scoped RBAC", func() {
18081881
cmd := &Command{
18091882
RenderProtos: false,

‎codegen/templates/chart/operator-deployment.yamltmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ spec:
248248
ports:
249249
[[- range $port := $operator.Service.Ports ]]
250250
- name: [[ lower $port.Name ]]
251-
port: {{ $[[ $operatorVar ]].ports.[[ $port.Name ]] }}
251+
port: {{ index $[[ $operatorVar ]] "ports" "[[ $port.Name ]]" }}
252252
[[- end ]]
253253
[[ end ]]
254254
{{- end }} {{/* define "[[ $operator.Name ]].serviceSpec" */}}

‎codegen/test/chart-svcport/Chart.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Code generated by skv2. DO NOT EDIT.
2+
3+
apiVersion: v1
4+
home: https://docs.solo.io/skv2/latest
5+
name: Painting Operator
6+
sources:
7+
- https://github.com/solo-io/skv2
8+
version: v0.0.1

‎codegen/test/chart-svcport/templates/_helpers.tpl‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Code generated by skv2. DO NOT EDIT.
2+
3+
4+
5+
{{/* Below are library functions provided by skv2 */}}
6+
7+
{{- /*
8+
9+
"skv2.utils.merge" takes an array of three values:
10+
- the top context
11+
- the yaml block that will be merged in (override)
12+
- the name of the base template (source)
13+
14+
note: the source must be a named template (helm partial). This is necessary for the merging logic.
15+
16+
The behaviour is as follows, to align with already existing helm behaviour:
17+
- If no source is found (template is empty), the merged output will be empty
18+
- If no overrides are specified, the source is rendered as is
19+
- If overrides are specified and source is not empty, overrides will be merged in to the source.
20+
21+
Overrides can replace / add to deeply nested dictionaries, but will completely replace lists.
22+
Examples:
23+
24+
┌─────────────────────┬───────────────────────┬────────────────────────┐
25+
│ Source (template) │ Overrides │ Result │
26+
├─────────────────────┼───────────────────────┼────────────────────────┤
27+
│ metadata: │ metadata: │ metadata: │
28+
│ labels: │ labels: │ labels: │
29+
│ app: gloo │ app: gloo1 │ app: gloo1 │
30+
│ cluster: useast │ author: infra-team │ author: infra-team │
31+
│ │ │ cluster: useast │
32+
├─────────────────────┼───────────────────────┼────────────────────────┤
33+
│ lists: │ lists: │ lists: │
34+
│ groceries: │ groceries: │ groceries: │
35+
│ - apple │ - grapes │ - grapes │
36+
│ - banana │ │ │
37+
└─────────────────────┴───────────────────────┴────────────────────────┘
38+
39+
skv2.utils.merge is a fork of a helm library chart function (https://github.com/helm/charts/blob/master/incubator/common/templates/_util.tpl).
40+
This includes some optimizations to speed up chart rendering time, and merges in a value (overrides) with a named template, unlike the upstream
41+
version, which merges two named templates.
42+
43+
*/ -}}
44+
{{- define "skv2.utils.merge" -}}
45+
{{- $top := first . -}}
46+
{{- $overrides := (index . 1) -}}
47+
{{- $tpl := fromYaml (include (index . 2) $top) -}}
48+
{{- if or (empty $overrides) (empty $tpl) -}}
49+
{{ include (index . 2) $top }} {{/* render source as is */}}
50+
{{- else -}}
51+
{{- $merged := merge $overrides $tpl -}}
52+
{{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
53+
{{- end -}}
54+
{{- end -}}

‎codegen/test/chart-svcport/templates/deployment.yaml‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Code generated by skv2. DO NOT EDIT.
2+
3+
4+
5+
{{- $painter := $.Values.painter }}
6+
---
7+
8+
{{- define "painter.deploymentSpec" }}
9+
10+
# Deployment manifest for painter
11+
12+
apiVersion: apps/v1
13+
kind: Deployment
14+
metadata:
15+
labels:
16+
app: painter
17+
annotations:
18+
app.kubernetes.io/name: painter
19+
name: painter
20+
namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
21+
spec:
22+
selector:
23+
matchLabels:
24+
app: painter
25+
template:
26+
metadata:
27+
labels:
28+
app: painter
29+
annotations:
30+
app.kubernetes.io/name: painter
31+
prometheus.io/path: /metrics
32+
prometheus.io/port: "9091"
33+
prometheus.io/scrape: "true"
34+
spec:
35+
serviceAccountName: painter
36+
containers:
37+
{{- $painter := $.Values.painter }}
38+
{{- $painterImage := $painter.image }}
39+
- name: painter
40+
image: {{ $painterImage.registry }}/{{ $painterImage.repository }}:{{ $painterImage.tag }}
41+
imagePullPolicy: {{ $painterImage.pullPolicy }}
42+
{{- if $painter.env }}
43+
env:
44+
{{ toYaml $painter.env | indent 10 }}
45+
{{- else if $painter.extraEnvs }}
46+
env:
47+
{{- end }}
48+
{{- range $name, $item := $painter.extraEnvs }}
49+
- name: {{ $name }}
50+
{{- $item | toYaml | nindent 12 }}
51+
{{- end }}
52+
resources:
53+
{{- if $painter.resources }}
54+
{{ toYaml $painter.resources | indent 10}}
55+
{{- else}}
56+
requests:
57+
cpu: 500m
58+
memory: 256Mi
59+
{{- end }}
60+
{{- /*
61+
Render securityContext configs if it is set.
62+
If securityContext is not set, render the default securityContext.
63+
If securityContext is set to 'false', render an empty map.
64+
*/}}
65+
securityContext:
66+
{{- if or ($painter.securityContext) (eq "map[]" (printf "%v" $painter.securityContext)) }}
67+
{{ toYaml $painter.securityContext | indent 10}}
68+
{{/* Because securityContext is nil by default we can only perform following conversion if it is a boolean. Skip conditional otherwise. */}}
69+
{{- else if eq (ternary $painter.securityContext true (eq "bool" (printf "%T" $painter.securityContext))) false }}
70+
{}
71+
{{- else}}
72+
runAsNonRoot: true
73+
{{- if not $painter.floatingUserId }}
74+
runAsUser: {{ printf "%.0f" (float64 $painter.runAsUser) }}
75+
{{- end }}
76+
readOnlyRootFilesystem: true
77+
allowPrivilegeEscalation: false
78+
capabilities:
79+
drop:
80+
- ALL
81+
{{- end }}
82+
{{- if $painterImage.pullSecret }}
83+
imagePullSecrets:
84+
- name: {{ $painterImage.pullSecret }}
85+
{{- end}}
86+
{{- end }} {{/* define "painter.deploymentSpec" */}}
87+
88+
{{/* Render painter deployment template with overrides from values*/}}
89+
{{ if $painter.enabled }}
90+
{{- $painterDeploymentOverrides := dict }}
91+
{{- if $painter.deploymentOverrides }}
92+
{{- $painterDeploymentOverrides = $painter.deploymentOverrides }}
93+
{{- end }}
94+
---
95+
{{ include "skv2.utils.merge" (list . $painterDeploymentOverrides "painter.deploymentSpec") }}
96+
{{- end }}
97+
---
98+
{{ if $painter.enabled }}
99+
apiVersion: v1
100+
kind: ServiceAccount
101+
metadata:
102+
labels:
103+
app: painter
104+
{{- if $painter.serviceAccount}}
105+
{{- if $painter.serviceAccount.extraAnnotations }}
106+
annotations:
107+
{{- range $key, $value := $painter.serviceAccount.extraAnnotations }}
108+
{{ $key }}: {{ $value }}
109+
{{- end }}
110+
{{- end }}
111+
{{- end}}
112+
name: painter
113+
namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
114+
{{- end }}
115+
116+
117+
{{- define "painter.serviceSpec"}}
118+
119+
# Service for painter
120+
{{/* Define variables in function scope */}}
121+
{{- $painter := $.Values.painter}}
122+
apiVersion: v1
123+
kind: Service
124+
metadata:
125+
labels:
126+
app: painter
127+
annotations:
128+
app.kubernetes.io/name: painter
129+
name: painter
130+
namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
131+
spec:
132+
selector:
133+
app: painter
134+
type: {{ $painter.serviceType }}
135+
ports:
136+
- name: foo-bar
137+
port: {{ index $painter "ports" "foo-bar" }}
138+
139+
{{- end }} {{/* define "painter.serviceSpec" */}}
140+
{{ if $painter.enabled }}
141+
{{/* Render painter service template with overrides from values*/}}
142+
{{- $painterServiceOverrides := dict }}
143+
{{- if $painter.serviceOverrides }}
144+
{{- $painterServiceOverrides = $painter.serviceOverrides }}
145+
{{- end }}
146+
147+
---
148+
149+
{{ include "skv2.utils.merge" (list . $painterServiceOverrides "painter.serviceSpec") }}
150+
{{- end }}
151+

‎codegen/test/chart-svcport/templates/rbac.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Code generated by skv2. DO NOT EDIT.
2+
3+

‎codegen/test/chart-svcport/values.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Code generated by skv2. DO NOT EDIT.
2+
3+
painter:
4+
deploymentOverrides: null
5+
enabled: true
6+
env: null
7+
extraEnvs: {}
8+
floatingUserId: false
9+
image:
10+
pullPolicy: IfNotPresent
11+
registry: quay.io/solo-io
12+
repository: painter
13+
tag: v0.0.0
14+
ports:
15+
foo-bar: 9900
16+
runAsUser: 10101
17+
serviceOverrides: null
18+
serviceType: ""
19+
sidecars: {}
20+

‎codegen/test/chart/conditional-sidecar/templates/deployment.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ spec:
197197
type: {{ $glooMgmtServer.serviceType }}
198198
ports:
199199
- name: grpc
200-
port: {{ $glooMgmtServer.ports.grpc }}
200+
port: {{ index $glooMgmtServer "ports" "grpc" }}
201201

202202
{{- end }} {{/* define "gloo-mgmt-server.serviceSpec" */}}
203203
{{ if $glooMgmtServer.enabled }}

0 commit comments

Comments
 (0)