Skip to content

Commit

Permalink
AC: use a RootDirectoryDigest for directory assets
Browse files Browse the repository at this point in the history 
Which means we avoid needing to calculate TreeDigest`s whilst still
creating Actions/ActionResults that encode that an asset is a directory.

Note: the bb-remote-asset server does not validate that the
RootDirectoryDigest actually references a Directory proto because
bb-remote-asset treats the digest as an opaque digest.

See #34 (comment)
  • Loading branch information
@harrysarson
harrysarson committed Jul 12, 2024
1 parent f0b6649 commit eabe6bb
Show file tree
Hide file tree
Showing 12 changed files with 368 additions and 61 deletions.
4 changes: 2 additions & 2 deletions pkg/fetch/caching_fetcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (cf *cachingFetcher) FetchBlob(ctx context.Context, req *remoteasset.FetchB

// Cache fetched blob with single URI
assetRef := storage.NewAssetReference([]string{response.Uri}, response.Qualifiers)
assetData := storage.NewAsset(response.BlobDigest, getDefaultTimestamp())
assetData := storage.NewBlobAsset(response.BlobDigest, getDefaultTimestamp())
err = cf.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return response, err
Expand Down Expand Up @@ -159,7 +159,7 @@ func (cf *cachingFetcher) FetchDirectory(ctx context.Context, req *remoteasset.F

// Cache fetched blob with single URI
assetRef := storage.NewAssetReference([]string{response.Uri}, response.Qualifiers)
assetData := storage.NewAsset(response.RootDirectoryDigest, getDefaultTimestamp())
assetData := storage.NewDirectoryAsset(response.RootDirectoryDigest, getDefaultTimestamp())
err = cf.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return response, err
Expand Down
9 changes: 7 additions & 2 deletions pkg/fetch/caching_fetcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ func TestFetchBlobCaching(t *testing.T) {
refDigest, err := storage.AssetReferenceToDigest(storage.NewAssetReference([]string{uri}, []*remoteasset.Qualifier{}), instanceName)
require.NoError(t, err)

t.Logf("Ref digest was %v", refDigest)

backend := mock.NewMockBlobAccess(ctrl)
assetStore := storage.NewBlobAccessAssetStore(backend, 16*1024*1024)
mockFetcher := mock.NewMockFetcher(ctrl)
Expand All @@ -56,6 +58,7 @@ func TestFetchBlobCaching(t *testing.T) {
require.NoError(t, err)
a := m.(*asset.Asset)
require.True(t, proto.Equal(a.Digest, blobDigest))
require.Equal(t, asset.Asset_BLOB, a.Type)
return nil
}).After(fetchBlobCall)
response, err := cachingFetcher.FetchBlob(ctx, request)
Expand All @@ -71,7 +74,7 @@ func TestFetchBlobCaching(t *testing.T) {
})

t.Run("Cached", func(t *testing.T) {
backend.EXPECT().Get(ctx, refDigest).Return(buffer.NewProtoBufferFromProto(storage.NewAsset(blobDigest, nil), buffer.UserProvided))
backend.EXPECT().Get(ctx, refDigest).Return(buffer.NewProtoBufferFromProto(storage.NewBlobAsset(blobDigest, nil), buffer.UserProvided))
response, err := cachingFetcher.FetchBlob(ctx, request)
require.Nil(t, err)
require.Equal(t, response.Status.Code, int32(codes.OK))
Expand Down Expand Up @@ -111,6 +114,7 @@ func TestFetchDirectoryCaching(t *testing.T) {
require.NoError(t, err)
a := m.(*asset.Asset)
require.True(t, proto.Equal(a.Digest, dirDigest))
require.Equal(t, asset.Asset_DIRECTORY, a.Type)
return nil
}).After(fetchDirectoryCall)
response, err := cachingFetcher.FetchDirectory(ctx, request)
Expand All @@ -126,7 +130,7 @@ func TestFetchDirectoryCaching(t *testing.T) {
})

t.Run("Cached", func(t *testing.T) {
backend.EXPECT().Get(ctx, refDigest).Return(buffer.NewProtoBufferFromProto(storage.NewAsset(dirDigest, nil), buffer.UserProvided))
backend.EXPECT().Get(ctx, refDigest).Return(buffer.NewProtoBufferFromProto(storage.NewBlobAsset(dirDigest, nil), buffer.UserProvided))
response, err := cachingFetcher.FetchDirectory(ctx, request)
require.Nil(t, err)
require.Equal(t, response.Status.Code, int32(codes.OK))
Expand Down Expand Up @@ -191,6 +195,7 @@ func TestCachingFetcherOldestContentAccepted(t *testing.T) {
SizeBytes: 234,
},
LastUpdated: ts,
Type: asset.Asset_BLOB,
}, buffer.UserProvided)
backend.EXPECT().Get(ctx, refDigest).Return(buf)
assetStore := storage.NewBlobAccessAssetStore(backend, 16*1024*1024)
Expand Down
106 changes: 85 additions & 21 deletions pkg/proto/asset/asset.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions pkg/proto/asset/asset.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,15 @@ message Asset {
// Time at which this Asset was last Push'd or Fetch'd from a remote into the
// store
google.protobuf.Timestamp last_updated = 3;

enum AssetType {
// Blob asset, e.g. from PushBlob
BLOB = 0;

// Directory asset, e.g. from PushDirectory
DIRECTORY = 1;
}

// The type of the asset.
AssetType type = 4;
}
8 changes: 4 additions & 4 deletions pkg/push/push_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *assetPushServer) PushBlob(ctx context.Context, req *remoteasset.PushBlo
}

assetRef := storage.NewAssetReference(req.Uris, req.Qualifiers)
assetData := storage.NewAsset(req.BlobDigest, req.ExpireAt)
assetData := storage.NewBlobAsset(req.BlobDigest, req.ExpireAt)
err = s.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return nil, err
Expand All @@ -49,7 +49,7 @@ func (s *assetPushServer) PushBlob(ctx context.Context, req *remoteasset.PushBlo
if len(req.Uris) > 1 {
for _, uri := range req.Uris {
assetRef := storage.NewAssetReference([]string{uri}, req.Qualifiers)
assetData := storage.NewAsset(req.BlobDigest, req.ExpireAt)
assetData := storage.NewBlobAsset(req.BlobDigest, req.ExpireAt)
err = s.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return nil, err
Expand All @@ -74,7 +74,7 @@ func (s *assetPushServer) PushDirectory(ctx context.Context, req *remoteasset.Pu
}

assetRef := storage.NewAssetReference(req.Uris, req.Qualifiers)
assetData := storage.NewAsset(req.RootDirectoryDigest, req.ExpireAt)
assetData := storage.NewDirectoryAsset(req.RootDirectoryDigest, req.ExpireAt)
err = s.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return nil, err
Expand All @@ -83,7 +83,7 @@ func (s *assetPushServer) PushDirectory(ctx context.Context, req *remoteasset.Pu
if len(req.Uris) > 1 {
for _, uri := range req.Uris {
assetRef := storage.NewAssetReference([]string{uri}, req.Qualifiers)
assetData := storage.NewAsset(req.RootDirectoryDigest, req.ExpireAt)
assetData := storage.NewDirectoryAsset(req.RootDirectoryDigest, req.ExpireAt)
err = s.assetStore.Put(ctx, assetRef, assetData, instanceName)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/push/push_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func TestPushServerPushBlobSuccess(t *testing.T) {
require.NoError(t, err)
a := m.(*asset.Asset)
require.True(t, proto.Equal(a.Digest, blobDigest))
require.Equal(t, asset.Asset_BLOB, a.Type)
return nil
})
assetStore := storage.NewBlobAccessAssetStore(backend, 16*1024*1024)
Expand Down Expand Up @@ -91,6 +92,7 @@ func TestPushServerPushDirectorySuccess(t *testing.T) {
require.NoError(t, err)
a := m.(*asset.Asset)
require.True(t, proto.Equal(a.Digest, rootDirectoryDigest))
require.Equal(t, asset.Asset_DIRECTORY, a.Type)
return nil
})
assetStore := storage.NewBlobAccessAssetStore(backend, 16*1024*1024)
Expand Down
Loading

0 comments on commit eabe6bb

Please sign in to comment.