diff --git a/cmd/spire-server/cli/bundle/bundle_test.go b/cmd/spire-server/cli/bundle/bundle_test.go index 86e06a6100..be6f799a32 100644 --- a/cmd/spire-server/cli/bundle/bundle_test.go +++ b/cmd/spire-server/cli/bundle/bundle_test.go @@ -303,10 +303,12 @@ func (s *BundleSuite) TestDelete() { s.Require().Equal(0, s.deleteCmd.Run([]string{"-id", "spiffe://domain1.test"})) s.Require().Equal("bundle deleted.\n", s.stdout.String()) - _, err := s.ds.FetchBundle(context.Background(), &datastore.FetchBundleRequest{ + resp, err := s.ds.FetchBundle(context.Background(), &datastore.FetchBundleRequest{ TrustDomain: "spiffe://domain1.test", }) - s.Require().EqualError(err, "no such bundle") + s.Require().NoError(err) + s.Require().NotNil(resp) + s.Require().Nil(resp.Bundle) } func (s *BundleSuite) TestDeleteWithRestrictMode() { diff --git a/pkg/server/ca/manager.go b/pkg/server/ca/manager.go index f4e61ef135..3fdf6729e9 100644 --- a/pkg/server/ca/manager.go +++ b/pkg/server/ca/manager.go @@ -215,7 +215,7 @@ func (m *manager) pruneBundle(ctx context.Context) error { return fmt.Errorf("fetch bundle: %v", err) } if resp.Bundle == nil { - return errors.New("no bundle in response") + return errors.New("bundle not found") } oldBundle := resp.Bundle diff --git a/pkg/server/endpoints/endpoints.go b/pkg/server/endpoints/endpoints.go index 50d04cdf04..40c149c343 100644 --- a/pkg/server/endpoints/endpoints.go +++ b/pkg/server/endpoints/endpoints.go @@ -237,7 +237,7 @@ func (e *endpoints) getCerts(ctx context.Context) ([]tls.Certificate, *x509.Cert return nil, nil, fmt.Errorf("get bundle from datastore: %v", err) } if resp.Bundle == nil { - return nil, nil, errors.New("response missing bundle") + return nil, nil, errors.New("bundle not found") } caCerts, err := x509.ParseCertificates(resp.Bundle.CaCerts) diff --git a/pkg/server/endpoints/node/handler.go b/pkg/server/endpoints/node/handler.go index 59220548c0..c2fcf1b38d 100644 --- a/pkg/server/endpoints/node/handler.go +++ b/pkg/server/endpoints/node/handler.go @@ -685,7 +685,7 @@ func (h *Handler) getBundle(ctx context.Context, trustDomain string) (*node.Bund return nil, fmt.Errorf("get bundle from datastore: %v", err) } if resp.Bundle == nil { - return nil, errors.New("response missing bundle") + return nil, errors.New("bundle not found") } return &node.Bundle{ diff --git a/pkg/server/endpoints/registration/handler.go b/pkg/server/endpoints/registration/handler.go index fea5f672eb..c2fd548638 100644 --- a/pkg/server/endpoints/registration/handler.go +++ b/pkg/server/endpoints/registration/handler.go @@ -246,7 +246,7 @@ func (h *Handler) FetchFederatedBundle( return nil, err } if resp.Bundle == nil { - return nil, errors.New("no bundle in response") + return nil, errors.New("bundle not found") } return ®istration.FederatedBundle{ @@ -374,7 +374,7 @@ func (h *Handler) FetchBundle( return nil, fmt.Errorf("get bundle from datastore: %v", err) } if resp.Bundle == nil { - return nil, errors.New("response has no bundle") + return nil, errors.New("bundle not found") } return ®istration.Bundle{CaCerts: resp.Bundle.CaCerts}, nil diff --git a/pkg/server/endpoints/registration/handler_test.go b/pkg/server/endpoints/registration/handler_test.go index 8f3c832451..d0cd2f65c6 100644 --- a/pkg/server/endpoints/registration/handler_test.go +++ b/pkg/server/endpoints/registration/handler_test.go @@ -125,7 +125,7 @@ func (s *HandlerSuite) TestFetchFederatedBundle() { {Id: "spiffe://example.org", CaCerts: "", Err: "federated bundle id cannot match server trust domain"}, {Id: "spiffe://otherdomain.org/spire/agent", CaCerts: "", Err: `"spiffe://otherdomain.org/spire/agent" is not a valid trust domain SPIFFE ID: path is not empty`}, {Id: "spiffe://otherdomain.org", CaCerts: "OTHERDOMAIN", Err: ""}, - {Id: "spiffe://yetotherdomain.org", CaCerts: "", Err: "no such bundle"}, + {Id: "spiffe://yetotherdomain.org", CaCerts: "", Err: "bundle not found"}, } for _, testCase := range testCases { @@ -248,8 +248,9 @@ func (s *HandlerSuite) TestDeleteFederatedBundle() { resp, err := s.ds.FetchBundle(context.Background(), &datastore.FetchBundleRequest{ TrustDomain: testCase.Id, }) - s.Require().EqualError(err, "no such bundle") - s.Require().Nil(resp) + s.Require().NoError(err) + s.Require().NotNil(resp) + s.Require().Nil(resp.Bundle) } } diff --git a/pkg/server/plugin/datastore/sql/sql.go b/pkg/server/plugin/datastore/sql/sql.go index cdbb15c962..4d1bde069f 100644 --- a/pkg/server/plugin/datastore/sql/sql.go +++ b/pkg/server/plugin/datastore/sql/sql.go @@ -594,7 +594,11 @@ func deleteBundle(tx *gorm.DB, req *datastore.DeleteBundleRequest) (*datastore.D // FetchBundle returns the bundle matching the specified Trust Domain. func fetchBundle(tx *gorm.DB, req *datastore.FetchBundleRequest) (*datastore.FetchBundleResponse, error) { model := new(Bundle) - if err := tx.Find(model, "trust_domain = ?", req.TrustDomain).Error; err != nil { + err := tx.Find(model, "trust_domain = ?", req.TrustDomain).Error + switch { + case err == gorm.ErrRecordNotFound: + return &datastore.FetchBundleResponse{}, nil + case err != nil: return nil, sqlError.Wrap(err) } diff --git a/pkg/server/plugin/datastore/sql/sql_test.go b/pkg/server/plugin/datastore/sql/sql_test.go index d0cae74314..eb6ff6e4a0 100644 --- a/pkg/server/plugin/datastore/sql/sql_test.go +++ b/pkg/server/plugin/datastore/sql/sql_test.go @@ -107,14 +107,20 @@ func (s *PluginSuite) TestBundleCRUD() { CaCerts: s.cert.Raw, } + // fetch non-existant + fresp, err := s.ds.FetchBundle(ctx, &datastore.FetchBundleRequest{TrustDomain: "spiffe://foo"}) + s.Require().NoError(err) + s.Require().NotNil(fresp) + s.Require().Nil(fresp.Bundle) + // create - _, err := s.ds.CreateBundle(ctx, &datastore.CreateBundleRequest{ + _, err = s.ds.CreateBundle(ctx, &datastore.CreateBundleRequest{ Bundle: bundle, }) s.Require().NoError(err) // fetch - fresp, err := s.ds.FetchBundle(ctx, &datastore.FetchBundleRequest{TrustDomain: "spiffe://foo"}) + fresp, err = s.ds.FetchBundle(ctx, &datastore.FetchBundleRequest{TrustDomain: "spiffe://foo"}) s.Require().NoError(err) s.Equal(bundle, fresp.Bundle) diff --git a/test/fakes/fakedatastore/fakedatastore.go b/test/fakes/fakedatastore/fakedatastore.go index 6bb4e9d239..a774031a3f 100644 --- a/test/fakes/fakedatastore/fakedatastore.go +++ b/test/fakes/fakedatastore/fakedatastore.go @@ -181,7 +181,7 @@ func (s *DataStore) FetchBundle(ctx context.Context, req *datastore.FetchBundleR bundle, ok := s.bundles[req.TrustDomain] if !ok { - return nil, ErrNoSuchBundle + return &datastore.FetchBundleResponse{}, nil } return &datastore.FetchBundleResponse{