Merge pull request #41 from gruntwork-io/yori-sidecar

Fix sidecar and additionalpaths rendering

Author: yorinasub17

charts/k8s-service/templates/deployment.yaml

@@ -148,7 +148,7 @@ spec:
           {{- end }}
           {{- if .Values.securityContext }}
           securityContext:
-{{ toYaml .Values.securityContext | indent 12 }}            
+{{ toYaml .Values.securityContext | indent 12 }}
           {{- end}}
           resources:
 {{ toYaml .Values.containerResources | indent 12 }}
@@ -227,7 +227,7 @@ spec:
 
         {{- range $key, $value := .Values.sideCarContainers }}
         - name: {{ $key }}
-{{ toYaml $value | indent 12 }}
+{{ toYaml $value | indent 10 }}
         {{- end }}
 
     {{- /* START IMAGE PULL SECRETS LOGIC */ -}}

charts/k8s-service/templates/ingress.yaml

@@ -46,7 +46,7 @@ spec:
           {{- range $additionalPathsHigherPriority }}
           - path: {{ .path }}
             backend:
-              serviceName: {{ .serviceName }}
+              serviceName: {{ if .serviceName }}{{ .serviceName }}{{ else }}{{ $fullName }}{{ end }}
               servicePort: {{ .servicePort }}
           {{- end }}
           - path: {{ $ingressPath }}
@@ -56,7 +56,7 @@ spec:
           {{- range $additionalPaths }}
           - path: {{ .path }}
             backend:
-              serviceName: {{ .serviceName }}
+              serviceName: {{ if .serviceName }}{{ .serviceName }}{{ else }}{{ $fullName }}{{ end }}
               servicePort: {{ .servicePort }}
           {{- end }}
     {{- end }}

test/k8s_service_template_test.go

@@ -454,6 +454,69 @@ func TestK8SServiceIngressAdditionalPathsHigherPriorityNoServiceName(t *testing.
 	assert.Equal(t, secondPath.Backend.ServicePort.StrVal, "app")
 }
 
+// Test that omitting a serviceName on additionalPaths reuses the application service name even when hosts is set
+func TestK8SServiceIngressWithHostsAdditionalPathsNoServiceName(t *testing.T) {
+	t.Parallel()
+
+	ingress := renderK8SServiceIngressWithSetValues(
+		t,
+		map[string]string{
+			"ingress.enabled":                        "true",
+			"ingress.path":                           "/app",
+			"ingress.servicePort":                    "app",
+			"ingress.hosts[0]":                       "chart-example.local",
+			"ingress.additionalPaths[0].path":        "/black-hole",
+			"ingress.additionalPaths[0].servicePort": "3000",
+		},
+	)
+	pathRules := ingress.Spec.Rules[0].HTTP.Paths
+	assert.Equal(t, len(pathRules), 2)
+
+	// The first path should be the main service path
+	firstPath := pathRules[0]
+	assert.Equal(t, firstPath.Path, "/app")
+	assert.Equal(t, strings.ToLower(firstPath.Backend.ServiceName), "release-name-linter")
+	assert.Equal(t, firstPath.Backend.ServicePort.StrVal, "app")
+
+	// The second path should be the black hole
+	secondPath := pathRules[1]
+	assert.Equal(t, secondPath.Path, "/black-hole")
+	assert.Equal(t, strings.ToLower(secondPath.Backend.ServiceName), "release-name-linter")
+	assert.Equal(t, secondPath.Backend.ServicePort.IntVal, int32(3000))
+}
+
+// Test that omitting a serviceName on additionalPathsHigherPriority reuses the application service name even when hosts
+// is set
+func TestK8SServiceIngressWithHostsAdditionalPathsHigherPriorityNoServiceName(t *testing.T) {
+	t.Parallel()
+
+	ingress := renderK8SServiceIngressWithSetValues(
+		t,
+		map[string]string{
+			"ingress.enabled":     "true",
+			"ingress.path":        "/app",
+			"ingress.servicePort": "app",
+			"ingress.hosts[0]":    "chart-example.local",
+			"ingress.additionalPathsHigherPriority[0].path":        "/black-hole",
+			"ingress.additionalPathsHigherPriority[0].servicePort": "3000",
+		},
+	)
+	pathRules := ingress.Spec.Rules[0].HTTP.Paths
+	assert.Equal(t, len(pathRules), 2)
+
+	// The first path should be the black hole
+	firstPath := pathRules[0]
+	assert.Equal(t, firstPath.Path, "/black-hole")
+	assert.Equal(t, strings.ToLower(firstPath.Backend.ServiceName), "release-name-linter")
+	assert.Equal(t, firstPath.Backend.ServicePort.IntVal, int32(3000))
+
+	// The second path should be the main service path
+	secondPath := pathRules[1]
+	assert.Equal(t, secondPath.Path, "/app")
+	assert.Equal(t, strings.ToLower(secondPath.Backend.ServiceName), "release-name-linter")
+	assert.Equal(t, secondPath.Backend.ServicePort.StrVal, "app")
+}
+
 // Test rendering Managed Certificate
 func TestK8SServiceManagedCertDomainNameAndName(t *testing.T) {
 	t.Parallel()
@@ -612,3 +675,17 @@ func TestK8SServiceIngressMultiCert(t *testing.T) {
 	assert.Equal(t, secondTlsHosts[0], "chart1-example-tls-host")
 	assert.Equal(t, secondTlsHosts[1], "chart1-example-tls-host2")
 }
+
+func TestK8SServiceSideCarContainersRendersCorrectly(t *testing.T) {
+	t.Parallel()
+	deployment := renderK8SServiceDeploymentWithSetValues(
+		t,
+		map[string]string{
+			"sideCarContainers.datadog.image": "datadog/agent:latest",
+		},
+	)
+	renderedContainers := deployment.Spec.Template.Spec.Containers
+	require.Equal(t, len(renderedContainers), 2)
+	sideCarContainer := renderedContainers[1]
+	assert.Equal(t, sideCarContainer.Image, "datadog/agent:latest")
+}