enhancements/config: Versioned Config Access - #2075
Conversation
Add four related enhancement proposals for a generalized VersionedConfigAccess[T] pattern that unifies how OpenShift handles version-evolving configuration defaults: - versioned-config-access.md: Generic framework in library-go - featuregate-versioned-config.md: FeatureGateAccess refactor - tls-versioned-config.md: Version-indexed TLS profiles - pki-versioned-config.md: Version-indexed PKI profiles
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe pull request adds enhancement proposals for generic versioned configuration access and its application to FeatureGates, PKI profiles, and TLS profiles. The proposals define APIs, materialization, synchronization, compatibility, testing, upgrades, and operations. ChangesVersioned Configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
enhancements/config/tls-versioned-config.md (2)
101-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefine the compatibility scope for
TLSProfilesState that “latest version” means the latest declaration compiled into that release. An old operator does not receive declarations added only to a newer release.
🤖 Prompt for 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. In `@enhancements/config/tls-versioned-config.md` around lines 101 - 108, Update the TLSProfiles compatibility documentation to define “latest version” as the latest declaration compiled into the current release, and explicitly state that older operators do not receive declarations introduced only in newer releases.
171-180: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument version format and matching semantics explicitly.
The proposal delegates version matching to
VersionedConfigAccess[ResolvedTLSProfile], which performs exact string matching (lines 221–225, 317–320). The materializer reads versions fromClusterVersion.status.historyandClusterVersion.status.desired.versionand writes them unchanged toAPIServerStatus.TLSProfiles[].version. The accessor then reads the same status entries. Both layers consume the same source, ensuring consistent format; however, add an explicit statement that version strings are obtained directly from ClusterVersion without normalization or transformation, and specify the expected format.🤖 Prompt for 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. In `@enhancements/config/tls-versioned-config.md` around lines 171 - 180, Update the TLS versioned configuration proposal to explicitly state that APIServerStatus.TLSProfiles[].version values are copied directly from ClusterVersion status history or desired.version without normalization or transformation, and document the expected version-string format used for exact matching.enhancements/config/pki-versioned-config.md (3)
337-338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SetChangeHandlernaming diverges from the domain-specific convention used elsewhere.Line 341-343 states that method names follow the
InitialFeatureGatesObserved-style domain-specific naming convention, yetSetChangeHandlerremains generic instead of e.g.SetPKIProfileChangeHandler. Confirm whether this is an intentional exception or an oversight, since the stated convention implies domain-specific naming for all public accessor methods.🤖 Prompt for 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. In `@enhancements/config/pki-versioned-config.md` around lines 337 - 338, Rename the public PKIProfileAccess method SetChangeHandler to a domain-specific name such as SetPKIProfileChangeHandler, and update all references and documentation consistently. Preserve the existing handler behavior and signature while aligning the API with the stated domain-specific naming convention.
596-599: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated sentence-opening word ("openshift/...") flagged by style checker.
Static analysis flags three successive list items beginning with the same phrase pattern ("openshift/api", "openshift/library-go", "openshift/cluster-config-operator"). This is a stylistic nit in a design doc and does not affect technical accuracy; optional rewording only.
🤖 Prompt for 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. In `@enhancements/config/pki-versioned-config.md` around lines 596 - 599, Reword the three successive list items in the component summary to avoid repeating “openshift/” at the start of each entry, while preserving the referenced components and their technical descriptions.Source: Linters/SAST tools
537-540: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExact version-string matching is fragile and blocks operator startup.
The failure mode states: "If an operator reports a version string that does not exactly match any status entry, the accessor returns an error and the operator blocks at startup." Exact string matching between an operator's reported desired version and status entries is brittle to formatting differences (e.g., leading
v, build metadata, or partial version strings) and could cause spurious startup failures that are hard to distinguish from real materializer failures.Consider clarifying the expected version string format/normalization contract between the accessor and status entries, and whether floor-based lookup (as used for the declaration table) is more appropriate here than exact match, to reduce operator startup flakiness.
🤖 Prompt for 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. In `@enhancements/config/pki-versioned-config.md` around lines 537 - 540, The versioned-config documentation should define a normalization contract for operator-reported versions and status entries, including handling of prefixes, build metadata, and partial versions. Update the described accessor behavior to use the agreed normalized or floor-based lookup semantics instead of requiring brittle exact string equality, while preserving errors for genuinely unsupported versions.
🤖 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 `@enhancements/config/featuregate-versioned-config.md`:
- Around line 153-181: The featureGateAdapter contract is incomplete unless it
preserves the existing FeatureGateAccess behavior. Define and implement its
callback mapping to FeatureChange with Previous nil on first observation,
preserve pre-initialization errors, slice copying, NewFeatureGate
panic-on-unknown behavior, post-Run setter panic, hardcoded initialization, and
reflect.DeepEqual ordering plus nil-versus-empty semantics; add tests covering
each case.
In `@enhancements/config/pki-versioned-config.md`:
- Around line 199-207: Define and document an explicit retention and pruning
policy for PKIStatus.Profiles, including the trigger and timing for removing
versions absent from ClusterVersion.status.history. Update the related open
question near the Profiles definition and the statement around
PKIStatus.Profiles so the claimed maximum of two entries is backed by verifiable
cleanup behavior.
In `@enhancements/config/tls-versioned-config.md`:
- Around line 209-218: Update the TLSProfiles status contract comments to use
“version” consistently instead of “payloadVersion,” including the list-key
description and the materializer and accessor sections. Keep the
+listMapKey=version annotation and VersionedTLSProfile field naming unchanged.
- Around line 581-590: Update the “Keep Static Maps for TLS” section to classify
X25519MLKEM768 consistently with the TLSProfileSpec field name used in the other
sections, identifying it as a group rather than a cipher. Ensure the wording
does not imply placing it in the cipher-suite list.
- Around line 493-504: Update the “Version Skew Strategy” section to define
compatibility requirements for all TLS profile fields—cipher suites, groups, and
minimum TLS version—instead of claiming cipher negotiation is inherently
version-independent. Specify that rollouts must follow an ordering that
preserves compatibility and require successful handshake tests for every old/new
client-server combination before proceeding.
- Around line 312-321: Update the Materializer Controller to watch and reconcile
the complete spec.tlsSecurityProfile, not only its type. For Custom profiles,
copy custom.Ciphers, custom.TLSVersion, and custom.DHParamSize into every
status.tlsProfiles[] entry and ensure CurrentTLSProfile() reflects changes. Add
validation and error status handling for missing or invalid Custom values.
- Around line 317-321: Clarify the TLS profile reconciliation contract around
ClusterVersion.status.history and status.desired.version: define the active
version set, remove apiserver.status.tlsProfiles entries for versions no longer
active, and specify that pruning occurs as upgrades converge. Update the
documented workflow and add an integration test covering at least three retained
history versions across multiple upgrades.
- Around line 480-491: The downgrade contract must explicitly define whether
losing status.tlsProfiles is acceptable when an old version reads or updates
APIServerStatus. Update the versioned API schema and downgrade documentation
around APIServerStatus and TLS materialization to state the intended behavior,
then add coverage for materialization, downgrade, an old-version status update,
re-upgrade, and subsequent status inspection; only retain the “No manual
cleanup” claim if these tests confirm it.
- Around line 286-303: Update the VersionedTLSProfiles floor-lookup
specification to state that requests below the minimum declared MinVersion
return an error, and define the error behavior for unparsable requested or
declaration versions. Extend the associated test plan to cover below-minimum
lookups, the exact minimum boundary, and malformed version strings.
In `@enhancements/config/versioned-config-access.md`:
- Around line 203-208: Update the API Extensions section to clarify that “API
extensions” refers only to Kubernetes API extensions, if that is the intended
scope. Explicitly classify the exported library-go symbols
VersionedConfigAccess, VersionedEntry, StatusExtractor, EqualityFunc,
NewVersionedConfigAccess, and NewHardcodedConfigAccess as public Go API, and add
API review, compatibility, and graduation criteria for them.
- Around line 190-201: Define an explicit retention policy for versioned status
entries in the configuration-access design, including a grace period or liveness
handshake that preserves entries needed by lagging, downgraded, or rollback
operators instead of relying only on ClusterVersion history. Update the
version-skew and pruning sections to reflect this policy, and add downgrade and
rollback test scenarios before GA.
- Around line 480-489: Clarify the missing-version startup flow in the
materializer documentation by stating whether the operator process retries,
restarts, or continues waiting after the timeout. Align the described outcome
with the actual behavior of Run(ctx) and InitialConfigObserved(), and remove the
contradictory claim that the same timed-out process later proceeds unless the
implementation supports it.
- Around line 259-264: Update the versioned configuration observation flow
documented by SetChangeHandler so the first resolved configuration initializes
state and closes InitialConfigObserved() without invoking the change handler.
Invoke the handler only for subsequent configuration changes, while preserving
the existing previous/current values for those changes. Add a test verifying the
initial observation closes InitialConfigObserved() and does not call the
handler.
- Around line 278-280: Define ownership semantics for the generic T
configuration values used by CurrentConfig and NewHardcodedConfigAccess: prevent
caller-owned slices, maps, and pointers from mutating cached state by requiring
immutable payloads or deep-copying values both when storing them and when
returning them. Apply the same guarantee to the additional configuration access
method referenced in the comment, while preserving equality and
change-notification behavior.
- Around line 359-371: Before implementing the floor-based lookup mechanism in
the materializer, establish the version comparison semantics that floor lookup
depends on. Define the version grammar and format rules, implement a version
parser and comparator that handles semantic versioning correctly (avoiding
lexical ordering pitfalls), specify how invalid versions and pre-release
versions are ordered, and add comprehensive boundary tests covering cases like
4.9 versus 4.10, pre-release versus release versions, and malformed version
strings. This foundational work must be completed and tested before the
materializer attempts to find the highest MinVersion entry less than or equal to
a requested version V.
---
Nitpick comments:
In `@enhancements/config/pki-versioned-config.md`:
- Around line 337-338: Rename the public PKIProfileAccess method
SetChangeHandler to a domain-specific name such as SetPKIProfileChangeHandler,
and update all references and documentation consistently. Preserve the existing
handler behavior and signature while aligning the API with the stated
domain-specific naming convention.
- Around line 596-599: Reword the three successive list items in the component
summary to avoid repeating “openshift/” at the start of each entry, while
preserving the referenced components and their technical descriptions.
- Around line 537-540: The versioned-config documentation should define a
normalization contract for operator-reported versions and status entries,
including handling of prefixes, build metadata, and partial versions. Update the
described accessor behavior to use the agreed normalized or floor-based lookup
semantics instead of requiring brittle exact string equality, while preserving
errors for genuinely unsupported versions.
In `@enhancements/config/tls-versioned-config.md`:
- Around line 101-108: Update the TLSProfiles compatibility documentation to
define “latest version” as the latest declaration compiled into the current
release, and explicitly state that older operators do not receive declarations
introduced only in newer releases.
- Around line 171-180: Update the TLS versioned configuration proposal to
explicitly state that APIServerStatus.TLSProfiles[].version values are copied
directly from ClusterVersion status history or desired.version without
normalization or transformation, and document the expected version-string format
used for exact matching.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8489d53c-0790-4f2a-9bc9-0a80c0d8e4d6
📒 Files selected for processing (4)
enhancements/config/featuregate-versioned-config.mdenhancements/config/pki-versioned-config.mdenhancements/config/tls-versioned-config.mdenhancements/config/versioned-config-access.md
| ```go | ||
| // FeatureGateAccess interface is UNCHANGED. All existing methods preserved. | ||
| type FeatureGateAccess interface { | ||
| SetChangeHandler(featureGateChangeHandlerFn FeatureGateChangeHandlerFunc) | ||
| Run(ctx context.Context) | ||
| InitialFeatureGatesObserved() <-chan struct{} | ||
| CurrentFeatureGates() (FeatureGate, error) | ||
| AreInitialFeatureGatesObserved() bool | ||
| } | ||
|
|
||
| // NewFeatureGateAccess signature is UNCHANGED. | ||
| func NewFeatureGateAccess( | ||
| desiredVersion, missingVersionMarker string, | ||
| clusterVersionInformer v1.ClusterVersionInformer, | ||
| featureGateInformer v1.FeatureGateInformer, | ||
| eventRecorder events.Recorder, | ||
| ) FeatureGateAccess { | ||
| // Internally delegates to: | ||
| inner := versionedconfig.NewVersionedConfigAccess[Features]( | ||
| desiredVersion, missingVersionMarker, | ||
| clusterVersionInformer.Informer(), | ||
| featureGateInformer.Informer(), | ||
| "cluster", | ||
| extractFeaturesFromFeatureGate, | ||
| featuresEqual, | ||
| eventRecorder, | ||
| ) | ||
| return &featureGateAdapter{inner: inner} | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target document ---'
sed -n '120,230p' enhancements/config/featuregate-versioned-config.md
printf '%s\n' '--- related files ---'
git ls-files | rg '(^|/)(featuregate|versioned.?config|config).*\\.(go|md)$' | head -200
printf '%s\n' '--- relevant symbols ---'
rg -n --glob '*.go' --glob '*.md' \
'type FeatureGateAccess|type FeatureGateChangeHandlerFunc|CurrentFeatureGates|SetChangeHandler|NewFeatureGateAccess|featuresEqual|extractFeaturesFromFeatureGate|NewVersionedConfigAccess|InitialFeatureGatesObserved|AreInitialFeatureGatesObserved' .Repository: openshift/enhancements
Length of output: 8213
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target document ---'
sed -n '120,230p' enhancements/config/featuregate-versioned-config.md
printf '%s\n' '--- related files ---'
git ls-files | rg '(^|/)(featuregate|versioned.?config|config).*\.(go|md)$' | head -200
printf '%s\n' '--- relevant symbols ---'
rg -n --glob '*.go' --glob '*.md' \
'type FeatureGateAccess|type FeatureGateChangeHandlerFunc|CurrentFeatureGates|SetChangeHandler|NewFeatureGateAccess|featuresEqual|extractFeaturesFromFeatureGate|NewVersionedConfigAccess|InitialFeatureGatesObserved|AreInitialFeatureGatesObserved' .Repository: openshift/enhancements
Length of output: 8696
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- feature-gate proposal overview and test plan ---'
sed -n '1,125p' enhancements/config/featuregate-versioned-config.md
sed -n '230,275p' enhancements/config/featuregate-versioned-config.md
printf '%s\n' '--- generic accessor contract ---'
sed -n '240,380p' enhancements/config/versioned-config-access.md
sed -n '380,475p' enhancements/config/versioned-config-access.md
printf '%s\n' '--- feature-gate API references in repository ---'
rg -n -C 5 --glob '*.md' \
'FeatureGateChangeHandlerFunc|type FeatureGate interface|panic-on-unknown|unknown feature|CurrentFeatureGates\(\)|AreInitialFeatureGatesObserved' .
printf '%s\n' '--- repository remotes ---'
git remote -vRepository: openshift/enhancements
Length of output: 28355
🏁 Script executed:
#!/bin/bash
set -eu
base='https://raw.githubusercontent.com/openshift/library-go/master/pkg/operator/configobserver/featuregates'
printf '%s\n' '--- existing feature-gate reader ---'
curl -fsSL "$base/simple_featuregate_reader.go" |
rg -n -C 8 \
'type FeatureGate|FeatureGateChangeHandlerFunc|func \(.*\) SetChangeHandler|func \(.*\) CurrentFeatureGates|func .*featuresEqual|func .*Enabled|func .*KnownFeatures|InitialFeatureGatesObserved|AreInitialFeatureGatesObserved|panic|not observed|zero'
printf '%s\n' '--- existing feature-gate tests ---'
for file in simple_featuregate_reader_test.go featuregate_reader_test.go; do
if curl -fsSL "$base/$file" > "/tmp/$file"; then
printf '%s\n' "--- $file ---"
rg -n -C 6 \
'SetChangeHandler|CurrentFeatureGates|Enabled|KnownFeatures|unknown|panic|observed|zero|equal|order|disabled' \
"/tmp/$file" || true
fi
done
printf '%s\n' '--- generic proposal behavior statements ---'
rg -n -C 4 \
'zero value|CurrentConfig\(\)|SetChangeHandler|InitialConfigObserved|equality|equal|order|slice' \
enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 20209
🏁 Script executed:
#!/bin/bash
set -eu
api='https://api.github.com/repos/openshift/library-go/contents/pkg/operator/configobserver/featuregates'
printf '%s\n' '--- feature-gates directory files ---'
curl -fsSL "$api" | jq -r '.[].name'
printf '%s\n' '--- reader implementation ---'
curl -fsSL \
'https://raw.githubusercontent.com/openshift/library-go/master/pkg/operator/configobserver/featuregates/simple_featuregate_reader.go' |
sed -n '1,290p'
printf '%s\n' '--- query wrapper candidates ---'
for file in featuregate.go feature_gate.go featuregate_types.go simple_featuregate.go; do
if curl -fsSL \
"https://raw.githubusercontent.com/openshift/library-go/master/pkg/operator/configobserver/featuregates/$file" \
> "/tmp/$file"; then
printf '%s\n' "--- $file ---"
rg -n -C 12 \
'type FeatureGate|func NewFeatureGate|func \(.*\) Enabled|func \(.*\) KnownFeatures|unknown|panic' \
"/tmp/$file" || true
fi
done
printf '%s\n' '--- complete relevant reader tests ---'
curl -fsSL \
'https://raw.githubusercontent.com/openshift/library-go/master/pkg/operator/configobserver/featuregates/simple_featuregate_reader_test.go' |
sed -n '1,390p'
printf '%s\n' '--- deterministic equality model ---'
python3 - <<'PY'
from dataclasses import dataclass
from typing import Optional
`@dataclass`
class Features:
enabled: Optional[list[str]]
disabled: Optional[list[str]]
def reflect_deep_equal(a: Features, b: Features) -> bool:
# Model Go reflect.DeepEqual for the two slice fields.
return a.enabled == b.enabled and a.disabled == b.disabled and (
(a.enabled is None) == (b.enabled is None)
and (a.disabled is None) == (b.disabled is None)
)
cases = [
("same order", Features(["A", "B"], []), Features(["A", "B"], [])),
("enabled reordered", Features(["A", "B"], []), Features(["B", "A"], [])),
("disabled reordered", Features([], ["A", "B"]), Features([], ["B", "A"])),
("nil vs empty enabled", Features(None, []), Features([], [])),
("nil vs empty disabled", Features([], None), Features([], [])),
]
for name, left, right in cases:
print(f"{name}: {reflect_deep_equal(left, right)}")
PYRepository: openshift/enhancements
Length of output: 22082
Define the featureGateAdapter contract before claiming behavioral compatibility.
Map generic callbacks to FeatureChange, including Previous == nil on first observation. Preserve the pre-initialization error, slice-copy behavior, NewFeatureGate panic-on-unknown semantics, post-Run setter panic, hardcoded initialization, and reflect.DeepEqual ordering and nil-vs-empty behavior. Add tests for these cases.
🤖 Prompt for 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.
In `@enhancements/config/featuregate-versioned-config.md` around lines 153 - 181,
The featureGateAdapter contract is incomplete unless it preserves the existing
FeatureGateAccess behavior. Define and implement its callback mapping to
FeatureChange with Previous nil on first observation, preserve
pre-initialization errors, slice copying, NewFeatureGate panic-on-unknown
behavior, post-Run setter panic, hardcoded initialization, and reflect.DeepEqual
ordering plus nil-versus-empty semantics; add tests covering each case.
| // payloadVersion. Operators must locate the version they are managing | ||
| // and use the resolved profile for certificate generation. | ||
| // A nil PKIProfile entry for a version means PKI is Unmanaged for that | ||
| // version, and operators should use their hardcoded defaults. | ||
| // +listType=map | ||
| // +listMapKey=version | ||
| // +optional | ||
| Profiles []VersionedPKIProfile `json:"profiles,omitempty"` | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Retention policy gap undermines the size-risk mitigation.
PKIStatus.Profiles has no defined pruning mechanism. Line 404 admits: "the exact cleanup timing needs definition." The risk mitigation at line 366-369 depends on "at most two version entries exist in status," but this claim only holds if pruning is implemented and timed correctly. Without a defined retention policy, entries for versions no longer present in ClusterVersion can accumulate indefinitely, especially across repeated upgrade/downgrade cycles or fleets with many concurrent version skews.
Resolve open question 2 before this proposal reaches Tech Preview. Define an explicit pruning trigger (e.g., "remove entries not present in ClusterVersion.status.history after N reconciliation cycles") so the size-risk mitigation is verifiable rather than aspirational.
Also applies to: 403-405
🤖 Prompt for 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.
In `@enhancements/config/pki-versioned-config.md` around lines 199 - 207, Define
and document an explicit retention and pruning policy for PKIStatus.Profiles,
including the trigger and timing for removing versions absent from
ClusterVersion.status.history. Update the related open question near the
Profiles definition and the statement around PKIStatus.Profiles so the claimed
maximum of two entries is backed by verifiable cleanup behavior.
| // tlsProfiles contains a list of resolved TLS security profiles keyed by | ||
| // payloadVersion. Operators must locate the version they are managing, | ||
| // find the resolved profile, and configure their operand accordingly. | ||
| // The resolved profile reflects the admin's selected profile type | ||
| // (Old, Intermediate, Modern, Custom) evaluated against the version-indexed | ||
| // declaration table for each version. | ||
| // +listType=map | ||
| // +listMapKey=version | ||
| // +optional | ||
| TLSProfiles []VersionedTLSProfile `json:"tlsProfiles,omitempty"` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use version consistently in the status contract.
The status comment says the list is keyed by payloadVersion, but the map key and struct field are version. Replace payloadVersion with version and use the same term in the materializer and accessor sections.
🤖 Prompt for 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.
In `@enhancements/config/tls-versioned-config.md` around lines 209 - 218, Update
the TLSProfiles status contract comments to use “version” consistently instead
of “payloadVersion,” including the list-key description and the materializer and
accessor sections. Keep the +listMapKey=version annotation and
VersionedTLSProfile field naming unchanged.
| #### Version-Indexed Declaration Table | ||
|
|
||
| The static `var TLSProfiles` map is replaced by a version-indexed declaration | ||
| table. Only versions where profiles change need entries; the lookup uses | ||
| floor-based resolution (the highest `MinVersion` entry at or below the | ||
| requested version): | ||
|
|
||
| ```go | ||
| // Pseudocode for the version-indexed TLS declaration table. | ||
| // Actual implementation uses the features/util.go builder pattern. | ||
| var VersionedTLSProfiles = map[TLSProfileType][]VersionedTLSDeclaration{ | ||
| TLSProfileIntermediateType: { | ||
| {MinVersion: "4.18", Spec: intermediateSpec_4_18}, | ||
| {MinVersion: "4.22", Spec: intermediateSpec_4_22}, // adds PQC groups | ||
| }, | ||
| // Old, Modern follow the same pattern | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file at the specified lines
find . -name "tls-versioned-config.md" -type fRepository: openshift/enhancements
Length of output: 206
🏁 Script executed:
# Read the file at the specified line ranges to understand context
if [ -f "enhancements/config/tls-versioned-config.md" ]; then
echo "=== Lines 286-303 (Version-Indexed Declaration Table) ===" && \
sed -n '286,303p' enhancements/config/tls-versioned-config.md && \
echo -e "\n=== Lines 330-341 (missingVersionMarker context) ===" && \
sed -n '330,341p' enhancements/config/tls-versioned-config.md && \
echo -e "\n=== Lines 404-406 (test context) ===" && \
sed -n '404,406p' enhancements/config/tls-versioned-config.md
else
echo "File not found"
fiRepository: openshift/enhancements
Length of output: 1774
🏁 Script executed:
# Search for missingVersionMarker in the entire file to understand its definition and usage
rg -n "missingVersionMarker" enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 213
🏁 Script executed:
# Search for behavior definitions related to missing/invalid versions, floor lookup, or no-match cases
rg -n -i "(no-match|missing.*version|malformed|unparsable|fallback|below.*first)" enhancements/config/tls-versioned-config.md | head -30Repository: openshift/enhancements
Length of output: 253
🏁 Script executed:
# Get line count and overall structure to understand the document scope
wc -l enhancements/config/tls-versioned-config.md && \
echo -e "\n=== TOC/Outline ===" && \
head -100 enhancements/config/tls-versioned-config.md | grep -E "^(#{1,4}|---)"Repository: openshift/enhancements
Length of output: 312
🏁 Script executed:
# Read broader context around lines 330-341 to understand how missingVersionMarker is used
sed -n '320,360p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 1833
🏁 Script executed:
# Check what "missing desired version" errors are mentioned at line 538
sed -n '535,545p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 649
🏁 Script executed:
# Search for error handling, fail, close, or startup behavior sections
rg -n -i "(error.*handling|fail.*clos|startup|invalid.*version|below.*first|edge.*case)" enhancements/config/tls-versioned-config.md | head -20Repository: openshift/enhancements
Length of output: 974
🏁 Script executed:
# Look for behavior when version is not found or floor lookup fails
rg -n -B2 -A2 "floor|not found|fallback" enhancements/config/tls-versioned-config.md | head -40Repository: openshift/enhancements
Length of output: 997
🏁 Script executed:
# Read test section completely (around 404-406) for context on what tests are planned
sed -n '395,425p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 1527
🏁 Script executed:
# Search for more context on missingVersionMarker usage and VersionedConfigAccess behavior
rg -n -B3 -A5 "VersionedConfigAccess" enhancements/config/tls-versioned-config.md | head -50Repository: openshift/enhancements
Length of output: 2791
🏁 Script executed:
# Look for explicit behavior definition around what happens when no floor entry exists
sed -n '275,325p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 1859
🏁 Script executed:
# Check if there's any discussion of edge cases or undefined behavior in Risks/Mitigations
sed -n '355,395p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 2119
🏁 Script executed:
# Check if versioned-config-access.md is referenced and might contain the missing-version behavior
find . -name "versioned-config-access.md" -type fRepository: openshift/enhancements
Length of output: 209
🏁 Script executed:
# Look for any other mentions of version resolution, edge cases, or undefined behavior in the current doc
rg -n -i "(undefined|unspecified|tbd|todo|future|may|need.*to|should)" enhancements/config/tls-versioned-config.md | head -20Repository: openshift/enhancements
Length of output: 880
🏁 Script executed:
# Check what the document says about default/fallback behavior more broadly
sed -n '510,560p' enhancements/config/tls-versioned-config.mdRepository: openshift/enhancements
Length of output: 2431
🏁 Script executed:
# Read versioned-config-access.md to see if it defines missing-version behavior
wc -l enhancements/config/versioned-config-access.md && \
echo "=== File found ===" && \
head -50 enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 2297
🏁 Script executed:
# Search for missing version, no match, or fallback behavior in versioned-config-access.md
rg -n -i "(missing|not found|no.*match|fallback|below|invalid)" enhancements/config/versioned-config-access.md | head -30Repository: openshift/enhancements
Length of output: 810
🏁 Script executed:
# Look for any error handling or edge case discussion in versioned-config-access.md
rg -n -B2 -A2 "(error|edge|fail|undefined|behavior)" enhancements/config/versioned-config-access.md | head -50Repository: openshift/enhancements
Length of output: 2351
🏁 Script executed:
# Read the exact section in versioned-config-access.md that defines the missing version behavior
sed -n '300,365p' enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 3324
🏁 Script executed:
# Check the accessor's initialization and error handling contract more carefully
sed -n '265,295p' enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 1111
🏁 Script executed:
# Check if there's any explicit discussion of what happens when version is below all entries
sed -n '330,380p' enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 2632
🏁 Script executed:
# Look for the complete floor-based lookup explanation in versioned-config-access.md
rg -n -B5 -A10 "highest.*MinVersion" enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 160
🏁 Script executed:
# Check what the test plan actually covers for edge cases
sed -n '410,450p' enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 1654
Specify floor-lookup behavior for versions below the minimum declared entry and add tests for malformed inputs.
The proposal documents that the materializer performs floor-based resolution to find the highest MinVersion entry at or below the requested version. However, it does not explicitly specify behavior when the requested version falls below all declared entries. Additionally, the test plan covers floor lookup for versions between entries (requesting 4.20 with entries at 4.18 and 4.22) but does not test the minimum-version boundary or malformed version strings.
Add explicit specification of the error condition when no floor entry exists and define handling for unparsable version inputs. Extend the test plan to cover these edge cases.
🤖 Prompt for 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.
In `@enhancements/config/tls-versioned-config.md` around lines 286 - 303, Update
the VersionedTLSProfiles floor-lookup specification to state that requests below
the minimum declared MinVersion return an error, and define the error behavior
for unparsable requested or declaration versions. Extend the associated test
plan to cover below-minimum lookups, the exact minimum boundary, and malformed
version strings.
| #### Materializer Controller | ||
|
|
||
| A new controller (~100 lines) in cluster-config-operator: | ||
|
|
||
| 1. Watches the APIServer spec for `spec.tlsSecurityProfile.type` changes. | ||
| 2. Reads all versions from `ClusterVersion.status.history` and | ||
| `ClusterVersion.status.desired.version`. | ||
| 3. For each version, performs a floor-based lookup in the declaration table | ||
| to find the appropriate profile. | ||
| 4. Writes resolved entries into `apiserver.status.tlsProfiles[]`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/config/tls-versioned-config.md"
printf '%s\n' '--- target sections ---'
sed -n '280,340p' "$file"
sed -n '390,445p' "$file"
printf '%s\n' '--- related references ---'
rg -n -i --glob '!vendor/**' \
'tlsSecurityProfile|tlsProfiles|CurrentTLSProfile|Materializer|materializer|Custom|cipher suites|minimum TLS|groups' .Repository: openshift/enhancements
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/config/tls-versioned-config.md"
for range in 120,165 200,245 288,330 345,375 390,430 510,545; do
sed -n "${range}p" "$file"
printf '\n---\n'
done
printf '%s\n' '--- analogous PKI materializer sections ---'
pki="enhancements/config/pki-versioned-config.md"
for range in 135,160 295,325 400,435 525,540; do
sed -n "${range}p" "$pki"
printf '\n---\n'
doneRepository: openshift/enhancements
Length of output: 15882
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/config/tls-versioned-config.md"
printf '%s\n' '--- TLS scope and API shape ---'
sed -n '1,125p' "$file"
sed -n '185,245p' "$file"
printf '%s\n' '--- all Custom-related TLS statements ---'
rg -n -C 3 'Custom|custom|TLSProfileSpec|cipherSuites|minTLSVersion|groups|ResolveTLSProfile' "$file"
printf '%s\n' '--- repository definitions or examples ---'
rg -n -C 2 --glob '*.go' --glob '*.yaml' --glob '*.yml' --glob '*.md' \
'type TLSProfileSpec|TLSProfileSpec struct|TLSProfileType|tlsSecurityProfile' \
--glob '!enhancements/config/tls-versioned-config.md' .Repository: openshift/enhancements
Length of output: 17497
🏁 Script executed:
#!/bin/bash
set -eu
file="enhancements/kube-apiserver/tls-config.md"
sed -n '70,180p' "$file"
printf '\n--- Custom examples and current profile map ---\n'
sed -n '235,275p' "$file"
printf '\n--- versioned-config watcher/reconciliation contract ---\n'
sed -n '300,430p' enhancements/config/versioned-config-access.mdRepository: openshift/enhancements
Length of output: 12082
Materialize and reconcile the complete spec.tlsSecurityProfile.
For Custom, copy spec.tlsSecurityProfile.custom (Ciphers, TLSVersion, and DHParamSize) into every status.tlsProfiles[] entry. Watching only .type leaves status.tlsProfiles and CurrentTLSProfile() stale when a Custom field changes. Define validation and error status for invalid or missing Custom values.
🤖 Prompt for 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.
In `@enhancements/config/tls-versioned-config.md` around lines 312 - 321, Update
the Materializer Controller to watch and reconcile the complete
spec.tlsSecurityProfile, not only its type. For Custom profiles, copy
custom.Ciphers, custom.TLSVersion, and custom.DHParamSize into every
status.tlsProfiles[] entry and ensure CurrentTLSProfile() reflects changes. Add
validation and error status handling for missing or invalid Custom values.
| ### API Extensions | ||
|
|
||
| This enhancement adds no API extensions. It is purely a library-go | ||
| infrastructure package. Domain-specific CRD status changes are documented in | ||
| the respective domain enhancements | ||
| ([TLS](tls-versioned-config.md), [PKI](pki-versioned-config.md)). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Treat the new library-go symbols as a public API.
The proposal says that this enhancement adds no API surface. It adds exported types and constructors: VersionedConfigAccess, VersionedEntry, StatusExtractor, EqualityFunc, NewVersionedConfigAccess, and NewHardcodedConfigAccess. Define API review, compatibility, and graduation criteria for this public Go package. If “API extensions” means Kubernetes API extensions only, state that scope.
Also applies to: 440-445
🤖 Prompt for 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.
In `@enhancements/config/versioned-config-access.md` around lines 203 - 208,
Update the API Extensions section to clarify that “API extensions” refers only
to Kubernetes API extensions, if that is the intended scope. Explicitly classify
the exported library-go symbols VersionedConfigAccess, VersionedEntry,
StatusExtractor, EqualityFunc, NewVersionedConfigAccess, and
NewHardcodedConfigAccess as public Go API, and add API review, compatibility,
and graduation criteria for them.
| // SetChangeHandler sets the function called when the resolved config | ||
| // changes. Must be called before Run. The default handler calls | ||
| // os.Exit(0), which causes the operator to restart and pick up the | ||
| // new configuration. On the first observation, the change handler is | ||
| // called with the zero value of T as the previous value. | ||
| SetChangeHandler(fn func(previous, current T)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Do not call the change handler for the initial observation.
Line [261] defines the default handler as os.Exit(0). Lines [263]-[264] state that the first observation calls this handler with the zero value. This causes operators that use the default handler to exit during normal startup. Call the handler only after initialization, or define a separate initial-observation callback. Add a test that the first observation closes InitialConfigObserved() without invoking the change handler.
🤖 Prompt for 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.
In `@enhancements/config/versioned-config-access.md` around lines 259 - 264,
Update the versioned configuration observation flow documented by
SetChangeHandler so the first resolved configuration initializes state and
closes InitialConfigObserved() without invoking the change handler. Invoke the
handler only for subsequent configuration changes, while preserving the existing
previous/current values for those changes. Add a test verifying the initial
observation closes InitialConfigObserved() and does not call the handler.
| // CurrentConfig returns the current version-matched configuration. | ||
| // Returns an error if the config has not yet been observed. | ||
| CurrentConfig() (T, error) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define ownership for T values.
Returning T by value does not copy slices, maps, or pointers. The FeatureGate payload contains slices, and NewHardcodedConfigAccess accepts caller-owned values. A caller can mutate cached configuration without an informer event, so equality and change notification no longer describe a stable value. Require immutable payloads or deep-copy values at storage and return boundaries.
Also applies to: 326-329
🤖 Prompt for 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.
In `@enhancements/config/versioned-config-access.md` around lines 278 - 280,
Define ownership semantics for the generic T configuration values used by
CurrentConfig and NewHardcodedConfigAccess: prevent caller-owned slices, maps,
and pointers from mutating cached state by requiring immutable payloads or
deep-copying values both when storing them and when returning them. Apply the
same guarantee to the additional configuration access method referenced in the
comment, while preserving equality and change-notification behavior.
| #### Sparse Version Tables and Floor-Based Lookup | ||
|
|
||
| Not every version needs an entry in the declaration table. The materializer | ||
| uses floor-based resolution: for a requested version `V`, it finds the highest | ||
| `MinVersion` entry that is less than or equal to `V`. This means that if a | ||
| domain's configuration changes in 4.22, only a `MinVersion: "4.22"` entry is | ||
| needed; all versions from 4.18 through 4.21 automatically resolve to the 4.18 | ||
| entry. | ||
|
|
||
| This is a property of the **materializer**, not the accessor. The accessor | ||
| always does exact version matching on the status entries. The materializer is | ||
| responsible for writing an entry for every version that the cluster knows about, | ||
| even if multiple versions resolve to the same configuration. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(versioned-config-access\.md|.*(config|materializ|version).*)$' | head -200
printf '%s\n' '--- referenced documentation ---'
sed -n '320,390p' enhancements/config/versioned-config-access.md
printf '%s\n' '--- version and materializer references ---'
rg -n -i 'floor|MinVersion|materializ|version(ed)? config|semantic.?version|semver|accessor|pre-release' . \
-g '!node_modules' -g '!dist' -g '!build' | head -300Repository: openshift/enhancements
Length of output: 35947
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- versioned-config-access declaration and test requirements ---'
sed -n '160,235p' enhancements/config/versioned-config-access.md
sed -n '390,490p' enhancements/config/versioned-config-access.md
printf '%s\n' '--- PKI version declaration and tests ---'
sed -n '280,325p' enhancements/config/pki-versioned-config.md
sed -n '395,435p' enhancements/config/pki-versioned-config.md
printf '%s\n' '--- TLS version declaration and tests ---'
sed -n '280,330p' enhancements/config/tls-versioned-config.md
sed -n '395,425p' enhancements/config/tls-versioned-config.md
printf '%s\n' '--- existing semantic-version parsing/comparison conventions ---'
rg -n -i 'Parse.*[Vv]ersion|semver|semantic version|go-semver|version\.Compare|Compare.*version|version.*Compare|blang/semver|Masterminds/semver' \
--glob '*.go' --glob '*.md' --glob 'go.mod' --glob 'go.sum' . | head -250Repository: openshift/enhancements
Length of output: 20114
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- ClusterVersion version format and validation ---'
sed -n '45,95p' dev-guide/cluster-version-operator/user/status.md
rg -n -i 'desired\.version|history.*version|version.*history|valid.*version|semver|semantic' \
dev-guide/cluster-version-operator enhancements/config --glob '*.md' | head -200
printf '%s\n' '--- module metadata for version libraries ---'
find . -maxdepth 3 \( -name 'go.mod' -o -name 'go.sum' \) -print
for f in $(find . -maxdepth 3 \( -name 'go.mod' -o -name 'go.sum' \) -print); do
printf '%s\n' "--- $f ---"
rg -n 'semver|version' "$f" | head -80 || true
done
printf '%s\n' '--- deterministic ordering probe ---'
python3 - <<'PY'
from functools import cmp_to_key
values = ["4.9", "4.10", "4.18", "4.22", "4.22-rc.1", "4.22.1", "not-a-version"]
print("lexical:", sorted(values))
def parse(v):
core, sep, pre = v.partition("-")
parts = core.split(".")
if len(parts) != 2 or any(not p.isdigit() for p in parts):
raise ValueError(v)
return (int(parts[0]), int(parts[1]), pre)
def compare(a, b):
pa, pb = parse(a), parse(b)
if pa[:2] != pb[:2]:
return (pa[:2] > pb[:2]) - (pa[:2] < pb[:2])
if not pa[2] or not pb[2]:
return (not pa[2]) - (not pb[2])
return (pa[2] > pb[2]) - (pa[2] < pb[2])
print("numeric core:", sorted(values[:-1], key=cmp_to_key(compare)))
try:
parse("not-a-version")
except ValueError as exc:
print("invalid input:", exc)
PYRepository: openshift/enhancements
Length of output: 5008
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository version-library metadata ---'
cat tools/go.mod
printf '%s\n' '--- tracked implementation candidates ---'
git ls-files | rg '(^|/)(.*(materializ|versioned.?config|featuregate|cluster.*version).*\.(go|yaml|yml))$' | head -200
printf '%s\n' '--- corrected boundary probe ---'
python3 - <<'PY'
from functools import cmp_to_key
values = ["4.9", "4.10", "4.18", "4.22", "4.22-rc.1", "4.22.1"]
print("lexical:", sorted(values))
def parse_major_minor(v):
core, _, prerelease = v.partition("-")
parts = core.split(".")
if len(parts) not in (2, 3) or any(not p.isdigit() for p in parts):
raise ValueError(v)
numbers = tuple(map(int, parts))
return numbers, prerelease
def compare(a, b):
na, prea = parse_major_minor(a)
nb, preb = parse_major_minor(b)
# This probe only demonstrates numeric core ordering and invalid-input handling.
if na != nb:
return (na > nb) - (na < nb)
if not prea or not preb:
return (not prea) - (not preb)
return (prea > preb) - (prea < preb)
print("numeric ordering:", sorted(values, key=cmp_to_key(compare)))
for candidate in ("not-a-version", "4", "4.x"):
try:
parse_major_minor(candidate)
except ValueError:
print("invalid rejected:", candidate)
PYRepository: openshift/enhancements
Length of output: 789
Define and test version ordering before implementing floor lookup. Specify the version grammar, parser, comparator, invalid-version behavior, and pre-release ordering. Add boundary tests for 4.9 versus 4.10, pre-release versus release, and malformed versions. Lexical ordering can select the wrong floor.
🤖 Prompt for 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.
In `@enhancements/config/versioned-config-access.md` around lines 359 - 371,
Before implementing the floor-based lookup mechanism in the materializer,
establish the version comparison semantics that floor lookup depends on. Define
the version grammar and format rules, implement a version parser and comparator
that handles semantic versioning correctly (avoiding lexical ordering pitfalls),
specify how invalid versions and pre-release versions are ordered, and add
comprehensive boundary tests covering cases like 4.9 versus 4.10, pre-release
versus release versions, and malformed version strings. This foundational work
must be completed and tested before the materializer attempts to find the
highest MinVersion entry less than or equal to a requested version V.
| 3. **The materializer runs ahead of operators.** Because cluster-config-operator | ||
| is updated early in the upgrade process (before most other operators), the | ||
| status entries for the new version are available before operators at the new | ||
| version start looking for them. | ||
|
|
||
| 4. **Missing version entries cause controlled startup delays, not crashes.** If | ||
| an operator starts before its version entry is available in status, the | ||
| accessor's `InitialConfigObserved` channel blocks. The operator's standard | ||
| timeout (typically 1 minute) converts this into a clear error. The | ||
| materializer will eventually write the entry, and the operator will proceed. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Define retry behavior after a missing status entry.
Lines [485]-[489] say that a one-minute timeout produces an error, then say that the operator will proceed when materialization completes. If the timeout terminates the process, that process cannot observe the later update. Specify whether startup retries, restarts, or continues waiting. Align this behavior with Run(ctx) and InitialConfigObserved().
🤖 Prompt for 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.
In `@enhancements/config/versioned-config-access.md` around lines 480 - 489,
Clarify the missing-version startup flow in the materializer documentation by
stating whether the operator process retries, restarts, or continues waiting
after the timeout. Align the described outcome with the actual behavior of
Run(ctx) and InitialConfigObserved(), and remove the contradictory claim that
the same timed-out process later proceeds unless the implementation supports it.
…rover @deads2k is in another org at the moment. @JoelSpeed is the active openshift/api approver and enhancement reviewer. Also removes duplicate @JoelSpeed entry from TLS EP api-approvers and updates reviewer descriptions to match @JoelSpeed's domain expertise.
1 participant
Summary
Four related enhancement proposals for a generalized
VersionedConfigAccess[T]pattern that unifies how OpenShift handles version-evolving configuration defaults.Note: These are bundled in a single PR for early feedback on the overall approach. They would normally be separate PRs, one per enhancement.
Enhancement Documents
VersionedConfigAccess[T]interface in library-goFeatureGateAccessonto the generic as validationAPIServerStatusadditionsconfig/v1PKIStatusadditionsconfig/v1alpha1Problem
Three configuration domains share the same fundamental problem: an admin selects an intent-based profile (FeatureSet, TLS profile type, PKI management mode), and the platform must resolve that intent into concrete values appropriate for the cluster's version. Today:
var TLSProfilesmap with no version indexing, no status, and no accessorEach domain independently reinvents (or fails to implement) the same three-layer pattern: declare, materialize, access.
Key Benefits
oc get apiserver cluster -o yamland see exactly what "Intermediate" TLS resolves to for their versionSummary by CodeRabbit
Documentation