Skip to content

test: fewer domain levels for knative services domain#3368

Merged
knative-prow[bot] merged 1 commit intoknative:mainfrom
matejvasek:change-ksvc-domain-tmpl
Jan 20, 2026
Merged

test: fewer domain levels for knative services domain#3368
knative-prow[bot] merged 1 commit intoknative:mainfrom
matejvasek:change-ksvc-domain-tmpl

Conversation

@matejvasek
Copy link
Contributor

@matejvasek matejvasek commented Jan 16, 2026 •
edited
Loading

Changes

  • Fewer domain levels for knative services url. Now test ksvc will be direct sub-domain of the cluster domain. This is useful when using wild-card certificate (one can have a cert like *.localtest.me but not like *.*.localtest.me).
  • Refactor: move logic for determining ksvc url into a function instead or having code littered with fmt.Sprintf(…).

@knative-prow
Copy link

knative-prow bot commented Jan 16, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prow
Copy link

knative-prow bot commented Jan 16, 2026

@matejvasek: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

Details

In response to this:

Patch domain-template in config to ensure ksvcs are direct subdomain of the cluster domain.
This is useful when using with wild-card certificates.

Changes

/kind

Fixes #

Release Note

Docs

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 the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026
@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 16, 2026
@codecov
Copy link

codecov bot commented Jan 16, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.73%. Comparing base (4f14a28) to head (3da92c9).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3368      +/-   ##
==========================================
+ Coverage   55.18%   55.73%   +0.55%     
==========================================
  Files         170      170              
  Lines       19963    19963              
==========================================
+ Hits        11016    11127     +111     
+ Misses       7865     7727     -138     
- Partials     1082     1109      +27
Flag Coverage Δ
e2e 39.74% <ø> (ø)
e2e go 35.47% <ø> (ø)
e2e node 31.12% <ø> (ø)
e2e python 35.12% <ø> (ø)
e2e quarkus 31.27% <ø> (ø)
e2e rust 30.73% <ø> (ø)
e2e springboot 29.18% <ø> (-0.03%) ⬇️
e2e typescript 31.23% <ø> (ø)
integration 18.64% <ø> (+1.22%) ⬆️
unit macos-14 45.11% <ø> (ø)
unit macos-latest 45.11% <ø> (ø)
unit ubuntu-24.04-arm 45.23% <ø> (ø)
unit ubuntu-latest 46.00% <ø> (ø)
unit windows-latest 45.13% <ø> (ø)

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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@matejvasek matejvasek force-pushed the change-ksvc-domain-tmpl branch from c103930 to 7e91dbf Compare January 16, 2026 16:49
@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 16, 2026
@matejvasek matejvasek force-pushed the change-ksvc-domain-tmpl branch from 7e91dbf to 240c43c Compare January 16, 2026 16:58
@matejvasek
Fewer domain levels for knative services domain
3da92c9 
Patch domain-template in config to ensure ksvcs are direct subdomain of
the cluster domain.
This is useful when using with wild-card certificates.

Signed-off-by: Matej Vašek <matejvasek@gmail.com>
@matejvasek matejvasek force-pushed the change-ksvc-domain-tmpl branch from 240c43c to 3da92c9 Compare January 16, 2026 17:24
@matejvasek matejvasek changed the title [WIP] [DNM] Fewer domain levels for knative services domain Fewer domain levels for knative services domain Jan 16, 2026
@matejvasek matejvasek marked this pull request as ready for review January 16, 2026 17:24
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026
@matejvasek matejvasek requested review from gauron99, lkingland and twoGiants and removed request for dsimansk and jrangelramos January 16, 2026 17:24
@matejvasek matejvasek changed the title Fewer domain levels for knative services domain test: fewer domain levels for knative services domain Jan 16, 2026
gauron99
gauron99 reviewed Jan 19, 2026
View reviewed changes
e2e/e2e_test.go
Comment on lines +1222 to +1224
// TODO get `default-external-scheme` from the config in cluster
const ksvcDefaultExternalScheme = `http`
// TODO get `domain-template` from the config in cluster
Copy link
Contributor

@gauron99 gauron99 Jan 19, 2026

Choose a reason for hiding this comment

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

Are we planning these TODOs in a subsequent PR?

Copy link
Contributor Author

@matejvasek matejvasek Jan 19, 2026

Choose a reason for hiding this comment

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

TBH I do not plan to do it soon. Even with this hardcoded value this is a lot better what what we had before.

Copy link
Contributor Author

@matejvasek matejvasek Jan 19, 2026

Choose a reason for hiding this comment

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

We can mark it as good first issue.

Copy link
Contributor

@gauron99 gauron99 Jan 20, 2026

Choose a reason for hiding this comment

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

yep that sounds good

@matejvasek matejvasek requested a review from gauron99 January 19, 2026 15:45
@gauron99
Copy link
Contributor

gauron99 commented Jan 20, 2026

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2026
@knative-prow
Copy link

knative-prow bot commented Jan 20, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauron99, matejvasek

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [gauron99,matejvasek]

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 merged commit b15ef8f into knative:main Jan 20, 2026
43 checks passed
@matejvasek
Copy link
Contributor Author

matejvasek commented Jan 20, 2026

cc @jrangelramos

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

Reviewers

@lkingland lkingland Awaiting requested review from lkingland

@twoGiants twoGiants Awaiting requested review from twoGiants

@gauron99 gauron99 Awaiting requested review from gauron99

Assignees

@gauron99 gauron99

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matejvasek @gauron99