Skip to content

Commit a1437f2

Browse files
committed
fix: update to add cache error test case
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
1 parent 0e2495e commit a1437f2

11 files changed

Lines changed: 265 additions & 12 deletions

File tree

.github/workflows/build.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ jobs:
3636
go-version: ${{ matrix.go-version }}
3737
check-latest: true
3838
- name: Check out code
39-
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
39+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
4040
- name: Cache Go modules
41-
uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2
41+
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
4242
id: go-mod-cache
4343
with:
4444
path: ~/go/pkg/mod

.github/workflows/codeql.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jobs:
4242
fail-fast: false
4343
steps:
4444
- name: Checkout repository
45-
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
45+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
4646
- name: Set up Go ${{ matrix.go-version }} environment
4747
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2
4848
with:

.github/workflows/release-github.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ jobs:
3838
go-version: ${{ matrix.go-version }}
3939
check-latest: true
4040
- name: Checkout
41-
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
41+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
4242
with:
4343
fetch-depth: 0
4444
- name: Set GoReleaser Previous Tag To Be Last Non Weekly Release

.github/workflows/scorecard.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ jobs:
4141

4242
steps:
4343
- name: "Checkout code"
44-
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # tag=4.1.7
44+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=4.2.2
4545
with:
4646
persist-credentials: false
4747

@@ -54,7 +54,7 @@ jobs:
5454
publish_results: true
5555

5656
- name: "Upload artifact"
57-
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # tag=v4.4.0
57+
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # tag=v4.4.3
5858
with:
5959
name: SARIF file
6060
path: results.sarif

cmd/notation/verify.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"github.com/spf13/cobra"
4040

4141
corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
42+
clicrl "github.com/notaryproject/notation/internal/crl"
4243
)
4344

