From 69f0cf6bb4727884af42f38c92858fdb114104de Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani <45800463+rchincha@users.noreply.github.com> Date: Tue, 29 Nov 2022 23:35:06 -0800 Subject: [PATCH] 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 * fix: also check if search is enabled Signed-off-by: Ramkumar Chinchani Signed-off-by: Catalin Hofnar Signed-off-by: Ramkumar Chinchani Co-authored-by: Catalin Hofnar --- .github/workflows/ecosystem-tools.yaml | 1 - examples/config-all-remote.json | 37 ++++++++++++ pkg/cli/extensions_test.go | 15 +---- pkg/cli/root.go | 30 ++++++++-- pkg/cli/root_test.go | 78 ++++++++++++++++++++++++++ test/blackbox/cloud-only.bats | 10 ++-- 6 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 examples/config-all-remote.json diff --git a/.github/workflows/ecosystem-tools.yaml b/.github/workflows/ecosystem-tools.yaml index 6c55cb12..7357339e 100644 --- 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 new file mode 100644 index 00000000..c3d5c0c4 --- /dev/null +++ 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 index 55e8a616..d82b3831 100644 --- 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 index 7629170f..9730f5a7 100644 --- 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 index 5b13e0db..ad6ecb19 100644 --- 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 index 571f07b1..7aa8952f 100644 --- 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" {