Let workers translate DownloadObject() NOT_FOUND to FAILED_PRECONDITION

This is something that Buildbarn already does for REv2, but it looks
like we're going to need this as well. For our fetching worker we don't
want to return NOT_FOUND in case any object referenced by its action
does not exist. We want to limit that return code to just 404s.

Author: EdSchouten

cmd/bonanza_builder/BUILD.bazel

@@ -30,6 +30,7 @@ go_library(
         "//pkg/remoteworker",
         "//pkg/storage/dag",
         "//pkg/storage/object",
+        "//pkg/storage/object/existenceprecondition",
         "//pkg/storage/object/grpc",
         "//pkg/storage/object/namespacemapping",
         "@com_github_buildbarn_bb_remote_execution//pkg/filesystem/pool",

cmd/bonanza_builder/main.go

@@ -35,6 +35,7 @@ import (
 	"bonanza.build/pkg/remoteworker"
 	"bonanza.build/pkg/storage/dag"
 	"bonanza.build/pkg/storage/object"
+	object_existenceprecondition "bonanza.build/pkg/storage/object/existenceprecondition"
 	object_grpc "bonanza.build/pkg/storage/object/grpc"
 	object_namespacemapping "bonanza.build/pkg/storage/object/namespacemapping"
 
@@ -74,8 +75,10 @@ func main() {
 		if err != nil {
 			return util.StatusWrap(err, "Failed to create storage gRPC client")
 		}
-		objectDownloader := object_grpc.NewGRPCDownloader(
-			object_pb.NewDownloaderClient(storageGRPCClient),
+		objectDownloader := object_existenceprecondition.NewDownloader(
+			object_grpc.NewGRPCDownloader(
+				object_pb.NewDownloaderClient(storageGRPCClient),
+			),
 		)
 		parsedObjectPool, err := model_parser.NewParsedObjectPoolFromConfiguration(configuration.ParsedObjectPool)
 		if err != nil {

cmd/bonanza_worker/BUILD.bazel

@@ -16,6 +16,7 @@ go_library(
         "//pkg/proto/storage/dag",
         "//pkg/proto/storage/object",
         "//pkg/remoteworker",
+        "//pkg/storage/object/existenceprecondition",
         "//pkg/storage/object/grpc",
         "@com_github_buildbarn_bb_remote_execution//pkg/clock",
         "@com_github_buildbarn_bb_remote_execution//pkg/filesystem/pool",

cmd/bonanza_worker/main.go

@@ -21,6 +21,7 @@ import (
 	dag_pb "bonanza.build/pkg/proto/storage/dag"
 	object_pb "bonanza.build/pkg/proto/storage/object"
 	"bonanza.build/pkg/remoteworker"
+	object_existenceprecondition "bonanza.build/pkg/storage/object/existenceprecondition"
 	object_grpc "bonanza.build/pkg/storage/object/grpc"
 
 	re_clock "github.com/buildbarn/bb-remote-execution/pkg/clock"
@@ -59,8 +60,10 @@ func main() {
 		if err != nil {
 			return util.StatusWrap(err, "Failed to create storage gRPC client")
 		}
-		objectDownloader := object_grpc.NewGRPCDownloader(
-			object_pb.NewDownloaderClient(storageGRPCClient),
+		objectDownloader := object_existenceprecondition.NewDownloader(
+			object_grpc.NewGRPCDownloader(
+				object_pb.NewDownloaderClient(storageGRPCClient),
+			),
 		)
 		parsedObjectPool, err := model_parser.NewParsedObjectPoolFromConfiguration(configuration.ParsedObjectPool)
 		if err != nil {

pkg/storage/object/existenceprecondition/BUILD.bazel

@@ -0,0 +1,13 @@
+load("@rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "existenceprecondition",
+    srcs = ["downloader.go"],
+    importpath = "bonanza.build/pkg/storage/object/existenceprecondition",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/storage/object",
+        "@org_golang_google_grpc//codes",
+        "@org_golang_google_grpc//status",
+    ],
+)

pkg/storage/object/existenceprecondition/downloader.go

@@ -0,0 +1,36 @@
+package existenceprecondition
+
+import (
+	"context"
+
+	"bonanza.build/pkg/storage/object"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+type downloader[TReference any] struct {
+	base object.Downloader[TReference]
+}
+
+// NewDownloader creates a decorator for object.Downloader that causes
+// all NOT_FOUND errors returned by DownloadObject() to be translated to
+// FAILED_PRECONDITION.
+//
+// This decorator can be used on workers that expect that all objects
+// references by actions that they receive are present in storage. If
+// objects cannot be read from storage, it's not desirable to let the
+// execution of the action itself fail with NOT_FOUND.
+func NewDownloader[TReference any](base object.Downloader[TReference]) object.Downloader[TReference] {
+	return &downloader[TReference]{
+		base: base,
+	}
+}
+
+func (d *downloader[TReference]) DownloadObject(ctx context.Context, reference TReference) (*object.Contents, error) {
+	contents, err := d.base.DownloadObject(ctx, reference)
+	if s := status.Convert(err); s.Code() == codes.NotFound {
+		return nil, status.Error(codes.FailedPrecondition, s.Message())
+	}
+	return contents, err
+}