CNTRLPLANE-3611: propagate tls profile to aws-pod-identity-webhook

[ricardomaraschini]

What this PR does / why we need it:

It was detected that the container aws-pod-identity-webhook isn't respecting the HostedControlPlane TLS profile. As part of the PQC work the following flags need to be set:

• --tls-min-version
  • needs to comply with the hcp apiserver min tls version.
• --tls-cipher-suites
  • needs to comply with the hcp apiserver cipher suites config.

Both flags are now set according to hcp.spec.configuration.apiServer.tlsSecurityProfile. Both of these flags influence how the container listens on port 4443.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Improvements
  • Pod identity webhook now respects HostedControlPlane TLS security profiles more precisely.
  • Minimum TLS version is applied when specified; cipher suites are included only if present.
  • TLS version values are normalized so configured versions are correctly interpreted and enforced.
• Tests
  • Added unit tests validating TLS version normalization and handling of unknown/empty inputs.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@ricardomaraschini: This pull request references CNTRLPLANE-3611 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

It was detected that the container aws-pod-identity-webhook isn't respecting the HostedControlPlane TLS profile. As part of the PQC work the following flags need to be set:

• --tls-min-version
• needs to comply with the hcp apiserver min tls version.
• --tls-cipher-suites
• needs to comply with the hcp apiserver cipher suites config.

Both flags are now set according to hcp.spec.configuration.apiServer.tlsSecurityProfile.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 55898573-76fd-4e52-9f1c-6955940aa152

📥 Commits

Reviewing files that changed from the base of the PR and between c117043 and 74ef7c2.

⛔ Files ignored due to path filters (2)

