diff --git a/pkg/libartifact/store/store.go b/pkg/libartifact/store/store.go index 4a690f5426..545bc2f998 100644 --- a/pkg/libartifact/store/store.go +++ b/pkg/libartifact/store/store.go @@ -29,6 +29,7 @@ import ( "github.com/containers/podman/v5/pkg/libartifact" libartTypes "github.com/containers/podman/v5/pkg/libartifact/types" "github.com/containers/storage/pkg/fileutils" + "github.com/containers/storage/pkg/lockfile" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/specs-go" specV1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -44,6 +45,7 @@ const ManifestSchemaVersion = 2 type ArtifactStore struct { SystemContext *types.SystemContext storePath string + lock *lockfile.LockFile } // NewArtifactStore is a constructor for artifact stores. Most artifact dealings depend on this. Store path is @@ -68,7 +70,14 @@ func NewArtifactStore(storePath string, sc *types.SystemContext) (*ArtifactStore if err := os.MkdirAll(baseDir, 0700); err != nil { return nil, err } + // Open the lockfile, creating if necessary + lock, err := lockfile.GetLockFile(filepath.Join(storePath, "index.lock")) + if err != nil { + return nil, err + } + artifactStore.lock = lock // if the index file is not present we need to create an empty one + // Do so after the lock to try and prevent races around store creation. if err := fileutils.Exists(artifactStore.indexPath()); err != nil && errors.Is(err, os.ErrNotExist) { if createErr := artifactStore.createEmptyManifest(); createErr != nil { return nil, createErr @@ -83,6 +92,9 @@ func (as ArtifactStore) Remove(ctx context.Context, name string) (*digest.Digest return nil, ErrEmptyArtifactName } + as.lock.Lock() + defer as.lock.Unlock() + // validate and see if the input is a digest artifacts, err := as.getArtifacts(ctx, nil) if err != nil { @@ -112,6 +124,10 @@ func (as ArtifactStore) Inspect(ctx context.Context, nameOrDigest string) (*liba if len(nameOrDigest) == 0 { return nil, ErrEmptyArtifactName } + + as.lock.RLock() + defer as.lock.Unlock() + artifacts, err := as.getArtifacts(ctx, nil) if err != nil { return nil, err @@ -122,6 +138,9 @@ func (as ArtifactStore) Inspect(ctx context.Context, nameOrDigest string) (*liba // List artifacts in the local store func (as ArtifactStore) List(ctx context.Context) (libartifact.ArtifactList, error) { + as.lock.RLock() + defer as.lock.Unlock() + return as.getArtifacts(ctx, nil) } @@ -134,6 +153,10 @@ func (as ArtifactStore) Pull(ctx context.Context, name string, opts libimage.Cop if err != nil { return "", err } + + as.lock.Lock() + defer as.lock.Unlock() + destRef, err := layout.NewReference(as.storePath, name) if err != nil { return "", err @@ -162,6 +185,10 @@ func (as ArtifactStore) Push(ctx context.Context, src, dest string, opts libimag if err != nil { return "", err } + + as.lock.Lock() + defer as.lock.Unlock() + srcRef, err := layout.NewReference(as.storePath, src) if err != nil { return "", err @@ -201,6 +228,14 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []en return nil, fmt.Errorf("cannot override filename with %s annotation", specV1.AnnotationTitle) } + locked := true + as.lock.Lock() + defer func() { + if locked { + as.lock.Unlock() + } + }() + // Check if artifact already exists artifacts, err := as.getArtifacts(ctx, nil) if err != nil { @@ -263,6 +298,11 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []en } defer imageDest.Close() + // Unlock around the actual pull of the blobs. + // This is ugly as hell, but should be safe. + locked = false + as.lock.Unlock() + // ImageDestination, in general, requires the caller to write a full image; here we may write only the added layers. // This works for the oci/layout transport we hard-code. for _, artifactBlob := range artifactBlobs { @@ -307,6 +347,9 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []en artifactManifest.Layers = append(artifactManifest.Layers, newLayer) } + as.lock.Lock() + locked = true + rawData, err := json.Marshal(artifactManifest) if err != nil { return nil, err @@ -669,6 +712,8 @@ func copyTrustedImageBlobToTarStream(ctx context.Context, imgSrc types.ImageSour } func (as ArtifactStore) createEmptyManifest() error { + as.lock.Lock() + defer as.lock.Unlock() index := specV1.Index{ MediaType: specV1.MediaTypeImageIndex, Versioned: specs.Versioned{SchemaVersion: ManifestSchemaVersion},