Deduplication branch

[hpyvovar-mycarrier]

Fix: Endpoint Deduplication in VirtualService
Problem
Language-specific endpoints (/api, /health) were duplicating user-defined endpoints, causing VirtualService validation errors.

Solution
Deduplication logic: Merge and deduplicate endpoints by normalized path (kind:path)
Refactored wildcard handling: Centralized in helm.processPrefixPath helper
Schema validation: Added allowedEndpoints to values.schema.json (validates kind enum, requires kind/match, allows metadata)
Conditional rendering: Forbidden rule only when endpoints exist.

Testing
Added 10 tests for deduplication logic.

[github-actions]

Helm Unit Test Results - charts/mycarrier-helm

162 tests   162 ✅  1m 40s ⏱️
 35 suites    0 💤
  1 files      0 ❌

Results for commit a109fd1.

♻️ This comment has been updated with latest results.

[Copilot]

Pull Request Overview

This PR implements deduplication logic for VirtualService endpoints to prevent validation errors caused by duplicate endpoint definitions. The changes ensure that language-specific defaults (e.g., /api, /health, /liveness for C#) and user-defined endpoints are merged and deduplicated based on their normalized path and match type (exact/prefix/regex).

Key Changes:

• Introduced deduplication logic that creates unique endpoint keys using kind:normalized_path format
• Centralized wildcard path processing in reusable helper functions
• Added schema validation for the allowedEndpoints array with support for both string and object formats

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
values.schema.json	Added JSON schema validation for allowedEndpoints array, supporting both string paths and objects with kind/match fields
allowed_endpoints_deduplication_test.yaml	Comprehensive test suite (10 tests) validating deduplication across exact/prefix/regex matches, wildcards, and case-sensitivity
_spec_virtualservice.tpl	Refactored endpoint detection logic and cleaned up template formatting (removed redundant whitespace/conditions)
_lang.tpl	Implemented core deduplication algorithm with path normalization, added helper functions for wildcard/prefix/name processing
Chart.yaml	Added Bitnami common chart dependency

[smunukutla-mycarrier]

The goal of the ticket was as follows:

• add default endpoints /health, /liveness, /api for csharp apis
• allow adding endpoints along with default
• provide a way to disable default endpoints
• deduplicate endpoints to make sure render is clean ✅

The first three have not been achieved yet. I was able to confirm deduplication works.

[bcarlock-mycarrier]

Please address the comments and ensure:

• /health is automatically added as an allowed endpoint when eq $.Values.global.language "csharp"
• /liveness is automatically added as an allowed endpoint when eq $.Values.global.language "csharp"
• /api is automatically added as an allowed endpoint when and (eq $.Values.global.language "csharp") ($fullName | contains "api"
• Make sure these default endpoints work with manually configured endpoints ie, if I add "/fakeendpoint" to "my-api" I should end up with /health, /liveness, /api, and /fakeendpoint
• There needs to be a way to disable the default endpoints, but that disable should not prevent the use of manually configured allowed endpoints.
• Tests that prove each item above

[Copilot]

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

[bcarlock-mycarrier]

Please see my comments

[bcarlock-mycarrier]

Since there is no difference between the true and false case we should skip the if statement entirely.

Suggested change

	{{- if contains "*" .match }}
	- name: {{ $fullName }}-allowed--{{ $endpointName }}
	{{- else }}
	- name: {{ $fullName }}-allowed-{{ $endpointName }}
	{{- end }}
	- name: {{ $fullName }}-allowed-{{ $endpointName }}

[bcarlock-mycarrier]

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 }}

[bcarlock-mycarrier]

@hpyvovar-mycarrier

DRY Violations in Proposed Helm Template Changes

Summary of DRY Violations

1. Repeated Endpoint Name Generation

   • Name formatting logic (replacing /, *, special characters) is duplicated for each endpoint type (exact, prefix, regex).

2. Multiple Conditional Blocks for Endpoint Types

   • Similar blocks for exact, prefix, and regex endpoint rendering, each handling match and name logic separately.

3. Redundant Helper Usage

   • Helper templates for path and name processing are called in multiple places with minor variations rather than centralized.

4. Repeated Route Block

   • The majority of the route YAML structure is repeated for different endpoint types, with only small changes in the rendered name/match.

5. Deduplication Logic

   • Deduplication keys and value assignments are handled separately for string and dict endpoints, resulting in similar code blocks.

---

Refactoring Suggestions

To address these DRY violations, consider the following refactor:

1. Centralize Endpoint Name Generation

{{/* Centralized endpoint name generation for all kinds */}}
{{- define "helm.renderEndpointName" -}}
{{- if eq .kind "regex" -}}
  {{- include "helm.processRegexEndpointName" .match -}}
{{- else -}}
  {{- include "helm.processEndpointName" .match -}}
{{- end -}}
{{- end -}}

2. Centralize Endpoint Match Rendering

{{/* Centralized endpoint match rendering for all kinds */}}
{{- define "helm.renderEndpointMatch" -}}
uri:
  {{- if eq .kind "prefix" -}}
    prefix: {{ include "helm.processPrefixPath" .match }}
  {{- else if eq .kind "regex" -}}
    regex: {{ .match }}
  {{- else -}}
    exact: {{ .match }}
  {{- end -}}
{{- end -}}

3. Centralize Endpoint Rule Block

{{/* Centralized HTTP rule rendering for a single endpoint */}}
{{- define "helm.renderEndpointRule" -}}
- name: {{ $.fullName }}-allowed-{{ include "helm.renderEndpointName" . }}
  match:
    - {{ include "helm.renderEndpointMatch" . | indent 6 | trim }}
  route:
    - 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 }}
  {{- with $.application.networking.istio.corsPolicy }}
  corsPolicy:
    {{ toYaml . | indent 4 | trim }}
  {{- end }}
  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 -}}

4. DRY Rendering Loop for All Endpoint Rules

After deduplication, simply loop and call the above centralized template:

{{- range $unique }}
  {{- include "helm.renderEndpointRule" (dict "kind" .kind "match" .match "fullName" $fullName "application" $.application) }}
{{- end }}

5. Forbidden Rule Stays Separate

The forbidden rule is unique and can remain a separate block.

---

Net effect:

• All endpoint name and match processing is centralized.
• The HTTP rule block is generated via a single template.
• The main loop is lean: deduplicate endpoints, then render using one reusable template.
• Maintenance and readability improve; DRY principle is restored.

[smunukutla-mycarrier]

Please fix the double hyphens and trailing hyphens.

Example:

• {{ $fullName }}-allowed--{{ $endpointName }} should be {{ $fullName }}-allowed-{{ $endpointName }}
• {{ $fullName }}-allowed- should be {{ $fullName }}-allowed

Please update the test cases as well since this change will break them.