4445
type verifyOpts struct {
@@ -238,15 +239,19 @@ func getVerifier(ctx context.Context) (notation.Verifier, error) {
238239
if err != nil {
239240
return nil, err
240241
}
242+
crlFetcher.DiscardCacheError = true // discard crl cache error
241243
cacheRoot, err := dir.CacheFS().SysPath(dir.PathCRLCache)
242244
if err != nil {
243245
return nil, err
244246
}
245-
crlFetcher.Cache, err = crl.NewFileCache(cacheRoot)
247+
fileCache, err := crl.NewFileCache(cacheRoot)
246248
if err != nil {
247249
return nil, err
248250
}
249-
crlFetcher.DiscardCacheError = true // discard cache error
251+
crlFetcher.Cache = &clicrl.CacheWithLog{
252+
Cache: fileCache,
253+
DiscardCacheError: crlFetcher.DiscardCacheError,
254+
}
250255
revocationCodeSigningValidator, err := revocation.NewWithOptions(revocation.Options{
251256
OCSPHTTPClient: ocspHttpClient,
252257
CRLFetcher: crlFetcher,

internal/crl/crl.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright The Notary Project Authors.
2+
// Licensed under the Apache License, Version 2.0 (the "License");
3+
// you may not use this file except in compliance with the License.
4+
// You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
14+
package crl
15+
16+
import (
17+
"context"
18+
"errors"
19+
"fmt"
20+
"os"
21+
"sync"
22+
23+
corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
24+
"github.com/notaryproject/notation-go/log"
25+
)
26+
27+
// CacheWithLog implements corecrl.Cache with logging
28+
type CacheWithLog struct {
29+
corecrl.Cache
30+
31+
//DiscardCacheError is set to true to enable logging the discard cache error
32+
//warning
33+
DiscardCacheError bool
34+
35+
// logDiscardCrlCacheErrorOnce guarantees the discard cache error
36+
// warning is logged only once
37+
logDiscardCrlCacheErrorOnce sync.Once
38+
}
39+
40+
// Get retrieves the CRL bundle with the given url
41+
func (c *CacheWithLog) Get(ctx context.Context, url string) (*corecrl.Bundle, error) {
42+
if c.Cache == nil {
43+
return nil, errors.New("cache cannot be nil")
44+
}
45+
logger := log.GetLogger(ctx)
46+
47+
bundle, err := c.Cache.Get(ctx, url)
48+
if err != nil && !errors.Is(err, corecrl.ErrCacheMiss) {
49+
if c.DiscardCacheError {
50+
c.logDiscardCrlCacheErrorOnce.Do(c.logDiscardCrlCacheError)
51+
}
52+
logger.Debug(err.Error())
53+
return nil, err
54+
}
55+
return bundle, err
56+
}
57+
58+
// Set stores the CRL bundle with the given url
59+
func (c *CacheWithLog) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error {
60+
if c.Cache == nil {
61+
return errors.New("cache cannot be nil")
62+
}
63+
logger := log.GetLogger(ctx)
64+
65+
err := c.Cache.Set(ctx, url, bundle)
66+
if err != nil {
67+
if c.DiscardCacheError {
68+
c.logDiscardCrlCacheErrorOnce.Do(c.logDiscardCrlCacheError)
69+
}
70+
logger.Debug(err.Error())
71+
return err
72+
}
73+
return nil
74+
}
75+
76+
// logDiscardCrlCacheError logs the warning when CRL cache error is
77+
// discarded
78+
func (c *CacheWithLog) logDiscardCrlCacheError() {
79+
fmt.Fprintln(os.Stderr, "Warning: CRL cache error discarded. Enable debug log through '-d' for error details.")
80+
}

internal/crl/crl_test.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Copyright The Notary Project Authors.
2+
// Licensed under the Apache License, Version 2.0 (the "License");
3+
// you may not use this file except in compliance with the License.
4+
// You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
14+
package crl
15+
16+
import (
17+
"context"
18+
"errors"
19+
"os"
20+
"sync"
21+
"testing"
22+
23+
corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
24+
)
25+
26+
func TestGet(t *testing.T) {
27+
cache := &CacheWithLog{}
28+
expectedErrMsg := "cache cannot be nil"
29+
_, err := cache.Get(context.Background(), "")
30+
if err == nil || err.Error() != expectedErrMsg {
31+
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
32+
}
33+
34+
cache = &CacheWithLog{
35+
Cache: &dummyCache{},
36+
}
37+
expectedErrMsg = "cache get failed"
38+
_, err = cache.Get(context.Background(), "")
39+
if err == nil || err.Error() != expectedErrMsg {
40+
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
41+
}
42+
43+
cache = &CacheWithLog{
44+
Cache: &dummyCache{
45+
cacheMiss: true,
46+
},
47+
}
48+
_, err = cache.Get(context.Background(), "")
49+
if err == nil || !errors.Is(err, corecrl.ErrCacheMiss) {
50+
t.Fatalf("expected error %q, but got %q", corecrl.ErrCacheMiss, err)
51+
}
52+
}
53+
54+
func TestSet(t *testing.T) {
55+
cache := &CacheWithLog{}
56+
expectedErrMsg := "cache cannot be nil"
57+
err := cache.Set(context.Background(), "", nil)
58+
if err == nil || err.Error() != expectedErrMsg {
59+
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
60+
}
61+
62+
cache = &CacheWithLog{
63+
Cache: &dummyCache{},
64+
}
65+
expectedErrMsg = "cache set failed"
66+
err = cache.Set(context.Background(), "", nil)
67+
if err == nil || err.Error() != expectedErrMsg {
68+
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
69+
}
70+
71+
cache = &CacheWithLog{
72+
Cache: &dummyCache{
73+
setSuccess: true,
74+
},
75+
}
76+
err = cache.Set(context.Background(), "", nil)
77+
if err != nil {
78+
t.Fatalf("expected nil error, but got %q", err)
79+
}
80+
}
81+
82+
func TestLogDiscardErrorOnce(t *testing.T) {
83+
cache := &CacheWithLog{
84+
Cache: &dummyCache{},
85+
DiscardCacheError: true,
86+
}
87+
oldStderr := os.Stderr
88+
defer func() {
89+
os.Stderr = oldStderr
90+
}()
91+
testFile, err := os.CreateTemp(t.TempDir(), "testNotation")
92+
if err != nil {
93+
t.Fatal(err)
94+
}
95+
defer testFile.Close()
96+
os.Stderr = testFile
97+
var wg sync.WaitGroup
98+
for i := 0; i < 3; i++ {
99+
wg.Add(1)
100+
go func() {
101+
defer wg.Done()
102+
cache.Get(context.Background(), "")
103+
cache.Set(context.Background(), "", nil)
104+
}()
105+
}
106+
wg.Wait()
107+
108+
b, err := os.ReadFile(testFile.Name())
109+
if err != nil {
110+
t.Fatal(err)
111+
}
112+
expectedMsg := "Warning: CRL cache error discarded. Enable debug log through '-d' for error details.\n"
113+
if string(b) != expectedMsg {
114+
t.Fatalf("expected to get %q, but got %q", expectedMsg, string(b))
115+
}
116+
}
117+
118+
type dummyCache struct {
119+
cacheMiss bool
120+
setSuccess bool
121+
}
122+
123+
func (d *dummyCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) {
124+
if d.cacheMiss {
125+
return nil, corecrl.ErrCacheMiss
126+
}
127+
return nil, errors.New("cache get failed")
128+
}
129+
130+
func (d *dummyCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error {
131+
if d.setSuccess {
132+
return nil
133+
}
134+
return errors.New("cache set failed")
135+
}

test/e2e/plugin/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ require (
1515
github.com/fxamacker/cbor/v2 v2.7.0 // indirect
1616
github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect
1717
github.com/go-ldap/ldap/v3 v3.4.8 // indirect
18-
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
18+
github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
1919
github.com/google/uuid v1.6.0 // indirect
2020
github.com/inconshreveable/mousetrap v1.1.0 // indirect
2121
github.com/notaryproject/tspclient-go v0.2.0 // indirect

test/e2e/plugin/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ github.com/go-ldap/ldap/v3 v3.4.8 h1:loKJyspcRezt2Q3ZRMq2p/0v8iOurlmeXDPw6fikSvQ
1414
github.com/go-ldap/ldap/v3 v3.4.8/go.mod h1:qS3Sjlu76eHfHGpUdWkAXQTw4beih+cHsco2jXlIXrk=
1515
github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
1616
github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
17-
github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
18-
github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
17+
github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo=
18+
github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
1919
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
2020
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
2121
github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=

test/e2e/run.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,4 @@ export NOTATION_E2E_PLUGIN_TAR_GZ_PATH=$CWD/plugin/bin/$PLUGIN_NAME.tar.gz
118118
export NOTATION_E2E_MALICIOUS_PLUGIN_ARCHIVE_PATH=$CWD/testdata/malicious-plugin
119119

120120
# run tests
121-
ginkgo -r -p -v
121+
ginkgo -r -p -v --focus "successfully completed with cache error in debug log"

0 commit comments

Comments
 (0)