WIP: resourceread: enable strict decoding for release payload manifests - #1444
WIP: resourceread: enable strict decoding for release payload manifests#1444machine424 wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
/pipeline required |
|
Scheduling tests matching the |
WalkthroughThe codec factories in resource reading and operator status handling now enable strict serialization. Decoding rejects unknown or invalid fields that non-strict decoding previously tolerated. ChangesStrict codec decoding
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 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: machine424 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/resourceread/resourceread.go`:
- Line 24: Update hack/generate-lib-resources.py so its generated
NewCodecFactory call includes serializer.EnableStrict, then regenerate
lib/resourceread/resourceread.go to reflect the generator output.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c9a8853c-ffec-475d-80b5-a4cab3aa3653
📒 Files selected for processing (2)
lib/resourceread/resourceread.gopkg/cvo/internal/operatorstatus.go
| var ( | ||
| scheme = runtime.NewScheme() | ||
| codecs = serializer.NewCodecFactory(scheme) | ||
| codecs = serializer.NewCodecFactory(scheme, serializer.EnableStrict) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'codecs = serializer.NewCodecFactory\(scheme, serializer.EnableStrict\)' \
hack/generate-lib-resources.py \
lib/resourceread/resourceread.goRepository: openshift/cluster-version-operator
Length of output: 172
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- tracked candidates ---'
git ls-files 'lib/resourceread/resourceread.go' 'hack/generate-lib-resources.py'
printf '%s\n' '--- generated file context ---'
nl -ba lib/resourceread/resourceread.go | sed -n '1,45p'
printf '%s\n' '--- generator references ---'
rg -n -C 4 'CodecFactory|EnableStrict|resourceread|generate' hack/generate-lib-resources.pyRepository: openshift/cluster-version-operator
Length of output: 334
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- generated file context ---'
cat -n lib/resourceread/resourceread.go | sed -n '1,45p'
printf '%s\n' '--- generator references ---'
rg -n -C 4 'CodecFactory|EnableStrict|resourceread|generate' hack/generate-lib-resources.pyRepository: openshift/cluster-version-operator
Length of output: 5422
Update the generator before regenerating the generated file.
hack/generate-lib-resources.py emits serializer.NewCodecFactory(scheme) while lib/resourceread/resourceread.go uses serializer.EnableStrict. Add the option to the generator, then regenerate the file.
🤖 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 `@lib/resourceread/resourceread.go` at line 24, Update
hack/generate-lib-resources.py so its generated NewCodecFactory call includes
serializer.EnableStrict, then regenerate lib/resourceread/resourceread.go to
reflect the generator output.
Source: Coding guidelines
|
@machine424: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Summary by CodeRabbit