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

only return metadata inside watch events #87

Merged
merged 1 commit into from
Jan 11, 2024
Merged

only return metadata inside watch events #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions pkg/registry/file/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func isPayloadFile(path string) bool {
return !IsMetadataFile(path)
}

func (s *StorageImpl) writeFiles(ctx context.Context, key string, obj runtime.Object, out runtime.Object) error {
func (s *StorageImpl) writeFiles(ctx context.Context, key string, obj runtime.Object, metaOut runtime.Object) error {
// do not alter obj
dup := obj.DeepCopyObject()
// set resourceversion
Expand Down Expand Up @@ -159,9 +159,9 @@ func (s *StorageImpl) writeFiles(ctx context.Context, key string, obj runtime.Ob
if err != nil {
return err
}
// eventually fill out
if out != nil {
err = json.Unmarshal(jsonBytes, out)
// eventually fill metaOut
if metaOut != nil {
err = json.Unmarshal(metadataBytes, metaOut)
if err != nil {
return err
}
Expand All @@ -172,7 +172,7 @@ func (s *StorageImpl) writeFiles(ctx context.Context, key string, obj runtime.Ob
// Create adds a new object at a key even when it already exists. 'ttl' is time-to-live
// in seconds (and is ignored). If no error is returned and out is not nil, out will be
// set to the read value from database.
func (s *StorageImpl) Create(ctx context.Context, key string, obj, out runtime.Object, _ uint64) error {
func (s *StorageImpl) Create(ctx context.Context, key string, obj, metaOut runtime.Object, _ uint64) error {
ctx, span := otel.Tracer("").Start(ctx, "StorageImpl.Create")
span.SetAttributes(attribute.String("key", key))
defer span.End()
Expand All @@ -183,13 +183,13 @@ func (s *StorageImpl) Create(ctx context.Context, key string, obj, out runtime.O
return errors.New(msg)
}
// write files
if err := s.writeFiles(ctx, key, obj, out); err != nil {
if err := s.writeFiles(ctx, key, obj, metaOut); err != nil {
logger.L().Ctx(ctx).Error("write files failed", helpers.Error(err), helpers.String("key", key))
return err
}

// publish event to watchers
s.watchDispatcher.Added(key, obj)
s.watchDispatcher.Added(key, metaOut)
return nil
}

Expand All @@ -198,7 +198,7 @@ func (s *StorageImpl) Create(ctx context.Context, key string, obj, out runtime.O
// If 'cachedExistingObject' is non-nil, it can be used as a suggestion about the
// current version of the object to avoid read operation from storage to get it.
// However, the implementations have to retry in case suggestion is stale.
func (s *StorageImpl) Delete(ctx context.Context, key string, out runtime.Object, _ *storage.Preconditions, _ storage.ValidateObjectFunc, _ runtime.Object) error {
func (s *StorageImpl) Delete(ctx context.Context, key string, metaOut runtime.Object, _ *storage.Preconditions, _ storage.ValidateObjectFunc, _ runtime.Object) error {
ctx, span := otel.Tracer("").Start(ctx, "StorageImpl.Delete")
span.SetAttributes(attribute.String("key", key))
defer span.End()
Expand All @@ -208,7 +208,7 @@ func (s *StorageImpl) Delete(ctx context.Context, key string, out runtime.Object
spanLock.End()
defer s.lock.Unlock()
// read json file
b, err := afero.ReadFile(s.appFs, p+JsonExt)
b, err := afero.ReadFile(s.appFs, p+MetadataExt)
if err != nil {
if errors.Is(err, afero.ErrFileNotFound) {
return storage.NewKeyNotFoundError(key, 0)
Expand All @@ -225,15 +225,15 @@ func (s *StorageImpl) Delete(ctx context.Context, key string, out runtime.Object
if err != nil {
logger.L().Ctx(ctx).Error("remove metadata file failed", helpers.Error(err), helpers.String("key", key))
}
// try to fill out
err = json.Unmarshal(b, out)
// try to fill metaOut
err = json.Unmarshal(b, metaOut)
if err != nil {
logger.L().Ctx(ctx).Error("json unmarshal failed", helpers.Error(err), helpers.String("key", key))
return err
}

// publish event to watchers
s.watchDispatcher.Deleted(key, out)
s.watchDispatcher.Deleted(key, metaOut)
return nil
}

Expand Down Expand Up @@ -408,7 +408,7 @@ func (s *StorageImpl) getStateFromObject(ctx context.Context, obj runtime.Object
//
// )
func (s *StorageImpl) GuaranteedUpdate(
ctx context.Context, key string, destination runtime.Object, ignoreNotFound bool,
ctx context.Context, key string, metaOut runtime.Object, ignoreNotFound bool,
preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, cachedExistingObject runtime.Object) error {
ctx, span := otel.Tracer("").Start(ctx, "StorageImpl.GuaranteedUpdate")
span.SetAttributes(attribute.String("key", key))
Expand All @@ -417,7 +417,7 @@ func (s *StorageImpl) GuaranteedUpdate(
// key preparation is skipped
// otel span tracking is skipped

v, err := conversion.EnforcePtr(destination)
v, err := conversion.EnforcePtr(metaOut)
if err != nil {
logger.L().Ctx(ctx).Error("unable to convert output object to pointer", helpers.Error(err), helpers.String("key", key))
return fmt.Errorf("unable to convert output object to pointer: %v", err)
Expand Down Expand Up @@ -499,11 +499,11 @@ func (s *StorageImpl) GuaranteedUpdate(
continue
}

// save to disk and fill into destination
err = s.writeFiles(ctx, key, ret, destination)
// save to disk and fill into metaOut
err = s.writeFiles(ctx, key, ret, metaOut)
if err == nil {
// Only successful updates should produce modification events
s.watchDispatcher.Modified(key, ret)
s.watchDispatcher.Modified(key, metaOut)
} else {
logger.L().Ctx(ctx).Error("write files failed", helpers.Error(err), helpers.String("key", key))
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/registry/file/storage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func TestStorageImpl_Delete(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
fs := afero.NewMemMapFs()
if tt.create {
fpath := getStoredPayloadFilepath(DefaultStorageRoot, tt.args.key)
fpath := getStoredMetadataFilepath(DefaultStorageRoot, tt.args.key)
_ = afero.WriteFile(fs, fpath, []byte(tt.content), 0644)
}
s := NewStorageImpl(fs, DefaultStorageRoot)
Expand Down Expand Up @@ -519,7 +519,6 @@ func TestStorageImpl_GuaranteedUpdate(t *testing.T) {
}
return
} else {
assert.Equal(t, tt.want, destination)
onDisk := &v1beta1.SBOMSPDXv2p3{}
err = s.Get(context.Background(), tt.args.key, storage.GetOptions{}, onDisk)
assert.NoError(t, err)
Expand Down
35 changes: 18 additions & 17 deletions pkg/registry/file/watch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ func TestFilesystemStoragePublishesToMatchingWatch(t *testing.T) {
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
Expand All @@ -150,23 +151,26 @@ func TestFilesystemStoragePublishesToMatchingWatch(t *testing.T) {
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
{
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
{
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
Expand All @@ -192,7 +196,8 @@ func TestFilesystemStoragePublishesToMatchingWatch(t *testing.T) {
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
Expand Down Expand Up @@ -235,7 +240,7 @@ func TestFilesystemStoragePublishesToMatchingWatch(t *testing.T) {
}

var ttl uint64 = 0
var out runtime.Object
out := &v1beta1.SBOMSPDXv2p3{}
for key, object := range tc.inputObjects {
_ = s.Create(ctx, key, object, out, ttl)
}
Expand Down Expand Up @@ -289,15 +294,17 @@ func TestFilesystemStorageWatchStop(t *testing.T) {
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
{
Type: watch.Added,
Object: &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "some-sbom",
Name: "some-sbom",
ResourceVersion: "1",
},
},
},
Expand Down Expand Up @@ -331,7 +338,7 @@ func TestFilesystemStorageWatchStop(t *testing.T) {

// Act out the creation operation
var ttl uint64 = 0
var out runtime.Object
out := &v1beta1.SBOMSPDXv2p3{}
for key, object := range tc.inputObjects {
_ = s.Create(ctx, key, object, out, ttl)
}
Expand Down Expand Up @@ -363,14 +370,8 @@ func TestFilesystemStorageWatchStop(t *testing.T) {
func TestWatchGuaranteedUpdateProducesMatchingEvents(t *testing.T) {
toto := &v1beta1.SBOMSPDXv2p3{
ObjectMeta: v1.ObjectMeta{
Name: "toto",
},
Spec: v1beta1.SBOMSPDXv2p3Spec{
Metadata: v1beta1.SPDXMeta{
Tool: v1beta1.ToolMeta{
Name: "titi",
},
},
Name: "toto",
ResourceVersion: "1",
},
}

Expand Down
Loading