Add a test for action_cache_server_proxy (#6943)

enterprise/server/action_cache_server_proxy/BUILD

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 package(default_visibility = ["//enterprise:__subpackages__"])
 
@@ -15,3 +15,17 @@ go_library(
         "//server/util/status",
     ],
 )
+
+go_test(
+    name = "action_cache_server_proxy_test",
+    size = "small",
+    srcs = ["action_cache_server_proxy_test.go"],
+    embed = [":action_cache_server_proxy"],
+    deps = [
+        "//proto:remote_execution_go_proto",
+        "//server/remote_cache/action_cache_server",
+        "//server/testutil/testenv",
+        "//server/util/status",
+        "@com_github_stretchr_testify//require",
+    ],
+)

enterprise/server/action_cache_server_proxy/action_cache_server_proxy.go

@@ -43,12 +43,16 @@ func NewActionCacheServerProxy(env environment.Env) (*ActionCacheServerProxy, er
 	}, nil
 }
 
-// TODO(iain): don't cache these locally
+// Action Cache entries are not content-addressable, so the value pointed to
+// by a given key may change in the backing cache. Thus, always fetch them from
+// the authoritative cache.
 func (s *ActionCacheServerProxy) GetActionResult(ctx context.Context, req *repb.GetActionResultRequest) (*repb.ActionResult, error) {
 	return s.remoteCache.GetActionResult(ctx, req)
 }
 
-// TODO(iain): don't cache these locally
+// Action Cache entries are not content-addressable, so the value pointed to
+// by a given key may change in the backing cache. Thus, don't cache them
+// locally when writing to the authoritative cache.
 func (s *ActionCacheServerProxy) UpdateActionResult(ctx context.Context, req *repb.UpdateActionResultRequest) (*repb.ActionResult, error) {
 	return s.remoteCache.UpdateActionResult(ctx, req)
 }

enterprise/server/action_cache_server_proxy/action_cache_server_proxy_test.go

@@ -0,0 +1,116 @@
+package action_cache_server_proxy
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/buildbuddy-io/buildbuddy/server/remote_cache/action_cache_server"
+	"github.com/buildbuddy-io/buildbuddy/server/testutil/testenv"
+	"github.com/buildbuddy-io/buildbuddy/server/util/status"
+	"github.com/stretchr/testify/require"
+
+	repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
+)
+
+func runACServer(ctx context.Context, t *testing.T) repb.ActionCacheClient {
+	env := testenv.GetTestEnv(t)
+	acServer, err := action_cache_server.NewActionCacheServer(env)
+	require.NoError(t, err)
+	grpcServer, runFunc := testenv.RegisterLocalGRPCServer(env)
+	repb.RegisterActionCacheServer(grpcServer, acServer)
+	go runFunc()
+	conn, err := testenv.LocalGRPCConn(ctx, env)
+	require.NoError(t, err)
+	t.Cleanup(func() { conn.Close() })
+	return repb.NewActionCacheClient(conn)
+}
+
+func runACProxy(ctx context.Context, client repb.ActionCacheClient, t *testing.T) repb.ActionCacheClient {
+	env := testenv.GetTestEnv(t)
+	env.SetActionCacheClient(client)
+	proxyServer, err := NewActionCacheServerProxy(env)
+	require.NoError(t, err)
+	grpcServer, runFunc := testenv.RegisterLocalGRPCServer(env)
+	repb.RegisterActionCacheServer(grpcServer, proxyServer)
+	go runFunc()
+	conn, err := testenv.LocalGRPCConn(ctx, env)
+	require.NoError(t, err)
+	t.Cleanup(func() { conn.Close() })
+	return repb.NewActionCacheClient(conn)
+}
+
+func update(ctx context.Context, client repb.ActionCacheClient, digest *repb.Digest, code int32, t *testing.T) {
+	req := repb.UpdateActionResultRequest{
+		ActionDigest:   digest,
+		DigestFunction: repb.DigestFunction_SHA256,
+		ActionResult:   &repb.ActionResult{ExitCode: code},
+	}
+	_, err := client.UpdateActionResult(ctx, &req)
+	require.NoError(t, err)
+}
+
+func get(ctx context.Context, client repb.ActionCacheClient, digest *repb.Digest, t *testing.T) *repb.ActionResult {
+	req := &repb.GetActionResultRequest{
+		ActionDigest:   digest,
+		DigestFunction: repb.DigestFunction_SHA256,
+	}
+	resp, err := client.GetActionResult(ctx, req)
+	require.NoError(t, err)
+	return resp
+}
+
+func TestActionCacheProxy(t *testing.T) {
+	ctx := context.Background()
+	ac := runACServer(ctx, t)
+	proxy := runACProxy(ctx, ac, t)
+
+	digestA := &repb.Digest{
+		Hash:      strings.Repeat("a", 64),
+		SizeBytes: 1024,
+	}
+	digestB := &repb.Digest{
+		Hash:      strings.Repeat("b", 64),
+		SizeBytes: 1024,
+	}
+
+	// DigestA shouldn't be present initially.
+	readReqA := &repb.GetActionResultRequest{
+		ActionDigest:   digestA,
+		DigestFunction: repb.DigestFunction_SHA256,
+	}
+	_, err := proxy.GetActionResult(ctx, readReqA)
+	require.True(t, status.IsNotFoundError(err))
+
+	// Write it through the proxy and confirm it's readable from the proxy and
+	// backing cache.
+	update(ctx, proxy, digestA, 1, t)
+	require.Equal(t, int32(1), get(ctx, ac, digestA, t).GetExitCode())
+	require.Equal(t, int32(1), get(ctx, proxy, digestA, t).GetExitCode())
+
+	// Change the action result, write it, and confirm the new result is
+	// readable from the proxy and backing cache.
+	update(ctx, proxy, digestA, 2, t)
+	require.Equal(t, int32(2), get(ctx, ac, digestA, t).GetExitCode())
+	require.Equal(t, int32(2), get(ctx, proxy, digestA, t).GetExitCode())
+
+	// DigestB shouldn't be present initially.
+	readReqB := &repb.GetActionResultRequest{
+		ActionDigest:   digestB,
+		DigestFunction: repb.DigestFunction_SHA256,
+	}
+	_, err = proxy.GetActionResult(ctx, readReqB)
+	require.True(t, status.IsNotFoundError(err))
+
+	// Write it to the backing cache and confirm it's readable from the proxy
+	// and backing cache.
+	update(ctx, ac, digestB, 999, t)
+	require.Equal(t, int32(999), get(ctx, ac, digestB, t).GetExitCode())
+	require.Equal(t, int32(999), get(ctx, proxy, digestB, t).GetExitCode())
+
+	// Change the action result, write it, and confirm the new result is
+	// readable from the proxy and backing cache.
+	update(ctx, ac, digestB, 998, t)
+	require.Equal(t, int32(998), get(ctx, ac, digestB, t).GetExitCode())
+	require.Equal(t, int32(998), get(ctx, proxy, digestB, t).GetExitCode())
+}