Skip to content

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

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

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

Conversation

@ccwienk

@ccwienk ccwienk commented Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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 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

ccwienk commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

/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
@ccwienk
fix(ocm): reject extraIdentity with non-string values via schema
4f62cdb 
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
@gardener-ci-robot

gardener-ci-robot commented May 29, 2026

Copy link
Copy Markdown

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 14d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as active with /lifecycle active
  • Mark this PR as fresh with /remove-lifecycle stale
  • Mark this PR as rotten with /lifecycle rotten
  • Close this PR with /close

/lifecycle stale

@gardener-prow gardener-prow Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 29, 2026
@ccwienk ccwienk requested a review from Michael5601 as a code owner June 17, 2026 11:13
Michael5601
Michael5601 approved these changes Jun 17, 2026
View reviewed changes

@Michael5601 Michael5601 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.

lgtm :)

@gardener-prow gardener-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2026
@gardener-prow

gardener-prow Bot commented Jun 17, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Michael5601

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

The pull request process is described 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 commented Jun 17, 2026

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: ec94fc5cf571e0be091ac565dbdbaf6588930fd9

@gardener-prow gardener-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 17, 2026
@ccwienk ccwienk removed the do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Michael5601 Michael5601 Michael5601 approved these changes
@TuanAnh17N TuanAnh17N Awaiting requested review from TuanAnh17N TuanAnh17N is a code owner

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. lgtm Indicates that a PR is ready to be merged. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ccwienk @gardener-ci-robot @Michael5601