Skip to content

Skip pre-tier-model versions in PLCC validators#27

Merged
fgiudici merged 1 commit into
release-engineering:mainfrom
kyle-walker:skip-eol-versions-only
Jul 3, 2026
Merged

Skip pre-tier-model versions in PLCC validators#27
fgiudici merged 1 commit into
release-engineering:mainfrom
kyle-walker:skip-eol-versions-only

Conversation

@kyle-walker

@kyle-walker kyle-walker commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Skip tier-specific validation for pre-tier-model versions

The three-tier lifecycle model (Platform Aligned / Platform Agnostic /
Rolling Stream) was introduced with OCP 4.14 GA on 2023-10-31. Versions
whose earliest phase predates this cutoff should not be held to tier
requirements that did not exist when they were published.

Add isPreTierModel(v) helper that checks whether a version's earliest
parseable phase start date falls before TierModelCutoffDate. Apply the
skip to all tier-specific validators (REQ-TIER-*, REQ-FIELD-02,
CUSTOM-04) while preserving universal invariants (date validation,
version name format, contiguity) for all versions regardless of age.

Assisted-by: Claude
Signed-off-by: Kyle Walker kwalker@redhat.com

@asergienk

Copy link
Copy Markdown
Collaborator

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:13 PM UTC · Completed 1:26 PM UTC
Commit: feb9bda · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [missing-authorization] pkg/plcc/validation.go — Non-trivial change (42+ lines, structural modification to 15 validators) with no linked issue. Changes add EOL version skipping behavior across the entire validator layer, which represents a functional change to data quality validation scope.
    Remediation: Link this PR to an issue that authorizes the scope change.

  • [missing-doc] docs/VALIDATION_RULES.md — The semantic and syntax validators sections document 15+ validators that check version-level requirements. This PR modifies these validators to skip versions with Type "End of life" via the new isVersionEOL() helper, but the documentation does not mention this skipping behavior. Users reading the validation rules would not understand that EOL versions are excluded from validation.
    Remediation: Add a note in VALIDATION_RULES.md explaining that validators skip versions with Type "End of life".

  • [constant-reuse-semantics] pkg/plcc/validation.go:624isVersionEOL compares Version.Type against PhaseEndOfLife, a constant defined for phase names. The constant value is "End of Life" (capital L) while the actual API Version.Type value may be "End of life" (lowercase l). The EqualFold comparison handles this mismatch but creates semantic coupling: PhaseEndOfLife is documented as a PLCC phase name constant yet is being used for a version type comparison.
    Remediation: Define a separate VersionTypeEOL constant (e.g., const VersionTypeEOL = "End of life") even if the string value is similar, to decouple version type semantics from phase name semantics.

Low

  • [architectural-coherence] pkg/plcc/validation.go — The change adds lifecycle-status-based skipping (EOL) within the validator layer. Per CLAUDE.md, validators check "data quality on raw plcc.Product values before FBC translation". Skipping versions by lifecycle status could be seen as a filtering concern rather than a data quality concern. However, existing validators already skip versions based on tier (Aligned, Agnostic, Rolling) which is also a lifecycle property, so this follows established patterns.

  • [test-inadequate] pkg/plcc/validation_test.go:123TestEOLVersionsSkipped verifies that all DefaultValidators pass on an EOL-only product, but the test fixture does not trigger every validator even without the EOL guard. For example, ValidateVersionNames passes because "1.0" is valid MAJOR.MINOR, and ValidatePhaseEndAfterStart passes because the dates are unparseable. The test does catch several validators (ValidateDatesStatic, ValidateDatesClean, ValidatePlatformAlignedPhases, ValidatePlatformAlignedOCP) but not all 15.
    Remediation: Use data that would fail more validators without the EOL guard (e.g., bad version name, invalid OCP format string) to strengthen coverage.


Labels: PR adds EOL version skipping feature to PLCC validators and has findings noting missing documentation updates.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread pkg/plcc/validation.go Outdated
return reasons
}

