feat: examples for library usage#2054
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdded a runnable suite of Go example tests and supporting docs/config that demonstrate blobs, descriptors, credentials, repositories, signing, OCI registry integration, and component transfer flows for the OCM Go bindings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f3683e4 to
dfab1bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
bindings/go/examples/Taskfile.yml (1)
6-9: Consider adding a Docker-free default test target.
testcurrently runs everything viareuse:run-go-test, which likely includes OCI tests requiring Docker. A separate default (test= short/local) plustest/fullwould make onboarding smoother.Suggested Taskfile refinement
tasks: test: cmds: - task: reuse:run-go-test + vars: + GO_TEST_FLAGS: -short + + test/full: + cmds: + - task: reuse:run-go-test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/examples/Taskfile.yml` around lines 6 - 9, The current Taskfile target "test" invokes the reusable target "reuse:run-go-test" which likely runs OCI/Docker-based tests; add a Docker-free default by changing "test" to run a short/local test target (e.g., call a new "test:short" or run "go test ./... -short") and add a separate "test/full" target that delegates to "reuse:run-go-test" for the full suite; update or create targets named "test", "test:short" (or "test/local") and "test/full" so onboarding runs fast by default while still allowing the full Docker-backed run via "test/full".bindings/go/examples/06_oci_test.go (1)
47-49: Skip when Docker is unavailable, not only in-short.Right now a plain
go teston a machine or CI runner without a healthy Docker daemon will fail hard instead of opting out cleanly. For a guided-tour package, these should either probe provider health andt.Skip, or live behind a dedicated tag/task.Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/examples/06_oci_test.go` around lines 47 - 49, The test currently only skips when testing.Short() is set but should also detect whether the Docker daemon is reachable; add a pre-check (e.g., a helper like dockerAvailable() called at the top of the test) that probes Docker health (docker client Ping or "docker info") and call t.Skip("skipping OCI registry test: Docker unavailable") if the probe fails, leaving the existing testing.Short() skip in place; apply the same change to the other test block that uses testing.Short()/t.Skip (the second occurrence around the other OCI test).
🤖 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/examples/07_transfer_test.go`:
- Around line 102-127: The test currently only asserts metadata; update the test
to also retrieve and assert the transferred resource payload by calling
targetRepo.GetLocalResource for the component/version and resource name
"my-resource", obtain the ReadCloser, read all bytes with io.ReadAll, close the
reader, and compare the resulting []byte to the existing resourceContent
variable using r.Equal; add appropriate r.NoError checks around
GetLocalResource, obtaining the ReadCloser, reading bytes, and defer closing to
ensure the actual payload was transferred (place this after the existing
targetRepo.GetComponentVersion assertions).
In `@bindings/go/examples/go.mod`:
- Around line 10-21: The examples module's tests run without initializing the
go.work workspace so they pull published modules instead of local siblings;
update the test task (bindings/go/examples:test) to depend on the repo-level
init/go.work task (add deps: ["init/go.work"] or equivalent) so
reuse:run-go-test runs after workspace initialization, or alternatively add
local `replace` directives in the examples' go.mod (like the pattern used in
bindings/go/oci/integration) to point the
ocm.software/open-component-model/bindings/go/* modules to their local paths;
modify either the task's deps or the go.mod replace entries and ensure the task
invoking reuse:run-go-test is the one updated.
In `@bindings/go/examples/README.md`:
- Around line 45-78: The fenced concept-map block in the README is missing a
language identifier (markdownlint MD040); update the opening fence for the ASCII
diagram in the README's concept map to include a language tag (e.g., change ```
to ```text) so the block is explicitly marked as plain text; locate the ASCII
diagram fenced block and only modify its opening backticks to include the
language identifier.
In `@bindings/go/README.md`:
- Around line 50-53: Update the README wording to remove the contradiction by
qualifying the "All examples are self-contained" sentence—make it explicit that
all examples are self-contained and require no external services except the "OCI
Registry" example which performs a full round-trip against a real OCI registry
using testcontainers (and is skipped with `-short`), i.e., change the phrase
"All examples are self-contained (no external services required) and run as part
of CI" to something like "All examples are self-contained (no external services
required) and run as part of CI, except the 'OCI Registry' example which
requires a real OCI registry/testcontainers and is skipped with `-short`."
In `@README.md`:
- Around line 97-99: The README has inconsistent capitalization in the sentence
listing language bindings; update the wording so "Bindings" and "Focus" use
normal sentence case (e.g., "bindings" and "focus") and ensure overall phrasing
reads smoothly—locate the sentence containing "[go](bindings/go). These Bindings
are also used by the OCM CLI and are our primary Focus." and change it to
standard capitalization and consistent tone to match the rest of the document.
---
Nitpick comments:
In `@bindings/go/examples/06_oci_test.go`:
- Around line 47-49: The test currently only skips when testing.Short() is set
but should also detect whether the Docker daemon is reachable; add a pre-check
(e.g., a helper like dockerAvailable() called at the top of the test) that
probes Docker health (docker client Ping or "docker info") and call
t.Skip("skipping OCI registry test: Docker unavailable") if the probe fails,
leaving the existing testing.Short() skip in place; apply the same change to the
other test block that uses testing.Short()/t.Skip (the second occurrence around
the other OCI test).
In `@bindings/go/examples/Taskfile.yml`:
- Around line 6-9: The current Taskfile target "test" invokes the reusable
target "reuse:run-go-test" which likely runs OCI/Docker-based tests; add a
Docker-free default by changing "test" to run a short/local test target (e.g.,
call a new "test:short" or run "go test ./... -short") and add a separate
"test/full" target that delegates to "reuse:run-go-test" for the full suite;
update or create targets named "test", "test:short" (or "test/local") and
"test/full" so onboarding runs fast by default while still allowing the full
Docker-backed run via "test/full".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76028f5c-f0d9-45f9-8ef6-6d1669642fd4
⛔ Files ignored due to path filters (1)
bindings/go/examples/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.github/config/wordlist.txtREADME.mdTaskfile.ymlbindings/go/README.mdbindings/go/examples/01_blob_test.gobindings/go/examples/02_descriptor_test.gobindings/go/examples/03_credentials_test.gobindings/go/examples/04_repository_test.gobindings/go/examples/05_signing_test.gobindings/go/examples/06_oci_test.gobindings/go/examples/07_transfer_test.gobindings/go/examples/README.mdbindings/go/examples/Taskfile.ymlbindings/go/examples/doc_test.gobindings/go/examples/go.modreuse.Taskfile.yml
frewilhelm
left a comment
There was a problem hiding this comment.
I liked it a lot. Just some nits
|
Cool examples, Like! I just found a small issue when executing the oci test on my colima based docker setup (could imagine that this also happens on other rootless docker setups and added a small callout. |
- feat(examples): add tested examples for credential resolution, blobs, and signing - chore(docs): update README with links to new examples Signed-off-by: Jakob Möller <contact@jakob-moeller.com>
Signed-off-by: Jakob Möller <contact@jakob-moeller.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
bindings/go/examples/07_transfer_test.go (1)
118-127:⚠️ Potential issue | 🟡 MinorDuplicate: transfer verification still checks metadata only.
This block still doesn’t assert that the transferred blob bytes match
resourceContent; it can pass even if payload transfer failed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/examples/07_transfer_test.go` around lines 118 - 127, The verification currently only checks metadata from GetComponentVersion (targetRepo.GetComponentVersion and got.Component.Resources) but does not validate blob contents; fetch the transferred resource blob (use the resource's access/digest from got.Component.Resources[0].Access or similar) via the target repository client (e.g., targetRepo's blob download/Read/GetBlob method) and assert its bytes equal the original resourceContent variable; update the test after the GetComponentVersion block to download the resource payload and r.Equal(resourceContent, downloadedBytes).bindings/go/examples/README.md (1)
45-45:⚠️ Potential issue | 🟡 MinorDuplicate: add a language tag to the concept-map fence (MD040).
Use a language identifier (e.g.,
text) on the opening fence to satisfy markdownlint.Suggested fix
-``` +```text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/examples/README.md` at line 45, The markdown code fence for the concept-map block in README.md is missing a language tag which triggers MD040; update the opening fence from ``` to include a language identifier (e.g., change the opening fence to ```text) so the concept-map fenced block has a language tag and satisfies markdownlint.
🤖 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/examples/07_transfer_test.go`:
- Around line 102-104: The call to BuildGraphDefinition uses the wrong
signature; replace the positional arguments (ctx, fromRef, targetSpec, resolver)
with the variadic options form by passing the transfer mapping via the
WithTransfer option (i.e., call BuildGraphDefinition(ctx,
transfer.WithTransfer(fromRef, targetSpec, resolver)) so the mappings are
provided as an Option); locate BuildGraphDefinition and change the call to use
transfer.WithTransfer (or the package's WithTransfer) to construct the
Option(s).
---
Duplicate comments:
In `@bindings/go/examples/07_transfer_test.go`:
- Around line 118-127: The verification currently only checks metadata from
GetComponentVersion (targetRepo.GetComponentVersion and got.Component.Resources)
but does not validate blob contents; fetch the transferred resource blob (use
the resource's access/digest from got.Component.Resources[0].Access or similar)
via the target repository client (e.g., targetRepo's blob download/Read/GetBlob
method) and assert its bytes equal the original resourceContent variable; update
the test after the GetComponentVersion block to download the resource payload
and r.Equal(resourceContent, downloadedBytes).
In `@bindings/go/examples/README.md`:
- Line 45: The markdown code fence for the concept-map block in README.md is
missing a language tag which triggers MD040; update the opening fence from ```
to include a language identifier (e.g., change the opening fence to ```text) so
the concept-map fenced block has a language tag and satisfies markdownlint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0168d173-9e72-46e0-8358-cfc213bad7bd
⛔ Files ignored due to path filters (1)
bindings/go/examples/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.github/config/wordlist.txtREADME.mdTaskfile.ymlbindings/go/README.mdbindings/go/examples/01_blob_test.gobindings/go/examples/02_descriptor_test.gobindings/go/examples/03_credentials_test.gobindings/go/examples/04_repository_test.gobindings/go/examples/05_signing_test.gobindings/go/examples/06_oci_test.gobindings/go/examples/07_transfer_test.gobindings/go/examples/README.mdbindings/go/examples/Taskfile.ymlbindings/go/examples/doc_test.gobindings/go/examples/go.modreuse.Taskfile.yml
✅ Files skipped from review due to trivial changes (10)
- .github/config/wordlist.txt
- bindings/go/examples/doc_test.go
- README.md
- bindings/go/README.md
- Taskfile.yml
- bindings/go/examples/03_credentials_test.go
- bindings/go/examples/go.mod
- bindings/go/examples/02_descriptor_test.go
- bindings/go/examples/01_blob_test.go
- bindings/go/examples/05_signing_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- bindings/go/examples/Taskfile.yml
- reuse.Taskfile.yml
- bindings/go/examples/06_oci_test.go
- Use transfer.WithTransfer/Component/FromRepository/ToRepositorySpec
instead of a hand-rolled singleRepoResolver struct
- Update transfer dependency to v0.0.0-20260326100711-8cef52a35973
which contains the redesigned N:M routing API
- Assert transferred resource payload in TestExample_TransferCTFtoCTF
- Remove unused newTestScheme() and its imports from 04_repository_test.go
- Remove redundant alias from repository import in 06_oci_test.go
- Fix misleading comment at step 5 in 06_oci_test.go
- Add test/full Taskfile target; default test runs with -short
- Fix missing language tag on fenced concept map block in examples README
- Fix contradictory "no external services" wording in bindings/go README;
add Transfer to the examples bullet list
- Fix capitalization in root README ("Bindings"/"Focus" → lowercase)
Signed-off-by: Jakob Möller <contact@jakob-moeller.com>
Adds TestExample_PrivateOCIRegistry to step 6, demonstrating how to authenticate against a password-protected registry using OCM's credential resolution system. The example shows deriving a consumer identity from the registry URL via credidentity.IdentityFromOCIRepository, wiring it into a StaticCredentialsResolver, and using it alongside auth.StaticCredential to push and retrieve a component version from a private registry. Closes the review request from frewilhelm on PR open-component-model#2054. Signed-off-by: Jakob Möller <contact@jakob-moeller.com>
…gistry test Signed-off-by: Jakob Möller <contact@jakob-moeller.com>
What this PR does / why we need it
Which issue(s) this PR fixes
creates an example tour
Testing
How to test the changes
described in the tour readme!
fix open-component-model/ocm-project#932