Repository navigation
feat(components): add support for disjunctive 'or' dependency requirements in linter rules - #1341
Conversation
…ments in linter rules Add flat 1-level `or` support to `DependencyRequirement` in `pkg/components/linter` so components in `compatibilityMatrix.componentDependencies` (e.g., `metrics` or `ingestion-gateway`) can declare alternative backend requirements (such as `redpanda` or `pubsub`) with independent SemVer constraints. Key changes: - Extend `DependencyRequirement` in `pkg/components/linter/rules.go` with `Or []DependencyRequirement` (`yaml:"or,omitempty" json:"or,omitempty"`). - Add strict schema validation in `ParseRules` to reject mutually exclusive fields (`component` or top-level `version` combined with `or`), empty requirements, nested `or` blocks, or `or` alternatives missing `component`. - Update `evaluateDependencyRequirement` in `pkg/components/linter/matrix.go` to evaluate `or` blocks with pure logical OR semantics: satisfied if at least one enabled alternative meets its SemVer constraint; emits `VERSION_INCOMPATIBILITY` only when enabled alternatives exist and all fail their version constraints; emits `REQUIRED_COMPONENT_MISSING` when all alternatives are missing/disabled (unless `optional: true`). - Add comprehensive table-driven unit tests in `rules_test.go` and `matrix_test.go` and update `cmd/component-linter/README.md`. TAG=agy CONV=22bf6868-8f46-4b28-8b7d-e36b1dfd7dcc
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for disjunctive requirements ("or") in the component linter's compatibility matrix. It updates the configuration parser to validate and support flat 1-level "or" lists of alternative requirements, implements the corresponding evaluation logic in the linter matrix, and adds comprehensive unit tests. Feedback on the changes suggests simplifying a loop in reportMissingOrComponents by replacing an if block containing a continue with a standard if/else block to improve readability.
Simplify the loop in `reportMissingOrComponents` by initializing the quoted component name first and conditionally appending the version constraint before a single `parts = append(parts, part)` call, eliminating the `continue` statement while keeping control flow strictly linear. Comment link: #1341 (comment) TAG=agy CONV=22bf6868-8f46-4b28-8b7d-e36b1dfd7dcc
There was a problem hiding this comment.
Code Review
This pull request introduces support for disjunctive requirements (or) in the component linter, allowing a dependency requirement to be satisfied by any of a list of alternative components. The changes include updates to the documentation, parser validation to prevent nested or invalid or structures, evaluation logic in the linter matrix, and comprehensive unit tests. The review feedback suggests a minor improvement in checkOrAlternatives to copy the loop variable before modifying its fields, which prevents potential bugs if the slice is refactored to pointers in the future.
Summary
Adds support for flat 1-level disjunctive (
or) dependency requirements inpkg/components/linter(compatibilityMatrix.componentDependencies). This allows components such asmetricsoringestion-gatewayto require at least one of multiple alternative backend components (e.g.,redpandaorpubsub), each with its own SemVer version constraint.Motivation & Context
Previously,
ComponentDependencyRule.Requiresinpkg/components/linterevaluated all listedDependencyRequiremententries as a strict logicalAND. When a component supports pluggable backends (such asredpandavs.pubsub), there was no way to express a required "at least one of" dependency inrules.yamlwithout making bothoptional: true(which fails to catch deployments where both backends are missing/disabled). Adding a flatorlist toDependencyRequirementenables static compatibility enforcement for alternative dependencies while remaining 100% backward-compatible with existingcomponentrules.Key Changes
pkg/components/linter/rules.go:Or []DependencyRequirement(yaml:"or,omitempty" json:"or,omitempty") toDependencyRequirement.validateRulesConfig,validateDependencyRequirement, andvalidateOrRequirementinParseRulesto enforce flat 1-levelorrules (rejecting entries that specify bothcomponentandor, top-levelversionalongsideor, neithercomponentnoror, nestedorblocks, ororalternatives with an emptycomponent).pkg/components/linter/matrix.go:evaluateDependencyRequirementwithevaluateOrDependencyRequirement,validateOrConstraints,checkOrAlternatives,allOrAlternativesOptional, andreportMissingOrComponents.ORevaluation: satisfied when at least one enabled alternative satisfies its SemVer constraint; reportsVERSION_INCOMPATIBILITYonly whenREQUIRED_COMPONENT_MISSINGwhen all alternatives are missing/disabled (unlessoptional: trueis set on the parent requirement or on all alternatives).pkg/components/linter/rules_test.go&pkg/components/linter/matrix_test.go:TestParseRules_OrRequirementsandTestMatrix_OrRequirements) covering schema validation errors and all matrix evaluation paths.cmd/component-linter/README.md:or:syntax and evaluation semantics under Section 8 (Compatibility Matrix Configuration (rules.yaml)).Assumptions
VERSION_INCOMPATIBILITY/REQUIRED_COMPONENT_MISSINGwhen an enabledoralternative has an unparseablespec.version— assumed to match existing single-dependency behavior inevaluateDependencyRequirement(wherevalidateSchemaalready emitsINVALID_SEMVERfor the malformed manifest) — if wrong, a malformed version on an enabled alternative would emit bothINVALID_SEMVERand a secondary compatibility error.Verification & Testing
bazel test //pkg/components/linter/... //cmd/component-linter/...bazel coverage --combined_report=lcov //pkg/components/linter/..../.githooks/pre-commitpkg/components/linter/rules.goand all new/modified lines inpkg/components/linter/matrix.go.Related Issues / Bugs
N/A
TAG=agy
CONV=22bf6868-8f46-4b28-8b7d-e36b1dfd7dcc