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.

Sign up for GitHub

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

Jump to bottom

feat: replace docker with oras #904

Merged
merged 4 commits into from
Sep 26, 2024
Merged

feat: replace docker with oras #904

merged 4 commits into from
Sep 26, 2024

Conversation

Skarlso
Copy link
Contributor

@Skarlso Skarlso commented Aug 30, 2024 •
edited
Loading

This is a highly Draft pull request understanding the nuances of the OCI package.

There are a LOT of things to do yet:

  • Figure out to nicely create repositories with hosts
  • Add authentication configuration to the OCI repository creation
  • Figure out which store to use and which implementation to use where
  • Figure out the least intrusive modification that still works
    • this will partially be done via declared interfaces to support previous behaviour

@github-actions github-actions bot added the size/l Large label Aug 30, 2024
@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch 2 times, most recently from 276de89 to 9ad6f27 Compare August 30, 2024 14:41
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Aug 30, 2024

Integration Tests for 1682667 run with result: Success ✅!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 30, 2024 •
edited
Loading

Mend Scan Summary: ❌

Repository: open-component-model/ocm

VIOLATION DESCRIPTION NUMBER OF VIOLATIONS
HIGH/CRITICAL SECURITY VULNERABILITIES 4
MAJOR UPDATES AVAILABLE 0
LICENSE REQUIRES REVIEW 2
HIGH RISK LICENSES 10
RESTRICTIED LICENSE FOR ON-PREMISE DELIVERY 0

Detailed Logs: mend-scan-> Generate Report
Mend UI

@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Aug 30, 2024

Integration Tests for 1682667 run with result: Success ✅!

@Skarlso
Copy link
Contributor Author

Skarlso commented Sep 2, 2024 •
edited
Loading

Okay, I made the thing work, but it's kinda an ugly workaround I don't much like. But we might have to live with. Let's see how it play out.

REGISTRY REPOSITORY                       KIND     TAG     DIGEST
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.1.2  sha256:0d88d0ee812e10ef2683956bbb6a3d13ff254fd5a80395a61850969672bc458a
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.6  sha256:1192b8e7b091a50d721a588cc8952bce28d8a903441399b24109da8106f9447f
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.4  sha256:1acbd43cec6e30007556af51057b4f9acb6266932f805350e9971f188f5dda54
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.4.1  sha256:1c99548424582f7c01bfe49f626ca0d4a89ba88723c78b782f922beff3451a71
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.1.1  sha256:3461e5031ebb93f97c4a0decd58bcc947f33ad7f7ef82ce30d8ffdc3540be59b
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.6.0  sha256:3695b88bae23f70f2b4378dd84e2d52ebb7abd2559a095f1f7dc813a6d8a63d5
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.4.2  sha256:3be7403dbe91031654ce30c4528babcc48c1b72e9697fadca2a0f3db73bf1c34
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.5.3  sha256:3f2a56b16b9167ed338a757cf079c78191ef1f4ceab83860b2d486bdf6a2c0ad
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.7.0  sha256:428efd9683e63365341601f3e4c647afe39433093ba0dec00edd83bb16aefe5b
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.5  sha256:494d41c43ed7ef1e9288faa61eb16cac42f282d1dc82ee292e9fc9e8509f6041
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.2  sha256:5018d246d97cd139990c731b307e55592805ca08296db17fe5742ca6d552d973
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.5.1  sha256:5d2d6273541ea6639e709333a9d1d2b9bcf14103de9005d7d5dedfb1002b71fc
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.5.0  sha256:5dda48e405509881bb709bd0f3dc071e39951f97145d90a6da3850e12720cd79
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.2.0  sha256:6bcdf8db8b903a852a7360078107cea686fbc8539b180a3a7d677408b5e3fb4a
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.11 sha256:734843cf22fe96add178b5c066dda49677145e5f8e13c7161054e67f21883afa
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.10 sha256:7f16a7d6bc790060e99c7973a5ac8ab2218221235497e4357933f6585c284787
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.1.0  sha256:80660ce2b58a7f6ee1429478009bcd4710a8c07d73da312b89f784a171e0f07a
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.8.0  sha256:82d91fbe73d72ac8001cb43ec7dcc371428bad6960a1d523a6c96f752431281c
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.0.1  sha256:8a20a73c484dfaea11328384d3bd00410778027144144d4d32a21721ef8d12a4
ghcr.io  skarlso/crd-bootstrap-controller manifest latest  sha256:c78d260d00a7a6dfc4e6174fd98bbe0445a87272cd15c7b8ac863befe40b17cc
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.9.0  sha256:c78d260d00a7a6dfc4e6174fd98bbe0445a87272cd15c7b8ac863befe40b17cc
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.7  sha256:c8ff7a9b501c97489f15ccfee70a1d34544ad60e17e475964a68ce8d65102d8e
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.3.9  sha256:d4ec1b234fd4de93f3de9c007f6602399fdea12816a6698beb10019c7d90523a
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.4.0  sha256:d9ec40eb5d3c74bb1bf94b5981d20170517f2adfcf82cef9511074c9be2dd9a9
ghcr.io  skarlso/crd-bootstrap-controller manifest v0.5.2  sha256:ea2f4ac283ddb91484530550f52aa20cd11c8fe20e6a2dfc9f58710684ccb9d4

Right now, there is a "problem" with ORAS where you can't fetch anything without first knowing the Size. We can gather the size, by doing a Resolve call beforehand. But that needed to include the REF where there is a GetBlobData call using OCI. That needed a bit of refactoring passing it around.

Now, docker also did this but it did it separately and included in the client. I might also make the same refactor here. But suffice to say, the Ref needs to be passed around for oras to work right now.

Now, doing a Resolve should be fine even if delay is set, because it's not a Fetch call. So no data is retrieved. But it does retrieve the size. So doing something like this works:

resolvedDescriptor, err := resolver.Resolve(dummyContext, ref+"@"+digest.String())

But it's kinda a hack and I don't necessarily like it. Let's see what oras peeps reply to my issue on fetching without size because removing their size check makes everything work supernicely. So I don't really understand why they would have that restriction ¯_(ツ)_/¯.

Let's see how it goes.

The issue oras-project/oras-go#822

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from dee7266 to 478f787 Compare September 3, 2024 06:48
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 3, 2024

Integration Tests for ca95e2f run with result: Success ✅!

1 similar comment
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 3, 2024

Integration Tests for ca95e2f run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from efac9b6 to 3bafb9e Compare September 3, 2024 07:28
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 3, 2024

Integration Tests for 327ca9c run with result: Success ✅!

@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 4, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from d66df62 to b0646fa Compare September 4, 2024 12:20
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 4, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from cd1234d to 7259b77 Compare September 4, 2024 12:29
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 4, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

1 similar comment
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 4, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from eab85ca to d633138 Compare September 4, 2024 14:20
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 4, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

@Skarlso Skarlso added the area/ipcei Important Project of Common European Interest label Sep 5, 2024
@Skarlso
Copy link
Contributor Author

Skarlso commented Sep 5, 2024

Credential access is working:

ocm --cred :type=OCIRegistry --cred :hostname=ghcr.io --cred username=skarlso --cred password=${GITHUB_TOKEN} oci artifacts get ghcr.io/open-component-model/git-sync-controller
CREDS:  {"password":"<reducted>","username":"skarlso"}
AUTH CREDS:  {skarlso <reducted>  }
REGISTRY REPOSITORY                               KIND     TAG              DIGEST
ghcr.io  open-component-model/git-sync-controller manifest build-automation sha256:6f73f48cb1efa62b8b0f5a239878cb5106b5ae15d6517c108173cc86fe19d706
ghcr.io  open-component-model/git-sync-controller manifest v0.1.0           sha256:8f01ab181700a5e1890078ea78b9d4215c2c891eaba25f220c0c1cc19809c876
ghcr.io  open-component-model/git-sync-controller manifest latest           sha256:8f01ab181700a5e1890078ea78b9d4215c2c891eaba25f220c0c1cc19809c876

The only problem is that it's not working with the docker file right now. It's not setting password as a token, which is required by the static credential check.

@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 5, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

1 similar comment
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 5, 2024

