From c6dfff131fa7605fde4832b6d36b3cd86975c24f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 16:33:18 +0200 Subject: [PATCH 01/14] cli/context/store: metadataStore.get(), .remove(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/metadata_test.go | 12 ++++++------ cli/context/store/metadatastore.go | 15 +++++++++------ cli/context/store/store.go | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/cli/context/store/metadata_test.go b/cli/context/store/metadata_test.go index 9de17408b058..b81f53b0f56f 100644 --- a/cli/context/store/metadata_test.go +++ b/cli/context/store/metadata_test.go @@ -41,7 +41,7 @@ func TestMetadataCreateGetRemove(t *testing.T) { assert.NilError(t, err) // create a new instance to check it does not depend on some sort of state testee = metadataStore{root: testDir, config: testCfg} - meta, err := testee.get(contextdirOf("test-context")) + meta, err := testee.get("test-context") assert.NilError(t, err) assert.DeepEqual(t, meta, testMeta) @@ -49,13 +49,13 @@ func TestMetadataCreateGetRemove(t *testing.T) { err = testee.createOrUpdate(expected2) assert.NilError(t, err) - meta, err = testee.get(contextdirOf("test-context")) + meta, err = testee.get("test-context") assert.NilError(t, err) assert.DeepEqual(t, meta, expected2) - assert.NilError(t, testee.remove(contextdirOf("test-context"))) - assert.NilError(t, testee.remove(contextdirOf("test-context"))) // support duplicate remove - _, err = testee.get(contextdirOf("test-context")) + assert.NilError(t, testee.remove("test-context")) + assert.NilError(t, testee.remove("test-context")) // support duplicate remove + _, err = testee.get("test-context") assert.Assert(t, IsErrContextDoesNotExist(err)) } @@ -121,7 +121,7 @@ func TestWithEmbedding(t *testing.T) { }, } assert.NilError(t, testee.createOrUpdate(Metadata{Metadata: testCtxMeta, Name: "test"})) - res, err := testee.get(contextdirOf("test")) + res, err := testee.get("test") assert.NilError(t, err) assert.Equal(t, testCtxMeta, res.Metadata) } diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go index 151852fa1bf0..92fc7d4724dd 100644 --- a/cli/context/store/metadatastore.go +++ b/cli/context/store/metadatastore.go @@ -55,9 +55,12 @@ func parseTypedOrMap(payload []byte, getter TypeGetter) (interface{}, error) { return reflect.ValueOf(typed).Elem().Interface(), nil } -func (s *metadataStore) get(id contextdir) (Metadata, error) { - contextDir := s.contextDir(id) - bytes, err := os.ReadFile(filepath.Join(contextDir, metaFile)) +func (s *metadataStore) get(name string) (Metadata, error) { + return s.getByID(contextdirOf(name)) +} + +func (s *metadataStore) getByID(id contextdir) (Metadata, error) { + bytes, err := os.ReadFile(filepath.Join(s.contextDir(id), metaFile)) if err != nil { return Metadata{}, convertContextDoesNotExist(err) } @@ -80,8 +83,8 @@ func (s *metadataStore) get(id contextdir) (Metadata, error) { return r, err } -func (s *metadataStore) remove(id contextdir) error { - contextDir := s.contextDir(id) +func (s *metadataStore) remove(name string) error { + contextDir := s.contextDir(contextdirOf(name)) return os.RemoveAll(contextDir) } @@ -95,7 +98,7 @@ func (s *metadataStore) list() ([]Metadata, error) { } var res []Metadata for _, dir := range ctxDirs { - c, err := s.get(contextdir(dir)) + c, err := s.getByID(contextdir(dir)) if err != nil { return nil, err } diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 19ad980a475d..d4657b99bd25 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -137,14 +137,14 @@ func (s *store) CreateOrUpdate(meta Metadata) error { func (s *store) Remove(name string) error { id := contextdirOf(name) - if err := s.meta.remove(id); err != nil { + if err := s.meta.remove(name); err != nil { return patchErrContextName(err, name) } return patchErrContextName(s.tls.removeAllContextData(id), name) } func (s *store) GetMetadata(name string) (Metadata, error) { - res, err := s.meta.get(contextdirOf(name)) + res, err := s.meta.get(name) patchErrContextName(err, name) return res, err } From f843c42c05b7f50dc4204cf10379e45dca26c87a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:09:59 +0200 Subject: [PATCH 02/14] cli/context/store: listRecursivelyMetadataDirs(): use filepath.Join() Looks like the intent here is to get the path of a subdirectory. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/metadatastore.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go index 92fc7d4724dd..64d516470399 100644 --- a/cli/context/store/metadatastore.go +++ b/cli/context/store/metadatastore.go @@ -2,7 +2,6 @@ package store import ( "encoding/json" - "fmt" "os" "path/filepath" "reflect" @@ -134,7 +133,7 @@ func listRecursivelyMetadataDirs(root string) ([]string, error) { return nil, err } for _, s := range subs { - result = append(result, fmt.Sprintf("%s/%s", fi.Name(), s)) + result = append(result, filepath.Join(fi.Name(), s)) } } } From d0398c423f881579dfa7971370960599a52ce22c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:21:01 +0200 Subject: [PATCH 03/14] cli/context/store: removeAllContextData(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. This also fixes a test which incorrectly used "names" as pseudo-IDs. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 5 ++--- cli/context/store/tlsstore.go | 4 ++-- cli/context/store/tlsstore_test.go | 14 ++++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index d4657b99bd25..03fac955156d 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -136,11 +136,10 @@ func (s *store) CreateOrUpdate(meta Metadata) error { } func (s *store) Remove(name string) error { - id := contextdirOf(name) if err := s.meta.remove(name); err != nil { return patchErrContextName(err, name) } - return patchErrContextName(s.tls.removeAllContextData(id), name) + return patchErrContextName(s.tls.removeAllContextData(name), name) } func (s *store) GetMetadata(name string) (Metadata, error) { @@ -151,7 +150,7 @@ func (s *store) GetMetadata(name string) (Metadata, error) { func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { id := contextdirOf(name) - if err := s.tls.removeAllContextData(id); err != nil { + if err := s.tls.removeAllContextData(name); err != nil { return patchErrContextName(err, name) } if data == nil { diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 8267e879644f..9b1eda5891db 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -55,8 +55,8 @@ func (s *tlsStore) removeAllEndpointData(contextID contextdir, endpointName stri return os.RemoveAll(s.endpointDir(contextID, endpointName)) } -func (s *tlsStore) removeAllContextData(contextID contextdir) error { - return os.RemoveAll(s.contextDir(contextID)) +func (s *tlsStore) removeAllContextData(name string) error { + return os.RemoveAll(s.contextDir(contextdirOf(name))) } func (s *tlsStore) listContextData(contextID contextdir) (map[string]EndpointFiles, error) { diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 5af9acda3b2b..80b5d0d5f295 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -45,26 +45,28 @@ func TestTlsListAndBatchRemove(t *testing.T) { "ep2": {"f1", "f2", "f3"}, } + const contextName = "test-ctx" + contextID := contextdirOf(contextName) for name, files := range all { for _, file := range files { - err := testee.createOrUpdate("test-ctx", name, file, []byte("data")) + err := testee.createOrUpdate(contextID, name, file, []byte("data")) assert.NilError(t, err) } } - resAll, err := testee.listContextData("test-ctx") + resAll, err := testee.listContextData(contextID) assert.NilError(t, err) assert.DeepEqual(t, resAll, all) - err = testee.removeAllEndpointData("test-ctx", "ep3") + err = testee.removeAllEndpointData(contextID, "ep3") assert.NilError(t, err) - resEp1ep2, err := testee.listContextData("test-ctx") + resEp1ep2, err := testee.listContextData(contextID) assert.NilError(t, err) assert.DeepEqual(t, resEp1ep2, ep1ep2) - err = testee.removeAllContextData("test-ctx") + err = testee.removeAllContextData(contextName) assert.NilError(t, err) - resEmpty, err := testee.listContextData("test-ctx") + resEmpty, err := testee.listContextData(contextID) assert.NilError(t, err) assert.DeepEqual(t, resEmpty, map[string]EndpointFiles{}) } From 42e275eaf60197bb207fd1f546b01567cf8d2841 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:29:01 +0200 Subject: [PATCH 04/14] cli/context/store: TestTlsCreateUpdateGetRemove(): use correct ID This test was depending on the fact that contextDir's are a string, and for the test is was using the context _name_ as a pseudo-ID. This patch updates the test to be more explicit where ID's and where names are used. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/tlsstore_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 80b5d0d5f295..ede8955d4e9a 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -8,26 +8,30 @@ import ( func TestTlsCreateUpdateGetRemove(t *testing.T) { testee := tlsStore{root: t.TempDir()} - _, err := testee.getData("test-ctx", "test-ep", "test-data") + + const contextName = "test-ctx" + contextID := contextdirOf(contextName) + + _, err := testee.getData(contextID, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) - err = testee.createOrUpdate("test-ctx", "test-ep", "test-data", []byte("data")) + err = testee.createOrUpdate(contextID, "test-ep", "test-data", []byte("data")) assert.NilError(t, err) - data, err := testee.getData("test-ctx", "test-ep", "test-data") + data, err := testee.getData(contextID, "test-ep", "test-data") assert.NilError(t, err) assert.Equal(t, string(data), "data") - err = testee.createOrUpdate("test-ctx", "test-ep", "test-data", []byte("data2")) + err = testee.createOrUpdate(contextID, "test-ep", "test-data", []byte("data2")) assert.NilError(t, err) - data, err = testee.getData("test-ctx", "test-ep", "test-data") + data, err = testee.getData(contextID, "test-ep", "test-data") assert.NilError(t, err) assert.Equal(t, string(data), "data2") - err = testee.remove("test-ctx", "test-ep", "test-data") + err = testee.remove(contextID, "test-ep", "test-data") assert.NilError(t, err) - err = testee.remove("test-ctx", "test-ep", "test-data") + err = testee.remove(contextID, "test-ep", "test-data") assert.NilError(t, err) - _, err = testee.getData("test-ctx", "test-ep", "test-data") + _, err = testee.getData(contextID, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) } From c3eb116f9cac44c9a6c752aacfffb1de59b4a390 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:36:21 +0200 Subject: [PATCH 05/14] cli/context/store: removeAllEndpointData(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 2 +- cli/context/store/tlsstore.go | 4 ++-- cli/context/store/tlsstore_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 03fac955156d..ed82e52c47d9 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -168,7 +168,7 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { id := contextdirOf(contextName) - if err := s.tls.removeAllEndpointData(id, endpointName); err != nil { + if err := s.tls.removeAllEndpointData(contextName, endpointName); err != nil { return patchErrContextName(err, contextName) } if data == nil { diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 9b1eda5891db..5d54ca8dfeaa 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -51,8 +51,8 @@ func (s *tlsStore) remove(contextID contextdir, endpointName, filename string) e return err } -func (s *tlsStore) removeAllEndpointData(contextID contextdir, endpointName string) error { - return os.RemoveAll(s.endpointDir(contextID, endpointName)) +func (s *tlsStore) removeAllEndpointData(name, endpointName string) error { + return os.RemoveAll(s.endpointDir(contextdirOf(name), endpointName)) } func (s *tlsStore) removeAllContextData(name string) error { diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index ede8955d4e9a..dd7c571ea96f 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -62,7 +62,7 @@ func TestTlsListAndBatchRemove(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, resAll, all) - err = testee.removeAllEndpointData(contextID, "ep3") + err = testee.removeAllEndpointData(contextName, "ep3") assert.NilError(t, err) resEp1ep2, err := testee.listContextData(contextID) assert.NilError(t, err) From 3b7f13a5e51d24f2fc33d107188ec46377b6b103 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:38:12 +0200 Subject: [PATCH 06/14] cli/context/store: createOrUpdate(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 6 ++---- cli/context/store/tlsstore.go | 3 ++- cli/context/store/tlsstore_test.go | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index ed82e52c47d9..56554dd01aef 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -149,7 +149,6 @@ func (s *store) GetMetadata(name string) (Metadata, error) { } func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { - id := contextdirOf(name) if err := s.tls.removeAllContextData(name); err != nil { return patchErrContextName(err, name) } @@ -158,7 +157,7 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { } for ep, files := range data.Endpoints { for fileName, data := range files.Files { - if err := s.tls.createOrUpdate(id, ep, fileName, data); err != nil { + if err := s.tls.createOrUpdate(name, ep, fileName, data); err != nil { return patchErrContextName(err, name) } } @@ -167,7 +166,6 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { } func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { - id := contextdirOf(contextName) if err := s.tls.removeAllEndpointData(contextName, endpointName); err != nil { return patchErrContextName(err, contextName) } @@ -175,7 +173,7 @@ func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string return nil } for fileName, data := range data.Files { - if err := s.tls.createOrUpdate(id, endpointName, fileName, data); err != nil { + if err := s.tls.createOrUpdate(contextName, endpointName, fileName, data); err != nil { return patchErrContextName(err, contextName) } } diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 5d54ca8dfeaa..47303607e460 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -23,7 +23,8 @@ func (s *tlsStore) filePath(contextID contextdir, endpointName, filename string) return filepath.Join(s.root, string(contextID), endpointName, filename) } -func (s *tlsStore) createOrUpdate(contextID contextdir, endpointName, filename string, data []byte) error { +func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []byte) error { + contextID := contextdirOf(name) epdir := s.endpointDir(contextID, endpointName) parentOfRoot := filepath.Dir(s.root) if err := os.MkdirAll(parentOfRoot, 0755); err != nil { diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index dd7c571ea96f..9b1606b80956 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -15,12 +15,12 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { _, err := testee.getData(contextID, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) - err = testee.createOrUpdate(contextID, "test-ep", "test-data", []byte("data")) + err = testee.createOrUpdate(contextName, "test-ep", "test-data", []byte("data")) assert.NilError(t, err) data, err := testee.getData(contextID, "test-ep", "test-data") assert.NilError(t, err) assert.Equal(t, string(data), "data") - err = testee.createOrUpdate(contextID, "test-ep", "test-data", []byte("data2")) + err = testee.createOrUpdate(contextName, "test-ep", "test-data", []byte("data2")) assert.NilError(t, err) data, err = testee.getData(contextID, "test-ep", "test-data") assert.NilError(t, err) @@ -53,7 +53,7 @@ func TestTlsListAndBatchRemove(t *testing.T) { contextID := contextdirOf(contextName) for name, files := range all { for _, file := range files { - err := testee.createOrUpdate(contextID, name, file, []byte("data")) + err := testee.createOrUpdate(contextName, name, file, []byte("data")) assert.NilError(t, err) } } From 0bcdff25717a69f9aabb5b4fcc73a3feec00db6d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:40:33 +0200 Subject: [PATCH 07/14] cli/context/store: getData(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 2 +- cli/context/store/tlsstore.go | 4 ++-- cli/context/store/tlsstore_test.go | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 56554dd01aef..97a2d93ea66c 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -186,7 +186,7 @@ func (s *store) ListTLSFiles(name string) (map[string]EndpointFiles, error) { } func (s *store) GetTLSData(contextName, endpointName, fileName string) ([]byte, error) { - res, err := s.tls.getData(contextdirOf(contextName), endpointName, fileName) + res, err := s.tls.getData(contextName, endpointName, fileName) return res, patchErrContextName(err, contextName) } diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 47303607e460..ec1dd43de255 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -36,8 +36,8 @@ func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []by return os.WriteFile(s.filePath(contextID, endpointName, filename), data, 0600) } -func (s *tlsStore) getData(contextID contextdir, endpointName, filename string) ([]byte, error) { - data, err := os.ReadFile(s.filePath(contextID, endpointName, filename)) +func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) { + data, err := os.ReadFile(s.filePath(contextdirOf(name), endpointName, filename)) if err != nil { return nil, convertTLSDataDoesNotExist(endpointName, filename, err) } diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 9b1606b80956..66b3d4ffd717 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -12,17 +12,17 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { const contextName = "test-ctx" contextID := contextdirOf(contextName) - _, err := testee.getData(contextID, "test-ep", "test-data") + _, err := testee.getData(contextName, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) err = testee.createOrUpdate(contextName, "test-ep", "test-data", []byte("data")) assert.NilError(t, err) - data, err := testee.getData(contextID, "test-ep", "test-data") + data, err := testee.getData(contextName, "test-ep", "test-data") assert.NilError(t, err) assert.Equal(t, string(data), "data") err = testee.createOrUpdate(contextName, "test-ep", "test-data", []byte("data2")) assert.NilError(t, err) - data, err = testee.getData(contextID, "test-ep", "test-data") + data, err = testee.getData(contextName, "test-ep", "test-data") assert.NilError(t, err) assert.Equal(t, string(data), "data2") @@ -31,7 +31,7 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { err = testee.remove(contextID, "test-ep", "test-data") assert.NilError(t, err) - _, err = testee.getData(contextID, "test-ep", "test-data") + _, err = testee.getData(contextName, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) } From 712cc9a1c7ee91521ed334d339878bd67aa3895c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:47:30 +0200 Subject: [PATCH 08/14] cli/context/store: remove(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/tlsstore.go | 6 ++++-- cli/context/store/tlsstore_test.go | 5 ++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index ec1dd43de255..777c9c04f015 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -44,8 +44,10 @@ func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) return data, nil } -func (s *tlsStore) remove(contextID contextdir, endpointName, filename string) error { - err := os.Remove(s.filePath(contextID, endpointName, filename)) +// remove removes a TLS data from an endpoint +// TODO(thaJeztah) tlsStore.remove() is not used anywhere outside of tests; should we use removeAllEndpointData() only? +func (s *tlsStore) remove(name, endpointName, filename string) error { + err := os.Remove(s.filePath(contextdirOf(name), endpointName, filename)) if os.IsNotExist(err) { return nil } diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 66b3d4ffd717..3f6b6c44438b 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -10,7 +10,6 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { testee := tlsStore{root: t.TempDir()} const contextName = "test-ctx" - contextID := contextdirOf(contextName) _, err := testee.getData(contextName, "test-ep", "test-data") assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) @@ -26,9 +25,9 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { assert.NilError(t, err) assert.Equal(t, string(data), "data2") - err = testee.remove(contextID, "test-ep", "test-data") + err = testee.remove(contextName, "test-ep", "test-data") assert.NilError(t, err) - err = testee.remove(contextID, "test-ep", "test-data") + err = testee.remove(contextName, "test-ep", "test-data") assert.NilError(t, err) _, err = testee.getData(contextName, "test-ep", "test-data") From 9720d5b4510e224ce666bad4e3b7032463e1c994 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 17:51:25 +0200 Subject: [PATCH 09/14] cli/context/store: listContextData(): accept name instead of ID This allows callers to just pass the name, and handle the conversion to ID and path internally. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 2 +- cli/context/store/tlsstore.go | 3 ++- cli/context/store/tlsstore_test.go | 7 +++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 97a2d93ea66c..299bdb9a7448 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -181,7 +181,7 @@ func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string } func (s *store) ListTLSFiles(name string) (map[string]EndpointFiles, error) { - res, err := s.tls.listContextData(contextdirOf(name)) + res, err := s.tls.listContextData(name) return res, patchErrContextName(err, name) } diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 777c9c04f015..30620b3f9760 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -62,7 +62,8 @@ func (s *tlsStore) removeAllContextData(name string) error { return os.RemoveAll(s.contextDir(contextdirOf(name))) } -func (s *tlsStore) listContextData(contextID contextdir) (map[string]EndpointFiles, error) { +func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error) { + contextID := contextdirOf(name) epFSs, err := os.ReadDir(s.contextDir(contextID)) if err != nil { if os.IsNotExist(err) { diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 3f6b6c44438b..9c4c79ebdab8 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -49,7 +49,6 @@ func TestTlsListAndBatchRemove(t *testing.T) { } const contextName = "test-ctx" - contextID := contextdirOf(contextName) for name, files := range all { for _, file := range files { err := testee.createOrUpdate(contextName, name, file, []byte("data")) @@ -57,19 +56,19 @@ func TestTlsListAndBatchRemove(t *testing.T) { } } - resAll, err := testee.listContextData(contextID) + resAll, err := testee.listContextData(contextName) assert.NilError(t, err) assert.DeepEqual(t, resAll, all) err = testee.removeAllEndpointData(contextName, "ep3") assert.NilError(t, err) - resEp1ep2, err := testee.listContextData(contextID) + resEp1ep2, err := testee.listContextData(contextName) assert.NilError(t, err) assert.DeepEqual(t, resEp1ep2, ep1ep2) err = testee.removeAllContextData(contextName) assert.NilError(t, err) - resEmpty, err := testee.listContextData(contextID) + resEmpty, err := testee.listContextData(contextName) assert.NilError(t, err) assert.DeepEqual(t, resEmpty, map[string]EndpointFiles{}) } From 38f54e792631f066bde65b85599df749d71089b4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 18:08:22 +0200 Subject: [PATCH 10/14] cli/context/store: remove filePath(), make contextDir() accept name removing the extra abstraction, and simplify use of contextDir() Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 5 ++--- cli/context/store/tlsstore.go | 34 ++++++++++++++-------------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 299bdb9a7448..f00599602c93 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -191,10 +191,9 @@ func (s *store) GetTLSData(contextName, endpointName, fileName string) ([]byte, } func (s *store) GetStorageInfo(contextName string) StorageInfo { - dir := contextdirOf(contextName) return StorageInfo{ - MetadataPath: s.meta.contextDir(dir), - TLSPath: s.tls.contextDir(dir), + MetadataPath: s.meta.contextDir(contextdirOf(contextName)), + TLSPath: s.tls.contextDir(contextName), } } diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 30620b3f9760..0c2fc4120670 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -11,33 +11,28 @@ type tlsStore struct { root string } -func (s *tlsStore) contextDir(id contextdir) string { - return filepath.Join(s.root, string(id)) +func (s *tlsStore) contextDir(name string) string { + return filepath.Join(s.root, string(contextdirOf(name))) } -func (s *tlsStore) endpointDir(contextID contextdir, name string) string { - return filepath.Join(s.root, string(contextID), name) -} - -func (s *tlsStore) filePath(contextID contextdir, endpointName, filename string) string { - return filepath.Join(s.root, string(contextID), endpointName, filename) +func (s *tlsStore) endpointDir(name, endpointName string) string { + return filepath.Join(s.contextDir(name), endpointName) } func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []byte) error { - contextID := contextdirOf(name) - epdir := s.endpointDir(contextID, endpointName) parentOfRoot := filepath.Dir(s.root) if err := os.MkdirAll(parentOfRoot, 0755); err != nil { return err } - if err := os.MkdirAll(epdir, 0700); err != nil { + endpointDir := s.endpointDir(name, endpointName) + if err := os.MkdirAll(endpointDir, 0700); err != nil { return err } - return os.WriteFile(s.filePath(contextID, endpointName, filename), data, 0600) + return os.WriteFile(filepath.Join(endpointDir, filename), data, 0600) } func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) { - data, err := os.ReadFile(s.filePath(contextdirOf(name), endpointName, filename)) + data, err := os.ReadFile(filepath.Join(s.endpointDir(name, endpointName), filename)) if err != nil { return nil, convertTLSDataDoesNotExist(endpointName, filename, err) } @@ -47,7 +42,7 @@ func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) // remove removes a TLS data from an endpoint // TODO(thaJeztah) tlsStore.remove() is not used anywhere outside of tests; should we use removeAllEndpointData() only? func (s *tlsStore) remove(name, endpointName, filename string) error { - err := os.Remove(s.filePath(contextdirOf(name), endpointName, filename)) + err := os.Remove(filepath.Join(s.endpointDir(name, endpointName), filename)) if os.IsNotExist(err) { return nil } @@ -55,16 +50,16 @@ func (s *tlsStore) remove(name, endpointName, filename string) error { } func (s *tlsStore) removeAllEndpointData(name, endpointName string) error { - return os.RemoveAll(s.endpointDir(contextdirOf(name), endpointName)) + return os.RemoveAll(s.endpointDir(name, endpointName)) } func (s *tlsStore) removeAllContextData(name string) error { - return os.RemoveAll(s.contextDir(contextdirOf(name))) + return os.RemoveAll(s.contextDir(name)) } func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error) { - contextID := contextdirOf(name) - epFSs, err := os.ReadDir(s.contextDir(contextID)) + contextDir := s.contextDir(name) + epFSs, err := os.ReadDir(contextDir) if err != nil { if os.IsNotExist(err) { return map[string]EndpointFiles{}, nil @@ -74,8 +69,7 @@ func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error r := make(map[string]EndpointFiles) for _, epFS := range epFSs { if epFS.IsDir() { - epDir := s.endpointDir(contextID, epFS.Name()) - fss, err := os.ReadDir(epDir) + fss, err := os.ReadDir(filepath.Join(contextDir, epFS.Name())) if err != nil { return nil, err } From de6020a240ff95c97150f07d7a0dd59981143868 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 22:18:51 +0200 Subject: [PATCH 11/14] cli/context/store: simplify error handling, and make it more idiomatic The package defined various special errors; these errors existed for two reasons; - being able to distinguish "not found" errors from other errors (as "not found" errors can be ignored in various cases). - to be able to update the context _name_ in the error message after the error was created. This was needed in cases where the name was not available at the location where the error was produced (e.g. only the "id" was present), and the helpers to detect "not found" errors did not support wrapped errors (so wrapping the error with a "name" could break logic); a `setContextName` interface and corresponding `patchErrContextName()` utility was created for this (which was a "creative", but not very standard approach). This patch: - Removes the special error-types, replacing them with errdefs definitions (which is a more common approach in our code-base to detect error types / classes). - Removes the internal utilities for error-handling, and deprecates the exported utilities (to allow external consumers to adjust their code). - Some errors have been enriched with detailed information (which may be useful for debugging / problem solving). - Note that in some cases, `patchErrContextName()` was called, but the code producing the error would never return a `setContextName` error, so would never update the error message. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 5 +- cli/command/context/create.go | 3 +- cli/command/context/remove.go | 4 +- cli/command/context/remove_test.go | 7 +- cli/command/context/use_test.go | 4 +- cli/command/defaultcontextstore.go | 29 ++------ cli/command/defaultcontextstore_test.go | 6 +- cli/context/store/metadata_test.go | 5 +- cli/context/store/metadatastore.go | 28 ++++---- cli/context/store/store.go | 89 ++++++------------------- cli/context/store/store_test.go | 8 ++- cli/context/store/tlsstore.go | 29 ++++---- cli/context/store/tlsstore_test.go | 5 +- cli/context/tlsdata.go | 6 +- 14 files changed, 91 insertions(+), 137 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index c2b5bcc7ab5e..b31c35785d82 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -28,6 +28,7 @@ import ( "github.com/docker/docker/api/types/registry" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" + "github.com/docker/docker/errdefs" "github.com/docker/go-connections/tlsconfig" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -462,8 +463,8 @@ func resolveContextName(opts *cliflags.CommonOptions, config *configfile.ConfigF } if config != nil && config.CurrentContext != "" { _, err := contextstore.GetMetadata(config.CurrentContext) - if store.IsErrContextDoesNotExist(err) { - return "", errors.Errorf("Current context %q is not found on the file system, please check your config file at %s", config.CurrentContext, config.Filename) + if errdefs.IsNotFound(err) { + return "", errors.Errorf("current context %q is not found on the file system, please check your config file at %s", config.CurrentContext, config.Filename) } return config.CurrentContext, err } diff --git a/cli/command/context/create.go b/cli/command/context/create.go index 647295e30855..7765a9490e86 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -10,6 +10,7 @@ import ( "github.com/docker/cli/cli/command/formatter/tabwriter" "github.com/docker/cli/cli/context/docker" "github.com/docker/cli/cli/context/store" + "github.com/docker/docker/errdefs" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -127,7 +128,7 @@ func checkContextNameForCreation(s store.Reader, name string) error { if err := store.ValidateContextName(name); err != nil { return err } - if _, err := s.GetMetadata(name); !store.IsErrContextDoesNotExist(err) { + if _, err := s.GetMetadata(name); !errdefs.IsNotFound(err) { if err != nil { return errors.Wrap(err, "error while getting existing contexts") } diff --git a/cli/command/context/remove.go b/cli/command/context/remove.go index 434e53e8fac2..9ba405d588e1 100644 --- a/cli/command/context/remove.go +++ b/cli/command/context/remove.go @@ -40,7 +40,7 @@ func RunRemove(dockerCli command.Cli, opts RemoveOptions, names []string) error if name == "default" { errs = append(errs, `default: context "default" cannot be removed`) } else if err := doRemove(dockerCli, name, name == currentCtx, opts.Force); err != nil { - errs = append(errs, fmt.Sprintf("%s: %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintln(dockerCli.Out(), name) } @@ -54,7 +54,7 @@ func RunRemove(dockerCli command.Cli, opts RemoveOptions, names []string) error func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error { if isCurrent { if !force { - return errors.New("context is in use, set -f flag to force remove") + return errors.Errorf("context %q is in use, set -f flag to force remove", name) } // fallback to DOCKER_HOST cfg := dockerCli.ConfigFile() diff --git a/cli/command/context/remove_test.go b/cli/command/context/remove_test.go index 300d3d2fdc4c..35e27781b2e5 100644 --- a/cli/command/context/remove_test.go +++ b/cli/command/context/remove_test.go @@ -6,8 +6,9 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" - "github.com/docker/cli/cli/context/store" + "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) func TestRemove(t *testing.T) { @@ -18,7 +19,7 @@ func TestRemove(t *testing.T) { _, err := cli.ContextStore().GetMetadata("current") assert.NilError(t, err) _, err = cli.ContextStore().GetMetadata("other") - assert.Check(t, store.IsErrContextDoesNotExist(err)) + assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) } func TestRemoveNotAContext(t *testing.T) { @@ -38,7 +39,7 @@ func TestRemoveCurrent(t *testing.T) { createTestContext(t, cli, "other") cli.SetCurrentContext("current") err := RunRemove(cli, RemoveOptions{}, []string{"current"}) - assert.ErrorContains(t, err, "current: context is in use, set -f flag to force remove") + assert.ErrorContains(t, err, `context "current" is in use, set -f flag to force remove`) } func TestRemoveCurrentForce(t *testing.T) { diff --git a/cli/command/context/use_test.go b/cli/command/context/use_test.go index 13be26425f9e..d7521abcdb58 100644 --- a/cli/command/context/use_test.go +++ b/cli/command/context/use_test.go @@ -11,8 +11,8 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" - "github.com/docker/cli/cli/context/store" "github.com/docker/cli/cli/flags" + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/homedir" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -47,7 +47,7 @@ func TestUse(t *testing.T) { func TestUseNoExist(t *testing.T) { cli := makeFakeCli(t) err := newUseCommand(cli).RunE(nil, []string{"test"}) - assert.Check(t, store.IsErrContextDoesNotExist(err)) + assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) } // TestUseDefaultWithoutConfigFile verifies that the CLI does not create diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index a0dd1b18653b..3677f711e52c 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -1,11 +1,10 @@ package command import ( - "fmt" - "github.com/docker/cli/cli/context/docker" "github.com/docker/cli/cli/context/store" cliflags "github.com/docker/cli/cli/flags" + "github.com/docker/docker/errdefs" "github.com/pkg/errors" ) @@ -107,7 +106,7 @@ func (s *ContextStoreWithDefault) List() ([]store.Metadata, error) { // CreateOrUpdate is not allowed for the default context and fails func (s *ContextStoreWithDefault) CreateOrUpdate(meta store.Metadata) error { if meta.Name == DefaultContextName { - return errors.New("default context cannot be created nor updated") + return errdefs.InvalidParameter(errors.New("default context cannot be created nor updated")) } return s.Store.CreateOrUpdate(meta) } @@ -115,7 +114,7 @@ func (s *ContextStoreWithDefault) CreateOrUpdate(meta store.Metadata) error { // Remove is not allowed for the default context and fails func (s *ContextStoreWithDefault) Remove(name string) error { if name == DefaultContextName { - return errors.New("default context cannot be removed") + return errdefs.InvalidParameter(errors.New("default context cannot be removed")) } return s.Store.Remove(name) } @@ -135,7 +134,7 @@ func (s *ContextStoreWithDefault) GetMetadata(name string) (store.Metadata, erro // ResetTLSMaterial is not implemented for default context and fails func (s *ContextStoreWithDefault) ResetTLSMaterial(name string, data *store.ContextTLSData) error { if name == DefaultContextName { - return errors.New("The default context store does not support ResetTLSMaterial") + return errdefs.InvalidParameter(errors.New("default context cannot be edited")) } return s.Store.ResetTLSMaterial(name, data) } @@ -143,7 +142,7 @@ func (s *ContextStoreWithDefault) ResetTLSMaterial(name string, data *store.Cont // ResetEndpointTLSMaterial is not implemented for default context and fails func (s *ContextStoreWithDefault) ResetEndpointTLSMaterial(contextName string, endpointName string, data *store.EndpointTLSData) error { if contextName == DefaultContextName { - return errors.New("The default context store does not support ResetEndpointTLSMaterial") + return errdefs.InvalidParameter(errors.New("default context cannot be edited")) } return s.Store.ResetEndpointTLSMaterial(contextName, endpointName, data) } @@ -176,29 +175,13 @@ func (s *ContextStoreWithDefault) GetTLSData(contextName, endpointName, fileName return nil, err } if defaultContext.TLS.Endpoints[endpointName].Files[fileName] == nil { - return nil, &noDefaultTLSDataError{endpointName: endpointName, fileName: fileName} + return nil, errdefs.NotFound(errors.Errorf("TLS data for %s/%s/%s does not exist", DefaultContextName, endpointName, fileName)) } return defaultContext.TLS.Endpoints[endpointName].Files[fileName], nil - } return s.Store.GetTLSData(contextName, endpointName, fileName) } -type noDefaultTLSDataError struct { - endpointName string - fileName string -} - -func (e *noDefaultTLSDataError) Error() string { - return fmt.Sprintf("tls data for %s/%s/%s does not exist", DefaultContextName, e.endpointName, e.fileName) -} - -// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound -func (e *noDefaultTLSDataError) NotFound() {} - -// IsTLSDataDoesNotExist satisfies github.com/docker/cli/cli/context/store.tlsDataDoesNotExist -func (e *noDefaultTLSDataError) IsTLSDataDoesNotExist() {} - // GetStorageInfo implements store.Store's GetStorageInfo func (s *ContextStoreWithDefault) GetStorageInfo(contextName string) store.StorageInfo { if contextName == DefaultContextName { diff --git a/cli/command/defaultcontextstore_test.go b/cli/command/defaultcontextstore_test.go index 4b00b49f9ec6..a3b1bff5835a 100644 --- a/cli/command/defaultcontextstore_test.go +++ b/cli/command/defaultcontextstore_test.go @@ -8,8 +8,10 @@ import ( "github.com/docker/cli/cli/context/docker" "github.com/docker/cli/cli/context/store" cliflags "github.com/docker/cli/cli/flags" + "github.com/docker/docker/errdefs" "github.com/docker/go-connections/tlsconfig" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/golden" ) @@ -153,6 +155,7 @@ func TestErrCreateDefault(t *testing.T) { Metadata: testContext{Bar: "baz"}, Name: "default", }) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) assert.Error(t, err, "default context cannot be created nor updated") } @@ -160,6 +163,7 @@ func TestErrRemoveDefault(t *testing.T) { meta := testDefaultMetadata() s := testStore(t, meta, store.ContextTLSData{}) err := s.Remove("default") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) assert.Error(t, err, "default context cannot be removed") } @@ -167,5 +171,5 @@ func TestErrTLSDataError(t *testing.T) { meta := testDefaultMetadata() s := testStore(t, meta, store.ContextTLSData{}) _, err := s.GetTLSData("default", "noop", "noop") - assert.Check(t, store.IsErrTLSDataDoesNotExist(err)) + assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) } diff --git a/cli/context/store/metadata_test.go b/cli/context/store/metadata_test.go index b81f53b0f56f..fcb46c54f567 100644 --- a/cli/context/store/metadata_test.go +++ b/cli/context/store/metadata_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" ) @@ -22,7 +23,7 @@ func testMetadata(name string) Metadata { func TestMetadataGetNotExisting(t *testing.T) { testee := metadataStore{root: t.TempDir(), config: testCfg} _, err := testee.get("noexist") - assert.Assert(t, IsErrContextDoesNotExist(err)) + assert.ErrorType(t, err, errdefs.IsNotFound) } func TestMetadataCreateGetRemove(t *testing.T) { @@ -56,7 +57,7 @@ func TestMetadataCreateGetRemove(t *testing.T) { assert.NilError(t, testee.remove("test-context")) assert.NilError(t, testee.remove("test-context")) // support duplicate remove _, err = testee.get("test-context") - assert.Assert(t, IsErrContextDoesNotExist(err)) + assert.ErrorType(t, err, errdefs.IsNotFound) } func TestMetadataRespectJsonAnnotation(t *testing.T) { diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go index 64d516470399..42edc8a7861b 100644 --- a/cli/context/store/metadatastore.go +++ b/cli/context/store/metadatastore.go @@ -7,7 +7,9 @@ import ( "reflect" "sort" + "github.com/docker/docker/errdefs" "github.com/fvbommel/sortorder" + "github.com/pkg/errors" ) const ( @@ -55,13 +57,20 @@ func parseTypedOrMap(payload []byte, getter TypeGetter) (interface{}, error) { } func (s *metadataStore) get(name string) (Metadata, error) { - return s.getByID(contextdirOf(name)) + m, err := s.getByID(contextdirOf(name)) + if err != nil { + return m, errors.Wrapf(err, "load context %q", name) + } + return m, nil } func (s *metadataStore) getByID(id contextdir) (Metadata, error) { bytes, err := os.ReadFile(filepath.Join(s.contextDir(id), metaFile)) if err != nil { - return Metadata{}, convertContextDoesNotExist(err) + if errors.Is(err, os.ErrNotExist) { + return Metadata{}, errdefs.NotFound(errors.Wrap(err, "context does not exist")) + } + return Metadata{}, err } var untyped untypedContextMetadata r := Metadata{ @@ -83,8 +92,10 @@ func (s *metadataStore) getByID(id contextdir) (Metadata, error) { } func (s *metadataStore) remove(name string) error { - contextDir := s.contextDir(contextdirOf(name)) - return os.RemoveAll(contextDir) + if err := os.RemoveAll(s.contextDir(contextdirOf(name))); err != nil { + return errors.Wrapf(err, "failed to remove metadata") + } + return nil } func (s *metadataStore) list() ([]Metadata, error) { @@ -99,7 +110,7 @@ func (s *metadataStore) list() ([]Metadata, error) { for _, dir := range ctxDirs { c, err := s.getByID(contextdir(dir)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to read metadata") } res = append(res, c) } @@ -140,13 +151,6 @@ func listRecursivelyMetadataDirs(root string) ([]string, error) { return result, nil } -func convertContextDoesNotExist(err error) error { - if os.IsNotExist(err) { - return &contextDoesNotExistError{} - } - return err -} - type untypedContextMetadata struct { Metadata json.RawMessage `json:"metadata,omitempty"` Endpoints map[string]json.RawMessage `json:"endpoints,omitempty"` diff --git a/cli/context/store/store.go b/cli/context/store/store.go index f00599602c93..3378d7144424 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -7,7 +7,6 @@ import ( "bytes" _ "crypto/sha256" // ensure ids can be computed "encoding/json" - "fmt" "io" "net/http" "path" @@ -137,20 +136,21 @@ func (s *store) CreateOrUpdate(meta Metadata) error { func (s *store) Remove(name string) error { if err := s.meta.remove(name); err != nil { - return patchErrContextName(err, name) + return errors.Wrapf(err, "failed to remove context %s", name) } - return patchErrContextName(s.tls.removeAllContextData(name), name) + if err := s.tls.removeAllContextData(name); err != nil { + return errors.Wrapf(err, "failed to remove context %s", name) + } + return nil } func (s *store) GetMetadata(name string) (Metadata, error) { - res, err := s.meta.get(name) - patchErrContextName(err, name) - return res, err + return s.meta.get(name) } func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { if err := s.tls.removeAllContextData(name); err != nil { - return patchErrContextName(err, name) + return err } if data == nil { return nil @@ -158,7 +158,7 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { for ep, files := range data.Endpoints { for fileName, data := range files.Files { if err := s.tls.createOrUpdate(name, ep, fileName, data); err != nil { - return patchErrContextName(err, name) + return err } } } @@ -167,27 +167,25 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { if err := s.tls.removeAllEndpointData(contextName, endpointName); err != nil { - return patchErrContextName(err, contextName) + return err } if data == nil { return nil } for fileName, data := range data.Files { if err := s.tls.createOrUpdate(contextName, endpointName, fileName, data); err != nil { - return patchErrContextName(err, contextName) + return err } } return nil } func (s *store) ListTLSFiles(name string) (map[string]EndpointFiles, error) { - res, err := s.tls.listContextData(name) - return res, patchErrContextName(err, name) + return s.tls.listContextData(name) } func (s *store) GetTLSData(contextName, endpointName, fileName string) ([]byte, error) { - res, err := s.tls.getData(contextName, endpointName, fileName) - return res, patchErrContextName(err, contextName) + return s.tls.getData(contextName, endpointName, fileName) } func (s *store) GetStorageInfo(contextName string) StorageInfo { @@ -206,7 +204,7 @@ func ValidateContextName(name string) error { return errors.New(`"default" is a reserved context name`) } if !restrictedNameRegEx.MatchString(name) { - return fmt.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern) + return errors.Errorf("context name %q is invalid, names are validated against regexp %q", name, restrictedNamePattern) } return nil } @@ -480,58 +478,18 @@ func importEndpointTLS(tlsData *ContextTLSData, path string, data []byte) error return nil } -type setContextName interface { - setContext(name string) -} - -type contextDoesNotExistError struct { - name string -} - -func (e *contextDoesNotExistError) Error() string { - return fmt.Sprintf("context %q does not exist", e.name) -} - -func (e *contextDoesNotExistError) setContext(name string) { - e.name = name -} - -// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound -func (e *contextDoesNotExistError) NotFound() {} - -type tlsDataDoesNotExist interface { - errdefs.ErrNotFound - IsTLSDataDoesNotExist() -} - -type tlsDataDoesNotExistError struct { - context, endpoint, file string -} - -func (e *tlsDataDoesNotExistError) Error() string { - return fmt.Sprintf("tls data for %s/%s/%s does not exist", e.context, e.endpoint, e.file) -} - -func (e *tlsDataDoesNotExistError) setContext(name string) { - e.context = name -} - -// NotFound satisfies interface github.com/docker/docker/errdefs.ErrNotFound -func (e *tlsDataDoesNotExistError) NotFound() {} - -// IsTLSDataDoesNotExist satisfies tlsDataDoesNotExist -func (e *tlsDataDoesNotExistError) IsTLSDataDoesNotExist() {} - -// IsErrContextDoesNotExist checks if the given error is a "context does not exist" condition +// IsErrContextDoesNotExist checks if the given error is a "context does not exist" condition. +// +// Deprecated: use github.com/docker/docker/errdefs.IsNotFound() func IsErrContextDoesNotExist(err error) bool { - _, ok := err.(*contextDoesNotExistError) - return ok + return errdefs.IsNotFound(err) } // IsErrTLSDataDoesNotExist checks if the given error is a "context does not exist" condition +// +// Deprecated: use github.com/docker/docker/errdefs.IsNotFound() func IsErrTLSDataDoesNotExist(err error) bool { - _, ok := err.(tlsDataDoesNotExist) - return ok + return errdefs.IsNotFound(err) } type contextdir string @@ -539,10 +497,3 @@ type contextdir string func contextdirOf(name string) contextdir { return contextdir(digest.FromString(name).Encoded()) } - -func patchErrContextName(err error, name string) error { - if typed, ok := err.(setContextName); ok { - typed.setContext(name) - } - return err -} diff --git a/cli/context/store/store_test.go b/cli/context/store/store_test.go index 3c9ddd6b7a06..310065dc6418 100644 --- a/cli/context/store/store_test.go +++ b/cli/context/store/store_test.go @@ -12,7 +12,9 @@ import ( "path" "testing" + "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) type endpoint struct { @@ -100,7 +102,7 @@ func TestRemove(t *testing.T) { })) assert.NilError(t, s.Remove("source")) _, err = s.GetMetadata("source") - assert.Check(t, IsErrContextDoesNotExist(err)) + assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) f, err := s.ListTLSFiles("source") assert.NilError(t, err) assert.Equal(t, 0, len(f)) @@ -115,7 +117,7 @@ func TestListEmptyStore(t *testing.T) { func TestErrHasCorrectContext(t *testing.T) { _, err := New(t.TempDir(), testCfg).GetMetadata("no-exists") assert.ErrorContains(t, err, "no-exists") - assert.Check(t, IsErrContextDoesNotExist(err)) + assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) } func TestDetectImportContentType(t *testing.T) { @@ -173,7 +175,7 @@ func TestImportZip(t *testing.T) { Name: "source", }) assert.NilError(t, err) - var files = []struct { + files := []struct { Name, Body string }{ {"meta.json", string(meta)}, diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 0c2fc4120670..9992481405fe 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -3,6 +3,9 @@ package store import ( "os" "path/filepath" + + "github.com/docker/docker/errdefs" + "github.com/pkg/errors" ) const tlsDir = "tls" @@ -34,7 +37,10 @@ func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []by func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) { data, err := os.ReadFile(filepath.Join(s.endpointDir(name, endpointName), filename)) if err != nil { - return nil, convertTLSDataDoesNotExist(endpointName, filename, err) + if os.IsNotExist(err) { + return nil, errdefs.NotFound(errors.Errorf("TLS data for %s/%s/%s does not exist", name, endpointName, filename)) + } + return nil, errors.Wrapf(err, "failed to read TLS data for endpoint %s", endpointName) } return data, nil } @@ -50,11 +56,17 @@ func (s *tlsStore) remove(name, endpointName, filename string) error { } func (s *tlsStore) removeAllEndpointData(name, endpointName string) error { - return os.RemoveAll(s.endpointDir(name, endpointName)) + if err := os.RemoveAll(s.endpointDir(name, endpointName)); err != nil { + return errors.Wrapf(err, "failed to remove TLS data for endpoint %s", endpointName) + } + return nil } func (s *tlsStore) removeAllContextData(name string) error { - return os.RemoveAll(s.contextDir(name)) + if err := os.RemoveAll(s.contextDir(name)); err != nil { + return errors.Wrapf(err, "failed to remove TLS data") + } + return nil } func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error) { @@ -64,14 +76,14 @@ func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error if os.IsNotExist(err) { return map[string]EndpointFiles{}, nil } - return nil, err + return nil, errors.Wrapf(err, "failed to list TLS files for context %s", name) } r := make(map[string]EndpointFiles) for _, epFS := range epFSs { if epFS.IsDir() { fss, err := os.ReadDir(filepath.Join(contextDir, epFS.Name())) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to list TLS files for endpoint %s", epFS.Name()) } var files EndpointFiles for _, fs := range fss { @@ -87,10 +99,3 @@ func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error // EndpointFiles is a slice of strings representing file names type EndpointFiles []string - -func convertTLSDataDoesNotExist(endpoint, file string, err error) error { - if os.IsNotExist(err) { - return &tlsDataDoesNotExistError{endpoint: endpoint, file: file} - } - return err -} diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index 9c4c79ebdab8..e0585bda5fe6 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -3,6 +3,7 @@ package store import ( "testing" + "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" ) @@ -12,7 +13,7 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { const contextName = "test-ctx" _, err := testee.getData(contextName, "test-ep", "test-data") - assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) + assert.ErrorType(t, err, errdefs.IsNotFound) err = testee.createOrUpdate(contextName, "test-ep", "test-data", []byte("data")) assert.NilError(t, err) @@ -31,7 +32,7 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { assert.NilError(t, err) _, err = testee.getData(contextName, "test-ep", "test-data") - assert.Equal(t, true, IsErrTLSDataDoesNotExist(err)) + assert.ErrorType(t, err, errdefs.IsNotFound) } func TestTlsListAndBatchRemove(t *testing.T) { diff --git a/cli/context/tlsdata.go b/cli/context/tlsdata.go index f8459fd40648..c758612a1dc0 100644 --- a/cli/context/tlsdata.go +++ b/cli/context/tlsdata.go @@ -45,14 +45,14 @@ func (data *TLSData) ToStoreTLSData() *store.EndpointTLSData { func LoadTLSData(s store.Reader, contextName, endpointName string) (*TLSData, error) { tlsFiles, err := s.ListTLSFiles(contextName) if err != nil { - return nil, errors.Wrapf(err, "failed to retrieve context tls files for context %q", contextName) + return nil, errors.Wrapf(err, "failed to retrieve TLS files for context %q", contextName) } if epTLSFiles, ok := tlsFiles[endpointName]; ok { var tlsData TLSData for _, f := range epTLSFiles { data, err := s.GetTLSData(contextName, endpointName, f) if err != nil { - return nil, errors.Wrapf(err, "failed to retrieve context tls data for file %q of context %q", f, contextName) + return nil, errors.Wrapf(err, "failed to retrieve TLS data (%s) for context %q", f, contextName) } switch f { case caKey: @@ -62,7 +62,7 @@ func LoadTLSData(s store.Reader, contextName, endpointName string) (*TLSData, er case keyKey: tlsData.Key = data default: - logrus.Warnf("unknown file %s in context %s tls bundle", f, contextName) + logrus.Warnf("unknown file in context %s TLS bundle: %s", contextName, f) } } return &tlsData, nil From 951bb481c055b6678bcbab42101c778b1e1a61a2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 23:30:04 +0200 Subject: [PATCH 12/14] cli/context/store: New(): return concrete type Go conventions are for interfaces to be defined on the receiver side, and for producers to return concrete types. This patch changes the constructor to return a concrete type. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 40 +++++++++++++++++++-------- cli/context/store/storeconfig.go | 2 +- cli/context/store/storeconfig_test.go | 10 ++++--- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 3378d7144424..8ec3c6ff875a 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -93,11 +93,11 @@ type ContextTLSData struct { // New creates a store from a given directory. // If the directory does not exist or is empty, initialize it -func New(dir string, cfg Config) Store { +func New(dir string, cfg Config) *ContextStore { metaRoot := filepath.Join(dir, metadataDir) tlsRoot := filepath.Join(dir, tlsDir) - return &store{ + return &ContextStore{ meta: &metadataStore{ root: metaRoot, config: cfg, @@ -108,12 +108,14 @@ func New(dir string, cfg Config) Store { } } -type store struct { +// ContextStore implements Store. +type ContextStore struct { meta *metadataStore tls *tlsStore } -func (s *store) List() ([]Metadata, error) { +// List return all contexts. +func (s *ContextStore) List() ([]Metadata, error) { return s.meta.list() } @@ -130,11 +132,13 @@ func Names(s Lister) ([]string, error) { return names, nil } -func (s *store) CreateOrUpdate(meta Metadata) error { +// CreateOrUpdate creates or updates metadata for the context. +func (s *ContextStore) CreateOrUpdate(meta Metadata) error { return s.meta.createOrUpdate(meta) } -func (s *store) Remove(name string) error { +// Remove deletes the context with the given name, if found. +func (s *ContextStore) Remove(name string) error { if err := s.meta.remove(name); err != nil { return errors.Wrapf(err, "failed to remove context %s", name) } @@ -144,11 +148,15 @@ func (s *store) Remove(name string) error { return nil } -func (s *store) GetMetadata(name string) (Metadata, error) { +// GetMetadata returns the metadata for the context with the given name. +// It returns an errdefs.ErrNotFound if the context was not found. +func (s *ContextStore) GetMetadata(name string) (Metadata, error) { return s.meta.get(name) } -func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { +// ResetTLSMaterial removes TLS data for all endpoints in the context and replaces +// it with the new data. +func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) error { if err := s.tls.removeAllContextData(name); err != nil { return err } @@ -165,7 +173,9 @@ func (s *store) ResetTLSMaterial(name string, data *ContextTLSData) error { return nil } -func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { +// ResetEndpointTLSMaterial removes TLS data for the given context and endpoint, +// and replaces it with the new data. +func (s *ContextStore) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { if err := s.tls.removeAllEndpointData(contextName, endpointName); err != nil { return err } @@ -180,15 +190,21 @@ func (s *store) ResetEndpointTLSMaterial(contextName string, endpointName string return nil } -func (s *store) ListTLSFiles(name string) (map[string]EndpointFiles, error) { +// ListTLSFiles returns the list of TLS files present for each endpoint in the +// context. +func (s *ContextStore) ListTLSFiles(name string) (map[string]EndpointFiles, error) { return s.tls.listContextData(name) } -func (s *store) GetTLSData(contextName, endpointName, fileName string) ([]byte, error) { +// GetTLSData reads, and returns the content of the given fileName for an endpoint. +// It returns an errdefs.ErrNotFound if the file was not found. +func (s *ContextStore) GetTLSData(contextName, endpointName, fileName string) ([]byte, error) { return s.tls.getData(contextName, endpointName, fileName) } -func (s *store) GetStorageInfo(contextName string) StorageInfo { +// GetStorageInfo returns the paths where the Metadata and TLS data are stored +// for the context. +func (s *ContextStore) GetStorageInfo(contextName string) StorageInfo { return StorageInfo{ MetadataPath: s.meta.contextDir(contextdirOf(contextName)), TLSPath: s.tls.contextDir(contextName), diff --git a/cli/context/store/storeconfig.go b/cli/context/store/storeconfig.go index 2a4bc57c1ce8..9c93ecbab2b1 100644 --- a/cli/context/store/storeconfig.go +++ b/cli/context/store/storeconfig.go @@ -19,7 +19,7 @@ func EndpointTypeGetter(name string, getter TypeGetter) NamedTypeGetter { } } -// Config is used to configure the metadata marshaler of the context store +// Config is used to configure the metadata marshaler of the context ContextStore type Config struct { contextType TypeGetter endpointTypes map[string]TypeGetter diff --git a/cli/context/store/storeconfig_test.go b/cli/context/store/storeconfig_test.go index 353d093650dd..e5b8c75686e5 100644 --- a/cli/context/store/storeconfig_test.go +++ b/cli/context/store/storeconfig_test.go @@ -6,10 +6,12 @@ import ( "gotest.tools/v3/assert" ) -type testCtx struct{} -type testEP1 struct{} -type testEP2 struct{} -type testEP3 struct{} +type ( + testCtx struct{} + testEP1 struct{} + testEP2 struct{} + testEP3 struct{} +) func TestConfigModification(t *testing.T) { cfg := NewConfig(func() interface{} { return &testCtx{} }, EndpointTypeGetter("ep1", func() interface{} { return &testEP1{} })) From 09c94c1c21cb2ed02d347934de85b6163dc62ddf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Sep 2022 22:21:31 +0200 Subject: [PATCH 13/14] cli/context/store: List(): don't interrupt listing for not-found errors There's no reason to stop listing contexts if a context does not exist while iterating over the directories, Signed-off-by: Sebastiaan van Stijn --- cli/context/store/metadatastore.go | 3 +++ cli/context/store/tlsstore.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go index 42edc8a7861b..5222897f3995 100644 --- a/cli/context/store/metadatastore.go +++ b/cli/context/store/metadatastore.go @@ -110,6 +110,9 @@ func (s *metadataStore) list() ([]Metadata, error) { for _, dir := range ctxDirs { c, err := s.getByID(contextdir(dir)) if err != nil { + if os.IsNotExist(err) { + continue + } return nil, errors.Wrap(err, "failed to read metadata") } res = append(res, c) diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index 9992481405fe..dbd1649b99a0 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -82,6 +82,9 @@ func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error for _, epFS := range epFSs { if epFS.IsDir() { fss, err := os.ReadDir(filepath.Join(contextDir, epFS.Name())) + if os.IsNotExist(err) { + continue + } if err != nil { return nil, errors.Wrapf(err, "failed to list TLS files for endpoint %s", epFS.Name()) } From cd7c493ea2cfb8c6db0beb65cf9830c8df23a9f9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 29 Sep 2022 14:55:29 +0200 Subject: [PATCH 14/14] cli/context/store: rename removeAllContextData(), removeAllEndpointData() The existing `remove()` was unused, and using that as name makes it more consistent with the metadata-store. Also renaming `removeAllEndpointData` to just `removeEndpoint`, as it's part of the TLS-store, which should already make it clear it's about (TLS)data. Signed-off-by: Sebastiaan van Stijn --- cli/context/store/store.go | 6 +++--- cli/context/store/tlsstore.go | 21 ++++++--------------- cli/context/store/tlsstore_test.go | 9 +++------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 8ec3c6ff875a..6042c2af7dba 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -142,7 +142,7 @@ func (s *ContextStore) Remove(name string) error { if err := s.meta.remove(name); err != nil { return errors.Wrapf(err, "failed to remove context %s", name) } - if err := s.tls.removeAllContextData(name); err != nil { + if err := s.tls.remove(name); err != nil { return errors.Wrapf(err, "failed to remove context %s", name) } return nil @@ -157,7 +157,7 @@ func (s *ContextStore) GetMetadata(name string) (Metadata, error) { // ResetTLSMaterial removes TLS data for all endpoints in the context and replaces // it with the new data. func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) error { - if err := s.tls.removeAllContextData(name); err != nil { + if err := s.tls.remove(name); err != nil { return err } if data == nil { @@ -176,7 +176,7 @@ func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) error // ResetEndpointTLSMaterial removes TLS data for the given context and endpoint, // and replaces it with the new data. func (s *ContextStore) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error { - if err := s.tls.removeAllEndpointData(contextName, endpointName); err != nil { + if err := s.tls.removeEndpoint(contextName, endpointName); err != nil { return err } if data == nil { diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go index dbd1649b99a0..ec46e7e58a06 100644 --- a/cli/context/store/tlsstore.go +++ b/cli/context/store/tlsstore.go @@ -45,30 +45,21 @@ func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) return data, nil } -// remove removes a TLS data from an endpoint -// TODO(thaJeztah) tlsStore.remove() is not used anywhere outside of tests; should we use removeAllEndpointData() only? -func (s *tlsStore) remove(name, endpointName, filename string) error { - err := os.Remove(filepath.Join(s.endpointDir(name, endpointName), filename)) - if os.IsNotExist(err) { - return nil +// remove deletes all TLS data for the given context. +func (s *tlsStore) remove(name string) error { + if err := os.RemoveAll(s.contextDir(name)); err != nil { + return errors.Wrapf(err, "failed to remove TLS data") } - return err + return nil } -func (s *tlsStore) removeAllEndpointData(name, endpointName string) error { +func (s *tlsStore) removeEndpoint(name, endpointName string) error { if err := os.RemoveAll(s.endpointDir(name, endpointName)); err != nil { return errors.Wrapf(err, "failed to remove TLS data for endpoint %s", endpointName) } return nil } -func (s *tlsStore) removeAllContextData(name string) error { - if err := os.RemoveAll(s.contextDir(name)); err != nil { - return errors.Wrapf(err, "failed to remove TLS data") - } - return nil -} - func (s *tlsStore) listContextData(name string) (map[string]EndpointFiles, error) { contextDir := s.contextDir(name) epFSs, err := os.ReadDir(contextDir) diff --git a/cli/context/store/tlsstore_test.go b/cli/context/store/tlsstore_test.go index e0585bda5fe6..511ba1d2f330 100644 --- a/cli/context/store/tlsstore_test.go +++ b/cli/context/store/tlsstore_test.go @@ -26,11 +26,8 @@ func TestTlsCreateUpdateGetRemove(t *testing.T) { assert.NilError(t, err) assert.Equal(t, string(data), "data2") - err = testee.remove(contextName, "test-ep", "test-data") + err = testee.removeEndpoint(contextName, "test-ep") assert.NilError(t, err) - err = testee.remove(contextName, "test-ep", "test-data") - assert.NilError(t, err) - _, err = testee.getData(contextName, "test-ep", "test-data") assert.ErrorType(t, err, errdefs.IsNotFound) } @@ -61,13 +58,13 @@ func TestTlsListAndBatchRemove(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, resAll, all) - err = testee.removeAllEndpointData(contextName, "ep3") + err = testee.removeEndpoint(contextName, "ep3") assert.NilError(t, err) resEp1ep2, err := testee.listContextData(contextName) assert.NilError(t, err) assert.DeepEqual(t, resEp1ep2, ep1ep2) - err = testee.removeAllContextData(contextName) + err = testee.remove(contextName) assert.NilError(t, err) resEmpty, err := testee.listContextData(contextName) assert.NilError(t, err)