Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: git Access, AccessMethod through BlobAccess #869

Merged
merged 28 commits into from
Jan 6, 2025

Conversation

jakobmoellerdev
Copy link
Contributor

@jakobmoellerdev jakobmoellerdev commented Aug 10, 2024

What this PR does / why we need it:

This is an extremely early (WIP) draft of implementing ctf archives in directory format tracked via git repositories, ultimately serving me as a playground to get familiar with OCM and also to play around with its feature set. So we do not need to merge this if there is no interest or disagreement on the implementation.

This PR has developed into the development of a pure access method and input method that can be used to work with arbitrary git repositories. Examples for a component constructor could be:

name: test.de/x
version: %s
provider:
  name: ocm
resources:
- name: hello-world-access
  type: git
  version: 0.0.1
  access:
    type: git
    commit: "7fd1a60b01f91b314f59955a4e4d4e80d8edf11d"
    ref: refs/heads/master
    repository: https://github.com/octocat/Hello-World.git
- name: hello-world-input
  type: git
  version: 0.0.1
  input:
    type: git
    ref: refs/heads/master
    commit: "7fd1a60b01f91b314f59955a4e4d4e80d8edf11d"
    repository: https://github.com/octocat/Hello-World.git

Not only does this allow using git repositories for storing blobs, it also attempts to add git downloader functionality for a generic git access method based on in memory cloning and I plan to add a OCM implementation on it as well. While the download is available, this is not laid out to the CLI, library only.

Currently the implementation is written so that any modification to the CTF in the work directory also triggers a dedicated commit, and any lookup triggers a refres/pull cycle on git.

For testing I use file:// URLs which makes it quite easy to verify E2E in the test suites.

TODO:

  • Implement Access Credential mapping
  • Make Git Client thread-safe, optionally in-memory and allow more customization (currently hard bount to one target ref that I am creating or syncing against, see tests for examples)
  • Expand Git Options - Authentication support for repo and client

Which issue(s) this PR fixes:

As this is my own "experimentation" issue, it doesnt necessarily fix anything, it might as well be moved to a plugin for example or be scrapped alltogether

Creates an access method for arbitrary git repositories using git-go.

@Skarlso
Copy link
Contributor

Skarlso commented Aug 13, 2024

Hello.

So this work happens to be implementing this issue open-component-model/ocm-project#239.

I'll assign it to you then.

@jakobmoellerdev jakobmoellerdev force-pushed the git-ctf branch 2 times, most recently from 35d58d8 to 85476f7 Compare August 20, 2024 15:05
@jakobmoellerdev jakobmoellerdev changed the title feat: git based ctf tracking repositories feat: git Access, AccessMethods, Repositories backed by CTF Aug 20, 2024
@jakobmoellerdev jakobmoellerdev changed the title feat: git Access, AccessMethods, Repositories backed by CTF feat: git Access, AccessMethod, Repositories backed by CTF Aug 20, 2024
@hilmarf hilmarf added the kind/feature new feature, enhancement, improvement, extension label Aug 26, 2024
@mandelsoft
Copy link
Contributor

Hi @jakobmoellerdev, concerning your idea of git backed CTF repositories, if I understand your approach correctly,
every change will create a new commit. I think this approach misses the need for a semantical bracket for a set of changes.
It is more like creating a commit for every changed line in a source file, which does not look like a good idea.
A useful bracket (which is already supported) could be the open/close of a CTF. The typical workflow is to open a CTF, apply some changes (for example with the ocm add cv command) and then close it again. So a more useful point in time to create a new commit should be the Close operation of a CTF.

@jakobmoellerdev
Copy link
Contributor Author

Hi @mandelsoft, thanks for taking a look.

I think the semantical equivalent to a git push would be to close the repository. In your description that would be

  1. Open CTF -> Git Clone / Checkout
  2. Apply Changes (e.g. add cv) -> Write/Update Files in CTF, Commit atomically
  3. Close CTF -> Git Push

Of course we can change the flow to

  1. Open CTF -> Git Clone / Checkout
  2. Apply Changes (e.g. add cv) -> Write/Update Files in CTF,
  3. Close CTF -> Git Commit all changes, Git Push

If you are concerned with that. In my view they are equivalent and the atomic commits are much more transparent when a change was made. However Im totally fine with your take as well, it shouldnt be too hard to change.

I think we could change it so that there is one commit that has all the current commit messages in the commit body. Then we will have 1 commit. WDYT?

@jakobmoellerdev jakobmoellerdev force-pushed the git-ctf branch 2 times, most recently from 46d6388 to 23271da Compare September 9, 2024 16:00
@jakobmoellerdev
Copy link
Contributor Author

this is ready for a first pass review. We might change the way the commits are generated (see discussion above) but generally i would still like some input as its my first take on a larger change in this codebase

