Split out OCI store #3371
Merged
Split out OCI store #3371
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jedevc
force-pushed
the
oci-split-out-store
branch
from
8e4b7a6
to
7e10a01
Compare
|
@jedevc Needs rebase. |
jedevc
force-pushed
the
oci-split-out-store
branch
from
7e10a01
to
c79e797
Compare
tonistiigi
approved these changes
Dec 13, 2022
We don't need multiple caps for a single feature - the caps that these were copied by were addded over time, we don't immediately need all of them to start with, only the main feature one is initially required. Signed-off-by: Justin Chadwell <me@jedevc.com>
SessionID is only used by the OCI resolver, so it shouldn't be included for normal docker image resolution. Signed-off-by: Justin Chadwell <me@jedevc.com>
This breaking api change refactors the LLB API to prevent reference mangling and demangling throughout OCI access. Once the session and store IDs have been determined in the dockerfile frontend, we keep them the same, and attach them as additional properties. This has the additional effect of making the actual reference used in the image resolution arbitrary, since we only parse and access the digest. The rest of the name can be selected to optimize for log readability. Signed-off-by: Justin Chadwell <me@jedevc.com>
Now that the reference is not significant to image resolution, with only the digest being used to determine what to lookup, the only effect of the dummy reference is to provide reasonable error messages. Since the name of the store is likely not significant, using that as the basis for the reference may be deceiving. We can use the original name of the image as the dummy reference, except with the targeted digest. We can't easily require that clients should expose the name of the store, since that may be a leak of the local path to the store, which may be private. Signed-off-by: Justin Chadwell <me@jedevc.com>
tonistiigi
force-pushed
the
oci-split-out-store
branch
from
c79e797
to
e2185a7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
⬆️ Follow-up to #2827 #3118
⚠️ Breaks API compatibility with previous OCI implementation (should be alright, as long as it merges before v0.11 is released)
This PR performs a couple of OCI refactors on-top of the ones already done in #3300:
Removes unnecessary caps - the feature only requires a single extra cap, instead of the 3 that were previously defined. Since the existence of the main OCI cap implied the existence of the other ones, we don't need to have the other ones, more caps should only be required as we increase the scope of the feature over time.
Removes the unnecessary SessionID attachment to the normal docker image resolver, since it's only relevant when loading from OCI stores in the OCI image resolver.
Refactors the store id out of the main image reference, and instead attaches the store id as a separate property. This prevent us from needing to do name mangling and de-mangling (preventing us from needing the
Hostnamecall).Finally, since the reference then becomes completely irrelevant except for the digest portion, we can name it appropriately to ensure that we get reasonable error messages. Instead of using the store name as the basis for the dummy ref, we keep the original name, which allows the client to perform content store mappings as it desires - this pairs well with build: refactor reference parsing for oci image layouts docker/buildx#1456, which uses randomly generated identifiers as the store names.
Before:
After: