feat: migrate compref from cli to oci by chrisbleyerSAP · Pull Request #1908 · open-component-model/open-component-model

chrisbleyerSAP · 2026-03-06T16:27:07Z

What this PR does / why we need it

As discussed with @fabianburth and @jakobmoellerdev, this copies over the compref package to bindings/go/oci. Next step will be to removecompref from the cli and import it via bindings/go/oci.

Verification

I have tested the changes locally by running task test

Summary by CodeRabbit

New Features
- Added a component reference parser with support for OCI and CTF repositories, repository type inference, normalization, and string/identity serialization.
- Added configuration options for CTF access mode and toggling semver compatibility checks.
Tests
- Comprehensive test suite covering parsing, repository detection, serialization, permutations, and error cases across OCI/CTF and file/archive inputs.

coderabbitai · 2026-03-06T16:27:30Z

📝 Walkthrough

Walkthrough

Adds a new Go package bindings/go/oci/compref implementing an OCM component reference parser (CompRef) with repository type inference (OCI/CTF), validation, runtime scheme wiring, configuration options, and a comprehensive test suite.

Changes

Cohort / File(s)	Summary
Core Implementation `bindings/go/oci/compref/compref.go`	New component reference parser: adds `Ref` type, regex validators (`ComponentRegex`, `VersionRegex`, `DigestRegex`), `Parse` and `ParseRepository`, repository type inference (`guessType`), Windows path/URL/archive helpers, runtime scheme registration and debug hooks.
Configuration Options `bindings/go/oci/compref/options.go`	Adds `Options` with `CTFAccessMode` and `IgnoreSemverCompatibility`, `Option` interface, `NewOptions`, `WithCTFAccessMode`, and `IgnoreSemverCompatibility` option constructors.
Test Suite `bindings/go/oci/compref/compref_test.go`	Extensive tests: table-driven parsing cases, permutation tests across types/repos/components/versions/digests, repository parsing checks (OCI/CTF/file/URL), error cases, and `Ref.String()` serialization tests.

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant Parser as Parse
    participant RepoParser as ParseRepository
    participant Inference as TypeInference
    participant Scheme as RuntimeScheme

    Client->>Parser: Parse(input, opts...)
    Parser->>Parser: validate fields (component/version/digest)
    Parser->>Inference: guessType(repoRef)
    Inference-->>Parser: inferred type (oci/ctf/other)
    Parser->>RepoParser: ParseRepository(repoRef, opts...)
    RepoParser->>Scheme: request typed repository (OCI/CTF)
    Scheme-->>RepoParser: runtime.Typed repository instance
    RepoParser-->>Parser: repository object
    Parser->>Parser: construct Ref (Type, Repository, Prefix, Component, Version, Digest)
    Parser-->>Client: return *Ref or error

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through code with nimble paws,

parsing refs and minding laws.
OCI, CTF—I'm on the case,
components found in every place.
A tiny hop, a joyful cheer!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: migrate compref from cli to oci' accurately and concisely describes the main change: migrating the compref package from the CLI to the OCI bindings directory.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

bindings/go/oci/compref/options.go (1)

14-19: Consider selective field copying in Apply to avoid overwriting non-zero values.

The current implementation copies all fields from o to opts, including zero values. If Apply is called with an Options that only intends to set CTFAccessMode, it will also reset IgnoreSemverCompatibility to false. This may be intentional for "replace" semantics, but typically option merging preserves non-zero fields.

🔧 Proposed selective merge approach

 func (o *Options) Apply(opts *Options) error {
 	if o != nil {
-		*opts = *o
+		if o.CTFAccessMode != "" {
+			opts.CTFAccessMode = o.CTFAccessMode
+		}
+		if o.IgnoreSemverCompatibility {
+			opts.IgnoreSemverCompatibility = o.IgnoreSemverCompatibility
+		}
 	}
 	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/compref/options.go` around lines 14 - 19, The Apply method on
Options currently does a blind struct copy which overwrites existing non-zero
fields; change Options.Apply to perform a selective merge instead: for each
option field (e.g., CTFAccessMode, IgnoreSemverCompatibility, and any other
fields on the Options struct), only set opts.Field = o.Field when o.Field is a
non-zero/meaningful value (for booleans consider using a pointer or explicit
presence flag if you need to distinguish false vs unset); implement this logic
inside Options.Apply so callers that only want to set CTFAccessMode won’t
inadvertently reset IgnoreSemverCompatibility.

bindings/go/oci/compref/compref.go (1)

160-165: Consider using NewOptions for consistency with ParseRepository.

The Parse function manually creates and applies options while ParseRepository uses NewOptions(opts...). Using NewOptions here would be more consistent and reduce code duplication.

♻️ Proposed refactor for consistency

-	parsedOptions := Options{}
-	for _, o := range opts {
-		if err := o.Apply(&parsedOptions); err != nil {
-			return nil, fmt.Errorf("failed to apply option: %w", err)
-		}
-	}
+	parsedOptions, err := NewOptions(opts...)
+	if err != nil {
+		return nil, fmt.Errorf("failed to apply option: %w", err)
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/compref/compref.go` around lines 160 - 165, The Parse
function currently builds parsedOptions via manual loop over opts (parsedOptions
:= Options{} and o.Apply(...)); replace that with a call to NewOptions(opts...)
to mirror ParseRepository and avoid duplication—call NewOptions(opts...)
(handling and returning any error) and use the returned Options value in place
of parsedOptions so Parse and ParseRepository use the same option construction
path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bindings/go/oci/compref/compref_test.go`:
- Around line 720-732: The test case for "CTF repository with Version and
Digest" has a typo: the Ref.Type is set to "ctv" instead of "ctf"; update the
Ref literal in the failing test (the test case struct in compref_test.go that
builds a Ref with Repository *ctfv1.Repository, Prefix, Component, Version,
Digest) to set Type: "ctf" so the test correctly represents a CTF repository.
- Around line 335-337: The equality assertion runs only when the error assertion
fails because the condition is inverted; change the control flow so that when
tc.err(t, err) indicates an error case you bail out and only run
r.Equalf(parsed, tc.expected, ...) when there was no error—i.e. replace the
inverted if with a guard that returns when tc.err(t, err) is true, then call
r.Equalf using parsed, tc.expected and tc.input.

---

Nitpick comments:
In `@bindings/go/oci/compref/compref.go`:
- Around line 160-165: The Parse function currently builds parsedOptions via
manual loop over opts (parsedOptions := Options{} and o.Apply(...)); replace
that with a call to NewOptions(opts...) to mirror ParseRepository and avoid
duplication—call NewOptions(opts...) (handling and returning any error) and use
the returned Options value in place of parsedOptions so Parse and
ParseRepository use the same option construction path.

In `@bindings/go/oci/compref/options.go`:
- Around line 14-19: The Apply method on Options currently does a blind struct
copy which overwrites existing non-zero fields; change Options.Apply to perform
a selective merge instead: for each option field (e.g., CTFAccessMode,
IgnoreSemverCompatibility, and any other fields on the Options struct), only set
opts.Field = o.Field when o.Field is a non-zero/meaningful value (for booleans
consider using a pointer or explicit presence flag if you need to distinguish
false vs unset); implement this logic inside Options.Apply so callers that only
want to set CTFAccessMode won’t inadvertently reset IgnoreSemverCompatibility.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: abe9728d-e2ac-42d4-805f-7375c2c5d193

📥 Commits

Reviewing files that changed from the base of the PR and between 0d09fa3 and eb4bea4.

📒 Files selected for processing (3)

bindings/go/oci/compref/compref.go
bindings/go/oci/compref/compref_test.go
bindings/go/oci/compref/options.go

Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com>

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bindings/go/oci/compref/compref_test.go`:
- Around line 304-309: The test's error assertion expects "invalid version
format" but Parse() emits "invalid semantic version %q in %q, must match %q";
update the test in compref_test.go (the failing case using the err func) to
assert the actual substring emitted by Parse(), e.g., check for "invalid
semantic version" (or match the full formatted message pattern returned by
Parse()), so the assertion aligns with the Parse() error text.

In `@bindings/go/oci/compref/compref.go`:
- Around line 203-206: Replace the direct slog.Warn call in the
IgnoreSemverCompatibility branch with the package Base logger used elsewhere so
the "realm":"compref" context is included: change the slog.Warn(...) invocation
to Base.Warn(...) (matching how ParseRepository uses Base.Debug) and keep the
same message and key/value pairs ("version", versionPart, "input",
originalInput).

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 839d2740-0d76-4832-9ea0-1dd7bb6f18f3

📥 Commits

Reviewing files that changed from the base of the PR and between eb4bea4 and 2fde89c.

📒 Files selected for processing (3)

bindings/go/oci/compref/compref.go
bindings/go/oci/compref/compref_test.go
bindings/go/oci/compref/options.go

🚧 Files skipped from review as they are similar to previous changes (1)

bindings/go/oci/compref/options.go

#### What this PR does / why we need it Follow up for: #1908. Removes compref from cli. ##### Verification - [x] I have tested the changes locally by running `task test`  ## Summary by CodeRabbit * **Refactor** * Component reference handling moved to the public OCI bindings, consolidating parsing/resolution into a shared implementation. * Legacy internal reference parser and its option surface were removed; CLI commands use the unified binding with no functional change. * Tests updated to the new bindings; some legacy internal tests were removed.  Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com>

@fabianburth

#### What this PR does / why we need it As discussed with @fabianburth and @jakobmoellerdev, this copies over the `compref` package to `bindings/go/oci`. Next step will be to remove`compref` from the cli and import it via `bindings/go/oci`. ##### Verification - [x] I have tested the changes locally by running `task test`  ## Summary by CodeRabbit * **New Features** * Added a component reference parser with support for OCI and CTF repositories, repository type inference, normalization, and string/identity serialization. * Added configuration options for CTF access mode and toggling semver compatibility checks. * **Tests** * Comprehensive test suite covering parsing, repository detection, serialization, permutations, and error cases across OCI/CTF and file/archive inputs.  Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com>

#### What this PR does / why we need it Follow up for: open-component-model#1908. Removes compref from cli. ##### Verification - [x] I have tested the changes locally by running `task test`  ## Summary by CodeRabbit * **Refactor** * Component reference handling moved to the public OCI bindings, consolidating parsing/resolution into a shared implementation. * Legacy internal reference parser and its option surface were removed; CLI commands use the unified binding with no functional change. * Tests updated to the new bindings; some legacy internal tests were removed.  Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com>

@fabianburth

#### What this PR does / why we need it As discussed with @fabianburth and @jakobmoellerdev, this copies over the `compref` package to `bindings/go/oci`. Next step will be to remove`compref` from the cli and import it via `bindings/go/oci`. ##### Verification - [x] I have tested the changes locally by running `task test`  ## Summary by CodeRabbit * **New Features** * Added a component reference parser with support for OCI and CTF repositories, repository type inference, normalization, and string/identity serialization. * Added configuration options for CTF access mode and toggling semver compatibility checks. * **Tests** * Comprehensive test suite covering parsing, repository detection, serialization, permutations, and error cases across OCI/CTF and file/archive inputs.  Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com> Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>

#### What this PR does / why we need it Follow up for: open-component-model#1908. Removes compref from cli. ##### Verification - [x] I have tested the changes locally by running `task test`  ## Summary by CodeRabbit * **Refactor** * Component reference handling moved to the public OCI bindings, consolidating parsing/resolution into a shared implementation. * Legacy internal reference parser and its option surface were removed; CLI commands use the unified binding with no functional change. * Tests updated to the new bindings; some legacy internal tests were removed.  Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com> Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>

#### What this PR does / why we need it Follow up for a coderabbitai [comment](#1908 (comment)) on a previous PR #### Which issue(s) this PR fixes  #### Testing ##### How to test the changes  ##### Verification - [ ] I have tested the changes locally by running `ocm`  ## Summary by CodeRabbit * **Tests** * Improved test error handling to stop and report immediately when parsing fails. * Added explicit non-null validation for parsed results on success. * Strengthened serialization checks by comparing entire objects for exact equality rather than partial string matching.  --------- Signed-off-by: Fabian Burth <fabian.burth@sap.com> Co-authored-by: Jakob Möller <jakob.moeller@sap.com>

chrisbleyerSAP requested a review from a team as a code owner March 6, 2026 16:27

github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/l Large labels Mar 6, 2026

coderabbitai Bot reviewed Mar 6, 2026

View reviewed changes

Comment thread bindings/go/oci/compref/compref_test.go

Comment thread bindings/go/oci/compref/compref_test.go

fabianburth approved these changes Mar 6, 2026

View reviewed changes

jakobmoellerdev approved these changes Mar 6, 2026

View reviewed changes

feat: migrate compref from cli to oci

2fde89c

Signed-off-by: Christoph Bleyer <christoph.bleyer@sap.com>

chrisbleyerSAP force-pushed the main branch from eb4bea4 to 2fde89c Compare March 8, 2026 11:04

coderabbitai Bot reviewed Mar 8, 2026

View reviewed changes

Comment thread bindings/go/oci/compref/compref_test.go

Comment thread bindings/go/oci/compref/compref.go

Merge branch 'main' into main

00904df

jakobmoellerdev merged commit 4c2ccaf into open-component-model:main Mar 8, 2026
18 checks passed

chrisbleyerSAP mentioned this pull request Mar 10, 2026

feat: remove compref package from cli #1941

Merged

1 task

fabianburth mentioned this pull request Mar 18, 2026

chore: improve error handling and assertions in tests #2009

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: migrate compref from cli to oci#1908

feat: migrate compref from cli to oci#1908
jakobmoellerdev merged 2 commits into
open-component-model:mainfrom
chrisbleyerSAP:main

chrisbleyerSAP commented Mar 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 6, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

chrisbleyerSAP commented Mar 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chrisbleyerSAP commented Mar 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 6, 2026 •

edited

Loading