@jakobmoellerdev jakobmoellerdev force-pushed the git-ctf branch 2 times, most recently from 593ea06 to 024b5e6 Compare September 25, 2024 11:28
@jakobmoellerdev jakobmoellerdev marked this pull request as ready for review September 25, 2024 12:31
@jakobmoellerdev jakobmoellerdev requested a review from a team as a code owner September 25, 2024 12:31
api/credentials/builtin/git/identity/identity.go Outdated Show resolved Hide resolved
api/credentials/builtin/git/identity/identity.go Outdated Show resolved Hide resolved
api/credentials/builtin/git/identity/identity.go Outdated Show resolved Hide resolved
api/credentials/builtin/git/identity/identity.go Outdated Show resolved Hide resolved
api/credentials/builtin/git/identity/identity_test.go Outdated Show resolved Hide resolved
api/ocm/extensions/accessmethods/git/README.md Outdated Show resolved Hide resolved
api/ocm/extensions/accessmethods/git/method.go Outdated Show resolved Hide resolved
api/ocm/extensions/accessmethods/git/method.go Outdated Show resolved Hide resolved
api/utils/accessio/downloader/git/downloader.go Outdated Show resolved Hide resolved
api/ocm/extensions/accessmethods/git/method.go Outdated Show resolved Hide resolved
@jakobmoellerdev jakobmoellerdev force-pushed the git-ctf branch 7 times, most recently from bf6ad2d to dd310b1 Compare October 8, 2024 10:38
@jakobmoellerdev
Copy link
Contributor Author

Just a quick update on this PR: I didnt get around to it yet, but I will probably remove the repo code because its unlikely to be used (as it all started for me to get into the OCM codebase). The AccessMethod etc can stay in and are still valid though.

@jakobmoellerdev jakobmoellerdev changed the title feat: git Access, AccessMethod, Repositories backed by CTF feat: git Access, AccessMethod backed by CTF Oct 24, 2024
@jakobmoellerdev jakobmoellerdev changed the title feat: git Access, AccessMethod backed by CTF feat: git Access, AccessMethod through BlobAccess Oct 24, 2024
@github-actions github-actions bot added area/documentation Documentation related component/ocm-cli OCM Command Line Interface kind/dependency dependency update, etc. size/l Large labels Nov 28, 2024
@Skarlso Skarlso self-assigned this Dec 4, 2024
Copy link
Contributor

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

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

I have a few questions, but generally, this is working. :D

api/oci/internal/uniform.go Show resolved Hide resolved
api/tech/git/auth.go Show resolved Hide resolved
api/tech/git/identity/identity.go Outdated Show resolved Hide resolved
api/tech/git/resolver.go Show resolved Hide resolved
api/tech/git/resolver.go Outdated Show resolved Hide resolved
api/utils/accessio/downloader/git/downloader.go Outdated Show resolved Hide resolved
api/utils/accessio/downloader/git/downloader.go Outdated Show resolved Hide resolved
api/tech/git/resolver.go Outdated Show resolved Hide resolved
@Skarlso
Copy link
Contributor

Skarlso commented Dec 6, 2024

sap/git-access-test/test.ca on ☁️   [email protected]
❯ l
total 4
drwxr-x--- 3 skarlso staff   96 Dec  6 15:12 blobs/
-rwxr-x--- 1 skarlso staff 1115 Dec  6 15:11 component-descriptor.yaml*
sap/git-access-test/test.ca on ☁️   [email protected]
➜ cd blobs/
git-access-test/test.ca/blobs on ☁️   [email protected]
➜ tar xzf sha256.c3bdcc7317c2afe8a4ef373af21404eece1563db4a51aaab70372455c12c3624
git-access-test/test.ca/blobs on ☁️   [email protected]
➜ cat README
Hello World!
git-access-test/test.ca/blobs on ☁️   [email protected]

@Skarlso
Copy link
Contributor

Skarlso commented Dec 6, 2024

There are a few more tests to do as well.

jakobmoellerdev and others added 2 commits December 16, 2024 17:50
@jakobmoellerdev jakobmoellerdev force-pushed the git-ctf branch 2 times, most recently from f78090d to f33ba3a Compare December 17, 2024 11:23
@hilmarf hilmarf added this to the 2025-Q1 milestone Dec 19, 2024
Copy link
Contributor

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

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

Amazing job!! 👍

@jakobmoellerdev jakobmoellerdev merged commit 48c3ea2 into open-component-model:main Jan 6, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation related component/ocm-cli OCM Command Line Interface kind/dependency dependency update, etc. kind/feature new feature, enhancement, improvement, extension size/l Large
Projects
Status: 🔒Closed
Development

Successfully merging this pull request may close these issues.

5 participants