Skip to content

Conversation

@prashanthjos
Copy link
Contributor

@prashanthjos prashanthjos commented Oct 23, 2025

Description

This PR changes support for the CLUSTER_DOMAIN environment variable to allow explicit configuration of the cluster domain, taking precedence over /etc/resolv.conf parsing.

Motivation

When using custom service mesh configurations (e.g., Istio with custom domains), the internal DNS domain may differ from the Kubernetes cluster domain. Currently, Knative derives the cluster domain by parsing /etc/resolv.conf, which doesn't work in these scenarios.

For example, a service mesh might use mesh.net internally while Kubernetes uses cluster.local. This PR enables users to explicitly set the cluster domain via environment variable.

Changes

network/domain.go : Modified getClusterDomainName() to check CLUSTER_DOMAIN environment variable first, then fall back to /etc/resolv.conf parsing

  • Maintains full backward compatibility - if env var is not set, behavior is identical to current implementation

Release Note

Edit CLUSTER_DOMAIN environment variable support to allow explicit configuration of cluster domain for custom service mesh configurations.

…omains

This change modifies the getClusterDomainName() function to prioritize
the CLUSTER_DOMAIN environment variable over /etc/resolv.conf parsing,
enabling explicit configuration of cluster domains.

This is particularly useful when using service mesh configurations where
the internal DNS domain (e.g., mesh.net) differs from the Kubernetes
cluster domain.

Changes:
- Modified getClusterDomainName() to check CLUSTER_DOMAIN env var first
- Falls back to /etc/resolv.conf parsing if env var is not set
- Updated tests to validate precedence behavior
- Added test cases for custom domain scenarios
@knative-prow knative-prow bot requested review from Leo6Leo and aslom October 23, 2025 01:43
@knative-prow
Copy link

knative-prow bot commented Oct 23, 2025

Welcome @prashanthjos! It looks like this is your first PR to knative/pkg 🎉

@knative-prow
Copy link

knative-prow bot commented Oct 23, 2025

Hi @prashanthjos. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@knative-prow knative-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 23, 2025
@dprotaso
Copy link
Member

We have a release next week and knative.dev/pkg release-1.120 has already been cut. Will revisit after.
/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 23, 2025
@dprotaso
Copy link
Member

When using custom service mesh configurations (e.g., Istio with custom domains), the internal DNS domain may differ from the Kubernetes cluster domain.

Do you have an example on this can be setup with Istio?

@prashanthjos
Copy link
Contributor Author

prashanthjos commented Oct 23, 2025

When using Istio with a custom mesh domain, we configured a ServiceEntry to route all internal service-to-service traffic through the Istio proxy instead of using Kubernetes default DNS resolution.

apiVersion: networking.istio.io/v1beta1
kind: ServiceEntry
metadata:
  name: mesh-internal-services
  namespace: istio-system
spec:
  # All traffic to *.mesh.net is routed through the Istio proxy
  hosts:
    - "*.mesh.net"
  
  # Traffic is routed to the loopback address where Istio proxy intercepts it
  # via iptables rules configured by istio-init
  addresses:
    - "127.0.0.6"  # Example loopback IP for mesh traffic
  
  location: MESH_INTERNAL
  resolution: STATIC
  
  endpoints:
    # The Istio proxy intercepts and routes based on mTLS identity
    - address: "127.0.0.6"

With Default Kubernetes DNS all the services are accessed via ..svc.cluster.local, but however with custom mesh DNS through ServiceEntry services are accessed via ..svc.mesh.net.

Istio Interception: When one service calls another service in our mesh

  • Istio's istio-init container modifies iptables rules in each pod (this is during initialization of Pod)
  • Traffic to *.mesh.net is intercepted at 127.0.0.6 (loopback)
  • The Envoy proxy handles mTLS, routing, policies, and telemetry
  • Traffic is then forwarded to the actual service

Without the CLUSTER_DOMAIN environment variable, Knative components would:

We have required VS and DR (generated by our custom implementations of the Knative network spec) in place so that Knative components work as expected.

We also created an issue to see how can use net-istio so that we don't write our own custom implementation but that is a long shot for us.

@prashanthjos
Copy link
Contributor Author

We have a release next week and knative.dev/pkg release-1.120 has already been cut. Will revisit after. /hold

@dprotaso can we expect this to be merged next week?

@prashanthjos
Copy link
Contributor Author

@dprotaso please do let me know if you have any questions. I would be happy to try any alternatives too before we can merge this PR.

@dprotaso
Copy link
Member

dprotaso commented Nov 3, 2025

Maintains full backward compatibility - if env var is not set, behavior is identical to current implementation

Technically this isn't accurate - setting the env var now will take precedence over /etc/resolve.conf

Though this env var is being used in tests (#2092) so it's probably ok to tweak this.

@dprotaso
Copy link
Member

dprotaso commented Nov 3, 2025

/ok-to-test

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 3, 2025
@dprotaso
Copy link
Member

dprotaso commented Nov 3, 2025

/hold cancel

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2025
Comment on lines 52 to 56
}, {
name: "env variable with custom domain",
resolvConf: ``,
env: "custom.local",
want: "custom.local",
Copy link
Member

Choose a reason for hiding this comment

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

This test seems unnecessary - I say we drop it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, I removed the test!

…omains

This change modifies the getClusterDomainName() function to prioritize
the CLUSTER_DOMAIN environment variable over /etc/resolv.conf parsing,
enabling explicit configuration of cluster domains.

This is particularly useful when using service mesh configurations where
the internal DNS domain (e.g., mesh.net) differs from the Kubernetes
cluster domain.

Changes:
- Modified getClusterDomainName() to check CLUSTER_DOMAIN env var first
- Falls back to /etc/resolv.conf parsing if env var is not set
- Updated tests to validate precedence behavior
- Added test cases for custom domain scenarios
@dprotaso
Copy link
Member

dprotaso commented Nov 4, 2025

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2025
@knative-prow
Copy link

knative-prow bot commented Nov 4, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, prashanthjos

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 4, 2025
@knative-prow knative-prow bot merged commit 874da3b into knative:main Nov 4, 2025
36 checks passed
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.58%. Comparing base (487f9df) to head (ed32ad4).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3283      +/-   ##
==========================================
- Coverage   74.96%   74.58%   -0.39%     
==========================================
  Files         188      188              
  Lines       10263     8184    -2079     
==========================================
- Hits         7694     6104    -1590     
+ Misses       2330     1840     -490     
- Partials      239      240       +1     

☔ 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.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants