Skip to content

OCPNODE-4114: Update TLSSecurityProfile doc to reflect CRI-O propagation and TLS 1.3 support #2747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
asahay19 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

OCPNODE-4114: Update TLSSecurityProfile doc to reflect CRI-O propagation and TLS 1.3 support #2747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion machineconfiguration/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,10 @@ type KubeletConfigSpec struct {

// If unset, the default is based on the apiservers.config.openshift.io/cluster resource.
// Note that only Old and Intermediate profiles are currently supported, and
// the maximum available minTLSVersion is VersionTLS12.
// the maximum available minTLSVersion is VersionTLS13.
// When set, this TLS configuration is applied to both the kubelet and CRI-O
// on nodes matching the pool selector. CRI-O receives the minimum TLS version
// via a drop-in configuration file managed by the ContainerRuntimeConfig controller.
Comment on lines 759 to +763
Copy link

@qodo-code-review qodo-code-review bot Mar 6, 2026

Choose a reason for hiding this comment

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

Action required

1. Tls profile docs contradict schema 🐞 Bug ✓ Correctness

The updated TLSSecurityProfile comment says only Old/Intermediate profiles are supported while also
claiming the maximum minTLSVersion is VersionTLS13, which implies Modern/Custom (TLS 1.3) support.
This contradicts the underlying TLSSecurityProfile schema and the generated CRD enum (which allow
Modern/Custom and VersionTLS13), making the API documentation misleading and potentially causing
users to assume unsupported/unsupported behavior incorrectly.
Agent Prompt 
### Issue description
`KubeletConfigSpec.TLSSecurityProfile` docs are contradictory: they say only `Old`/`Intermediate` are supported, but also claim the max `minTLSVersion` is `VersionTLS13` (which implies `Modern`/`Custom`). This conflicts with the schema/CRD which allows `Modern`/`Custom` and `VersionTLS13`, and may mislead users.

### Issue Context
This PR is documentation-focused, so correctness/clarity of the API docstrings is the core deliverable. The CRD schema is broader than the current prose, so the prose must clearly state what the operator actually supports/propagates.

### Fix Focus Areas
- machineconfiguration/v1/types.go[758-765]
- machineconfiguration/v1/zz_generated.swagger_doc_generated.go[247-251]
- machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs.crd.yaml[111-117]
- machineconfiguration/v1/zz_generated.featuregated-crd-manifests/kubeletconfigs.machineconfiguration.openshift.io/AAA_ungated.yaml[112-118]
- payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml[111-117]

(After updating the source comment, re-run the repo’s generation scripts as appropriate so generated CRDs/swagger docs remain consistent.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +760 to +763
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clarify supported-profile scope and CRI-O propagation here.

Combined with the preceding sentence, this now reads inconsistently: it still says only Old/Intermediate are supported, but also advertises VersionTLS13 as the max minTLSVersion, and it says the TLS config applies to CRI-O even though the next sentence narrows that to only the minimum TLS version. Please tighten the wording so users do not infer that Modern/custom profiles or custom cipher settings are fully honored for CRI-O. This same text is mirrored into the generated CRD description, so update both in lockstep.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@machineconfiguration/v1/types.go` around lines 760 - 763, Update the comment
and mirrored CRD description around the TLS profile and CRI-O propagation to
clearly state that only Old and Intermediate named profiles are supported for
full TLS/profile config, that VersionTLS13 is the highest allowed value for
minTLSVersion, and that CRI-O only receives the minimum TLS version
(minTLSVersion) via a drop-in file managed by the ContainerRuntimeConfig
controller — it does NOT receive Modern or custom profiles or custom cipher
suites or other profile-specific settings; change text near the minTLSVersion
comment and the corresponding CRD description to explicitly limit CRI-O
propagation to minTLSVersion only and to warn that Modern/custom profiles and
cipher settings are not applied to CRI-O.

// +optional
TLSSecurityProfile *configv1.TLSSecurityProfile `json:"tlsSecurityProfile,omitempty"`
}
Expand Down
5 changes: 4 additions & 1 deletion ...iguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ spec:
description: |-
If unset, the default is based on the apiservers.config.openshift.io/cluster resource.
Note that only Old and Intermediate profiles are currently supported, and
the maximum available minTLSVersion is VersionTLS12.
the maximum available minTLSVersion is VersionTLS13.
When set, this TLS configuration is applied to both the kubelet and CRI-O
on nodes matching the pool selector. CRI-O receives the minimum TLS version
via a drop-in configuration file managed by the ContainerRuntimeConfig controller.
properties:
custom:
description: |-
Expand Down
5 changes: 4 additions & 1 deletion ...turegated-crd-manifests/kubeletconfigs.machineconfiguration.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ spec:
description: |-
If unset, the default is based on the apiservers.config.openshift.io/cluster resource.
Note that only Old and Intermediate profiles are currently supported, and
the maximum available minTLSVersion is VersionTLS12.
the maximum available minTLSVersion is VersionTLS13.
When set, this TLS configuration is applied to both the kubelet and CRI-O
on nodes matching the pool selector. CRI-O receives the minimum TLS version
via a drop-in configuration file managed by the ContainerRuntimeConfig controller.
properties:
custom:
description: |-
Expand Down
2 changes: 1 addition & 1 deletion machineconfiguration/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ spec:
description: |-
If unset, the default is based on the apiservers.config.openshift.io/cluster resource.
Note that only Old and Intermediate profiles are currently supported, and
the maximum available minTLSVersion is VersionTLS12.
the maximum available minTLSVersion is VersionTLS13.
When set, this TLS configuration is applied to both the kubelet and CRI-O
on nodes matching the pool selector. CRI-O receives the minimum TLS version
via a drop-in configuration file managed by the ContainerRuntimeConfig controller.
properties:
custom:
description: |-
Expand Down