Skip to content

feat: add targetRef field to ZTunnel CRD#1259

Merged
istio-testing merged 1 commit intoistio-ecosystem:mainfrom
dgn:ztunnel-targetref
Apr 16, 2026
Merged

feat: add targetRef field to ZTunnel CRD#1259
istio-testing merged 1 commit intoistio-ecosystem:mainfrom
dgn:ztunnel-targetref

Conversation

@dgn
Copy link
Copy Markdown
Collaborator

@dgn dgn commented Oct 7, 2025

This adds a targetRef field to the ZTunnel CRD, allowing users to keep their ZTunnel config in sync with what they already defined in the Istio or IstioRevision CRD.

@dgn dgn requested a review from a team as a code owner October 7, 2025 08:57
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 85.71429% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.19%. Comparing base (8f7ce71) to head (eaa8de5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
controllers/ztunnel/ztunnel_controller.go 86.66% 4 Missing and 2 partials ⚠️
pkg/reconcile/ztunnel.go 64.70% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1259      +/-   ##
==========================================
- Coverage   81.26%   81.19%   -0.08%     
==========================================
  Files          46       46              
  Lines        2493     2542      +49     
==========================================
+ Hits         2026     2064      +38     
- Misses        344      349       +5     
- Partials      123      129       +6     
Flag Coverage Δ
integration-tests 71.17% <80.95%> (+0.09%) ⬆️
unit-tests 53.38% <30.95%> (-0.41%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dgn dgn force-pushed the ztunnel-targetref branch 5 times, most recently from bcd9f0d to b655b0e Compare October 7, 2025 09:50
@dgn
Copy link
Copy Markdown
Collaborator Author

dgn commented Oct 7, 2025

This should support most use cases including RevisionBased update strategies already. Another thing we could add is making the IstioRevisionController aware of the link - currently, users have to configure the ZTunnel namespace on their Istio/IstioRevision:

spec:
  values:
    pilot:
      trustedZtunnelNamespace: ztunnel

This could be inferred at install time by looking for a ZTunnel resource that has a targetRef pointing to this IstioRevision and then pulling the spec.namespace value from that ZTunnel.

return fmt.Errorf("failed to apply profile: %w", err)
}

// Apply values from referenced IstioRevision first
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would make sense to warn users when setting the same config in both ztunnel and Istio to different values?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to warn users - their settings will always take precedence. By default, they should only configure the version and targetRef

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a warning might imply that something is wrong. Can we add a log message saying that user setting from ztunnel are used?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. as we're just merging the fields I don't think we have the ability to give that sort of granular feedback. or rather, we'd have to add a bunch of logic in order to do that. maybe just documenting the behavior is enough?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, works for me.

BeforeAll(func() {
common.CreateIstio(k, version.Name, `
values:
global:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be added to samples?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the global setting. I will add the targetRef to the sample though

Comment thread tests/e2e/ambient/ambient_test.go
@dgn dgn force-pushed the ztunnel-targetref branch from b655b0e to 88ed7f4 Compare October 8, 2025 15:32
Copy link
Copy Markdown
Contributor

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dgn.

Comment thread api/v1alpha1/ztunnel_types.go Outdated
return fmt.Errorf("failed to apply profile: %w", err)
}

// Apply values from referenced IstioRevision first
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a warning might imply that something is wrong. Can we add a log message saying that user setting from ztunnel are used?

Comment thread pkg/revision/references.go
var revisionName string
switch ref.Kind {
case v1.IstioRevisionKind:
revisionName = ref.Name
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add input validation for targetReference?

Comment thread controllers/ztunnel/ztunnel_controller.go
version: %s
namespace: %s
targetRef:
kind: Istio
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we will require a test case without the targetRef?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have tests that verify that user input takes precedence so it should be okay

@sridhargaddam
Copy link
Copy Markdown
Contributor

This should support most use cases including RevisionBased update strategies already. Another thing we could add is making the IstioRevisionController aware of the link - currently, users have to configure the ZTunnel namespace on their Istio/IstioRevision:

spec:
  values:
    pilot:
      trustedZtunnelNamespace: ztunnel

This could be inferred at install time by looking for a ZTunnel resource that has a targetRef pointing to this IstioRevision and then pulling the spec.namespace value from that ZTunnel.

This is a good idea.

Comment thread api/v1alpha1/ztunnel_types.go Outdated
@zmiklank
Copy link
Copy Markdown
Contributor

Hi @dgn, do you think it could be useful to add info about this API change into ZTunnel CRD docs too?

@dgn dgn force-pushed the ztunnel-targetref branch from 88ed7f4 to 926dd12 Compare October 21, 2025 14:59
@dgn dgn force-pushed the ztunnel-targetref branch from 926dd12 to 101ce49 Compare March 6, 2026 15:39
@dgn dgn changed the title [PoC] feat: add targetRef field to ZTunnel CRD feat: add targetRef field to ZTunnel CRD Mar 6, 2026
@dgn dgn force-pushed the ztunnel-targetref branch from 101ce49 to 3abc798 Compare March 10, 2026 14:06
@dgn
Copy link
Copy Markdown
Collaborator Author

dgn commented Mar 10, 2026

Hi @dgn, do you think it could be useful to add info about this API change into ZTunnel CRD docs too?

done!

Comment thread api/v1/ztunnel_types.go
Comment thread controllers/ztunnel/ztunnel_controller.go Outdated
spec:
version: v1.29.0
namespace: ztunnel
targetRef:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me if we wanted to populate the spec.version as well from the targetRef?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it, but I guess we'd want to have a placeholder version in that case? (e.g. "fromParent")

Comment thread controllers/ztunnel/ztunnel_controller.go
@dgn dgn force-pushed the ztunnel-targetref branch 2 times, most recently from 8796de3 to 28e28a5 Compare April 9, 2026 14:38
@dgn
Copy link
Copy Markdown
Collaborator Author

dgn commented Apr 9, 2026

Hi @dgn, do you think it could be useful to add info about this API change into ZTunnel CRD docs too?

Done!

@dgn
Copy link
Copy Markdown
Collaborator Author

dgn commented Apr 9, 2026

All comments addressed, PTAL 🙂

Copy link
Copy Markdown
Contributor

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good - just some minor comments.

Comment thread controllers/ztunnel/ztunnel_controller.go
Comment thread tests/integration/api/istiorevision_test.go
@dgn dgn force-pushed the ztunnel-targetref branch 3 times, most recently from 16ede15 to 09b6637 Compare April 15, 2026 19:02
@dgn dgn force-pushed the ztunnel-targetref branch 2 times, most recently from 2f830ed to c7984c5 Compare April 15, 2026 19:43
Copy link
Copy Markdown
Contributor

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @dgn

@sridhargaddam
Copy link
Copy Markdown
Contributor

I've added "/hold" so that @nrfox (or others) can take a final look. Feel free to remove the hold if needed.

@dgn dgn force-pushed the ztunnel-targetref branch from c7984c5 to 073c4d5 Compare April 16, 2026 12:39
@dgn
Copy link
Copy Markdown
Collaborator Author

dgn commented Apr 16, 2026

/retest

This adds a `targetRef` field to the ZTunnel CRD, allowing users to keep
their ZTunnel config in sync with what they already defined in the
`Istio` or `IstioRevision` CRD.

Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
@dgn dgn force-pushed the ztunnel-targetref branch from 073c4d5 to eaa8de5 Compare April 16, 2026 13:46
@istio-testing istio-testing merged commit 820aa80 into istio-ecosystem:main Apr 16, 2026
17 of 18 checks passed
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Apr 17, 2026
* upstream/main:
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845)
  feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259)
  Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844)
  Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695)
  Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840)
  Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833)
  Update kustomization files with registry.istio.io (istio-ecosystem#1829)
  Improve testing images tags for OLM and Operator images (istio-ecosystem#1819)
  Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820)
  Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644)
  introduce crd-schema-checker (istio-ecosystem#1055)
  Add kubebuilder validation for revisionTagTargetRef (istio-ecosystem#1774)
  Handle errors in Helm discovery client (istio-ecosystem#1812)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1808)
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Apr 20, 2026
* upstream/main:
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1851)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1850)
  Add documentation for resource customization (istio-ecosystem#1292)
  refactor error and status condition handling (istio-ecosystem#1807)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845)
  feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259)
  Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844)
  Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695)
  Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840)
  Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833)
  Update kustomization files with registry.istio.io (istio-ecosystem#1829)
  Improve testing images tags for OLM and Operator images (istio-ecosystem#1819)
  Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820)
  Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644)
  introduce crd-schema-checker (istio-ecosystem#1055)
  Add kubebuilder validation for revisionTagTargetRef (istio-ecosystem#1774)
  Handle errors in Helm discovery client (istio-ecosystem#1812)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1808)
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Apr 21, 2026
* upstream/main: (23 commits)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1856)
  Modify "download-charts" script for alpha/beta releases (istio-ecosystem#1852)
  Add operator `TLSConfig` and sync with APIServer TLS profile on openshift (istio-ecosystem#1513)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1851)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1850)
  Add documentation for resource customization (istio-ecosystem#1292)
  refactor error and status condition handling (istio-ecosystem#1807)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845)
  feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259)
  Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844)
  Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695)
  Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840)
  Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833)
  Update kustomization files with registry.istio.io (istio-ecosystem#1829)
  Improve testing images tags for OLM and Operator images (istio-ecosystem#1819)
  Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820)
  Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644)
  introduce crd-schema-checker (istio-ecosystem#1055)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants