Skip to content

Commit b1e4687

Browse files
author
Patrick Zheng
authored
fix: remove empty trust store directory after notation cert delete and notation cert cleanup-test (#1243)
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
1 parent 4b873a7 commit b1e4687

5 files changed

Lines changed: 171 additions & 3 deletions

File tree

‎cmd/notation/cert/delete.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ func deleteCerts(opts *certDeleteOpts) error {
9696
}
9797
return nil
9898
}
99+
99100
// Delete a certain certificate with path storeType/namedStore/cert
100101
cert := opts.cert
101102
if cert == "" {

‎cmd/notation/internal/truststore/truststore.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ func AddCert(path, storeType, namedStore string, display bool) error {
7171
if _, err := os.Stat(filepath.Join(trustStorePath, filepath.Base(certPath))); err == nil {
7272
return errors.New("certificate already exists in the Trust Store")
7373
}
74+
7475
// add cert to trust store
7576
_, err = osutil.CopyToDir(certPath, trustStorePath)
7677
if err != nil {
@@ -81,7 +82,6 @@ func AddCert(path, storeType, namedStore string, display bool) error {
8182
if display {
8283
fmt.Printf("Successfully added %s to named store %s of type %s\n", filepath.Base(certPath), namedStore, storeType)
8384
}
84-
8585
return nil
8686
}
8787

@@ -114,7 +114,6 @@ func ListCerts(root string, depth int) ([]string, error) {
114114
}); err != nil {
115115
return nil, err
116116
}
117-
118117
return certPaths, nil
119118
}
120119

@@ -163,6 +162,7 @@ func DeleteAllCerts(storeType, namedStore string, confirmed bool) error {
163162
if err = os.RemoveAll(path); err != nil {
164163
return err
165164
}
165+
166166
// write out on success
167167
fmt.Printf("Successfully deleted %s\n", path)
168168
return nil
@@ -189,6 +189,20 @@ func DeleteCert(storeType, namedStore, cert string, confirmed bool) error {
189189
if err = os.Remove(path); err != nil {
190190
return err
191191
}
192+
193+
// remove the trust store directory if empty
194+
storePath := filepath.Dir(path)
195+
empty, err := osutil.IsDirEmpty(storePath)
196+
if err != nil {
197+
// in this case, empty is always false
198+
fmt.Fprintf(os.Stderr, "Warning: failed to check if the trust store directory %s is empty: %v\n", storePath, err)
199+
}
200+
if empty {
201+
if err := os.Remove(storePath); err != nil {
202+
fmt.Fprintf(os.Stderr, "Warning: failed to remove the empty trust store directory %s: %v\n", storePath, err)
203+
}
204+
}
205+
192206
// write out on success
193207
fmt.Printf("Successfully deleted %s from trust store %s of type %s\n", cert, namedStore, storeType)
194208
return nil

‎internal/osutil/file.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package osutil
1616
import (
1717
"crypto/sha256"
1818
"encoding/hex"
19+
"errors"
1920
"fmt"
2021
"io"
2122
"io/fs"
@@ -154,3 +155,19 @@ func ValidateSHA256Sum(path string, checksum string) error {
154155
}
155156
return nil
156157
}
158+
159+
// IsDirEmpty checks if directory dir is empty.
160+
// It returns true and nil error if and only if dir is empty.
161+
func IsDirEmpty(dir string) (bool, error) {
162+
d, err := os.Open(dir)
163+
if err != nil {
164+
return false, err
165+
}
166+
defer d.Close()
167+
168+
_, err = d.Readdirnames(1)
169+
if errors.Is(err, io.EOF) {
170+
return true, nil
171+
}
172+
return false, err
173+
}

‎internal/osutil/file_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"os"
1919
"path/filepath"
2020
"runtime"
21+
"strings"
2122
"testing"
2223
)
2324

@@ -269,3 +270,41 @@ func TestValidateChecksum(t *testing.T) {
269270
t.Fatalf("expected nil err, but got %v", err)
270271
}
271272
}
273+
274+
func TestIsDirEmpty(t *testing.T) {
275+
t.Run("empty dir", func(t *testing.T) {
276+
tempDir := t.TempDir()
277+
isEmpty, err := IsDirEmpty(tempDir)
278+
if err != nil {
279+
t.Fatalf("expected nil error, but got %s", err)
280+
}
281+
if !isEmpty {
282+
t.Fatal("should be empty")
283+
}
284+
})
285+
286+
t.Run("dir does not exist", func(t *testing.T) {
287+
nonExistDir := "invalid"
288+
_, err := IsDirEmpty(nonExistDir)
289+
if err == nil {
290+
t.Fatalf("expected error, but got nil")
291+
}
292+
})
293+
294+
t.Run("failed to read content of dir", func(t *testing.T) {
295+
if runtime.GOOS == "windows" {
296+
t.Skip("skipping test on Windows")
297+
}
298+
tempDir := t.TempDir()
299+
if err := os.Chmod(tempDir, 0000); err != nil {
300+
t.Fatal(err)
301+
}
302+
defer os.Chmod(tempDir, 0700)
303+
304+
expectedErrorMsg := "permission denied"
305+
_, err := IsDirEmpty(tempDir)
306+
if err == nil || !strings.Contains(err.Error(), expectedErrorMsg) {
307+
t.Fatalf("expected permission denied, but got %s", err)
308+
}
309+
})
310+
}

‎test/e2e/suite/command/cert.go‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,42 @@ var _ = Describe("notation cert", func() {
4040
MatchKeyWords(
4141
"Successfully deleted",
4242
)
43+
44+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e")
45+
if _, err := os.Stat(trustStorePath); err == nil {
46+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
47+
}
4348
})
4449
})
4550

46-
It("delete a specfic cert", func() {
51+
It("delete a specific cert and the empty trust store directory", func() {
4752
Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
4853
notation.Exec("cert", "delete", "--type", "ca", "--store", "e2e", "e2e.crt", "-y").
4954
MatchKeyWords(
5055
"Successfully deleted e2e.crt from trust store e2e of type ca",
5156
)
57+
58+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e")
59+
if _, err := os.Stat(trustStorePath); err == nil {
60+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
61+
}
62+
})
63+
})
64+
65+
It("delete a specific cert from trust store containing more than one certificates", func() {
66+
Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
67+
notation.Exec("cert", "add", "--type", "ca", "--store", "e2e", filepath.Join(NotationE2ELocalKeysDir, "expired_e2e.crt")).
68+
MatchKeyWords("Successfully added following certificates")
69+
70+
notation.Exec("cert", "delete", "--type", "ca", "--store", "e2e", "expired_e2e.crt", "-y").
71+
MatchKeyWords(
72+
"Successfully deleted expired_e2e.crt from trust store e2e of type ca",
73+
)
74+
75+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e")
76+
if _, err := os.Stat(trustStorePath); err != nil {
77+
Fail(fmt.Sprintf("trust store directory %s should still exist", trustStorePath))
78+
}
5279
})
5380
})
5481

@@ -58,6 +85,32 @@ var _ = Describe("notation cert", func() {
5885
MatchErrKeyWords(
5986
"failed to delete the certificate file",
6087
)
88+
89+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e")
90+
if _, err := os.Stat(trustStorePath); err != nil {
91+
Fail(fmt.Sprintf("trust store directory %s should still exist", trustStorePath))
92+
}
93+
})
94+
})
95+
96+
It("delete a specific cert but failed to delete the empty trust store directory", func() {
97+
Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
98+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e")
99+
100+
// Remove read permission for trustStorePath
101+
if err := os.Chmod(trustStorePath, 0300); err != nil {
102+
Fail(err.Error())
103+
}
104+
defer os.Chmod(trustStorePath, 0700)
105+
106+
notation.Exec("cert", "delete", "--type", "ca", "--store", "e2e", "e2e.crt", "-y").
107+
MatchKeyWords(
108+
"Successfully deleted e2e.crt from trust store e2e of type ca",
109+
).
110+
MatchErrKeyWords(
111+
fmt.Sprintf("Warning: failed to check if the trust store directory %s is empty", trustStorePath),
112+
"permission denied",
113+
)
61114
})
62115
})
63116

@@ -129,6 +182,11 @@ var _ = Describe("notation cert", func() {
129182
"Successfully deleted certificate file:", "e2e-test.crt",
130183
"Cleanup completed successfully",
131184
)
185+
186+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
187+
if _, err := os.Stat(trustStorePath); err == nil {
188+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
189+
}
132190
})
133191
})
134192

@@ -153,6 +211,11 @@ var _ = Describe("notation cert", func() {
153211
"Successfully deleted certificate file:", "e2e-test.crt",
154212
"Cleanup completed successfully",
155213
)
214+
215+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
216+
if _, err := os.Stat(trustStorePath); err == nil {
217+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
218+
}
156219
})
157220
})
158221

@@ -195,6 +258,11 @@ var _ = Describe("notation cert", func() {
195258
"Successfully deleted certificate file:", "e2e-test.crt",
196259
"Cleanup completed successfully",
197260
)
261+
262+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
263+
if _, err := os.Stat(trustStorePath); err == nil {
264+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
265+
}
198266
})
199267
})
200268

@@ -221,6 +289,10 @@ var _ = Describe("notation cert", func() {
221289
MatchKeyWords(
222290
"Successfully deleted e2e-test.crt from trust store e2e-test of type ca",
223291
)
292+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
293+
if _, err := os.Stat(trustStorePath); err == nil {
294+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
295+
}
224296

225297
notation.Exec("cert", "cleanup-test", "e2e-test", "-y").
226298
MatchKeyWords(
@@ -249,6 +321,11 @@ var _ = Describe("notation cert", func() {
249321
"Successfully deleted certificate file:", "e2e-test.crt",
250322
"Cleanup completed successfully",
251323
)
324+
325+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
326+
if _, err := os.Stat(trustStorePath); err == nil {
327+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
328+
}
252329
})
253330
})
254331

@@ -268,6 +345,11 @@ var _ = Describe("notation cert", func() {
268345
fmt.Sprintf("Certificate file %s does not exist", localCertPath),
269346
"Cleanup completed successfully",
270347
)
348+
349+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
350+
if _, err := os.Stat(trustStorePath); err == nil {
351+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
352+
}
271353
})
272354
})
273355

@@ -302,6 +384,11 @@ var _ = Describe("notation cert", func() {
302384
"failed to delete certificate e2e-test.crt from trust store e2e-test of type ca",
303385
"permission denied",
304386
)
387+
388+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
389+
if _, err := os.Stat(trustStorePath); err != nil {
390+
Fail(fmt.Sprintf("trust store directory %s should still exist", trustStorePath))
391+
}
305392
})
306393
})
307394

@@ -318,6 +405,11 @@ var _ = Describe("notation cert", func() {
318405
"failed to remove key e2e-test from the key list",
319406
"permission denied",
320407
)
408+
409+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
410+
if _, err := os.Stat(trustStorePath); err == nil {
411+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
412+
}
321413
})
322414
})
323415

@@ -334,6 +426,11 @@ var _ = Describe("notation cert", func() {
334426
fmt.Sprintf("failed to delete key file %s", filepath.Join(localKeysDir, "e2e-test.key")),
335427
"permission denied",
336428
)
429+
430+
trustStorePath := vhost.AbsolutePath(NotationDirName, TrustStoreDirName, "x509", TrustStoreTypeCA, "e2e-test")
431+
if _, err := os.Stat(trustStorePath); err == nil {
432+
Fail(fmt.Sprintf("empty trust store directory %s should be deleted", trustStorePath))
433+
}
337434
})
338435
})
339436
})

0 commit comments

Comments
 (0)