Skip to content

feat(oci): enhance repository provider with user agent, temp dir, and caching support#963

Merged
fabianburth merged 15 commits into
open-component-model:mainfrom
fabianburth:fix/multi-ctf-race-oci
Oct 7, 2025
Merged

feat(oci): enhance repository provider with user agent, temp dir, and caching support#963
fabianburth merged 15 commits into
open-component-model:mainfrom
fabianburth:fix/multi-ctf-race-oci

Conversation

@fabianburth

@fabianburth fabianburth commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Core features:

  • Implements a storeCache, enabling repository caching based on path to enable sharing a lock.
    This (hopefully) fixes the current bugs related to race conditions.
  • Prevents digests from being used as tag and correspondingly prevents list to attempt to resolve artifacts with empty tags. This should fix the race condition between AddComponentVersion and ListComponentVersion.

Summary:

  • Introduces new options (WithUserAgent, WithTempDir) for more flexible configuration of repository providers.
  • Adds a default UserAgent value (DefaultCreator).
  • Updates NewFromCTFRepoV1 to return both repository and store, allowing to cache the store.
  • Refactors OpenCTFByFileExtension for better modularity by separating format detection into DiscoverCTFFormatFromPath.

Implications

  • Improves control over temporary working directories and user agent settings.
  • Enhances compatibility and performance when working with CTF archives.
  • Existing repository logic adapts seamlessly to changes, maintaining backward compatibility for most use cases.

Which issue(s) this PR fixes

Contributes to open-component-model/ocm-project#564

@fabianburth fabianburth requested a review from a team as a code owner September 30, 2025 09:03
@github-actions github-actions Bot added !BREAKING-CHANGE! Breaking change in API or ocm-cli or spec kind/feature new feature, enhancement, improvement, extension size/m Medium labels Sep 30, 2025
@fabianburth fabianburth force-pushed the fix/multi-ctf-race-oci branch from 762a948 to d440648 Compare September 30, 2025 09:42
@fabianburth fabianburth marked this pull request as draft September 30, 2025 11:27
Comment thread bindings/go/oci/repository/repository_test.go Outdated
#### What this PR does / why we need it
Introduces options for configuring the `CachingComponentVersionRepositoryProvider`:
- **UserAgent**: Ability to set a custom user agent header.
- **TempDir**: Define a temporary directory for repository operations.

#### Key changes
- Added an `Options` struct and corresponding `WithTempDir` and `WithUserAgent` helpers.
- Updated repository provider constructor to accept these options.
- Enhanced caching capabilities with an internal `storeCache`.
- Adjusted `NewFromCTFRepoV1` to return both the repository and store, simplifying repository reuse.
- Validation checks for unsupported archive operations on read-write access.

#### Implications
This enhances flexibility by supporting configurable repository behavior, improving cache management, and enforcing operation safety on restricted formats.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
#### Why this change was made
Replaced the `ctf` module dependency with a forked version hosted on GitHub:
- Ensures compatibility with recent updates or fixes in the forked version.
- Removes reliance on the older `ocm.software` hosted variant.

#### Key changes
- Introduced `replace` directive in `go.mod` to point to the forked dependency.
- Updated `go.sum` to reflect the new checksums for the replaced module.
- Removed outdated `ctf` module references.

#### Implications
- Resolves potential issues associated with the older dependency version.
- Aligns the project with the updated forked repository to improve maintainability.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
…chive handling

---

#### Why this change was made
- Updated `ctf.OpenCTFByFileExtension` to discard unused return values for clarity and correctness.
- Fixed potential misuse in the credential cache retrieval logic by safely accessing the `store`.

---

#### Key changes
- Switched to explicitly ignoring the unused `format` value from `ctf.OpenCTFByFileExtension` for better readability.
- Removed unnecessary variable destructuring in `credential_cache.go` to prevent silent errors.

---

#### Implications
- Enhances code maintainability by removing misleading or redundant constructs.
- Reduces the chance of introducing unexpected behavior in future updates.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
---

#### Why this change was made
- Inserted a blank line in `go.mod` to follow standard formatting conventions.

#### Key changes
- Added a blank line between the `replace` directive and `require` block for improved readability.

#### Implications
- Ensures adherence to Go module formatting best practices.
- Enhances the readability and maintainability of `go.mod`.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
---

#### Why this change was made
Refactored the repository provider and caching mechanisms for improved readability, maintainability, and safety:
- Simplified store caching with `loadOrStore` to reduce boilerplate code and ensure thread safety.
- Modularized `NewFromCTFRepoV1` by delegating store creation to `NewStoreFromCTFRepoV1`, enhancing composability.

---

#### Key changes
- Replaced `storeCache.get` and `storeCache.add` methods with `loadOrStore` for atomic cache operations.
- Decoupled store creation logic into `NewStoreFromCTFRepoV1` for easier reuse.
- Updated concurrency handling in `Resolve` to use mutual exclusion for safety.
- Adjusted dependencies in `go.mod` and `go.sum` to use `ctf v0.3.0`.

---

#### Implications
- Reduces redundancy and potential race conditions in cache handling.
- Makes repository creation logic modular and easier to maintain.
- Updates dependency for compatibility with recent changes.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
---

#### Why this change was made
- Removed the redundant third return value (`store`) in `NewFromCTFRepoV1` calls within repository tests.
- Adjusted test logic to align with the updated function signature.

---

#### Key changes
- Refactored `repository_test.go` to exclude the unused `store` variable.
- Updated assertions to maintain proper error handling without the removed return value.

---

#### Implications
- Enhances code clarity and reduces unnecessary complexity in test cases.
- Aligns with recent modularization and simplifications in repository creation logic.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
---

#### Why this change was made
- Improved concurrency safety in test cases for `ComponentVersion` operations.
- Introduced a thread-safe `storeCache` to manage repository stores.
- Replaced `tags` locking mechanism in `store.go` for stricter mutual exclusion.

---

#### Key changes
- Added a new `storeCache` implementation to `provider` for atomic store handling, ensuring consistent behavior across multiple threads.
- Refactored tests to simulate concurrent access scenarios with isolated versions to prevent blob write races.
- Enhanced `Tags` method in `store.go` to use exclusive locking for race prevention.
- Removed redundant `storeCache` implementation from `credential_cache.go`.

---

#### Implications
- Improves test reliability in simulated concurrent repository operations.
- Reduces the risk of data corruption and races in repository store caching and blob management.
- Simplifies and centralizes store caching logic for better maintainability.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth fabianburth force-pushed the fix/multi-ctf-race-oci branch from b51005a to ee6d166 Compare October 6, 2025 13:53
@fabianburth fabianburth self-assigned this Oct 6, 2025
Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@jakobmoellerdev jakobmoellerdev marked this pull request as ready for review October 6, 2025 15:13
ikhandamirov
ikhandamirov previously approved these changes Oct 7, 2025
Comment thread bindings/go/oci/ctf/store.go Outdated
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
ikhandamirov
ikhandamirov previously approved these changes Oct 7, 2025

@jakobmoellerdev jakobmoellerdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty much only nits on docs due to the complexity of the change. change itself is lgtm

Comment thread bindings/go/oci/ctf/store.go
Comment thread bindings/go/oci/ctf/store.go Outdated
Comment thread bindings/go/oci/ctf/store.go Outdated
Comment thread bindings/go/oci/repository/provider/options.go
Comment thread bindings/go/oci/repository/provider/provider.go Outdated
Comment thread bindings/go/oci/repository/provider/provider.go
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth fabianburth changed the title feat(oci)!: enhance repository provider with user agent, temp dir, and caching support feat(oci): enhance repository provider with user agent, temp dir, and caching support Oct 7, 2025
@fabianburth fabianburth removed the !BREAKING-CHANGE! Breaking change in API or ocm-cli or spec label Oct 7, 2025
@fabianburth fabianburth merged commit 0f6fd5a into open-component-model:main Oct 7, 2025
22 checks passed
@fabianburth fabianburth deleted the fix/multi-ctf-race-oci branch October 7, 2025 09:16
matthiasbruns pushed a commit that referenced this pull request Oct 7, 2025
<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it
This PR fixes is supposed to fix the race conditions occuring when
multiple ctf instances operate on the same ctf archive. The actual fix
is provided in
#964
and
#963.
Here, a the archive instance and its lock are shared between multiple
ctf repositories operating on the archive.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Contributes to
open-component-model/ocm-project#564

---------

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature new feature, enhancement, improvement, extension size/m Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants