A-1414: support hosted cache

internal/cache/save.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/buildkite/agent/v3/api"
@@ -394,10 +395,12 @@ func validateCacheStore(storeType, bucketURL string) error {
 		if bucketURL == "" {
 			return fmt.Errorf("BUILDKITE_AGENT_CACHE_STORE_URL must be set for the %s cache store", storeType)
 		}
-		// Note: We allow both s3:// and file:// for S3 store (file:// is for local testing)
-	case store.LocalHostedAgents:
-		if bucketURL != "" {
-			return fmt.Errorf("NSC store should not have bucket URL set, got: %s", bucketURL)
+		// s3:// for S3, nsc:// for Namespace artifact storage, and file:// for
+		// local testing.
+		switch scheme, _, _ := strings.Cut(bucketURL, "://"); scheme {
+		case "s3", "nsc", "file":
+		default:
+			return fmt.Errorf("unsupported cache store URL scheme %q for the %s cache store", scheme, storeType)
 		}
 	}
 

internal/cache/store/blob.go

@@ -3,6 +3,7 @@ package store
 import (
 	"context"
 	"fmt"
+	"strings"
 )
 
 // Blob interface defines the operations for blob storage
@@ -17,9 +18,16 @@ type Blob interface {
 func NewBlobStore(ctx context.Context, store, bucketURL string) (Blob, error) {
 	switch store {
 	case AgentManaged:
-		return NewS3Blob(ctx, bucketURL)
-	case LocalHostedAgents:
-		return NewNscStore()
+		scheme, _, _ := strings.Cut(bucketURL, "://")
+		switch scheme {
+		case nscScheme:
+			return NewNscStore(bucketURL)
+		case "file":
+			// Supported only for local testing, kept consistent with validateCacheStore.
+			return NewLocalFileBlob(ctx, bucketURL)
+		default:
+			return NewS3Blob(ctx, bucketURL)
+		}
 	case LocalFileStore:
 		return NewLocalFileBlob(ctx, bucketURL)
 	default:

internal/cache/store/file_test.go

@@ -571,3 +571,15 @@ func TestNewBlobStoreLocalFile(t *testing.T) {
 		t.Errorf("expected LocalFileBlob type, got %T", blob)
 	}
 }
+
+// file:// is accepted for agent_managed (local testing) and must reach the local
+// file store rather than the S3 store, matching validateCacheStore.
+func TestNewBlobStoreAgentManagedFile(t *testing.T) {
+	blob, err := NewBlobStore(t.Context(), AgentManaged, fileURL(t.TempDir()))
+	if err != nil {
+		t.Fatalf("NewBlobStore: %v", err)
+	}
+	if _, ok := blob.(*LocalFileBlob); !ok {
+		t.Errorf("expected LocalFileBlob type, got %T", blob)
+	}
+}

internal/cache/store/nsc.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -16,13 +17,62 @@ import (
 	"go.opentelemetry.io/otel/attribute"
 )
 
+// nscScheme is the URL scheme that routes an agent-managed cache store to NSC.
+const nscScheme = "nsc"
+
+// nscDefaultExpiry is the default artifact expiry passed to nsc artifact upload.
+// Cache entries are content-addressed and short-lived, so we cap storage growth
+// rather than relying on NSC's no-expiry default.
+//
+// TODO: 24h is a temporary stopgap. We ultimately want parity with the S3 path,
+// where the expiry can be extended (e.g. refreshed on cache hits) instead of a
+// fixed lifetime.
+const nscDefaultExpiry = "24h"
+
+// commandRunner executes an external command. It is a seam so tests can assert
+// the arguments passed to the nsc CLI without invoking the real binary.
+type commandRunner func(ctx context.Context, workingDir string, args ...string) (*CommandResult, error)
+
 // NscStore implements the Blob interface for NSC artifact storage which uses the nsc CLI tool
 // https://namespace.so/docs/reference/cli/artifact-download
 // https://namespace.so/docs/reference/cli/artifact-upload
-type NscStore struct{}
+type NscStore struct {
+	// namespace is taken from the nsc://<namespace> cache store URL and passed
+	// as --namespace to the nsc CLI. It is always non-empty.
+	namespace string
+	run       commandRunner
+}
+
+// NewNscStore creates a store backed by the nsc CLI. The namespace is parsed
+// from the nsc://<namespace> cache store URL and is required.
+func NewNscStore(bucketURL string) (*NscStore, error) {
+	namespace, err := parseNscNamespace(bucketURL)
+	if err != nil {
+		return nil, err
+	}
+	return &NscStore{namespace: namespace, run: runCommand}, nil
+}
+
+// parseNscNamespace extracts the namespace from an nsc://<namespace> cache store
+// URL. It errors on a non-nsc URL or a missing namespace.
+func parseNscNamespace(bucketURL string) (string, error) {
+	u, err := url.Parse(bucketURL)
+	if err != nil {
+		return "", fmt.Errorf("invalid cache store URL %q: %w", bucketURL, err)
+	}
+	if u.Scheme != nscScheme {
+		return "", fmt.Errorf("expected %s:// cache store URL, got %q", nscScheme, bucketURL)
+	}
+	if u.Host == "" {
+		return "", fmt.Errorf("nsc:// URL must include a namespace, e.g. nsc://my-namespace")
+	}
+	return u.Host, nil
+}
 
-func NewNscStore() (*NscStore, error) {
-	return &NscStore{}, nil
+// artifactArgs builds the nsc CLI argument list, including --namespace.
+func (n *NscStore) artifactArgs(args ...string) []string {
+	cmd := append([]string{"nsc", "artifact"}, args...)
+	return append(cmd, "--namespace", n.namespace)
 }
 
 // validateFilePath validates that a file path is safe for use in commands
@@ -98,7 +148,7 @@ func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferI
 	start := time.Now()
 
 	// Execute nsc artifact upload command
-	result, err := runCommand(ctx, "", "nsc", "artifact", "upload", filePath, key)
+	result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscDefaultExpiry)...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to execute nsc upload command: %w", err)
 	}
@@ -146,7 +196,7 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
 	start := time.Now()
 
 	// Execute nsc artifact download command
-	result, err := runCommand(ctx, "", "nsc", "artifact", "download", key, filePath)
+	result, err := n.run(ctx, "", n.artifactArgs("download", key, filePath)...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to execute nsc download command: %w", err)
 	}
@@ -200,7 +250,7 @@ func runCommand(ctx context.Context, workingDir string, args ...string) (*Comman
 
 	// #nosec G204 - args are validated by callers (validateFilePath, validateKey)
 	// and this function is internal to the store package with controlled usage
-	cmd := exec.Command(args[0], args[1:]...)
+	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
 	var stdout, stderr bytes.Buffer
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr

internal/cache/store/nsc_test.go

@@ -1,10 +1,13 @@
 package store
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestNscStore_Interface(t *testing.T) {
@@ -211,107 +214,100 @@ func TestRunCommandValidation(t *testing.T) {
 	}
 }
 
-// TestNscStore_MockUpload tests the Upload method with mocked command execution
-// Note: This test will fail if nsc is not installed, but shows the structure
-func TestNscStore_Upload_Validation(t *testing.T) {
-	store, err := NewNscStore()
-	if err != nil {
-		t.Fatalf("NewNscStore: %v", err)
+func TestParseNscNamespace(t *testing.T) {
+	tests := []struct {
+		name      string
+		url       string
+		namespace string
+		wantErr   bool
+	}{
+		{name: "nsc with namespace", url: "nsc://my-namespace", namespace: "my-namespace"},
+		{name: "not nsc", url: "s3://my-bucket", wantErr: true},
+		{name: "nsc without namespace", url: "nsc://", wantErr: true},
+		{name: "invalid url", url: "nsc://host:notaport", wantErr: true},
 	}
 
-	ctx := t.Context()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			namespace, err := parseNscNamespace(tt.url)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("parseNscNamespace(%q) err = %v, wantErr %v", tt.url, err, tt.wantErr)
+			}
+			if namespace != tt.namespace {
+				t.Errorf("parseNscNamespace(%q) namespace = %q, want %q", tt.url, namespace, tt.namespace)
+			}
+		})
+	}
+}
 
-	// Create a temporary test file
-	tmpDir, err := os.MkdirTemp("", "nsc-test")
-	if err != nil {
-		t.Fatalf("MkdirTemp: %v", err)
+// fakeRunner records the args of the last command and returns a successful result.
+func fakeRunner(captured *[]string) commandRunner {
+	return func(_ context.Context, _ string, args ...string) (*CommandResult, error) {
+		*captured = args
+		return &CommandResult{}, nil
 	}
-	defer func() { _ = os.RemoveAll(tmpDir) }()
+}
 
+func TestNscStore_PassesNamespace(t *testing.T) {
+	ctx := t.Context()
+
+	tmpDir := t.TempDir()
 	testFile := filepath.Join(tmpDir, "test.txt")
-	err = os.WriteFile(testFile, []byte("test content"), 0o600)
-	if err != nil {
+	if err := os.WriteFile(testFile, []byte("test content"), 0o600); err != nil {
 		t.Fatalf("WriteFile: %v", err)
 	}
 
-	// Test invalid file path
-	_, err = store.Upload(ctx, "invalid;path", "valid-key")
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "invalid file path") {
-		t.Errorf("error %q does not contain %q", err.Error(), "invalid file path")
-	}
+	var captured []string
+	store := &NscStore{namespace: "my-namespace", run: fakeRunner(&captured)}
 
-	// Test invalid key
-	_, err = store.Upload(ctx, testFile, "invalid key with spaces")
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "invalid key") {
-		t.Errorf("error %q does not contain %q", err.Error(), "invalid key")
+	if _, err := store.Upload(ctx, testFile, "key"); err != nil {
+		t.Fatalf("Upload: %v", err)
 	}
 
-	// Test valid inputs (will fail with nsc command error, but validation passes)
-	_, err = store.Upload(ctx, testFile, "valid-key")
-	// This will error because nsc command likely doesn't exist or isn't configured
-	// but the error should be about command execution, not validation
-	if err != nil {
-		if strings.Contains(err.Error(), "invalid file path") {
-			t.Errorf("error %q should not contain %q", err.Error(), "invalid file path")
-		}
-		if strings.Contains(err.Error(), "invalid key") {
-			t.Errorf("error %q should not contain %q", err.Error(), "invalid key")
-		}
+	wantArgs := []string{"nsc", "artifact", "upload", testFile, "key", "--expires_in", "24h", "--namespace", "my-namespace"}
+	if diff := cmp.Diff(wantArgs, captured); diff != "" {
+		t.Errorf("upload args mismatch (-want +got):\n%s", diff)
 	}
 }
 
-// TestNscStore_Download_Validation tests the Download method validation
-func TestNscStore_Download_Validation(t *testing.T) {
-	store, err := NewNscStore()
-	if err != nil {
-		t.Fatalf("NewNscStore: %v", err)
+func TestNewNscStore_RequiresNamespace(t *testing.T) {
+	if _, err := NewNscStore("nsc://"); err == nil {
+		t.Error(`NewNscStore("nsc://"): expected error, got nil`)
 	}
+}
 
+// TestNscStore_ValidationShortCircuits ensures unsafe inputs are rejected before
+// the CLI is ever invoked.
+func TestNscStore_ValidationShortCircuits(t *testing.T) {
 	ctx := t.Context()
+	ran := false
+	store := &NscStore{namespace: "ns", run: func(context.Context, string, ...string) (*CommandResult, error) {
+		ran = true
+		return &CommandResult{}, nil
+	}}
 
-	tmpDir, err := os.MkdirTemp("", "nsc-test")
-	if err != nil {
-		t.Fatalf("MkdirTemp: %v", err)
+	if _, err := store.Upload(ctx, "invalid;path", "valid-key"); err == nil {
+		t.Error("Upload with unsafe path: expected error, got nil")
 	}
-	defer func() { _ = os.RemoveAll(tmpDir) }()
-
-	destFile := filepath.Join(tmpDir, "download.txt")
-
-	// Test invalid key
-	_, err = store.Download(ctx, "invalid key with spaces", destFile)
-	if err == nil {
-		t.Fatal("expected error, got nil")
+	if _, err := store.Download(ctx, "invalid key with spaces", "dest.txt"); err == nil {
+		t.Error("Download with unsafe key: expected error, got nil")
 	}
-	if !strings.Contains(err.Error(), "invalid key") {
-		t.Errorf("error %q does not contain %q", err.Error(), "invalid key")
+	if ran {
+		t.Error("nsc CLI should not run when input validation fails")
 	}
+}
 
-	// Test invalid file path
-	_, err = store.Download(ctx, "valid-key", "invalid;path")
-	if err == nil {
-		t.Fatal("expected error, got nil")
+func TestNewBlobStore_NscScheme(t *testing.T) {
+	blob, err := NewBlobStore(t.Context(), AgentManaged, "nsc://my-namespace")
+	if err != nil {
+		t.Fatalf("NewBlobStore: %v", err)
 	}
-	if !strings.Contains(err.Error(), "invalid file path") {
-		t.Errorf("error %q does not contain %q", err.Error(), "invalid file path")
+	nsc, ok := blob.(*NscStore)
+	if !ok {
+		t.Fatalf("NewBlobStore returned %T, want *NscStore", blob)
 	}
-
-	// Test valid inputs (will fail with nsc command error, but validation passes)
-	_, err = store.Download(ctx, "valid-key", destFile)
-	// This will error because nsc command likely doesn't exist or isn't configured
-	// but the error should be about command execution, not validation
-	if err != nil {
-		if strings.Contains(err.Error(), "invalid key") {
-			t.Errorf("error %q should not contain %q", err.Error(), "invalid key")
-		}
-		if strings.Contains(err.Error(), "invalid file path") {
-			t.Errorf("error %q should not contain %q", err.Error(), "invalid file path")
-		}
+	if nsc.namespace != "my-namespace" {
+		t.Errorf("namespace = %q, want %q", nsc.namespace, "my-namespace")
 	}
 }
 
@@ -323,7 +319,8 @@ func TestNscStore_Integration(t *testing.T) {
 		t.Skip("Skipping NSC integration test (set NSC_INTEGRATION_TEST=1 to run)")
 	}
 
-	store, err := NewNscStore()
+	// "main" is the nsc CLI's default namespace.
+	store, err := NewNscStore("nsc://main")
 	if err != nil {
 		t.Fatalf("NewNscStore: %v", err)
 	}

internal/cache/store/store.go

@@ -5,8 +5,6 @@ import (
 )
 
 const (
-	// local hosted agents store type
-	LocalHostedAgents = "local_hosted_agents"
 	// local file store type
 	LocalFileStore = "local_file"
 	// agent-managed store type (cache v2): the agent manages the blob store
@@ -25,7 +23,7 @@ type TransferInfo struct {
 
 func IsValidStore(storeType string) bool {
 	switch storeType {
-	case LocalHostedAgents, LocalFileStore, AgentManaged:
+	case LocalFileStore, AgentManaged:
 		return true
 	default:
 		return false