Skip to content

feat: Support Istio Virtual Service - #626

Open
LeoDOD wants to merge 14 commits into
stakater:mainfrom
LeoDOD:feature/support-virtualservice
Open

LeoDOD wants to merge 14 commits into
stakater:mainfrom
LeoDOD:feature/support-virtualservice

Conversation

@LeoDOD

@LeoDOD LeoDOD commented Aug 28, 2026 •

Copy link
Copy Markdown

Changes

  • added virtualService section to values.yaml with full documentation (hosts, gateways, exportTo, http, tls, tcp, routes)
  • added virtualService schema block to values.schema.json
  • created templates/virtualservice.yaml : supports single-VS and multi-VS (routes map) modes; http/tls/tcp are toYaml pass-through.
  • API version auto-detects networking.istio.io/v1 vs v1beta1
  • Added test to validate the new template.

Remarks

  • virtualService.enabled defaults to false to be fully backwards compatible
  • when routes is omitted, a single VirtualService is rendered using the top-level virtualService fields; when routes is a map, one VirtualService is rendered per entry (name suffix skipped when key is "default")

@LeoDOD LeoDOD changed the title Feature/support Istio VirtualService feat: Support Istio Virtual Service Aug 28, 2026
LeoDOD and others added 6 commits September 22, 2026 17:56
Added configuration for Istio VirtualService.
Added tests for Istio VirtualService configuration including rendering conditions, API version handling, and route specifications.
- Introduced a comprehensive schema for the VirtualService configuration, including properties such as enabled, exportTo, gateways, hosts, http, routes, tcp, and tls.
- Enhanced descriptions for each property to clarify usage and requirements.
- Removed the previous, less detailed VirtualService schema to streamline the configuration process.
@LeoDOD
LeoDOD force-pushed the feature/support-virtualservice branch from a82198c to 2e1ef21 Compare September 22, 2026 21:56
@aslafy-z

aslafy-z commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the PR, the template and tests are in good shape. Three things block merge, the rest are suggestions.

Blocking

1. Wildcard hosts render invalid YAML. templates/virtualservice.yaml emits hosts, gateways and exportTo entries as - {{ . }} unquoted, so *.example.com (the exact usage documented in values.yaml) and exportTo: ["*"] will fail. Please switch to {{ . | quote }} or render the lists with toYaml, and add a test case with a wildcard host and exportTo: ["*"].

2. Feature is not enabled in CI values. Per CONTRIBUTING, new features must be enabled in ci/values.yaml or ci/values-crds.yaml so the snapshot and render jobs cover them. Please add an enabled virtualService example (including the routes map mode) to ci/values-crds.yaml, add networking.istio.io/v1 and networking.istio.io/v1/VirtualService to the crds profile api-versions list in .github/workflows/ci.yaml and to the capabilities list in tests/common_test.yaml, then run mise run test -u and commit the updated snapshot.

3. Labels and annotations. Every other networking template (ingress.yaml, httproute.yaml) supports additionalLabels and annotations. Please add virtualService.additionalLabels and virtualService.annotations (and the per-route equivalents) using the same pattern, then regenerate README and schema.

Suggestions

  • apiVersion fallback. Other CRD templates either gate on Capabilities.APIVersions.Has or fail when the CRD is absent (see httproute.yaml). This template silently emits v1beta1 whenever the CRD is not present, which is what helm template and GitOps renders always get. Either adopt the fail pattern or document in the values comment that offline rendering always yields v1beta1.
  • Require hosts. enabled: true with defaults renders an empty spec:. A required "virtualService.hosts is required" per route would give a clearer error.
  • Name length in routes mode. The -<key> suffix is appended after application.name is already truncated to 63 characters, so long names overflow. Wrap the result in trunc 63 | trimSuffix "-".
  • Schema typing. exportTo is (list, null) while hosts and gateways are (list) with the same [] default. Align them.

Happy to re-review once the two blocking items are in.

@LeoDOD

LeoDOD commented Sep 25, 2026 •

Copy link
Copy Markdown
Author

I appreciate the feedback!
I addressed the blockers and also the suggestions.
Let me know if there is anything that needs fixing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants