EC-1694: feat(olm): add date-based FIPS annotation exemption for legacy bundles#1703
Open
robnester-rh wants to merge 1 commit intoconforma:mainfrom
Open
EC-1694: feat(olm): add date-based FIPS annotation exemption for legacy bundles#1703robnester-rh wants to merge 1 commit intoconforma:mainfrom
robnester-rh wants to merge 1 commit intoconforma:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds RFC3339 validation for CSV Changes
Sequence DiagramsequenceDiagram
participant CSV as ClusterServiceVersion
participant Parser as RFC3339 Parser
participant Config as Rule-data (cutoff)
participant Policy as OLM Policy
CSV->>Parser: read metadata.annotations.createdAt
Parser-->>CSV: parsed timestamp / null
alt parsed == null
Policy->>CSV: emit olm.malformed_created_at (warn)
Policy->>Policy: treat as non-exempt → enforce FIPS check
else parsed valid
Parser->>Config: request fips_exempt_created_before
Config-->>Policy: return cutoff timestamp
Policy->>Parser: compare createdAt vs cutoff
alt createdAt < cutoff
Policy-->>CSV: mark as FIPS-exempt (skip FIPS-value check)
else
Policy-->>CSV: enforce FIPS feature annotation requirement
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@policy/release/olm/olm.rego`:
- Around line 679-695: The current logic silently treats an invalid
lib.rule_data("fips_exempt_created_before") as non-parsable and simply causes
the exemption to be false; instead, add explicit validation for the cutoff
value: in _fips_exempt_legacy_bundle (which currently reads cutoff_date :=
lib.rule_data("fips_exempt_created_before")), assert cutoff_date is a non-empty
string and that _parse_rfc3339_safe(cutoff_date) returns a non-null value before
calling _date_before_cutoff; if parsing fails, emit an explicit configuration
error (e.g., a clear deny or config validation rule) referencing the bad
override so misconfiguration surfaces immediately rather than silently disabling
the exemption.
- Around line 115-117: The rule currently uses created_at :=
object.get(manifest.metadata.annotations, _created_at_annotation, "") which
conflates a missing annotation and an explicitly empty string; change the lookup
to use a null default (e.g. created_at :=
object.get(manifest.metadata.annotations, _created_at_annotation, null)) and
then require that created_at is not null (annotation present) before validating
its format with _parse_rfc3339_safe(created_at) == null so that
present-but-empty values trigger the olm.malformed_created_at warning while
genuinely-missing annotations are treated separately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fdc9a29-e89d-4da2-8c47-5fae09ede2fd
📒 Files selected for processing (6)
antora/docs/modules/ROOT/pages/packages/release_olm.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/lib/rule_data.regopolicy/release/olm/olm.regopolicy/release/olm/olm_test.rego
robnester-rh
force-pushed
the
EC-1694
branch
3 times, most recently
from
3b484de to
ed52aab
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
robnester-rh
force-pushed
the
EC-1694
branch
3 times, most recently
from
a962436 to
aa6b359
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@policy/release/olm/olm.rego`:
- Around line 407-414: The rule iterates "some path, manifest in
input.image.files" and hard-codes endswith(path, ".yaml"), bypassing the
existing CSV manifest selector and causing inconsistent warnings; update the
comprehension to iterate the canonical CSV manifest collection (use
_csv_manifests as the source instead of input.image.files) and drop the
hard-coded ".yaml" path check so the rule only evaluates manifests already
selected/filtered by _csv_manifests (leave the remaining logic using
manifest.metadata.annotations, _parse_rfc3339_safe(created_at), and
metadata.result_helper intact).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc31128d-b539-4b41-a56e-6ad1c4c7ee54
📒 Files selected for processing (6)
antora/docs/modules/ROOT/pages/packages/release_olm.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/lib/rule_data/rule_data.regopolicy/release/olm/olm.regopolicy/release/olm/olm_test.rego
✅ Files skipped from review due to trivial changes (3)
- antora/docs/modules/ROOT/pages/release_policy.adoc
- antora/docs/modules/ROOT/partials/release_policy_nav.adoc
- policy/release/olm/olm_test.rego
🚧 Files skipped from review as they are similar to previous changes (2)
- policy/lib/rule_data/rule_data.rego
- antora/docs/modules/ROOT/pages/packages/release_olm.adoc
Bundles created before the FIPS compliance requirement date (default: 2025-01-31) are now exempt from FIPS annotation checks. The cutoff date is configurable via the fips_exempt_created_before rule data. Also adds a warning when the createdAt annotation has an invalid RFC3339 format, and validates that the fips_exempt_created_before rule data is a valid RFC3339 timestamp when provided. Co-authored-by: Cursor <noreply@cursor.com> Made-with: Cursor
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createdAtannotation to determine bundle creation datecreatedAthas invalid RFC3339 format (fail closed for security)fips_exempt_created_beforerule_dataBackground
Per EC-1694, bundles created before the FIPS compliance requirement date should be exempt from the FIPS annotation check. The implementation uses date-based exemption (via
createdAtannotation) rather than OCP version-based exemption, since the FIPS requirement applies to all OCP versions.Test plan
createdAtannotation is NOT exemptmake fmtpasses