Skip to content

Commit

Permalink
fix: update to add cache error test case
Browse files Browse the repository at this point in the history
Signed-off-by: Junjie Gao <[email protected]>
  • Loading branch information
JeyJeyGao committed Nov 12, 2024
1 parent 0e2495e commit a1437f2
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 12 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ jobs:
go-version: ${{ matrix.go-version }}
check-latest: true
- name: Check out code
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Cache Go modules
uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
id: go-mod-cache
with:
path: ~/go/pkg/mod
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
fail-fast: false
steps:
- name: Checkout repository
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Set up Go ${{ matrix.go-version }} environment
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release-github.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
go-version: ${{ matrix.go-version }}
check-latest: true
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
- name: Set GoReleaser Previous Tag To Be Last Non Weekly Release
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:

steps:
- name: "Checkout code"
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # tag=4.1.7
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=4.2.2
with:
persist-credentials: false

Expand All @@ -54,7 +54,7 @@ jobs:
publish_results: true

- name: "Upload artifact"
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # tag=v4.4.0
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # tag=v4.4.3
with:
name: SARIF file
path: results.sarif
Expand Down
9 changes: 7 additions & 2 deletions cmd/notation/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/spf13/cobra"

corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
clicrl "github.com/notaryproject/notation/internal/crl"
)

type verifyOpts struct {
Expand Down Expand Up @@ -238,15 +239,19 @@ func getVerifier(ctx context.Context) (notation.Verifier, error) {
if err != nil {
return nil, err
}
crlFetcher.DiscardCacheError = true // discard crl cache error
cacheRoot, err := dir.CacheFS().SysPath(dir.PathCRLCache)
if err != nil {
return nil, err
}
crlFetcher.Cache, err = crl.NewFileCache(cacheRoot)
fileCache, err := crl.NewFileCache(cacheRoot)
if err != nil {
return nil, err
}
crlFetcher.DiscardCacheError = true // discard cache error
crlFetcher.Cache = &clicrl.CacheWithLog{
Cache: fileCache,
DiscardCacheError: crlFetcher.DiscardCacheError,
}
revocationCodeSigningValidator, err := revocation.NewWithOptions(revocation.Options{
OCSPHTTPClient: ocspHttpClient,
CRLFetcher: crlFetcher,
Expand Down
80 changes: 80 additions & 0 deletions internal/crl/crl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright The Notary Project Authors.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package crl

import (
"context"
"errors"
"fmt"
"os"
"sync"

corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
"github.com/notaryproject/notation-go/log"
)

// CacheWithLog implements corecrl.Cache with logging
type CacheWithLog struct {
corecrl.Cache

//DiscardCacheError is set to true to enable logging the discard cache error
//warning
DiscardCacheError bool

// logDiscardCrlCacheErrorOnce guarantees the discard cache error
// warning is logged only once
logDiscardCrlCacheErrorOnce sync.Once
}

// Get retrieves the CRL bundle with the given url
func (c *CacheWithLog) Get(ctx context.Context, url string) (*corecrl.Bundle, error) {
if c.Cache == nil {
return nil, errors.New("cache cannot be nil")
}
logger := log.GetLogger(ctx)

bundle, err := c.Cache.Get(ctx, url)
if err != nil && !errors.Is(err, corecrl.ErrCacheMiss) {
if c.DiscardCacheError {
c.logDiscardCrlCacheErrorOnce.Do(c.logDiscardCrlCacheError)
}
logger.Debug(err.Error())
return nil, err
}
return bundle, err
}

// Set stores the CRL bundle with the given url
func (c *CacheWithLog) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error {
if c.Cache == nil {
return errors.New("cache cannot be nil")
}
logger := log.GetLogger(ctx)

err := c.Cache.Set(ctx, url, bundle)
if err != nil {
if c.DiscardCacheError {
c.logDiscardCrlCacheErrorOnce.Do(c.logDiscardCrlCacheError)
}
logger.Debug(err.Error())
return err
}
return nil
}

// logDiscardCrlCacheError logs the warning when CRL cache error is
// discarded
func (c *CacheWithLog) logDiscardCrlCacheError() {
fmt.Fprintln(os.Stderr, "Warning: CRL cache error discarded. Enable debug log through '-d' for error details.")
}
135 changes: 135 additions & 0 deletions internal/crl/crl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Copyright The Notary Project Authors.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package crl

import (
"context"
"errors"
"os"
"sync"
"testing"

corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
)

func TestGet(t *testing.T) {
cache := &CacheWithLog{}
expectedErrMsg := "cache cannot be nil"
_, err := cache.Get(context.Background(), "")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
}

cache = &CacheWithLog{
Cache: &dummyCache{},
}
expectedErrMsg = "cache get failed"
_, err = cache.Get(context.Background(), "")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
}

cache = &CacheWithLog{
Cache: &dummyCache{
cacheMiss: true,
},
}
_, err = cache.Get(context.Background(), "")
if err == nil || !errors.Is(err, corecrl.ErrCacheMiss) {
t.Fatalf("expected error %q, but got %q", corecrl.ErrCacheMiss, err)
}
}

func TestSet(t *testing.T) {
cache := &CacheWithLog{}
expectedErrMsg := "cache cannot be nil"
err := cache.Set(context.Background(), "", nil)
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
}

cache = &CacheWithLog{
Cache: &dummyCache{},
}
expectedErrMsg = "cache set failed"
err = cache.Set(context.Background(), "", nil)
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("expected error %q, but got %q", expectedErrMsg, err)
}

cache = &CacheWithLog{
Cache: &dummyCache{
setSuccess: true,
},
}
err = cache.Set(context.Background(), "", nil)
if err != nil {
t.Fatalf("expected nil error, but got %q", err)
}
}

func TestLogDiscardErrorOnce(t *testing.T) {
cache := &CacheWithLog{
Cache: &dummyCache{},
DiscardCacheError: true,
}
oldStderr := os.Stderr
defer func() {
os.Stderr = oldStderr
}()
testFile, err := os.CreateTemp(t.TempDir(), "testNotation")
if err != nil {
t.Fatal(err)
}
defer testFile.Close()
os.Stderr = testFile
var wg sync.WaitGroup
for i := 0; i < 3; i++ {
wg.Add(1)
go func() {
defer wg.Done()
cache.Get(context.Background(), "")
cache.Set(context.Background(), "", nil)
}()
}
wg.Wait()

b, err := os.ReadFile(testFile.Name())
if err != nil {
t.Fatal(err)
}
expectedMsg := "Warning: CRL cache error discarded. Enable debug log through '-d' for error details.\n"
if string(b) != expectedMsg {
t.Fatalf("expected to get %q, but got %q", expectedMsg, string(b))
}
}

type dummyCache struct {
cacheMiss bool
setSuccess bool
}

func (d *dummyCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) {
if d.cacheMiss {
return nil, corecrl.ErrCacheMiss
}
return nil, errors.New("cache get failed")
}

func (d *dummyCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error {
if d.setSuccess {
return nil
}
return errors.New("cache set failed")
}
2 changes: 1 addition & 1 deletion test/e2e/plugin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/fxamacker/cbor/v2 v2.7.0 // indirect
github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect
github.com/go-ldap/ldap/v3 v3.4.8 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/notaryproject/tspclient-go v0.2.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/plugin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ github.com/go-ldap/ldap/v3 v3.4.8 h1:loKJyspcRezt2Q3ZRMq2p/0v8iOurlmeXDPw6fikSvQ
github.com/go-ldap/ldap/v3 v3.4.8/go.mod h1:qS3Sjlu76eHfHGpUdWkAXQTw4beih+cHsco2jXlIXrk=
github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo=
github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,4 @@ export NOTATION_E2E_PLUGIN_TAR_GZ_PATH=$CWD/plugin/bin/$PLUGIN_NAME.tar.gz
export NOTATION_E2E_MALICIOUS_PLUGIN_ARCHIVE_PATH=$CWD/testdata/malicious-plugin

# run tests
ginkgo -r -p -v
ginkgo -r -p -v --focus "successfully completed with cache error in debug log"
33 changes: 33 additions & 0 deletions test/e2e/suite/scenario/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package scenario_test

import (
"os"

. "github.com/notaryproject/notation/test/e2e/internal/notation"
"github.com/notaryproject/notation/test/e2e/internal/utils"
. "github.com/notaryproject/notation/test/e2e/suite/common"
Expand Down Expand Up @@ -128,4 +130,35 @@ var _ = Describe("notation CRL revocation check", Serial, func() {
)
})
})

It("successfully completed with cache error in debug log", func() {
Host(CRLOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
notation.Exec("sign", artifact.ReferenceWithDigest()).
MatchKeyWords(SignSuccessfully)

utils.LeafCRLUnrevoke()
utils.IntermediateCRLUnrevoke()

if err := os.MkdirAll(vhost.AbsolutePath(".cache"), 0500); err != nil {
Fail(err.Error())
}
defer os.Chmod(vhost.AbsolutePath(".cache"), 0700)

// verify without cache
notation.Exec("verify", artifact.ReferenceWithDigest(), "-d").
MatchKeyWords(
VerifySuccessfully,
).
MatchErrKeyWords(
"CRL file cache miss",
"Retrieving crl bundle from file cache with key",
"Storing crl bundle to file cache with key",
"OCSP check failed with unknown error and fallback to CRL check for certificate #2",
).
NoMatchErrKeyWords(
"is revoked",
)
})

})
})

0 comments on commit a1437f2

Please sign in to comment.