Skip to content

Commit 2fe75a9

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request cri-o#9159 from saschagrunert/artifact-subpath
Support artifact mount sub paths
2 parents f85046f + 2ac913d commit 2fe75a9

File tree

6 files changed

+173
-2
lines changed

6 files changed

+173
-2
lines changed

‎server/artifacts_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package server_test
2+
3+
import (
4+
"context"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
"github.com/cri-o/cri-o/internal/ociartifact"
10+
"github.com/cri-o/cri-o/server"
11+
)
12+
13+
// The actual test suite.
14+
var _ = t.Describe("Artifacts", func() {
15+
const artifact = "my-artifact"
16+
var (
17+
ctx = context.Background()
18+
paths = []ociartifact.BlobMountPath{
19+
{Name: "1"},
20+
{Name: "dir-1/2"},
21+
{Name: "dir-1/3"},
22+
{Name: "dir-2/4"},
23+
{Name: "dir-2/5"},
24+
}
25+
)
26+
27+
It("should succeed with empty sub path", func() {
28+
// Given
29+
30+
// When
31+
res, err := server.FilterMountPathsBySubPath(ctx, artifact, "", paths)
32+
33+
// Then
34+
Expect(err).NotTo(HaveOccurred())
35+
Expect(res).To(Equal(paths))
36+
})
37+
38+
It("should succeed with filtered sub path", func() {
39+
// Given
40+
41+
// When
42+
res, err := server.FilterMountPathsBySubPath(ctx, artifact, "dir-1", paths)
43+
44+
// Then
45+
Expect(err).NotTo(HaveOccurred())
46+
Expect(res).To(Equal([]ociartifact.BlobMountPath{
47+
{Name: "2"},
48+
{Name: "3"},
49+
}))
50+
})
51+
52+
It("should succeed with '.' sub path", func() {
53+
// Given
54+
55+
// When
56+
res, err := server.FilterMountPathsBySubPath(ctx, artifact, ".", paths)
57+
58+
// Then
59+
Expect(err).NotTo(HaveOccurred())
60+
Expect(res).To(Equal(paths))
61+
})
62+
63+
It("should succeed with './' prefix in sub path", func() {
64+
// Given
65+
66+
// When
67+
res, err := server.FilterMountPathsBySubPath(ctx, artifact, "./dir-1", paths)
68+
69+
// Then
70+
Expect(err).NotTo(HaveOccurred())
71+
Expect(res).To(Equal([]ociartifact.BlobMountPath{
72+
{Name: "2"},
73+
{Name: "3"},
74+
}))
75+
})
76+
77+
It("should fail if sub path is not existing", func() {
78+
// Given
79+
80+
// When
81+
res, err := server.FilterMountPathsBySubPath(ctx, artifact, "dir-3", paths)
82+
83+
// Then
84+
Expect(err).To(HaveOccurred())
85+
Expect(res).To(BeNil())
86+
})
87+
})

‎server/container_create_linux.go‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/cri-o/cri-o/internal/lib/sandbox"
2828
"github.com/cri-o/cri-o/internal/log"
2929
"github.com/cri-o/cri-o/internal/oci"
30+
"github.com/cri-o/cri-o/internal/ociartifact"
3031
crioann "github.com/cri-o/cri-o/pkg/annotations"
3132
)
3233

@@ -217,12 +218,17 @@ func (s *Server) addOCIBindMounts(ctx context.Context, ctr ctrfactory.Container,
217218

218219
continue
219220
}
221+
222+
// Don't fallback to an image mount if we already encounter an ErrImageVolumeMountFailed error
223+
if errors.Is(err, crierrors.ErrImageVolumeMountFailed) {
224+
return nil, nil, nil, err
225+
}
226+
227+
log.Warnf(ctx, "Artifact mount failed, falling back to image mount: %v", err)
220228
} else {
221229
log.Debugf(ctx, "Skipping artifact mount because OCI artifact mount support is disabled")
222230
}
223231

224-
log.Warnf(ctx, "Artifact mount failed with %s. Falling back to image mount", err)
225-
226232
volume, safeMount, err := s.mountImage(ctx, specgen, imageVolumesPath, m, runDir)
227233
if err != nil {
228234
return nil, nil, nil, fmt.Errorf("%w: %w", crierrors.ErrImageVolumeMountFailed, err)
@@ -428,6 +434,12 @@ func (s *Server) mountArtifact(ctx context.Context, specgen *generate.Generator,
428434
selinuxRelabel = false
429435
}
430436

437+
paths, err = FilterMountPathsBySubPath(ctx, m.Image.Image, m.ImageSubPath, paths)
438+
if err != nil {
439+
// This error will get reported directly to the end user
440+
return nil, err
441+
}
442+
431443
for _, path := range paths {
432444
dest, err := securejoin.SecureJoin(m.ContainerPath, path.Name)
433445
if err != nil {
@@ -463,6 +475,34 @@ func (s *Server) mountArtifact(ctx context.Context, specgen *generate.Generator,
463475
return volumes, nil
464476
}
465477

478+
func FilterMountPathsBySubPath(ctx context.Context, artifact, subPath string, paths []ociartifact.BlobMountPath) (filteredPaths []ociartifact.BlobMountPath, err error) {
479+
if subPath == "" || subPath == "." {
480+
return paths, nil
481+
}
482+
483+
cleanSubPath := filepath.Clean(subPath) + "/"
484+
485+
if !slices.ContainsFunc(paths, func(val ociartifact.BlobMountPath) bool {
486+
return strings.HasPrefix(val.Name, cleanSubPath)
487+
}) {
488+
return nil, fmt.Errorf("%w: sub path %q does not exist in OCI artifact volume %q", crierrors.ErrImageVolumeMountFailed, subPath, artifact)
489+
}
490+
491+
for _, path := range paths {
492+
if !strings.HasPrefix(path.Name, cleanSubPath) {
493+
log.Debugf(ctx, "Skipping to mount artifact path %q because it's not a sub path of %q", path.Name, subPath)
494+
495+
continue
496+
}
497+
498+
newPath := strings.TrimPrefix(path.Name, cleanSubPath)
499+
log.Debugf(ctx, "Modifying artifact mount path from %q to %q because of user specified sub path %q", path.Name, newPath, cleanSubPath)
500+
filteredPaths = append(filteredPaths, ociartifact.BlobMountPath{Name: newPath, SourcePath: path.SourcePath})
501+
}
502+
503+
return filteredPaths, nil
504+
}
505+
466506
// mountImage adds required image mounts to the provided spec generator and returns a corresponding ContainerVolume.
467507
func (s *Server) mountImage(ctx context.Context, specgen *generate.Generator, imageVolumesPath string, m *types.Mount, runDir string) (*oci.ContainerVolume, *safeMountInfo, error) {
468508
if m == nil || m.Image == nil || m.Image.Image == "" || m.ContainerPath == "" {

‎test/oci_artifacts.bats‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ function teardown() {
1313

1414
ARTIFACT_REPO=quay.io/crio/artifact
1515
ARTIFACT_IMAGE="$ARTIFACT_REPO:singlefile"
16+
ARTIFACT_IMAGE_SUBPATH="$ARTIFACT_REPO:subpath"
1617

1718
@test "should be able to pull and list an OCI artifact" {
1819
start_crio
@@ -225,3 +226,43 @@ EOF
225226

226227
rm -f "$ARTIFACT_CONFIG"
227228
}
229+
230+
@test "should be able to mount OCI Artifact with sub path" {
231+
start_crio
232+
crictl pull $ARTIFACT_IMAGE_SUBPATH
233+
pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json)
234+
jq --arg ARTIFACT_IMAGE_SUBPATH "$ARTIFACT_IMAGE_SUBPATH" \
235+
'.mounts = [ {
236+
container_path: "/root/artifact",
237+
image: { image: $ARTIFACT_IMAGE_SUBPATH },
238+
image_sub_path: "subpath"
239+
} ] |
240+
.command = ["sleep", "3600"]' \
241+
"$TESTDATA"/container_config.json > "$TESTDIR/container_config.json"
242+
ctr_id=$(crictl create "$pod_id" "$TESTDIR/container_config.json" "$TESTDATA/sandbox_config.json")
243+
crictl start "$ctr_id"
244+
245+
# The artifact should get mounted with the correct sub path
246+
run crictl exec --sync "$ctr_id" cat /root/artifact/2
247+
[[ "$output" == "2" ]]
248+
249+
run crictl exec --sync "$ctr_id" cat /root/artifact/3
250+
[[ "$output" == "3" ]]
251+
}
252+
253+
@test "should fail to mount OCI Artifact with sub path if not existing" {
254+
start_crio
255+
crictl pull $ARTIFACT_IMAGE_SUBPATH
256+
pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json)
257+
jq --arg ARTIFACT_IMAGE_SUBPATH "$ARTIFACT_IMAGE_SUBPATH" \
258+
'.mounts = [ {
259+
container_path: "/root/artifact",
260+
image: { image: $ARTIFACT_IMAGE_SUBPATH },
261+
image_sub_path: "subpath-not-existing"
262+
} ] |
263+
.command = ["sleep", "3600"]' \
264+
"$TESTDATA"/container_config.json > "$TESTDIR/container_config.json"
265+
run ! crictl create "$pod_id" "$TESTDIR/container_config.json" "$TESTDATA/sandbox_config.json"
266+
267+
[[ "$output" == *"ImageVolumeMountFailed"*"does not exist in OCI artifact volume"* ]]
268+
}

‎test/testdata/artifacts/push-oci-artifacts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ cd "$ARTIFACT_DIR"
1414
oras push "$REPOSITORY:singlefile" --artifact-type application/x.test.test.test.v1 artifact.txt
1515
oras push "$REPOSITORY:exec" --artifact-type application/x.test.test.test.v1 artifact.sh
1616
oras push "$REPOSITORY:multiplefiles" --artifact-type application/x.test.test.test.v1 artifact.txt artifact.sh
17+
oras push "$REPOSITORY:subpath" --artifact-type application/x.test.test.test.v1 subpath/2:text/plain subpath/3:text/plain

‎test/testdata/artifacts/subpath/2‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
2

‎test/testdata/artifacts/subpath/3‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
3

0 commit comments

Comments
 (0)