func isVersionEOL(v Version) bool {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] constant-reuse-semantics

isVersionEOL compares Version.Type against PhaseEndOfLife, a constant defined for phase names. The constant value is 'End of Life' (capital L) while the actual API Version.Type value may be 'End of life' (lowercase l). The EqualFold comparison handles this mismatch but creates semantic coupling: PhaseEndOfLife is documented as a PLCC phase name constant yet is being used for a version type comparison.

Suggested fix: Define a separate VersionTypeEOL constant (e.g., const VersionTypeEOL = "End of life") even if the string value is similar, to decouple version type semantics from phase name semantics.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure, since the field is a string we would catch EOL strings with different capitalization. I would leave it as it is.

Comment thread pkg/plcc/validation_test.go
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment enhancement New feature or request labels Jul 1, 2026

@fgiudici fgiudici left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR looks good here! Thanks @kyle-walker !
Would be worth to update docs/VALIDATION_RULES.md and adding few more tests, but something we can take care later, fine to merge from my side!

@asergienk asergienk marked this pull request as draft July 1, 2026 14:47
@asergienk

Copy link
Copy Markdown
Collaborator

@kyle-walker FYI: Converted to Draft - let's hold off on merging based on the concerns that were brought up in the weekly sync call. Francesco will start a thread in #wg-update-planner.

@joelanford

Copy link
Copy Markdown
Contributor

I'm curious how this would change the actual FBC that is produced? We still need EoL versions to be present in the FBC so that we can inform customers they are using EoL versions. And I believe we have some semantic invariants that we've promised to consumers of the FBC-ified lifecycle data:

  1. Each version has at least one phase
  2. All dates of all phases are static and follow a standard date format
  3. All phases are contiguous and non-overlapping.

@perdasilva correct me if I'm wrong, but consumers will report a version as end of life if the current date is greater than the end date of the last phase.

@kyle-walker

Copy link
Copy Markdown
Contributor Author

Just to echo, the thought here was primarily to avoid a large number of validation failures due to older definitions that didn't adhere to various modern standards. Since the majority of the failures that I was trying to avoid where specifically around tiering, and that only was introduced with 4.14, changing this to limit the Tier validations to be skipped for versions before that.

@kyle-walker kyle-walker force-pushed the skip-eol-versions-only branch from 3a45e93 to a648c29 Compare July 1, 2026 20:17
@kyle-walker kyle-walker changed the title Skip End of Life versions in PLCC validators Skip pre-tier-model versions in PLCC validators Jul 1, 2026
The three-tier lifecycle model (Platform Aligned / Platform Agnostic /
Rolling Stream) was introduced with OCP 4.14 GA on 2023-10-31. Versions
whose earliest phase predates this cutoff should not be held to tier
requirements that did not exist when they were published.

Add isPreTierModel(v) helper that checks whether a version's earliest
parseable phase start date falls before TierModelCutoffDate. Apply the
skip to all tier-specific validators (REQ-TIER-*, REQ-FIELD-02,
CUSTOM-04) while preserving universal invariants (date validation,
version name format, contiguity) for all versions regardless of age.

Assisted-by: Claude
@kyle-walker kyle-walker force-pushed the skip-eol-versions-only branch from a648c29 to 4da8a16 Compare July 1, 2026 20:34
@joelanford

Copy link
Copy Markdown
Contributor

/lgtm

@kyle-walker kyle-walker marked this pull request as ready for review July 2, 2026 13:22

@fgiudici fgiudici left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread pkg/plcc/validation.go
earliest = t
}
}
return earliest.IsZero() || earliest.Before(TierModelCutoffDate)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NOTE: this means we will skip also versions that have no Phases with parsable dates BUT filters not part of the "isPreTierModel" check will catch them.

@fgiudici fgiudici merged commit 02c45b9 into release-engineering:main Jul 3, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REQ-TIER-ALL-02: skip End-of-Life versions in tier validation

4 participants