Repository: deislabs/ratify
Pull request #160 (2022-04-06 23:56:56)

Update validating webhook timeout to 7 seconds
- update configmap, and README to set validating webhook timeout to 7 seconds
- add executor config to set timeout to 6800ms

Related Gatekeeper issue: https://github.com/open-policy-agent/gatekeeper/issues/1956

akashsinghal: @susanshi would appreciate a review when you have the chance. thanks!

Patch:
diff --git a/README.md b/README.md
index 20ea41d..09115d8 100644
--- a/README.md
+++ b/README.md
@@ -26,6 +26,8 @@ by its developers, nor is it "supported" software.
 
 Try out ratify in Kuberenetes through Gatekeeper as the admission controller.
 
+Prerequisite: Kubernetes v1.20 or higher
+
 - Setup Gatekeeper with [external data](https://open-policy-agent.github.io/gatekeeper/website/docs/externaldata)
 
 ```bash
@@ -35,9 +37,11 @@ helm install gatekeeper/gatekeeper  \
     --name-template=gatekeeper \
     --namespace gatekeeper-system --create-namespace \
     --set enableExternalData=true \
-    --set controllerManager.dnsPolicy=ClusterFirst,audit.dnsPolicy=ClusterFirst
+    --set validatingWebhookTimeoutSeconds=7
 ```
 
+NOTE: `validatingWebhookTimeoutSeconds` increased from 3 to 7 so all Ratify operations complete in complex scenarios. Kubernetes v1.20 or higher is REQUIRED to increase timeout.  
+
 - Deploy ratify and a `demo` constraint on gatekeeper
 
 ```bash
diff --git a/charts/ratify-gatekeeper/values.yaml b/charts/ratify-gatekeeper/values.yaml
index 34d444d..0fcf5f0 100644
--- a/charts/ratify-gatekeeper/values.yaml
+++ b/charts/ratify-gatekeeper/values.yaml
@@ -8,7 +8,7 @@ gatekeeper:
   auditFromCache: false
   disableMutation: false
   disableValidatingWebhook: false
-  validatingWebhookTimeoutSeconds: 3
+  validatingWebhookTimeoutSeconds: 7
   validatingWebhookFailurePolicy: Ignore
   validatingWebhookCheckIgnoreFailurePolicy: Fail
   enableDeleteOperations: false
diff --git a/charts/ratify/templates/configmap.yaml b/charts/ratify/templates/configmap.yaml
index a2ee985..3521645 100644
--- a/charts/ratify/templates/configmap.yaml
+++ b/charts/ratify/templates/configmap.yaml
@@ -5,6 +5,9 @@ metadata:
 data:
   config.json: |
     {
+      "executor": {
+        "requestTimeout": 6800
+      },
       "stores": {
         "version": "1.0.0",
         "plugins": [
diff --git a/cmd/ratify/cmd/serve.go b/cmd/ratify/cmd/serve.go
index d90bcd0..070b03d 100644
--- a/cmd/ratify/cmd/serve.go
+++ b/cmd/ratify/cmd/serve.go
@@ -92,6 +92,7 @@ func serve(opts serveCmdOptions) error {
 		Verifiers:      verifiers,
 		ReferrerStores: stores,
 		PolicyEnforcer: policyEnforcer,
+		Config:         &cf.ExecutorConfig,
 	}
 
 	if opts.httpServerAddress != "" {
diff --git a/pkg/utils/certificateUtils.go b/pkg/utils/certificateUtils.go
index 3ba82f9..94bf764 100644
--- a/pkg/utils/certificateUtils.go
+++ b/pkg/utils/certificateUtils.go
@@ -17,27 +17,63 @@ package utils
 
 import (
 	"crypto/x509"
+	"io/fs"
 	"os"
+	"strings"
 
 	"path/filepath"
 
+	"github.com/deislabs/ratify/pkg/homedir"
 	"github.com/notaryproject/notation-go-lib/crypto/cryptoutil"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
+// Return list of certificates loaded from path
+// when path is a directory, this method loads all certs in directory and resolve symlink if needed
 func GetCertificatesFromPath(path string) ([]*x509.Certificate, error) {
-
 	var certs []*x509.Certificate
+	fileMap := map[string]bool{} //a map to track path of physical files
+
+	path = ReplaceHomeShortcut(path)
 
 	err := filepath.Walk(path, func(file string, info os.FileInfo, err error) error {
-		if !info.IsDir() {
-			cert, certError := cryptoutil.ReadCertificateFile(file) // ReadCertificateFile returns empty if file was not a certificate
-			if certError != nil {
-				return certError
+
+		targetFileInfo := info
+		targetFilePath := file
+
+		if info == nil || err != nil {
+			logrus.Warnf("Invalid path '%v' skipped, error %v", file, err)
+			return nil
+		}
+
+		// In a cluster environment, each mounted file results in a physical file and a symlink
+		// check if file is a link and get the actual file path
+		if isSymbolicLink(info) {
+			targetFilePath, err = filepath.EvalSymlinks(file)
+			if err != nil || len(targetFilePath) == 0 {
+				logrus.Errorf("Unable to resolve symbolic link %v , error '%v'", file, err)
+				return nil
 			}
-			if cert != nil {
-				certs = append(certs, cert...)
+
+			targetFileInfo, err = os.Lstat(targetFilePath)
+
+			if err != nil {
+				logrus.Errorf("error getting file info for path '%v', error '%v'", targetFilePath, err)
+				return nil
+			}
+		}
+
+		// if filepath.EvalSymlinks fails to resolve multi level sym link, skip this file
+		if targetFileInfo != nil && !targetFileInfo.IsDir() && !isSymbolicLink(targetFileInfo) {
+			if _, ok := fileMap[targetFilePath]; !ok {
+				certs, err = loadCertFile(targetFileInfo, targetFilePath, certs, fileMap)
+				if err != nil {
+					return errors.Wrap(err, "error reading certificate file "+targetFilePath)
+				}
 			}
 		}
+
 		return nil
 	})
 
@@ -45,5 +81,38 @@ func GetCertificatesFromPath(path string) ([]*x509.Certificate, error) {
 		return nil, err
 	}
 
+	logrus.Infof("%v notary verification certificates loaded from path '%v'", len(certs), path)
 	return certs, nil
 }
+
+func isSymbolicLink(info fs.FileInfo) bool {
+	return info.Mode()&os.ModeSymlink != 0
+}
+
+func loadCertFile(fileInfo fs.FileInfo, filePath string, certificate []*x509.Certificate, fileMap map[string]bool) ([]*x509.Certificate, error) {
+	cert, certError := cryptoutil.ReadCertificateFile(filePath) // ReadCertificateFile returns empty if file was not a certificate
+	if certError != nil {
+		return certificate, certError
+	}
+	if cert != nil {
+		certificate = append(certificate, cert...)
+		fileMap[filePath] = true
+	}
+
+	return certificate, nil
+}
+
+// Replace the shortcut prefix in a path with the home directory
+// For example in a unix os, ~/.config/ becomes /home/azureuser/.config after replacement
+func ReplaceHomeShortcut(path string) string {
+	shortcutPrefix := homedir.GetShortcutString() + string(os.PathSeparator)
+	if strings.HasPrefix(path, shortcutPrefix) {
+		home := homedir.Get()
+		if len(home) > 0 {
+			return strings.Replace(path, homedir.GetShortcutString(), home, 1) // replace 1 instance
+		} else {
+			logrus.Warningf("Path '%v' replacement failed , value of Home dir '%v'", path, home)
+		}
+	}
+	return path
+}
diff --git a/plugins/verifier/cosign/cosign.go b/plugins/verifier/cosign/cosign.go
index 4a4111c..ffe91cc 100644
--- a/plugins/verifier/cosign/cosign.go
+++ b/plugins/verifier/cosign/cosign.go
@@ -26,6 +26,7 @@ import (
 	"github.com/deislabs/ratify/pkg/common"
 	"github.com/deislabs/ratify/pkg/ocispecs"
 	"github.com/deislabs/ratify/pkg/referrerstore"
+	"github.com/deislabs/ratify/pkg/utils"
 	"github.com/deislabs/ratify/pkg/verifier"
 	"github.com/deislabs/ratify/pkg/verifier/plugin/skel"
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -133,7 +134,8 @@ func signatures(ctx context.Context, img string, keyRef string, config *PluginIn
 }
 
 func loadPublicKey(ctx context.Context, keyRef string) (verifier signature.Verifier, err error) {
-	raw, err := ioutil.ReadFile(filepath.Clean(keyRef))
+	keyPath := filepath.Clean(utils.ReplaceHomeShortcut(keyRef))
+	raw, err := ioutil.ReadFile(keyPath)
 	if err != nil {
 		return nil, err
 	}


Test patch:
diff --git a/pkg/utils/certificateUtil_test.go b/pkg/utils/certificateUtil_test.go
index cb0e07a..7ab5c24 100644
--- a/pkg/utils/certificateUtil_test.go
+++ b/pkg/utils/certificateUtil_test.go
@@ -18,8 +18,34 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+
+	"github.com/deislabs/ratify/pkg/homedir"
 )
 
+func TestHomepathReplacement(t *testing.T) {
+
+	sampleFoldername := "test"
+	testPath := homedir.GetShortcutString() + string(os.PathSeparator) + sampleFoldername
+
+	result := ReplaceHomeShortcut(testPath)
+	home, err := os.UserHomeDir()
+	expectedPath := home + "/" + sampleFoldername
+
+	if result != expectedPath {
+		t.Fatalf("sample input %v ,result expected to be %v, actual result %v, error %v", testPath, expectedPath, result, err)
+	}
+}
+
+func TestReadCertificatesFromPath_InvalidPath(t *testing.T) {
+
+	files, err := GetCertificatesFromPath("/invalid/path")
+
+	expectedFileCount := 0
+	if len(files) != expectedFileCount || err != nil {
+		t.Fatalf("response length expected to be %v, actual %v, error %v", expectedFileCount, len(files), err)
+	}
+}
+
 func TestReadCertificatesFromPath_NestedDirectory(t *testing.T) {
 	// Setup to create nested directory structure
 	testDir := "TestDirectory"
@@ -50,6 +76,56 @@ func TestReadCertificatesFromPath_NestedDirectory(t *testing.T) {
 	}
 }
 
+func TestReadFilesFromPath_SymbolicLink(t *testing.T) {
+
+	// Setup
+	currPath, _ := os.Getwd()
+	testDirName := "TestDirectory"
+	absTestDirPath := currPath + string(os.PathSeparator) + testDirName + string(os.PathSeparator)
+	testFile1 := absTestDirPath + "file1.Crt"
+
+	setupDirectoryForTesting(t, testDirName)
+	createCertFile(t, testFile1)
+
+	symlink := absTestDirPath + "symlink"
+	os.Symlink(testFile1, symlink)
+	files, err := GetCertificatesFromPath(symlink)
+
+	// Teardown
+	os.RemoveAll(absTestDirPath)
+
+	// Validate
+	if len(files) != 1 || err != nil {
+		t.Fatalf("response length expected to be 1, actual %v, error %v", len(files), err)
+	}
+}
+
+func TestReadFilesFromPath_MultilevelSymbolicLink(t *testing.T) {
+
+	// Setup
+	currPath, _ := os.Getwd()
+	testDirName := "TestDirectory"
+	absTestDirPath := currPath + string(os.PathSeparator) + testDirName + string(os.PathSeparator)
+	testFile1 := absTestDirPath + "file1.Crt"
+
+	setupDirectoryForTesting(t, testDirName)
+	createCertFile(t, testFile1)
+
+	symlink := absTestDirPath + "symlink"
+	twoLevelSymlink := absTestDirPath + "symlink2"
+	os.Symlink(testFile1, symlink)
+	os.Symlink(symlink, twoLevelSymlink)
+	files, err := GetCertificatesFromPath(twoLevelSymlink)
+
+	// Teardown
+	os.RemoveAll(absTestDirPath)
+
+	// validate
+	if len(files) != 1 || err != nil {
+		t.Fatalf("response length expected to be 1, actual %v, error %v", len(files), err)
+	}
+}
+
 func TestReadFilesFromPath_SingleFile(t *testing.T) {
 	// Setup
 	testDir := "TestDirectory"