Skip to content

fix(ocm): reject extraIdentity with non-string values via schema#1591

Open
ccwienk wants to merge 1 commit intomasterfrom
fix/extra-identity-int-coercion
Open

fix(ocm): reject extraIdentity with non-string values via schema#1591
ccwienk wants to merge 1 commit intomasterfrom
fix/extra-identity-int-coercion

Conversation

@ccwienk
Copy link
Copy Markdown
Member

@ccwienk ccwienk commented Apr 29, 2026

Summary

  • YAML `safe_load` parses unquoted integers (e.g. `architecture: 386`) as `int`; downstream code calling `ComponentDescriptor.from_dict()` then crashes with a confusing `dacite.WrongTypeError`
  • Root cause fix: add `additionalProperties: {type: string}` to `identityAttribute` in the OCM schema so documents with non-string `extraIdentity` values are rejected at validation time with a clear error
  • The primary fix is in the upstream repository (quoting the GHA expression), this PR ensures the schema enforces the invariant for any caller

Changes

  • `ocm/ocm-component-descriptor-schema.yaml`: `identityAttribute` now requires all values to be strings
  • `test/ocm/componentmodel_test.py`: regression test verifies `ComponentDescriptor.validate()` rejects a descriptor with `architecture: 386` (integer)

Test plan

  • `pytest test/ocm/` — all 17 tests pass

@ccwienk ccwienk requested a review from TuanAnh17N as a code owner April 29, 2026 12:53
@gardener-prow
Copy link
Copy Markdown

gardener-prow Bot commented Apr 29, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zkdev for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow Bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 29, 2026
@ccwienk
Copy link
Copy Markdown
Member Author

ccwienk commented Apr 29, 2026

/hold (still wip)

@gardener-prow gardener-prow Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2026
YAML safe_load parses unquoted integers (e.g. architecture: 386) as int.
Add additionalProperties: {type: string} to identityAttribute in the OCM
schema so such documents are rejected at validation time rather than
silently coerced.
@ccwienk ccwienk force-pushed the fix/extra-identity-int-coercion branch from e4f5c78 to 4f62cdb Compare April 29, 2026 13:07
@ccwienk ccwienk changed the title fix(ocm): coerce extraIdentity values to str on deserialisation fix(ocm): reject extraIdentity with non-string values via schema Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant