fix(config): warn if cve is used with remote storage driver (#1034)

* fix(config): warn if cve is used with remote storage driver Signed-off-by: Catalin Hofnar <[email protected]> * fix: also check if search is enabled Signed-off-by: Ramkumar Chinchani <[email protected]> Signed-off-by: Catalin Hofnar <[email protected]> Signed-off-by: Ramkumar Chinchani <[email protected]> Co-authored-by: Catalin Hofnar <[email protected]>
project-zot · Nov 30, 2022 · 69f0cf6 · 69f0cf6
1 parent 72abab4
commit 69f0cf6
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 24 deletions.
diff --git a/.github/workflows/ecosystem-tools.yaml b/.github/workflows/ecosystem-tools.yaml
@@ -66,7 +66,6 @@ jobs:
           echo "Startup complete"          
       - name: Run cloud-only tests
         run: |
-            sudo mkdir /zot
             make test-cloud-only
         env:
           AWS_ACCESS_KEY_ID: fake

diff --git a/examples/config-all-remote.json b/examples/config-all-remote.json
@@ -0,0 +1,37 @@
+{
+    "distSpecVersion": "1.0.1-dev",
+    "storage": {
+        "dedupe": true,
+        "remoteCache": true,
+        "storageDriver": {
+            "name": "s3",
+            "rootdirectory": "/zot",
+            "region": "us-east-2",
+            "regionendpoint": "localhost:4566",
+            "bucket": "zot-storage",
+            "secure": false,
+            "skipverify": false
+        },
+        "cacheDriver": {
+            "name": "dynamodb",
+            "endpoint": "http://localhost:4566",
+            "region": "us-east-2",
+            "tableName": "BlobTable"
+        }
+    },
+    "http": {
+        "address": "127.0.0.1",
+        "port": "8080"
+    },
+    "log": {
+        "level": "debug"
+    },
+    "extensions": {
+      "search": {
+        "enable": false,
+        "cve": {
+          "updateInterval": "2h"
+        }
+      }
+    }
+}
diff --git a/pkg/cli/extensions_test.go b/pkg/cli/extensions_test.go
@@ -637,17 +637,12 @@ func TestServeSearchEnabled(t *testing.T) {
 		logPath, err := runCLIWithConfig(tempDir, content)
 		So(err, ShouldBeNil)
 		// to avoid data race when multiple go routines write to trivy DB instance.
-		WaitTillTrivyDBDownloadStarted(tempDir)
 		defer os.Remove(logPath) // clean up
 
-		substring := "\"Extensions\":{\"Search\":{\"Enable\":true,\"CVE\":{\"UpdateInterval\":86400000000000}},\"Sync\":null,\"Metrics\":null,\"Scrub\":null,\"Lint\":null}" //nolint:lll // gofumpt conflicts with lll
+		substring := "\"Extensions\":{\"Search\":{\"Enable\":true,\"CVE\":null},\"Sync\":null,\"Metrics\":null,\"Scrub\":null,\"Lint\":null}" //nolint:lll // gofumpt conflicts with lll
 		found, err := readLogFileAndSearchString(logPath, substring, readLogFileTimeout)
 		So(found, ShouldBeTrue)
 		So(err, ShouldBeNil)
-
-		found, err = readLogFileAndSearchString(logPath, "updating the CVE database", readLogFileTimeout)
-		So(found, ShouldBeTrue)
-		So(err, ShouldBeNil)
 	})
 }
 
@@ -730,17 +725,11 @@ func TestServeSearchEnabledNoCVE(t *testing.T) {
 		logPath, err := runCLIWithConfig(tempDir, content)
 		So(err, ShouldBeNil)
 		defer os.Remove(logPath) // clean up
-		// to avoid data race when multiple go routines write to trivy DB instance.
-		WaitTillTrivyDBDownloadStarted(tempDir)
 
-		substring := "\"Extensions\":{\"Search\":{\"Enable\":true,\"CVE\":{\"UpdateInterval\":86400000000000}},\"Sync\":null,\"Metrics\":null,\"Scrub\":null,\"Lint\":null}" //nolint:lll // gofumpt conflicts with lll
+		substring := "\"Extensions\":{\"Search\":{\"Enable\":true,\"CVE\":null},\"Sync\":null,\"Metrics\":null,\"Scrub\":null,\"Lint\":null}" //nolint:lll // gofumpt conflicts with lll
 		found, err := readLogFileAndSearchString(logPath, substring, readLogFileTimeout)
 		So(found, ShouldBeTrue)
 		So(err, ShouldBeNil)
-
-		found, err = readLogFileAndSearchString(logPath, "updating the CVE database", readLogFileTimeout)
-		So(found, ShouldBeTrue)
-		So(err, ShouldBeNil)
 	})
 }
 

diff --git a/pkg/cli/root.go b/pkg/cli/root.go
@@ -306,6 +306,28 @@ func validateCacheConfig(cfg *config.Config) error {
 	return nil
 }
 
+func validateExtensionsConfig(cfg *config.Config) error {
+	//nolint:lll
+	if cfg.Storage.StorageDriver != nil && cfg.Extensions != nil && cfg.Extensions.Search != nil &&
+		cfg.Extensions.Search.Enable != nil && *cfg.Extensions.Search.Enable && cfg.Extensions.Search.CVE != nil {
+		log.Warn().Err(errors.ErrBadConfig).Msg("CVE functionality can't be used with remote storage. Please disable CVE")
+
+		return errors.ErrBadConfig
+	}
+
+	for _, subPath := range cfg.Storage.SubPaths {
+		//nolint:lll
+		if subPath.StorageDriver != nil && cfg.Extensions != nil && cfg.Extensions.Search != nil &&
+			cfg.Extensions.Search.Enable != nil && *cfg.Extensions.Search.Enable && cfg.Extensions.Search.CVE != nil {
+			log.Warn().Err(errors.ErrBadConfig).Msg("CVE functionality can't be used with remote storage. Please disable CVE")
+
+			return errors.ErrBadConfig
+		}
+	}
+
+	return nil
+}
+
 func validateConfiguration(config *config.Config) error {
 	if err := validateHTTP(config); err != nil {
 		return err
@@ -331,6 +353,10 @@ func validateConfiguration(config *config.Config) error {
 		return err
 	}
 
+	if err := validateExtensionsConfig(config); err != nil {
+		return err
+	}
+
 	// check authorization config, it should have basic auth enabled or ldap
 	if config.HTTP.RawAccessControl != nil {
 		// checking for anonymous policy only authorization config: no users, no policies but anonymous policy
@@ -449,10 +475,6 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper) {
 			if config.Extensions.Search.Enable == nil {
 				config.Extensions.Search.Enable = &defaultVal
 			}
-
-			if config.Extensions.Search.CVE == nil {
-				config.Extensions.Search.CVE = &extconf.CVEConfig{UpdateInterval: 24 * time.Hour} //nolint: gomnd
-			}
 		}
 
 		if config.Extensions.Metrics != nil {

diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go
@@ -112,6 +112,84 @@ func TestVerify(t *testing.T) {
 		So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic)
 	})
 
+	Convey("Test verify CVE warn for remote storage", t, func(c C) {
+		tmpfile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpfile.Name()) // clean up
+
+		content := []byte(`{
+			"storage":{
+				"rootDirectory":"/tmp/zot",
+				"dedupe":true,
+				"remoteCache":false,
+				"storageDriver":{
+					"name":"s3",
+					"rootdirectory":"/zot",
+					"region":"us-east-2",
+					"bucket":"zot-storage",
+					"secure":true,
+					"skipverify":false
+				}
+			},
+			"http":{
+				"address":"127.0.0.1",
+				"port":"8080"
+			},
+			"extensions":{
+				"search": {
+					"enable": true,
+					"cve": {
+						"updateInterval": "24h"
+					}
+				}
+			}
+		}`)
+		err = os.WriteFile(tmpfile.Name(), content, 0o0600)
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpfile.Name()}
+		So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic)
+
+		content = []byte(`{
+			"storage":{
+				"rootDirectory":"/tmp/zot",
+				"dedupe":true,
+				"remoteCache":false,
+				"subPaths":{
+					"/a": {
+						"rootDirectory": "/tmp/zot1",
+						"dedupe": false,
+						"storageDriver":{
+							"name":"s3",
+							"rootdirectory":"/zot-a",
+							"region":"us-east-2",
+							"bucket":"zot-storage",
+							"secure":true,
+							"skipverify":false
+						}
+					}
+				}
+			},
+			"http":{
+				"address":"127.0.0.1",
+				"port":"8080"
+			},
+			"extensions":{
+				"search": {
+					"enable": true,
+					"cve": {
+						"updateInterval": "24h"
+					}
+				}
+			}
+		}`)
+		err = os.WriteFile(tmpfile.Name(), content, 0o0600)
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpfile.Name()}
+		So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic)
+	})
+
 	Convey("Test cached db config", t, func(c C) {
 		tmpfile, err := os.CreateTemp("", "zot-test*.json")
 		So(err, ShouldBeNil)

diff --git a/test/blackbox/cloud-only.bats b/test/blackbox/cloud-only.bats
@@ -52,9 +52,7 @@ function setup() {
             }
         },
 		"search": {
-			"cve": {
-				"updateInterval": "2h"
-			}
+            "enable": true
 		},
 		"scrub": {
 			"enable": true,
@@ -64,7 +62,7 @@ function setup() {
 }
 EOF
     awslocal s3 --region "us-east-2" mb s3://zot-storage
-    awslocal dynamodb --endpoint-url "http://localhost:4566" --region "us-east-2" create-table --table-name "BlobTable" --attribute-definitions AttributeName=Digest,AttributeType=S --key-schema AttributeName=Digest,KeyType=HASH --provisioned-throughput ReadCapacityUnits=10,WriteCapacityUnits=5
+    awslocal dynamodb --region "us-east-2" create-table --table-name "BlobTable" --attribute-definitions AttributeName=Digest,AttributeType=S --key-schema AttributeName=Digest,KeyType=HASH --provisioned-throughput ReadCapacityUnits=10,WriteCapacityUnits=5
     zot_serve_strace ${zot_config_file}
     wait_zot_reachable "http://127.0.0.1:8080/v2/_catalog"
 }
@@ -73,8 +71,8 @@ function teardown() {
     local zot_root_dir=${BATS_FILE_TMPDIR}/zot
     zot_stop
     rm -rf ${zot_root_dir}
-    aws s3 --endpoint-url "http://localhost:4566" rb s3://"zot-storage" --force
-    aws dynamodb --endpoint-url "http://localhost:4566" --region "us-east-2" delete-table --table-name "BlobTable"
+    awslocal s3 rb s3://"zot-storage" --force
+    awslocal dynamodb --region "us-east-2" delete-table --table-name "BlobTable"
 }
 
 @test "check for local disk writes" {