Integration Tests for 90f15d2 run with result: Success ✅!

@Skarlso Skarlso marked this pull request as ready for review September 6, 2024 07:39
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 9, 2024

Integration Tests for 9025236 run with result: Success ✅!

@Skarlso Skarlso mentioned this pull request Sep 9, 2024
Closed
@Skarlso
Copy link
Contributor Author

Skarlso commented Sep 11, 2024

This is odd. Looks like linter is failing locally, but doesn't fail in Github. Very interesting.

@Skarlso
Copy link
Contributor Author

Skarlso commented Sep 17, 2024

Yes we are waiting for Uwe.

mandelsoft
mandelsoft requested changes Sep 19, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
api/credentials/utils.go Show resolved Hide resolved
api/oci/cpi/support/artifact.go Outdated Show resolved Hide resolved
api/oci/cpi/support/flavor_index.go Outdated Show resolved Hide resolved
api/oci/extensions/repositories/ocireg/blobs.go Outdated Show resolved Hide resolved
api/oci/extensions/repositories/ocireg/repository.go Show resolved Hide resolved
api/ocm/tools/signing/digest.go Show resolved Hide resolved
api/version/version.go Show resolved Hide resolved
cmds/ocm/common/output/table.go Show resolved Hide resolved
docs/reference/ocm_add_componentversions.md Outdated Show resolved Hide resolved
@Skarlso Skarlso requested a review from a team as a code owner September 20, 2024 08:53
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from 5b09779 to f5d50c8 Compare September 20, 2024 09:06
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from f5d50c8 to 0181bdd Compare September 20, 2024 09:19
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from 0181bdd to e4b1c43 Compare September 20, 2024 09:24
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from 19fc20a to 72cb3ff Compare September 20, 2024 09:45
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from 72cb3ff to 821e5e5 Compare September 20, 2024 10:12
@ocmbot OCMBot
Copy link
Contributor

ocmbot bot commented Sep 20, 2024

Integration Tests for 7b3e61b run with result: Success ✅!

@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from a8f9a35 to ed725a6 Compare September 24, 2024 07:22
@Skarlso Skarlso force-pushed the replace-oci-package-with-something-else branch from 1acb53f to d6356db Compare September 25, 2024 11:49
@Skarlso Skarlso merged commit 3cfa228 into main Sep 26, 2024
19 checks passed
@Skarlso Skarlso deleted the replace-oci-package-with-something-else branch September 26, 2024 11:07
@Skarlso Skarlso mentioned this pull request Oct 4, 2024
Closed
1 task
@frewilhelm frewilhelm mentioned this pull request Oct 17, 2024
Merged
frewilhelm added a commit that referenced this pull request Oct 17, 2024
@frewilhelm @Skarlso
fix: ignore ORA error instead of containerd error (#975)
ec6e928 
Related to #872 and
#904

---------

Co-authored-by: Gergely Brautigam <[email protected]>
Skarlso added a commit that referenced this pull request Oct 22, 2024
@frewilhelm @Skarlso
fix: ignore ORA error instead of containerd error (#975)
01d9ca7 
Related to #872 and
#904

---------

Co-authored-by: Gergely Brautigam <[email protected]>
Skarlso added a commit that referenced this pull request Oct 22, 2024
@Skarlso
Revert "feat: replace docker with oras (#904)"
d93468a 
This reverts commit 3cfa228.
Skarlso added a commit that referenced this pull request Oct 24, 2024
@Skarlso @jakobmoellerdev
Revert "feat: replace docker with oras (#904)" (#1005)
c76a2ed 
This reverts commit 3cfa228.

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

---------

Co-authored-by: Jakob Möller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hilmarf hilmarf hilmarf left review comments

@frewilhelm frewilhelm frewilhelm left review comments

@mandelsoft mandelsoft mandelsoft approved these changes

@fabianburth fabianburth Awaiting requested review from fabianburth

Assignees
No one assigned
Labels
area/ipcei Important Project of Common European Interest size/l Large
Projects
OCM Backlog Board
Status: 🔒Closed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Skarlso @hilmarf @morri-son @mandelsoft @frewilhelm