From 78341a3a9a5676afe37dc2164fb1688663012753 Mon Sep 17 00:00:00 2001 From: Luna Duclos Date: Thu, 18 Aug 2016 18:28:43 +0200 Subject: [PATCH] Add error parameter to storage.SiteExists() --- caddytls/client.go | 7 ++++++- caddytls/config.go | 7 ++++++- caddytls/config_test.go | 2 +- caddytls/filestorage.go | 13 ++++++++----- caddytls/storage.go | 2 +- caddytls/storagetest/memorystorage.go | 4 ++-- caddytls/storagetest/storagetest.go | 23 ++++++++++++++++++++--- caddytls/tls_test.go | 16 +++++++++++++--- 8 files changed, 57 insertions(+), 17 deletions(-) diff --git a/caddytls/client.go b/caddytls/client.go index 70eccc22..97393195 100644 --- a/caddytls/client.go +++ b/caddytls/client.go @@ -287,7 +287,12 @@ func (c *ACMEClient) Revoke(name string) error { return err } - if !storage.SiteExists(name) { + siteExists, err := storage.SiteExists(name) + if err != nil { + return err + } + + if !siteExists { return errors.New("no certificate and key for " + name) } diff --git a/caddytls/config.go b/caddytls/config.go index f1dcff84..ab793552 100644 --- a/caddytls/config.go +++ b/caddytls/config.go @@ -138,7 +138,12 @@ func (c *Config) obtainCertName(name string, allowPrompts bool) error { return err } - if !c.Managed || !HostQualifies(name) || storage.SiteExists(name) { + siteExists, err := storage.SiteExists(name) + if err != nil { + return err + } + + if !c.Managed || !HostQualifies(name) || siteExists { return nil } diff --git a/caddytls/config_test.go b/caddytls/config_test.go index 7152d76d..f5450bb7 100644 --- a/caddytls/config_test.go +++ b/caddytls/config_test.go @@ -115,7 +115,7 @@ func TestStorageForCustomNil(t *testing.T) { type fakeStorage string -func (s fakeStorage) SiteExists(domain string) bool { +func (s fakeStorage) SiteExists(domain string) (bool, error) { panic("no impl") } diff --git a/caddytls/filestorage.go b/caddytls/filestorage.go index 32d001fa..bb86958a 100644 --- a/caddytls/filestorage.go +++ b/caddytls/filestorage.go @@ -121,16 +121,19 @@ func (s FileStorage) readFile(file string) ([]byte, error) { // SiteExists implements Storage.SiteExists by checking for the presence of // cert and key files. -func (s FileStorage) SiteExists(domain string) bool { +func (s FileStorage) SiteExists(domain string) (bool, error) { _, err := os.Stat(s.siteCertFile(domain)) - if err != nil { - return false + if os.IsNotExist(err) { + return false, nil + } else if err != nil { + return false, err } + _, err = os.Stat(s.siteKeyFile(domain)) if err != nil { - return false + return false, err } - return true + return true, nil } // LoadSite implements Storage.LoadSite by loading it from disk. If it is not diff --git a/caddytls/storage.go b/caddytls/storage.go index e3336794..d2c1ff65 100644 --- a/caddytls/storage.go +++ b/caddytls/storage.go @@ -39,7 +39,7 @@ type Storage interface { // SiteExists returns true if this site exists in storage. // Site data is considered present when StoreSite has been called // successfully (without DeleteSite having been called, of course). - SiteExists(domain string) bool + SiteExists(domain string) (bool, error) // LoadSite obtains the site data from storage for the given domain and // returns it. If data for the domain does not exist, the diff --git a/caddytls/storagetest/memorystorage.go b/caddytls/storagetest/memorystorage.go index cc1a48b5..138342d5 100644 --- a/caddytls/storagetest/memorystorage.go +++ b/caddytls/storagetest/memorystorage.go @@ -49,9 +49,9 @@ func NewInMemoryStorage() *InMemoryStorage { } // SiteExists implements caddytls.Storage.SiteExists in memory. -func (s *InMemoryStorage) SiteExists(domain string) bool { +func (s *InMemoryStorage) SiteExists(domain string) (bool, error) { _, siteExists := s.Sites[domain] - return siteExists + return siteExists, nil } // Clear completely clears all values associated with this storage. diff --git a/caddytls/storagetest/storagetest.go b/caddytls/storagetest/storagetest.go index 0400aded..9c11d77b 100644 --- a/caddytls/storagetest/storagetest.go +++ b/caddytls/storagetest/storagetest.go @@ -113,7 +113,12 @@ func (s *StorageTest) TestSiteExists() error { defer s.runPostTest() // Should not exist at first - if s.SiteExists("example.com") { + siteExists, err := s.SiteExists("example.com") + if err != nil { + return err + } + + if siteExists { return errors.New("Site should not exist") } @@ -121,7 +126,13 @@ func (s *StorageTest) TestSiteExists() error { if err := s.StoreSite("example.com", simpleSiteData); err != nil { return err } - if !s.SiteExists("example.com") { + + siteExists, err = s.SiteExists("example.com") + if err != nil { + return err + } + + if !siteExists { return errors.New("Expected site to exist") } @@ -129,7 +140,13 @@ func (s *StorageTest) TestSiteExists() error { if err := s.DeleteSite("example.com"); err != nil { return err } - if s.SiteExists("example.com") { + + siteExists, err = s.SiteExists("example.com") + if err != nil { + return err + } + + if siteExists { return errors.New("Site should not exist after delete") } return nil diff --git a/caddytls/tls_test.go b/caddytls/tls_test.go index 89b22aca..fb7d59dc 100644 --- a/caddytls/tls_test.go +++ b/caddytls/tls_test.go @@ -135,11 +135,16 @@ func TestExistingCertAndKey(t *testing.T) { domain := "example.com" - if storage.SiteExists(domain) { + siteExists, err := storage.SiteExists(domain) + if err != nil { + t.Fatalf("Could not determine whether site exists: %v", err) + } + + if siteExists { t.Errorf("Did NOT expect %v to have existing cert or key, but it did", domain) } - err := saveCertResource(storage, acme.CertificateResource{ + err = saveCertResource(storage, acme.CertificateResource{ Domain: domain, PrivateKey: []byte("key"), Certificate: []byte("cert"), @@ -148,7 +153,12 @@ func TestExistingCertAndKey(t *testing.T) { t.Fatalf("Expected no error, got: %v", err) } - if !storage.SiteExists(domain) { + siteExists, err = storage.SiteExists(domain) + if err != nil { + t.Fatalf("Could not determine whether site exists: %v", err) + } + + if !siteExists { t.Errorf("Expected %v to have existing cert and key, but it did NOT", domain) } }