Skip to content

Commit

Permalink
Merge pull request #584 from azdagron/reg-api-del-bundle-mode
Browse files Browse the repository at this point in the history
Add mode flag to bundle deletion
  • Loading branch information
azdagron authored Sep 7, 2018
2 parents 8927a86 + 41bedb0 commit 216acdc
Show file tree
Hide file tree
Showing 10 changed files with 353 additions and 65 deletions.
40 changes: 40 additions & 0 deletions cmd/spire-server/cli/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/mitchellh/cli"
"github.com/spiffe/spire/pkg/common/pemutil"
"github.com/spiffe/spire/proto/common"
"github.com/spiffe/spire/proto/server/datastore"
"github.com/spiffe/spire/test/fakes/fakedatastore"
"github.com/spiffe/spire/test/fakes/fakeregistrationclient"
Expand Down Expand Up @@ -273,6 +274,8 @@ func (s *BundleSuite) TestDeleteHelp() {
s.Require().Equal(`Usage of bundle delete:
-id string
SPIFFE ID of the trust domain
-mode string
Deletion mode: one of restrict, delete, or dissociate (default "restrict")
-serverAddr string
Address of the SPIRE server (default "localhost:8081")
`, s.stderr.String())
Expand All @@ -283,6 +286,14 @@ func (s *BundleSuite) TestDeleteWithoutID() {
s.Require().Equal("id is required\n", s.stderr.String())
}

func (s *BundleSuite) TestDeleteWithUnsupportedMode() {
s.Require().Equal(1, s.deleteCmd.Run([]string{
"-id", "spiffe://domain1.test",
"-mode", "whatever",
}))
s.Require().Equal("unsupported mode \"whatever\"\n", s.stderr.String())
}

func (s *BundleSuite) TestDelete() {
s.createBundle(&datastore.Bundle{
TrustDomain: "spiffe://domain1.test",
Expand All @@ -298,6 +309,27 @@ func (s *BundleSuite) TestDelete() {
s.Require().EqualError(err, "no such bundle")
}

func (s *BundleSuite) TestDeleteWithRestrictMode() {
s.createBundle(&datastore.Bundle{
TrustDomain: "spiffe://domain1.test",
CaCerts: s.cert1.Raw,
})
s.createRegistrationEntry(&datastore.RegistrationEntry{
ParentId: "spiffe://example.test/spire/agent/foo",
SpiffeId: "spiffe://example.test/blog",
Selectors: []*common.Selector{{Type: "foo", Value: "bar"}},
FederatesWith: []string{"spiffe://domain1.test"},
})

s.Require().Equal(1, s.deleteCmd.Run([]string{"-id", "spiffe://domain1.test"}))
s.Require().Equal("rpc error: code = Unknown desc = cannot delete bundle; federated with 1 registration entries\n", s.stderr.String())

_, err := s.ds.FetchBundle(context.Background(), &datastore.FetchBundleRequest{
TrustDomain: "spiffe://domain1.test",
})
s.Require().Nil(err)
}

func (s *BundleSuite) assertBundleSet(extraArgs ...string) {
rc := s.setCmd.Run(append([]string{"-id", "spiffe://otherdomain.test"}, extraArgs...))
s.Require().Equal(0, rc)
Expand All @@ -319,3 +351,11 @@ func (s *BundleSuite) createBundle(bundle *datastore.Bundle) {
})
s.Require().NoError(err)
}

func (s *BundleSuite) createRegistrationEntry(entry *common.RegistrationEntry) *common.RegistrationEntry {
resp, err := s.ds.CreateRegistrationEntry(context.Background(), &datastore.CreateRegistrationEntryRequest{
Entry: entry,
})
s.Require().NoError(err)
return resp.Entry
}
34 changes: 32 additions & 2 deletions cmd/spire-server/cli/bundle/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@ import (
"context"
"errors"
"flag"
"fmt"

"github.com/mitchellh/cli"
"github.com/spiffe/spire/pkg/common/idutil"
"github.com/spiffe/spire/proto/api/registration"
)

const (
deleteBundleRestrict = "restrict"
deleteBundleDissociate = "dissociate"
deleteBundleDelete = "delete"
)

// NewDeleteCommand creates a new "delete" subcommand for "bundle" command.
func NewDeleteCommand() cli.Command {
return newDeleteCommand(defaultEnv, newClients)
Expand All @@ -22,6 +29,9 @@ func newDeleteCommand(env *env, clientsMaker clientsMaker) cli.Command {
type deleteCommand struct {
// SPIFFE ID of the trust domain bundle
id string

// Deletion mode.
mode string
}

func (c *deleteCommand) name() string {
Expand All @@ -34,6 +44,7 @@ func (c *deleteCommand) synopsis() string {

func (c *deleteCommand) appendFlags(fs *flag.FlagSet) {
fs.StringVar(&c.id, "id", "", "SPIFFE ID of the trust domain")
fs.StringVar(&c.mode, "mode", deleteBundleRestrict, fmt.Sprintf("Deletion mode: one of %s, %s, or %s", deleteBundleRestrict, deleteBundleDelete, deleteBundleDissociate))
}

func (c *deleteCommand) run(ctx context.Context, env *env, clients *clients) error {
Expand All @@ -45,11 +56,30 @@ func (c *deleteCommand) run(ctx context.Context, env *env, clients *clients) err
return err
}

if _, err := clients.r.DeleteFederatedBundle(ctx, &registration.FederatedBundleID{
Id: c.id,
mode, err := deleteModeFromFlag(c.mode)
if err != nil {
return err
}

if _, err := clients.r.DeleteFederatedBundle(ctx, &registration.DeleteFederatedBundleRequest{
Id: c.id,
Mode: mode,
}); err != nil {
return err
}

return env.Println("bundle deleted.")
}

func deleteModeFromFlag(mode string) (registration.DeleteFederatedBundleRequest_Mode, error) {
switch mode {
case "", deleteBundleRestrict:
return registration.DeleteFederatedBundleRequest_RESTRICT, nil
case deleteBundleDissociate:
return registration.DeleteFederatedBundleRequest_DISSOCIATE, nil
case deleteBundleDelete:
return registration.DeleteFederatedBundleRequest_DELETE, nil
default:
return registration.DeleteFederatedBundleRequest_RESTRICT, fmt.Errorf("unsupported mode %q", mode)
}
}
1 change: 1 addition & 0 deletions doc/spire_server.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ Deletes bundle data for a trust domain. This command cannot be used to delete th
| Command | Action | Default |
|:--------------|:-------------------------------------------------------------------|:---------------|
| `-id` | The trust domain SPIFFE ID of the bundle to delete. | |
| `-mode` | One of: `restrict`, `dissociate`, `delete`. `restrict` prevents the bundle from being deleted if it is associated to registration entries (i.e. federated with). `dissociate` allows the bundle to be deleted and removes the association from registration entries. `delete` deletes the bundle as well as associated registration entries. | `restrict` |

## Architecture

Expand Down
20 changes: 19 additions & 1 deletion pkg/server/endpoints/registration/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (h *Handler) UpdateFederatedBundle(
}

func (h *Handler) DeleteFederatedBundle(
ctx context.Context, request *registration.FederatedBundleID) (
ctx context.Context, request *registration.DeleteFederatedBundleRequest) (
response *common.Empty, err error) {

if request.Id == h.TrustDomain.String() {
Expand All @@ -294,9 +294,15 @@ func (h *Handler) DeleteFederatedBundle(
return nil, err
}

mode, err := convertDeleteBundleMode(request.Mode)
if err != nil {
return nil, err
}

ds := h.getDataStore()
if _, err := ds.DeleteBundle(ctx, &datastore.DeleteBundleRequest{
TrustDomain: request.Id,
Mode: mode,
}); err != nil {
return nil, err
}
Expand Down Expand Up @@ -382,3 +388,15 @@ func (h *Handler) isEntryUnique(ctx context.Context, ds datastore.DataStore, ent
func (h *Handler) getDataStore() datastore.DataStore {
return h.Catalog.DataStores()[0]
}

func convertDeleteBundleMode(in registration.DeleteFederatedBundleRequest_Mode) (datastore.DeleteBundleRequest_Mode, error) {
switch in {
case registration.DeleteFederatedBundleRequest_RESTRICT:
return datastore.DeleteBundleRequest_RESTRICT, nil
case registration.DeleteFederatedBundleRequest_DISSOCIATE:
return datastore.DeleteBundleRequest_DISSOCIATE, nil
case registration.DeleteFederatedBundleRequest_DELETE:
return datastore.DeleteBundleRequest_DELETE, nil
}
return datastore.DeleteBundleRequest_RESTRICT, fmt.Errorf("unhandled delete mode %q", in)
}
2 changes: 1 addition & 1 deletion pkg/server/endpoints/registration/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (s *HandlerSuite) TestDeleteFederatedBundle() {
})

for _, testCase := range testCases {
response, err := s.handler.DeleteFederatedBundle(context.Background(), &registration.FederatedBundleID{
response, err := s.handler.DeleteFederatedBundle(context.Background(), &registration.DeleteFederatedBundleRequest{
Id: testCase.Id,
})

Expand Down
34 changes: 33 additions & 1 deletion proto/api/registration/README_pb.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@

- [registration.proto](#registration.proto)
- [Bundle](#spire.api.registration.Bundle)
- [DeleteFederatedBundleRequest](#spire.api.registration.DeleteFederatedBundleRequest)
- [FederatedBundle](#spire.api.registration.FederatedBundle)
- [FederatedBundleID](#spire.api.registration.FederatedBundleID)
- [JoinToken](#spire.api.registration.JoinToken)
Expand All @@ -78,6 +79,7 @@
- [SpiffeID](#spire.api.registration.SpiffeID)
- [UpdateEntryRequest](#spire.api.registration.UpdateEntryRequest)

- [DeleteFederatedBundleRequest.Mode](#spire.api.registration.DeleteFederatedBundleRequest.Mode)


- [Registration](#spire.api.registration.Registration)
Expand Down Expand Up @@ -1110,6 +1112,22 @@ CA Bundle of the server



<a name="spire.api.registration.DeleteFederatedBundleRequest"/>

### DeleteFederatedBundleRequest



| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| id | [string](#string) | | |
| mode | [DeleteFederatedBundleRequest.Mode](#spire.api.registration.DeleteFederatedBundleRequest.Mode) | | |






<a name="spire.api.registration.FederatedBundle"/>

### FederatedBundle
Expand Down Expand Up @@ -1219,6 +1237,20 @@ A type with the id with want to update plus values to modify.




<a name="spire.api.registration.DeleteFederatedBundleRequest.Mode"/>

### DeleteFederatedBundleRequest.Mode
Mode controls the delete behavior if there are other records
associated with the bundle (e.g. registration entries).

| Name | Number | Description |
| ---- | ------ | ----------- |
| RESTRICT | 0 | RESTRICT prevents the bundle from being deleted in the presence of associated entries |
| DELETE | 1 | DELETE deletes the bundle and associated entries |
| DISSOCIATE | 2 | DISSOCIATE deletes the bundle and dissociates associated entries |





Expand All @@ -1243,7 +1275,7 @@ A type with the id with want to update plus values to modify.
| FetchFederatedBundle | [FederatedBundleID](#spire.api.registration.FederatedBundleID) | [FederatedBundle](#spire.api.registration.FederatedBundleID) | Retrieves a single federated bundle |
| ListFederatedBundles | [spire.common.Empty](#spire.common.Empty) | [FederatedBundle](#spire.common.Empty) | Retrieves Federated bundles for all the Federated SPIFFE IDs. |
| UpdateFederatedBundle | [FederatedBundle](#spire.api.registration.FederatedBundle) | [spire.common.Empty](#spire.api.registration.FederatedBundle) | Updates a particular Federated Bundle. Useful for rotation. |
| DeleteFederatedBundle | [FederatedBundleID](#spire.api.registration.FederatedBundleID) | [spire.common.Empty](#spire.api.registration.FederatedBundleID) | Delete a particular Federated Bundle. Used to destroy inter-domain trust. |
| DeleteFederatedBundle | [DeleteFederatedBundleRequest](#spire.api.registration.DeleteFederatedBundleRequest) | [spire.common.Empty](#spire.api.registration.DeleteFederatedBundleRequest) | Delete a particular Federated Bundle. Used to destroy inter-domain trust. |
| CreateJoinToken | [JoinToken](#spire.api.registration.JoinToken) | [JoinToken](#spire.api.registration.JoinToken) | Create a new join token |
| FetchBundle | [spire.common.Empty](#spire.common.Empty) | [Bundle](#spire.common.Empty) | Retrieves the CA bundle. |

Expand Down
Loading

0 comments on commit 216acdc

Please sign in to comment.