-
Notifications
You must be signed in to change notification settings - Fork 0
Deduplication branch #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cd944c9
a7c249f
3deaee5
6d65d0f
85cb775
b45e0af
35f8f0f
cbe0274
3288653
fdad4f5
fcd2978
0ee105f
cef7cfa
685c518
10f7e81
d911d0f
8222d81
86cbcfd
3dc2858
64995c6
ba8f522
22a9569
7ce53c3
66fb776
25bab05
a109fd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -196,17 +196,60 @@ | |||||||||||||
{{/* | ||||||||||||||
SINGLE SOURCE OF TRUTH: Language-specific endpoint definitions | ||||||||||||||
Returns YAML array that can be converted to Go data structures with fromYamlArray | ||||||||||||||
Expects context with: .Values, .fullName, .application | ||||||||||||||
*/}} | ||||||||||||||
{{- define "helm.lang.endpoint.list" -}} | ||||||||||||||
{{- if eq .Values.global.language "csharp" -}} | ||||||||||||||
{{- $disableDefaults := dig "networking" "istio" "disableDefaultEndpoints" false .application -}} | ||||||||||||||
{{- if and (eq .Values.global.language "csharp") (not $disableDefaults) -}} | ||||||||||||||
- kind: "exact" | ||||||||||||||
match: "/liveness" | ||||||||||||||
- kind: "exact" | ||||||||||||||
match: "/health" | ||||||||||||||
{{- if contains "api" (.fullName | lower) }} | ||||||||||||||
- kind: "prefix" | ||||||||||||||
match: "/api" | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
|
||||||||||||||
{{/* | ||||||||||||||
Helper function to normalize paths for deduplication | ||||||||||||||
Removes all trailing wildcards and slashes | ||||||||||||||
*/}} | ||||||||||||||
{{- define "helm.normalizePath" -}} | ||||||||||||||
{{- . | trimSuffix "*" | trimSuffix "/" | trimSuffix "*" | trimSuffix "/" -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
|
||||||||||||||
{{/* | ||||||||||||||
Helper function to process prefix paths for Istio VirtualService | ||||||||||||||
Converts wildcard patterns to Istio-compatible prefix paths by replacing * with / | ||||||||||||||
Generic algorithm handles all patterns consistently without hardcoded special cases | ||||||||||||||
*/}} | ||||||||||||||
{{- define "helm.processPrefixPath" -}} | ||||||||||||||
{{- $path := . -}} | ||||||||||||||
{{- /* Replace all * with / for Istio prefix matching */ -}} | ||||||||||||||
{{- $processed := $path | replace "*" "/" -}} | ||||||||||||||
{{- /* Remove duplicate slashes that may result from replacement */ -}} | ||||||||||||||
{{- $processed | replace "//" "/" -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
|
||||||||||||||
{{/* | ||||||||||||||
Helper function to process endpoint names - replaces special characters for Kubernetes naming | ||||||||||||||
Replaces * with "wildcard", / with -, and removes leading/trailing dashes | ||||||||||||||
*/}} | ||||||||||||||
{{- define "helm.processEndpointName" -}} | ||||||||||||||
{{- $name := . -}} | ||||||||||||||
{{- $name = $name | replace "*" "wildcard" | replace "/" "-" | trimAll "-" -}} | ||||||||||||||
{{- $name -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
|
||||||||||||||
{{/* | ||||||||||||||
Helper function to process regex endpoint names for Kubernetes naming conventions | ||||||||||||||
Replaces regex special characters with dashes and removes leading/trailing dashes | ||||||||||||||
*/}} | ||||||||||||||
{{- define "helm.processRegexEndpointName" -}} | ||||||||||||||
{{- . | regexReplaceAll "[/\\.\\?\\+\\*]" "-" | regexReplaceAll "[\\^$]" "" | trimAll "-" -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
|
||||||||||||||
{{/* | ||||||||||||||
Helper template to generate VirtualService HTTP rules for language-specific and user-defined endpoints | ||||||||||||||
|
@@ -217,98 +260,148 @@ This template generates the complete HTTP rules as strings to avoid duplication | |||||||||||||
{{- $mergedEndpoints := list -}} | ||||||||||||||
|
||||||||||||||
{{/* Add language-specific endpoints first using centralized template */}} | ||||||||||||||
{{- $langEndpointsYaml := include "helm.lang.endpoint.list" . -}} | ||||||||||||||
{{- if $langEndpointsYaml -}} | ||||||||||||||
{{- $langEndpoints := fromYamlArray $langEndpointsYaml -}} | ||||||||||||||
{{- $mergedEndpoints = concat $mergedEndpoints $langEndpoints -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- $contextWithFullName := dict "Values" $.Values "application" $.application "fullName" $fullName }} | ||||||||||||||
{{- $langEndpointsYaml := include "helm.lang.endpoint.list" $contextWithFullName | trim -}} | ||||||||||||||
{{- if ne $langEndpointsYaml "" }} | ||||||||||||||
{{- $langEndpoints := fromYamlArray $langEndpointsYaml -}} | ||||||||||||||
{{- $mergedEndpoints = concat $mergedEndpoints $langEndpoints -}} | ||||||||||||||
{{- end }} | ||||||||||||||
|
||||||||||||||
{{/* Add user-defined endpoints */}} | ||||||||||||||
{{- if and .application.networking .application.networking.istio.allowedEndpoints -}} | ||||||||||||||
{{- $mergedEndpoints = concat $mergedEndpoints .application.networking.istio.allowedEndpoints -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- $mergedEndpoints = concat $mergedEndpoints .application.networking.istio.allowedEndpoints -}} | ||||||||||||||
{{- end }} | ||||||||||||||
|
||||||||||||||
{{/* Exit early if no endpoints to process */}} | ||||||||||||||
{{- if not $mergedEndpoints -}} | ||||||||||||||
{{- else -}} | ||||||||||||||
|
||||||||||||||
{{/* Generate HTTP rules for each endpoint */}} | ||||||||||||||
{{/* Deduplicate endpoints by kind+match */}} | ||||||||||||||
{{- $seen := dict -}} | ||||||||||||||
{{- $unique := list -}} | ||||||||||||||
{{- range $mergedEndpoints -}} | ||||||||||||||
{{- if typeIs "string" . }} | ||||||||||||||
{{/* Legacy format support - treat as exact match */}} | ||||||||||||||
- name: {{ $fullName }}-allowed-{{ . | replace "/" "-" | replace "*" "wildcard" | trimSuffix "-" }} | ||||||||||||||
{{- if typeIs "string" . -}} | ||||||||||||||
{{- if contains "*" . -}} | ||||||||||||||
{{- $normalizedPath := include "helm.normalizePath" . -}} | ||||||||||||||
{{- $key := printf "prefix:%s" $normalizedPath -}} | ||||||||||||||
{{- if not (hasKey $seen $key) -}} | ||||||||||||||
{{- $_ := set $seen $key true -}} | ||||||||||||||
{{/* Store original match for proper rendering, but track normalized key */}} | ||||||||||||||
{{- $unique = append $unique (dict "kind" "prefix" "match" . "normalized" $normalizedPath) -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- else -}} | ||||||||||||||
{{- $key := printf "exact:%s" . -}} | ||||||||||||||
{{- if not (hasKey $seen $key) -}} | ||||||||||||||
{{- $_ := set $seen $key true -}} | ||||||||||||||
{{- $unique = append $unique (dict "kind" "exact" "match" .) -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- else -}} | ||||||||||||||
{{/* For dict endpoints, normalize match field for comparison */}} | ||||||||||||||
{{- $normalizedMatch := .match -}} | ||||||||||||||
{{- if eq .kind "prefix" -}} | ||||||||||||||
{{- $normalizedMatch = include "helm.normalizePath" .match -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- $key := printf "%s:%s" .kind $normalizedMatch -}} | ||||||||||||||
{{- if not (hasKey $seen $key) -}} | ||||||||||||||
{{- $_ := set $seen $key true -}} | ||||||||||||||
{{/* Store original match but track normalized */}} | ||||||||||||||
{{- if eq .kind "prefix" -}} | ||||||||||||||
{{- $unique = append $unique (dict "kind" .kind "match" .match "normalized" $normalizedMatch) -}} | ||||||||||||||
{{- else -}} | ||||||||||||||
{{- $unique = append $unique . -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end }} | ||||||||||||||
|
||||||||||||||
{{/* render HTTP rules */}} | ||||||||||||||
{{- range $unique }} | ||||||||||||||
{{- if eq .kind "prefix" }} | ||||||||||||||
{{/* Use original match for name generation to preserve wildcards */}} | ||||||||||||||
{{- $endpointName := include "helm.processEndpointName" .match -}} | ||||||||||||||
{{/* Use double dash for wildcard patterns, single dash for simple prefixes */}} | ||||||||||||||
{{- if contains "*" .match }} | ||||||||||||||
- name: {{ $fullName }}-allowed--{{ $endpointName }} | ||||||||||||||
{{- else }} | ||||||||||||||
- name: {{ $fullName }}-allowed-{{ $endpointName }} | ||||||||||||||
{{- end }} | ||||||||||||||
Comment on lines
+324
to
+328
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since there is no difference between the true and false case we should skip the if statement entirely.
Suggested change
|
||||||||||||||
match: | ||||||||||||||
- uri: | ||||||||||||||
{{- if contains "*" . }} | ||||||||||||||
prefix: {{ . | replace "*" "" }} | ||||||||||||||
{{- else }} | ||||||||||||||
exact: {{ . }} | ||||||||||||||
{{- end }} | ||||||||||||||
- uri: | ||||||||||||||
prefix: {{ include "helm.processPrefixPath" .match }} | ||||||||||||||
{{- else if eq .kind "exact" }} | ||||||||||||||
{{- $processedMatch := .match | replace "/" "-" | trimAll "-" }} | ||||||||||||||
{{- if $processedMatch }} | ||||||||||||||
- name: {{ $fullName }}-allowed--{{ $processedMatch }} | ||||||||||||||
{{- else }} | ||||||||||||||
{{/* New format with kind and match fields */}} | ||||||||||||||
- name: {{ $fullName }}-allowed-{{ .match | replace "/" "-" | replace "*" "wildcard" | replace "(" "" | replace ")" "" | replace "|" "-" | replace "." "-" | replace "?" "-" | replace "+" "-" | replace "^" "" | replace "$" "" | trimSuffix "-" }} | ||||||||||||||
- name: {{ $fullName }}-allowed- | ||||||||||||||
{{- end }} | ||||||||||||||
match: | ||||||||||||||
- uri: | ||||||||||||||
exact: {{ .match }} | ||||||||||||||
{{- else if eq .kind "regex" }} | ||||||||||||||
{{- $processedMatch := include "helm.processRegexEndpointName" .match }} | ||||||||||||||
- name: {{ $fullName }}-allowed--{{ $processedMatch }} | ||||||||||||||
match: | ||||||||||||||
- uri: | ||||||||||||||
{{- if eq .kind "exact" }} | ||||||||||||||
exact: {{ .match }} | ||||||||||||||
{{- else if eq .kind "prefix" }} | ||||||||||||||
prefix: {{ .match }} | ||||||||||||||
{{- else if eq .kind "regex" }} | ||||||||||||||
regex: {{ .match }} | ||||||||||||||
{{- else }} | ||||||||||||||
{{/* Default to exact if kind is not recognized */}} | ||||||||||||||
exact: {{ .match }} | ||||||||||||||
{{- end }} | ||||||||||||||
- uri: | ||||||||||||||
regex: {{ .match }} | ||||||||||||||
{{- end }} | ||||||||||||||
Comment on lines
+320
to
348
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The body of the match rule is identical except for the key of the operation, which means we can better follow the DRY principle by condensing all 3 of these into this: - name: {{ $fullName }}-allowed--{{ if eq .kind "prefix" }}{{ $endpointName }}{{ else }}{{ $processedMatch }}{{ end }}
match:
- uri:
{{- if eq .kind "prefix" }}
prefix: {{ include "helm.processPrefixPath" .match }}
{{- else if eq .kind "exact" }}
exact: {{ .match }}
{{- else if eq .kind "regex" }}
regex: {{ .match }}
{{ end }} |
||||||||||||||
route: | ||||||||||||||
{{- if and $.application.service $.application.service.ports }} | ||||||||||||||
{{- range $.application.service.ports }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }} | ||||||||||||||
port: | ||||||||||||||
number: {{ .port }} | ||||||||||||||
weight: 100 | ||||||||||||||
{{- if (eq $.application.deploymentType "rollout") }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }}-preview | ||||||||||||||
port: | ||||||||||||||
number: {{ .port }} | ||||||||||||||
weight: 0 | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- else }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }} | ||||||||||||||
port: | ||||||||||||||
number: {{ default 8080 (dig "ports" "http" nil $.application) }} | ||||||||||||||
weight: 100 | ||||||||||||||
{{- if (eq $.application.deploymentType "rollout") }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }}-preview | ||||||||||||||
port: | ||||||||||||||
number: {{ default 8080 (dig "ports" "http" nil $.application) }} | ||||||||||||||
weight: 0 | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- if $.application.networking }} | ||||||||||||||
{{- if $.application.networking.istio.enabled }} | ||||||||||||||
headers: | ||||||||||||||
{{- if $.application.networking }} | ||||||||||||||
{{- if and $.application.networking.istio.responseHeaders }} | ||||||||||||||
{{- with $.application.networking.istio.responseHeaders }} | ||||||||||||||
{{ toYaml . | indent 4 | trim }} | ||||||||||||||
{{- if and $.application.service $.application.service.ports }} | ||||||||||||||
{{- range $.application.service.ports }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }} | ||||||||||||||
port: | ||||||||||||||
number: {{ .port }} | ||||||||||||||
weight: 100 | ||||||||||||||
{{- if eq $.application.deploymentType "rollout" }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }}-preview | ||||||||||||||
port: | ||||||||||||||
number: {{ .port }} | ||||||||||||||
weight: 0 | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- else }} | ||||||||||||||
{{ include "helm.istioIngress.responseHeaders" $ | indent 4 | trim }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }} | ||||||||||||||
port: | ||||||||||||||
number: {{ default 8080 (dig "ports" "http" nil $.application) }} | ||||||||||||||
weight: 100 | ||||||||||||||
{{- if eq $.application.deploymentType "rollout" }} | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }}-preview | ||||||||||||||
port: | ||||||||||||||
number: {{ default 8080 (dig "ports" "http" nil $.application) }} | ||||||||||||||
weight: 0 | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- with $.application.networking.istio.corsPolicy }} | ||||||||||||||
corsPolicy: | ||||||||||||||
{{ toYaml . | indent 4 | trim }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{- end }} | ||||||||||||||
{{/* Safely access service properties with default values if not defined */}} | ||||||||||||||
timeout: {{ default "151s" (dig "service" "timeout" "151s" $.application) }} | ||||||||||||||
retries: | ||||||||||||||
retryOn: {{ default "5xx,reset" (dig "service" "retryOn" "5xx,reset" $.application) }} | ||||||||||||||
attempts: {{ default 3 (dig "service" "attempts" 3 $.application) }} | ||||||||||||||
perTryTimeout: {{ default "50s" (dig "service" "perTryTimeout" "50s" $.application) }} | ||||||||||||||
{{- end }} | ||||||||||||||
|
||||||||||||||
- name: {{ $fullName }}-forbidden | ||||||||||||||
route: | ||||||||||||||
- destination: | ||||||||||||||
host: {{ $fullName }} | ||||||||||||||
port: | ||||||||||||||
number: {{ default 8080 (dig "ports" "http" nil $.application) }} | ||||||||||||||
fault: | ||||||||||||||
delay: | ||||||||||||||
fixedDelay: 29s | ||||||||||||||
percentage: | ||||||||||||||
value: 100 | ||||||||||||||
abort: | ||||||||||||||
httpStatus: 403 | ||||||||||||||
percentage: | ||||||||||||||
value: 100 | ||||||||||||||
{{- end -}} | ||||||||||||||
{{- end -}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the double hyphens and trailing hyphens.
Example:
Please update the test cases as well since this change will break them.