Skip to content

Commit

Permalink
Prevent error when pushing and deleting SVID (#2620)
Browse files Browse the repository at this point in the history
Signed-off-by: Marcos Yacob <marcos.yacob@hpe.com>
  • Loading branch information
MarcosDY authored Nov 9, 2021
1 parent 29d49be commit 3a4d646
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 4 deletions.
5 changes: 5 additions & 0 deletions pkg/agent/manager/storecache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ func (c *Cache) UpdateEntries(update *cache.UpdateEntries, checkSVID func(*commo
continue
}

if record.entry == nil {
// Entry waiting to be removed on platform
continue
}

c.c.Log.WithFields(logrus.Fields{
telemetry.Entry: id,
telemetry.SPIFFEID: record.entry.SpiffeId,
Expand Down
18 changes: 14 additions & 4 deletions pkg/agent/svid/store/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
telemetry_store "github.com/spiffe/spire/pkg/common/telemetry/agent/store"
"github.com/spiffe/spire/pkg/common/util"
"github.com/spiffe/spire/proto/spire/common"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

const (
Expand Down Expand Up @@ -112,13 +114,21 @@ func (s *SVIDStoreService) deleteSVID(ctx context.Context, log logrus.FieldLogge
return false
}

if err := svidStore.DeleteX509SVID(ctx, metadata); err != nil {
err = svidStore.DeleteX509SVID(ctx, metadata)

switch status.Code(err) {
case codes.OK:
log.Debug("SVID deleted successfully")
return true

case codes.InvalidArgument:
log.WithError(err).Debug("Failed to delete SVID because of malformed selectors")
return true

default:
log.WithError(err).Error("Failed to delete SVID")
return false
}

log.Debug("SVID deleted successfully")
return true
}

// storeSVID creates or updates an SVID using SVIDStore plugin. It get the plugin name from entry selectors
Expand Down
82 changes: 82 additions & 0 deletions pkg/agent/svid/store/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"github.com/spiffe/spire/test/spiretest"
"github.com/spiffe/spire/test/util"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

var (
Expand Down Expand Up @@ -203,6 +205,86 @@ func TestRunDeleteSecrets(t *testing.T) {
},
},
},
{
name: "delete fails because unexpected selectors",
stores: map[string]*fakeSVIDStore{
"store1": {
name: "store1",
err: status.Error(codes.InvalidArgument, "no valid selector"),
},
},
readyRecords: []*storecache.Record{
{
ID: "foh",
HandledEntry: &common.RegistrationEntry{
EntryId: "foh",
SpiffeId: "spiffe://example.org/foh",
Selectors: []*common.Selector{
{Type: "store1", Value: "a:1"},
{Type: "store1", Value: "i:1"},
},
},
Bundles: map[spiffeid.TrustDomain]*bundleutil.Bundle{
td: bundle,
},
ExpiresAt: now,
Revision: 1,
},
},
logs: []spiretest.LogEntry{
{
Level: logrus.DebugLevel,
Message: "Failed to delete SVID because of malformed selectors",
Data: logrus.Fields{
telemetry.RevisionNumber: "1",
telemetry.Entry: "foh",
telemetry.SPIFFEID: "spiffe://example.org/foh",
telemetry.SVIDStore: "store1",
logrus.ErrorKey: "rpc error: code = InvalidArgument desc = no valid selector",
},
},
},
},
{
name: "failed to delete using store",
stores: map[string]*fakeSVIDStore{
"store1": {
name: "store1",
err: status.Error(codes.Internal, "oh! no"),
},
},
readyRecords: []*storecache.Record{
{
ID: "foh",
HandledEntry: &common.RegistrationEntry{
EntryId: "foh",
SpiffeId: "spiffe://example.org/foh",
Selectors: []*common.Selector{
{Type: "store1", Value: "a:1"},
{Type: "store1", Value: "i:1"},
},
},
Bundles: map[spiffeid.TrustDomain]*bundleutil.Bundle{
td: bundle,
},
ExpiresAt: now,
Revision: 1,
},
},
logs: []spiretest.LogEntry{
{
Level: logrus.ErrorLevel,
Message: "Failed to delete SVID",
Data: logrus.Fields{
telemetry.RevisionNumber: "1",
telemetry.Entry: "foh",
telemetry.SPIFFEID: "spiffe://example.org/foh",
telemetry.SVIDStore: "store1",
logrus.ErrorKey: "rpc error: code = Internal desc = oh! no",
},
},
},
},
{
name: "selectors has changes",
stores: map[string]*fakeSVIDStore{
Expand Down

0 comments on commit 3a4d646

Please sign in to comment.