• control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml is excluded by !**/testdata/**
• control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml is excluded by !**/testdata/**

📒 Files selected for processing (2)

• control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go
• control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go

---

📝 Walkthrough

Walkthrough

The pull request refactors the applyAWSPodIdentityWebhookContainer function to build the aws-pod-identity-webhook container command dynamically by conditionally appending TLS configuration flags based on the HostedControlPlane TLS security profile. It adds --tls-min-version and --tls-cipher-suites flags when configured. A new unexported convertTLSVersion helper translates Kubernetes TLS version enums (TLS10–TLS13) into numeric version strings (1.0–1.3).

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main change: propagating TLS profile configuration to the aws-pod-identity-webhook container, which is the core objective of this pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No Ginkgo tests in the PR. The added tests use standard Go testing with stable, deterministic test names containing no dynamic values or generated identifiers.
Test Structure And Quality	✅ Passed	The check targets Ginkgo test code with Describe/It blocks. The test added (TestConvertTLSVersion) uses standard Go testing with Gomega assertions, not Ginkgo patterns.
Topology-Aware Scheduling Compatibility	✅ Passed	PR only adds TLS configuration flags to aws-pod-identity-webhook container command; no scheduling constraints, affinity rules, node selectors, or replica count changes that assume HA topology.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests added. Only a standard Go unit test (TestConvertTLSVersion) for TLS version conversion was added, with no IPv4 or external connectivity requirements.
No-Weak-Crypto	✅ Passed	No weak crypto algorithms, custom implementations, or non-constant-time secret comparisons found. Uses standard Go TLS with strong cipher suites.
Container-Privileges	✅ Passed	PR adds TLS configuration to aws-pod-identity-webhook without introducing privileged settings, hostNetwork/hostPID/hostIPC, SYS_ADMIN capabilities, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs	✅ Passed	PR doesn't add logging of sensitive data. Changes involve building container command flags with TLS configuration numbers and cipher suite names, which aren't sensitive.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ricardomaraschini
Once this PR has been reviewed and has the lgtm label, please assign muraee 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:

• OWNERS

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go`:
- Around line 315-319: The conversion of TLS enums can fail silently: when
config.MinTLSVersion(hcp.Spec.Configuration.GetTLSSecurityProfile()) yields a
value but convertTLSVersion(...) returns "", add explicit handling to surface
that failure; e.g., after calling convertTLSVersion(tlsMinVersion) detect empty
string and either log a warning/error (including the original tlsMinVersion, the
HostedControlPlane identity from hcp, and that the --tls-min-version flag will
be omitted) or return/propagate a validation error so the misconfiguration is
visible; update the block around convertTLSVersion, tlsMinVersion, and the
command append of "--tls-min-version" to include this log/validation.
- Around line 357-370: Add a new table-driven unit test named
TestConvertTLSVersion in package kas (file deployment_test.go) that calls the
helper convertTLSVersion with inputs "VersionTLS10", "VersionTLS11",
"VersionTLS12", "VersionTLS13", an unknown string like "VersionTLS99", and the
empty string, asserting returned values "1.0", "1.1", "1.2", "1.3", and ""
respectively; use t.Run for each case and fail the test when the actual return
from convertTLSVersion does not match the expected value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: aea43979-896a-48e1-b5b6-ce4da195d70c

📥 Commits

Reviewing files that changed from the base of the PR and between c3fac8c and ce4c692.

⛔ Files ignored due to path filters (2)

• control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml is excluded by !**/testdata/**
• control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml is excluded by !**/testdata/**

📒 Files selected for processing (1)

• control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go

[codecov]

Codecov Report

❌ Patch coverage is 36.36364% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.54%. Comparing base (832b848) to head (74ef7c2).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...ontrollers/hostedcontrolplane/v2/kas/deployment.go	36.36%	21 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8713      +/-   ##
==========================================
+ Coverage   41.50%   41.54%   +0.04%     
==========================================
  Files         758      758              
  Lines       93689    93859     +170     
==========================================
+ Hits        38882    38998     +116     
- Misses      52070    52116      +46     
- Partials     2737     2745       +8     

Files with missing lines	Coverage Δ
...ontrollers/hostedcontrolplane/v2/kas/deployment.go	28.57% <36.36%> (+1.78%)	⬆️

... and 9 files with indirect coverage changes

Flag	Coverage Δ
cmd-support	34.96% <ø> (+0.10%)	⬆️
cpo-hostedcontrolplane	43.61% <36.36%> (+0.01%)	⬆️
cpo-other	43.17% <ø> (ø)
hypershift-operator	51.62% <ø> (+0.05%)	⬆️
other	31.56% <ø> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci]

@ricardomaraschini: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

[hypershift-jira-solve-ci]

I now have all the information needed. The codecov.yml has no explicit coverage threshold configured, which means codecov uses the project's base coverage as the patch target.

Test Failure Analysis Complete

Job Information

• Prow Job: codecov/patch (Codecov GitHub check, not a Prow CI job)
• Build ID: 80550655918 (GitHub check run ID)
• PR: #8713 — CNTRLPLANE-3611: propagate tls profile to aws-pod-identity-webhook
• Target: codecov patch coverage gate
• Result: failure — 36.36% patch coverage vs 41.50% target

Test Failure Analysis

Error

codecov/patch: 36.36% of diff hit (target 41.50%)
21 lines in control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go
have no test coverage.

Summary

The codecov/patch check failed because only 12 of 33 changed lines (36.36%) in deployment.go are covered by tests, falling below the project's base coverage target of 41.50%. The PR adds TLS profile propagation logic to the applyAWSPodIdentityWebhookContainer function and a new convertTLSVersion helper. While convertTLSVersion is thoroughly tested by the new TestConvertTLSVersion unit test (covering all switch branches), the 21 modified lines inside applyAWSPodIdentityWebhookContainer — including the command slice construction, the MinTLSVersion conditional, and the CipherSuites conditional — have zero direct unit test coverage.

Root Cause

This is a code coverage gate violation, not a test execution failure. The specific cause:

1. convertTLSVersion IS covered (12 lines) — TestConvertTLSVersion in deployment_test.go exercises all 6 switch cases (TLS 1.0–1.3, unknown, empty). These lines pass codecov instrumentation.

2. applyAWSPodIdentityWebhookContainer is NOT covered (21 lines) — No unit test calls this function directly. The 21 uncovered lines are:

   • Lines building the command slice (the --annotation-prefix, --in-cluster, --kubeconfig, --logtostderr, --port, --aws-default-region, --tls-cert, --tls-key, --token-audience args)
   • The config.MinTLSVersion() conditional that calls convertTLSVersion and appends --tls-min-version
   • The config.CipherSuites() conditional that appends --tls-cipher-suites
   • The Command: command assignment

3. Fixture tests don't count for codecov — The existing TestControlPlaneComponents test validates the output YAML via fixture files (the two zz_fixture_*.yaml files were correctly updated), but codecov requires Go unit tests that execute the function body under instrumentation. Fixture comparison doesn't register as line-level coverage of deployment.go.

4. No explicit threshold configured — The project's codecov.yml has no coverage.patch.default.target setting, so codecov defaults to requiring patch coverage ≥ the project base coverage (41.50%). The shortfall is 5.14 percentage points.

Recommendations

1. Add a unit test for applyAWSPodIdentityWebhookContainer — Create a test that calls the function with a mock HostedControlPlane and asserts the resulting container spec. Suggested test cases:

   • Default TLS profile (Intermediate) → expects --tls-min-version=1.2 and --tls-cipher-suites=... in Command
   • Custom TLS profile (Old) → expects --tls-min-version=1.0 with broader cipher list
   • Nil/empty TLS security profile → expects no TLS args appended
   • This would cover all 21 missing lines and bring patch coverage above the threshold

2. The TestConvertTLSVersion test is well-written — No changes needed; it covers 100% of the helper function.

3. Alternative (not recommended): Add an explicit lower patch target in codecov.yml under coverage.patch.default.target, but this papers over the missing test rather than improving coverage of the new TLS logic.

Evidence

Evidence	Detail
Check conclusion	failure — 36.36% patch coverage vs 41.50% target
Affected file	control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go
Lines covered	12 of 33 — all in convertTLSVersion (via TestConvertTLSVersion)
Lines uncovered	21 — all in applyAWSPodIdentityWebhookContainer (command construction + TLS conditionals)
Test added	TestConvertTLSVersion — 6 test cases covering TLS 1.0–1.3, unknown, empty
Test missing	No unit test for applyAWSPodIdentityWebhookContainer
Codecov config	No explicit coverage.patch threshold in codecov.yml; defaults to project base (41.50%)
Codecov bot comment	"Patch coverage is 36.36364% with 21 lines in your changes missing coverage"
Fixture tests	Both zz_fixture_*.yaml files updated correctly with --tls-min-version=1.2 and --tls-cipher-suites=...

---