diff --git a/coordinator/coordapi.go b/coordinator/coordapi.go index a9d79ca29..89dac488a 100644 --- a/coordinator/coordapi.go +++ b/coordinator/coordapi.go @@ -72,9 +72,7 @@ func (s *coordAPIServer) SetManifest(_ context.Context, req *coordapi.SetManifes s.policyTextStore.Set(policy.Hash(), policy) } - if err := s.manifSetGetter.SetManifest(m); err != nil { - return nil, status.Errorf(codes.Internal, "setting manifest: %v", err) - } + s.manifSetGetter.SetManifest(m) resp := &coordapi.SetManifestResponse{ CACert: s.caChainGetter.GetRootCACert(), @@ -137,7 +135,7 @@ type certChainGetter interface { } type manifestSetGetter interface { - SetManifest(*manifest.Manifest) error + SetManifest(*manifest.Manifest) GetManifests() []*manifest.Manifest } diff --git a/coordinator/mesh.go b/coordinator/mesh.go index 1dad23b68..1851bdedb 100644 --- a/coordinator/mesh.go +++ b/coordinator/mesh.go @@ -24,7 +24,7 @@ type meshAuthority struct { ca *ca.CA certs map[string][]byte certsMux sync.RWMutex - manifests appendableList[manifest.Manifest] + manifests appendableList[*manifest.Manifest] logger *slog.Logger } @@ -32,7 +32,7 @@ func newMeshAuthority(ca *ca.CA, log *slog.Logger) *meshAuthority { return &meshAuthority{ ca: ca, certs: make(map[string][]byte), - manifests: new(appendable.Appendable[manifest.Manifest]), + manifests: new(appendable.Appendable[*manifest.Manifest]), logger: log.WithGroup("mesh-authority"), } } @@ -129,12 +129,12 @@ func (m *meshAuthority) GetManifests() []*manifest.Manifest { return m.manifests.All() } -func (m *meshAuthority) SetManifest(mnfst *manifest.Manifest) error { - return m.manifests.Append(mnfst) +func (m *meshAuthority) SetManifest(mnfst *manifest.Manifest) { + m.manifests.Append(mnfst) } type appendableList[T any] interface { - Append(*T) error - All() []*T - Latest() (*T, error) + Append(T) + All() []T + Latest() (T, error) } diff --git a/internal/appendable/appendable.go b/internal/appendable/appendable.go index 4a2e02cfa..dd9772788 100644 --- a/internal/appendable/appendable.go +++ b/internal/appendable/appendable.go @@ -2,41 +2,36 @@ package appendable import ( "errors" - "fmt" "sync" ) // Appendable is a thread-safe list that can be appended to. type Appendable[T any] struct { - list []*T + list []T mux sync.RWMutex } // Append adds a value to the list. -func (a *Appendable[T]) Append(value *T) error { - if value == nil { - return fmt.Errorf("nil value of type %T cannot be appended", value) - } +func (a *Appendable[T]) Append(value T) { a.mux.Lock() defer a.mux.Unlock() a.list = append(a.list, value) - return nil } // All returns all values in the list. -func (a *Appendable[T]) All() []*T { +func (a *Appendable[T]) All() []T { a.mux.RLock() defer a.mux.RUnlock() return a.list } // Latest returns the latest value in the list. -func (a *Appendable[T]) Latest() (*T, error) { +func (a *Appendable[T]) Latest() (T, error) { a.mux.RLock() defer a.mux.RUnlock() if len(a.list) == 0 { - return nil, errors.New("appendable is empty") + return *new(T), errors.New("appendable is empty") } return a.list[len(a.list)-1], nil diff --git a/internal/appendable/appendable_test.go b/internal/appendable/appendable_test.go new file mode 100644 index 000000000..28a151210 --- /dev/null +++ b/internal/appendable/appendable_test.go @@ -0,0 +1,106 @@ +package appendable_test + +import ( + "sync" + "testing" + + "github.com/edgelesssys/nunki/internal/appendable" + "github.com/stretchr/testify/assert" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + +func TestAppendable(t *testing.T) { + t.Run("new appendable is empty", func(t *testing.T) { + assert := assert.New(t) + + a := appendable.Appendable[string]{} + assert.Empty(a.All()) + }) + + t.Run("append and all", func(t *testing.T) { + assert := assert.New(t) + + a := appendable.Appendable[string]{} + a.Append("foo") + a.Append("bar") + + values := a.All() + assert.Len(values, 2) + assert.Contains(values, "foo") + assert.Contains(values, "bar") + }) + + t.Run("latest", func(t *testing.T) { + assert := assert.New(t) + + a := appendable.Appendable[string]{} + a.Append("foo") + a.Append("bar") + + v, err := a.Latest() + assert.NoError(err) + assert.EqualValues("bar", v) + }) + + t.Run("latest empty", func(t *testing.T) { + assert := assert.New(t) + + a := appendable.Appendable[string]{} + _, err := a.Latest() + assert.Error(err) + }) +} + +func TestAppendableConcurrent(t *testing.T) { + t.Run("append all latest", func(t *testing.T) { + assert := assert.New(t) + + a := appendable.Appendable[string]{} + + var wg sync.WaitGroup + + appendElem := func(e string) { + defer wg.Done() + a.Append(e) + } + all := func() { + defer wg.Done() + _ = a.All() + } + latest := func() { + defer wg.Done() + _, _ = a.Latest() + } + + wg.Add(18) + go latest() + go latest() + go latest() + go appendElem("foo") + go appendElem("bar") + go appendElem("baz") + go all() + go all() + go all() + go appendElem("foo") + go appendElem("bar") + go appendElem("baz") + go latest() + go latest() + go latest() + go all() + go all() + go all() + wg.Wait() + + values := a.All() + assert.Len(values, 6) + assert.Contains(values, "foo") + assert.Contains(values, "bar") + assert.Contains(values, "baz") + }) +}