Skip to content

Support comma-separated package names in PLCC product entries#28

Open
kyle-walker wants to merge 3 commits into
release-engineering:mainfrom
kyle-walker:comma-separated-packages
Open

Support comma-separated package names in PLCC product entries#28
kyle-walker wants to merge 3 commits into
release-engineering:mainfrom
kyle-walker:comma-separated-packages

Conversation

@kyle-walker

Copy link
Copy Markdown
Contributor

A single PLCC product (e.g. odf-operator) may track multiple catalog package names. The package field can now hold a comma-separated list, and FilterByPackageNames matches against any value in the list.

FBC output emits one package entry per name, sharing the same lifecycle data. ValidateNoDuplicates checks each name individually.

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

A single PLCC product (e.g. odf-operator) may track multiple catalog
package names. The package field can now hold a comma-separated list,
and FilterByPackageNames matches against any value in the list.

FBC output emits one package entry per name, sharing the same lifecycle
data. ValidateNoDuplicates checks each name individually.

Assisted-by: Claude
Signed-off-by: Kyle Walker <kwalker@redhat.com
@kyle-walker kyle-walker requested a review from a team as a code owner July 1, 2026 11:42
@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 · ❌ Failure · Started 1:11 PM UTC · Completed 1:23 PM UTC
Commit: feb9bda · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

High

  • [logic-error] pkg/plcc/validation.go:252Catalog.Validate strict-mode filtering still looks up rejections by the raw p.Package field (e.g. "odf-operator,mcg-operator"), but ValidateNoDuplicates now keys rejections by individual split package names (e.g. "odf-operator"). The lookup rejections[p.Package] will never match a split key, so duplicate products are silently kept even in strict mode.
    Remediation: Change the strict-mode loop in Validate to iterate over p.Packages() and check if any individual name is in the rejections map.

  • [logic-error] cmd/plcc2fbc/main.go:146writeSplit uses product.Package directly as a directory name via filepath.Join(dir, product.Package). When Package contains a comma-separated list like "odf-operator,mcg-operator", this creates a single directory with that literal string as its name instead of creating separate per-package directories. This is inconsistent with the PR's intent to treat each comma-separated value as a distinct package.
    Remediation: Iterate over product.Packages() in writeSplit so each individual package name gets its own output directory, or restructure so writeSplit receives the already-expanded packages from Translate.

Medium

  • [scope-creep] pkg/fbc/fbc.go:89 — The change introduces a data model transformation (one Product to N FBC Packages) that is not reflected in AGENTS.md, which documents the data flow as one-to-one. See also: [stale-doc] findings below.
    Remediation: Update AGENTS.md Data Flow section to document the 1:N mapping.

  • [architectural-inconsistency] pkg/plcc/plcc.go:47 — The Packages() method overloads the Package string field to mean a comma-separated list. The Product struct defines Package as a single string with JSON tag "package", implying a single value. This approach works but couples parsing logic to business logic.
    Remediation: Consider introducing a Packages []string field or normalizing at PLCC load time to maintain the one-Product-one-package invariant.

  • [coherence-gap] pkg/plcc/validation.go:528ValidateNoDuplicates now checks individual names from comma-separated lists, but rejection semantics are unclear: if one name in "odf-operator,mcg-operator" is duplicated, the entire Product is rejected. Combined with the strict-mode lookup bug above, this creates inconsistent behavior.
    Remediation: Clarify and document validation semantics for partial duplicates.

  • [coherence-gap] pkg/plcc/plcc.go:159FilterByPackageNames includes a Product if any of its comma-separated package names matches. Using --package odf-operator will also emit mcg-operator FBC entries if the Product's Package field is "odf-operator,mcg-operator". This may surprise users.
    Remediation: Document the new filter behavior. Consider whether filtering should operate on the expanded package list instead.

  • [stale-doc] docs/VALIDATION_RULES.md:95REQ-VAL-01 description does not clarify that it now checks individual names from comma-separated values.
    Remediation: Update to note comma-separated expansion behavior.

  • [stale-doc] docs/VALIDATION_RULES.md:48 — Table entry "Package maps to multiple products" does not address comma-separated package name behavior.
    Remediation: Clarify duplicate detection behavior with comma-separated package names.

Low

  • [logic-error] cmd/plcc2fbc/main.go:269 — Per-product PLCC validation reporting uses product.Package as the PackageName, logging the raw compound string rather than individual names. Cosmetic but potentially confusing.

  • [logic-error] pkg/plcc/plcc.go:171SortByPackage sorts by the raw Package field. Deterministic but may produce unintuitive ordering for comma-separated values.

  • [loop-iteration-idiom] pkg/plcc/plcc.go:156 — Uses index-based iteration (for i := range c.Data) where the existing codebase pattern uses value-based iteration (for _, p := range c.Data).

  • [helper-function-naming] pkg/fbc/fbc.go:111newPackageWithName uses a suffix pattern not seen elsewhere in the codebase.

  • [incomplete-doc] docs/FBC_SCHEMA.md:29 — Package field documentation does not mention comma-separated PLCC source values (though FBC output always contains single names).

  • [missing-doc] schema-examples/fbc-schema-example.yaml:10 — Comment # [MAPPED] from product.package does not note comma-separated expansion.


Labels: PR adds a new feature (comma-separated package support) and has stale documentation findings.

@asergienk

Copy link
Copy Markdown
Collaborator

@kyle-walker thanks for the PR! fullsend flagged some valid issues - could you please take a look?

Catalog.Validate strict-mode looked up rejections by the raw
Product.Package field, but ValidateNoDuplicates keys rejections by
individual split names. The lookup never matched, so duplicates were
silently kept. Fix by iterating over Product.Packages() in the
strict-mode filter loop.

writeSplit used Product.Package as a directory name, creating a single
directory with the literal comma-containing string. Fix by iterating
over Product.Packages() and writing each individual package to its own
directory with a single-name product copy.

Also fix per-product validation logging to report individual package
names instead of the raw compound string.

Assisted-by: Claude
Document the 1:N Product-to-FBC-Package mapping in the AGENTS.md data
flow diagram. Clarify REQ-VAL-01 duplicate detection and the filtering
summary table to reflect comma-separated name expansion.

Assisted-by: Claude
@kyle-walker

Copy link
Copy Markdown
Contributor Author

@asergienk - Happy to!

Just merged a couple of changes that should handle the risks raised. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants