Skip to content

[release-1.21] Add network/tls package for TLS configuration#3337

Open
Fedosin wants to merge 4 commits intoknative:release-1.21from
Fedosin:backport-tls-release-1.21
Open

[release-1.21] Add network/tls package for TLS configuration#3337
Fedosin wants to merge 4 commits intoknative:release-1.21from
Fedosin:backport-tls-release-1.21

Conversation

@Fedosin
Copy link
Contributor

@Fedosin Fedosin commented Mar 18, 2026

Changes

Backport of the following PRs from main to release-1.21:

/kind enhancement

Fixes #

Release Note


Docs


Fedosin and others added 4 commits March 18, 2026 13:25
…native#3324)

* feat: add shared tls package for reading TLS config from environment

Extract TLS configuration parsing into a reusable knative.dev/pkg/tls
package so that any Knative component (not just webhooks) can read
TLS_MIN_VERSION, TLS_MAX_VERSION, TLS_CIPHER_SUITES, and
TLS_CURVE_PREFERENCES from environment variables with an optional prefix.

The webhook package is updated to use the new tls package, extending env
var support from just WEBHOOK_TLS_MIN_VERSION to all four WEBHOOK_TLS_*
variables. Programmatic Options values continue to take precedence over
environment variables.

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>

* fix: address review feedback on tls package

Reduce the public API surface of the tls package by unexporting
ParseVersion, ParseCipherSuites, and ParseCurvePreferences since they
are implementation details of NewConfigFromEnv.

Also validate that TLS max version is not smaller than min version in
webhook.New(), document the Options TLS field precedence
(programmatic > env vars > defaults), and broaden TestConfig_TLSConfig
to exercise the full NewConfigFromEnv → TLSConfig path.

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>

---------

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
DefaultConfigFromEnv replaces NewConfigFromEnv by returning a full
default tls.Config with overrides from env vars. This avoids specifying
e.g. the TLS MinVersion explicitely.
…ity (knative#3331)

The TLS configuration package is moved from tls/ to network/tls/ to
co-locate it with the rest of the networking code. The old tls/ package
now re-exports all public symbols as deprecated aliases so that existing
consumers continue to compile without changes. The webhook package is
updated to import from the new location directly.

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
@knative-prow
Copy link

knative-prow bot commented Mar 18, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Fedosin
Once this PR has been reviewed and has the lgtm label, please assign matzew for approval. For more information see the Code Review Process.

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

Details 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 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 18, 2026
@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

❌ Patch coverage is 88.65979% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.85%. Comparing base (4a022ed) to head (1415e55).

Files with missing lines Patch % Lines
webhook/webhook.go 68.57% 7 Missing and 4 partials ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.21    #3337      +/-   ##
================================================
+ Coverage         74.64%   74.85%   +0.20%     
================================================
  Files               188      189       +1     
  Lines              8207     8275      +68     
================================================
+ Hits               6126     6194      +68     
  Misses             1841     1841              
  Partials            240      240              

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

Copy link
Contributor

@linkvt linkvt left a comment

Choose a reason for hiding this comment

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

Same as in the already reviewed PRs 👍

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@Fedosin
Copy link
Contributor Author

Fedosin commented Mar 18, 2026

/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 Mar 18, 2026
@Fedosin
Copy link
Contributor Author

Fedosin commented Mar 18, 2026

/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 Mar 18, 2026
@dprotaso
Copy link
Member

What's the intent here are we hoping to pull this into specific repos which we release? If so what bug are we fixing?

Or do you just want these changes in a release branch for other reasons?

@dprotaso
Copy link
Member

To clarify not looking to block this - just looking for clarity on what's the subsequent plan

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

Labels

kind/enhancement lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants