Skip to content

feat: use knative.dev/pkg/tls for reconciler TLS configuration#16431

Merged
knative-prow[bot] merged 1 commit intoknative:mainfrom
Fedosin:reconciler-tls
Mar 5, 2026
Merged

feat: use knative.dev/pkg/tls for reconciler TLS configuration#16431
knative-prow[bot] merged 1 commit intoknative:mainfrom
Fedosin:reconciler-tls

Conversation

@Fedosin
Copy link
Contributor

@Fedosin Fedosin commented Mar 3, 2026

Proposed Changes

Replace the hand-rolled tlsMinVersionFromEnv helper in the revision reconciler with the shared knative.dev/pkg/tls package, allowing TLS settings to be configured via TAG_TO_DIGEST_TLS_MIN_VERSION, TAG_TO_DIGEST_TLS_MAX_VERSION, TAG_TO_DIGEST_TLS_CIPHER_SUITES, and TAG_TO_DIGEST_TLS_CURVE_PREFERENCES environment variables. The default minimum version is bumped to TLS 1.3 now that quay.io supports it.

Release Note

The tag-to-digest resolver now reads TLS settings from environment variables (TAG_TO_DIGEST_TLS_MIN_VERSION, TAG_TO_DIGEST_TLS_MAX_VERSION, TAG_TO_DIGEST_TLS_CIPHER_SUITES, TAG_TO_DIGEST_TLS_CURVE_PREFERENCES) via the shared knative.dev/pkg/tls package. The default minimum TLS version is bumped from 1.2 to 1.3.

@knative-prow knative-prow bot requested review from dprotaso and skonto March 3, 2026 16:12
@knative-prow knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 3, 2026
@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.22%. Comparing base (42495d4) to head (eb4c84f).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16431      +/-   ##
==========================================
+ Coverage   80.21%   80.22%   +0.01%     
==========================================
  Files         217      217              
  Lines       13511    13503       -8     
==========================================
- Hits        10838    10833       -5     
+ Misses       2307     2304       -3     
  Partials      366      366              

☔ 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

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

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

This is out of scope as this is a TLS client and we should take care of servers only.

I have it in my notes under "Potential Future Work". But now that you did it we might as well keep it! 😆 👍

I have only one comment.

/approve
/lgtm
/hold for other reviewers

@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 3, 2026
@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2026
Copy link

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

Replace the hand-rolled tlsMinVersionFromEnv helper in the revision
reconciler with the shared knative.dev/pkg/tls package, allowing TLS
settings to be configured via TAG_TO_DIGEST_TLS_MIN_VERSION,
TAG_TO_DIGEST_TLS_MAX_VERSION, TAG_TO_DIGEST_TLS_CIPHER_SUITES, and
TAG_TO_DIGEST_TLS_CURVE_PREFERENCES environment variables. The default
minimum version is bumped to TLS 1.3 now that quay.io supports it.

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2026
@Fedosin
Copy link
Contributor Author

Fedosin commented Mar 5, 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 5, 2026
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.

Looks good to me! Bit unsure if we still need the test but on the other hand we make sure that env TLS parsing is configured correctly in the code - so I guess all good 👍

/lgtm

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

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@dprotaso
Copy link
Member

dprotaso commented Mar 5, 2026

/lgtm
/approve

@knative-prow
Copy link

knative-prow bot commented Mar 5, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, Fedosin, twoGiants

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:

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 Mar 5, 2026
@knative-prow knative-prow bot merged commit cff5211 into knative:main Mar 5, 2026
70 checks passed
Fedosin added a commit to Fedosin/serving that referenced this pull request Mar 20, 2026
… TLS

Backport of the following PRs from main to release-1.21:
- knative#16424 feat: use knative.dev/pkg/tls for activator TLS configuration
- knative#16425 feat: use knative.dev/pkg/tls for queue-proxy TLS configuration
- knative#16431 feat: use knative.dev/pkg/tls for reconciler TLS configuration
- knative#16458 Update TLS import path to knative.dev/pkg/network/tls

Replace hardcoded tls.VersionTLS13 in the activator, queue-proxy, and
tag-to-digest resolver with the shared knative.dev/pkg/network/tls
package, allowing TLS settings (min/max version, cipher suites, curve
preferences) to be configured via environment variables:
  - ACTIVATOR_TLS_*
  - QUEUE_PROXY_TLS_*
  - TAG_TO_DIGEST_TLS_*

Add four new keys to the config-deployment ConfigMap
(queue-sidecar-tls-min-version, queue-sidecar-tls-max-version,
queue-sidecar-tls-cipher-suites, queue-sidecar-tls-curve-preferences)
and forward them as QUEUE_PROXY_TLS_* environment variables in
makeQueueContainer.

The default remains TLS 1.3 when no env var is set. The tag-to-digest
resolver default is bumped from TLS 1.2 to TLS 1.3.

knative/pkg dependency: knative/pkg#3337

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
knative-prow bot pushed a commit that referenced this pull request Mar 20, 2026
… TLS (#16482)

Backport of the following PRs from main to release-1.21:
- #16424 feat: use knative.dev/pkg/tls for activator TLS configuration
- #16425 feat: use knative.dev/pkg/tls for queue-proxy TLS configuration
- #16431 feat: use knative.dev/pkg/tls for reconciler TLS configuration
- #16458 Update TLS import path to knative.dev/pkg/network/tls

Replace hardcoded tls.VersionTLS13 in the activator, queue-proxy, and
tag-to-digest resolver with the shared knative.dev/pkg/network/tls
package, allowing TLS settings (min/max version, cipher suites, curve
preferences) to be configured via environment variables:
  - ACTIVATOR_TLS_*
  - QUEUE_PROXY_TLS_*
  - TAG_TO_DIGEST_TLS_*

Add four new keys to the config-deployment ConfigMap
(queue-sidecar-tls-min-version, queue-sidecar-tls-max-version,
queue-sidecar-tls-cipher-suites, queue-sidecar-tls-curve-preferences)
and forward them as QUEUE_PROXY_TLS_* environment variables in
makeQueueContainer.

The default remains TLS 1.3 when no env var is set. The tag-to-digest
resolver default is bumped from TLS 1.2 to TLS 1.3.

knative/pkg dependency: knative/pkg#3337

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants