internal/database, deploy: modify validation order for deploy
Instead of checking that the current database index is valid before deploy
(which ensures that an invalid database can't be fixed), check that
the new database to deploy is valid (including with respect to the reports
in the old database) before deploy. If it is valid, deploy it, then check
afterwards that the newly deployed database is valid.
This allows issues caused by, e.g., incomplete deploys, to be fixed
without manual intervention, while still alerting us if there is a
persistent problem.
Change-Id: Ib57901bcada5a4b823d184bef25a64559a8e9e4a
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/541615
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/cmd/checkdeploy/main.go b/cmd/checkdeploy/main.go
index 46b0bda..edbc3a7 100644
--- a/cmd/checkdeploy/main.go
+++ b/cmd/checkdeploy/main.go
@@ -28,7 +28,7 @@
if *existingPath == "" {
log.Fatalf("flag -existing must be set")
}
- if err := db.Validate(*newPath, *existingPath); err != nil {
+ if err := db.ValidateDeploy(*newPath, *existingPath); err != nil {
log.Fatal(err)
}
if *newLegacyPath != "" {
diff --git a/deploy/build.yaml b/deploy/build.yaml
index 9a9e742..1706cc5 100644
--- a/deploy/build.yaml
+++ b/deploy/build.yaml
@@ -46,7 +46,7 @@
entrypoint: bash
args: ["-c", "go run ./cmd/gendb -out /workspace/db"]
- - id: Validate
+ - id: PreValidate
name: golang:1.19.2
entrypoint: bash
args:
@@ -57,3 +57,15 @@
name: gcr.io/cloud-builders/gsutil
entrypoint: bash
args: ["./deploy/gcp-deploy.sh"]
+
+ - id: CopyDeployed
+ name: gcr.io/cloud-builders/gsutil
+ entrypoint: bash
+ args:
+ - -ec
+ - gsutil -q -m cp -r gs://go-vulndb /workspace/deployed
+
+ - id: PostValidate
+ name: golang:1.19.2
+ entrypoint: bash
+ args: ["-c", "go run ./cmd/checkdb /workspace/deployed/go-vulndb"]
diff --git a/internal/database/all_test.go b/internal/database/all_test.go
index ab3cd72..06fa325 100644
--- a/internal/database/all_test.go
+++ b/internal/database/all_test.go
@@ -32,7 +32,7 @@
if diff := cmp.Diff(newDB, loaded); diff != "" {
t.Errorf("new/write/load mismatch (-new, +loaded):\n%s", diff)
}
- if err := Validate(tmpDir, tmpDir); err != nil {
+ if err := ValidateDeploy(tmpDir, tmpDir); err != nil {
t.Error(err)
}
}
@@ -59,7 +59,7 @@
if diff := cmp.Diff(fromRepo, loaded); diff != "" {
t.Errorf("fromRepo/write/load mismatch (-fromRepo, +loaded):\n%s", diff)
}
- if err := Validate(tmpDir, tmpDir); err != nil {
+ if err := ValidateDeploy(tmpDir, tmpDir); err != nil {
t.Error(err)
}
}
@@ -97,7 +97,7 @@
t.Errorf("unexpected diff: (-created,+loaded):\n%s", diff)
}
- if err := Validate(dir, dir); err != nil {
+ if err := ValidateDeploy(dir, dir); err != nil {
t.Error(err)
}
}
diff --git a/internal/database/validate.go b/internal/database/validate.go
index 32ac666..214c9f8 100644
--- a/internal/database/validate.go
+++ b/internal/database/validate.go
@@ -6,15 +6,16 @@
import (
"fmt"
+ "path/filepath"
"golang.org/x/vulndb/internal/derrors"
"golang.org/x/vulndb/internal/osv"
)
-// Validate checks that the databases in newPath and oldPath are
-// both valid databases, and that the database in newPath can
-// be safely deployed on top of the database in oldPath.
-func Validate(newPath, oldPath string) (err error) {
+// ValidateDeploy checks that the database in newPath is a valid database,
+// and that the database in newPath can be safely deployed on top of the
+// database in oldPath.
+func ValidateDeploy(newPath, oldPath string) (err error) {
derrors.Wrap(&err, "Validate(new=%s, old=%s)", newPath, oldPath)
new, err := Load(newPath)
@@ -22,7 +23,7 @@
return err
}
- old, err := Load(oldPath)
+ old, err := RawLoad(filepath.Join(oldPath, idDir))
if err != nil {
return err
}
diff --git a/internal/database/validate_test.go b/internal/database/validate_test.go
index b33be98..670d14a 100644
--- a/internal/database/validate_test.go
+++ b/internal/database/validate_test.go
@@ -26,7 +26,7 @@
t.Run("valid", func(t *testing.T) {
// Adding more entries is OK.
- if err := Validate(big, small); err != nil {
+ if err := ValidateDeploy(big, small); err != nil {
t.Error(err)
}
})
@@ -46,15 +46,10 @@
old: small,
new: invalid,
},
- {
- name: "invalid old db",
- old: invalid,
- new: big,
- },
}
for _, test := range failTests {
t.Run(test.name, func(t *testing.T) {
- if err := Validate(test.new, test.old); err == nil {
+ if err := ValidateDeploy(test.new, test.old); err == nil {
t.Error("expected error, got nil")
}
})
