Skip to content

fix!: move verification to resolution service#1707

Merged
jakobmoellerdev merged 110 commits into
open-component-model:mainfrom
frewilhelm:move-verification
Mar 12, 2026
Merged

fix!: move verification to resolution service#1707
jakobmoellerdev merged 110 commits into
open-component-model:mainfrom
frewilhelm:move-verification

Conversation

@frewilhelm

@frewilhelm frewilhelm commented Feb 2, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

This PR refactors signature verification logic by moving it from the component controller to the resolution service (worker pool). This centralizes verification and improves separation of concerns.

Key Changes
  • Worker Pool: Implements complete signature verification logic with new ErrNotSafelyDigestible error type to distinguish safely digestible components from verification failures.
  • Controllers: Remove verification methods and delegate to resolution service; updated error handling to gracefully handle non-digestible components.
  • Cache Repository: Passes verification configurations, the digest specification of a component reference from the parent component, and signing registry to worker pool for centralized handling and trusted cache keys.
  • Component reference integrity: If the component reference provides a digest specification, we re-generate the digest of the child component descriptor to ensure integrity.
Benefits
  • Centralized verification logic reduces duplication across Component, Resource, and Deployer controllers
  • Improved error handling with consistent behavior across all controllers
  • Better separation of concerns: controllers orchestrate, resolution service verifies

Which issue(s) this PR fixes

Fixes open-component-model/ocm-project#787

Summary by CodeRabbit

  • New Features

    • Digest- and signature-aware component resolution and verification.
    • Helm nested-signed example with sample repository, component, resource, deployer and signing keys.
    • Resource reconciliation Interval field for finer cadence control.

  • Improvements

    • Non-fatal event signaling for digest/integrity issues.
    • Metrics now include verification state for better observability.
    • CRD description clarified for the signature field.
    • New condition reporting for component drift resolution.

  • Tests

    • Expanded signing, verification, nested-resolution and cache/metrics test coverage.

@frewilhelm
move verification to resolution service
d7c65ea 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm frewilhelm requested a review from Skarlso February 2, 2026 17:25
@frewilhelm frewilhelm marked this pull request as ready for review February 3, 2026 14:34
@frewilhelm frewilhelm requested a review from a team as a code owner February 3, 2026 14:34
@frewilhelm frewilhelm marked this pull request as draft February 3, 2026 14:38
fabianburth
fabianburth reviewed Feb 5, 2026
View reviewed changes
Comment thread kubernetes/controller/internal/controller/component/component_controller.go Outdated
fabianburth
fabianburth reviewed Feb 6, 2026
View reviewed changes
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go Outdated
@fabianburth
fix: increase event severity for not digestible
c1d3b20 
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth
fix: add verifications to cache key
6b6b31a 
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth
Merge branch 'main' into move-verification
cb2d89d
@fabianburth
fix: move verifications and signing registry into new cached repo call
90826c1 
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth
Merge remote-tracking branch 'frewilhelm/move-verification' into move…
2fb67f6 
…-verification
@fabianburth
fix: add json tag to verification struct to fix linting
6364f7e 
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth
Merge branch 'main' into move-verification
a9a48d5
@fabianburth
fix: regenerate crds
367947e 
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth
Merge remote-tracking branch 'frewilhelm/move-verification' into move…
37b7237 
…-verification
@fabianburth
Merge branch 'main' into move-verification
5af3e15
frewilhelm
frewilhelm commented Feb 9, 2026
View reviewed changes
Comment thread kubernetes/controller/internal/controller/resource/resource_controller.go
@frewilhelm
add signed and nested example
729d617 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
add digest for building cache keys
07d4ed1 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
Merge branch 'main' of https://github.com/open-component-model/open-c…
babc464 
…omponent-model into move-verification
@frewilhelm
move digestible check
38e3330 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
jakobmoellerdev
jakobmoellerdev reviewed Mar 11, 2026
View reviewed changes
Comment thread kubernetes/controller/api/v1alpha1/condition_types.go
Comment thread kubernetes/controller/internal/controller/component/component_controller.go Outdated
Comment thread kubernetes/controller/internal/controller/deployer/deployer_controller.go Outdated
Comment thread kubernetes/controller/internal/controller/resource/resource_controller.go Outdated
Comment thread kubernetes/controller/internal/ocm/resource.go
Comment thread kubernetes/controller/internal/resolution/workerpool/workerpool.go
Comment thread kubernetes/controller/internal/resolution/cached_repository.go
Comment thread kubernetes/controller/internal/resolution/cached_repository.go Outdated
Comment thread kubernetes/controller/internal/resolution/cached_repository.go
Comment thread kubernetes/controller/internal/resolution/doc.go Outdated
matthiasbruns
matthiasbruns reviewed Mar 11, 2026
View reviewed changes
Comment thread kubernetes/controller/internal/ocm/resource.go
@frewilhelm
remove linebreak
758b608 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
extract functions for more readability
dc910cc 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
add doc to function
fb369b9 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
document signing registry
915348c 
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm
Merge branch 'main' of https://github.com/open-component-model/open-c…
1996352 
…omponent-model into move-verification
@Skarlso

Skarlso commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

I'm doing some nicification here and there and a general facelift going over the entire change.

I'll try to make it brief though. Hopefully, I can actually reduce the complexity here somewhat. I see that we actually don't need some of these things that we did here. So I'm going to give this a general facelift.

I will avoid changing too much though because I don't want people to have to re-read the entire thing. So I'll do one commit that you can view that will contain all the changes.

@Skarlso
chore: refactored and simplified some of the calls and function param…
83bf3da 
…eters

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso

Skarlso commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Here are the refactored changes that hopefully make the code a bit better and more readable.
83bf3da

I found a couple of things:

  • newCacheBackedRepository removed -> this was literally not needed
  • ResolveReferencePath params -> we already had RepositoryOptions that contained everything
  • findMatchingReference and extractDigest -> extracted from the ResolveReferencePath loop
  • identityFunc -> this was a straight duplicate of ocm.IdentityFuncIgnoreVersion()
  • some duplication on constructing RepositoryOptions in the deployer's getEffectiveComponentDescriptor
  • fixed bug -> errReferencePath was actually wrapping the wrong error :D

@Skarlso
update the doc for the resolution service
4ebd6db 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@frewilhelm frewilhelm requested a review from fabianburth March 12, 2026 12:06
@jakobmoellerdev jakobmoellerdev enabled auto-merge (squash) March 12, 2026 15:40
@jakobmoellerdev jakobmoellerdev merged commit d2a4a88 into open-component-model:main Mar 12, 2026
20 checks passed
This was referenced Mar 24, 2026
Merged
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 13, 2026
Merged
1 task
This was referenced Apr 22, 2026
Merged
Merged
@coderabbitai coderabbitai Bot mentioned this pull request May 27, 2026
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@matthiasbruns matthiasbruns matthiasbruns approved these changes

@fabianburth fabianburth fabianburth approved these changes

@jakobmoellerdev jakobmoellerdev jakobmoellerdev approved these changes

@Skarlso Skarlso Awaiting requested review from Skarlso

Assignees

No one assigned

Labels

!BREAKING-CHANGE! Breaking change in API or ocm-cli or spec kind/bugfix Bug size/l Large size/m Medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Move component verification to resolution service

6 participants

@frewilhelm @Skarlso @fabianburth @matthiasbruns @jakobmoellerdev @ieepirzy