diff --git a/README.md b/README.md index 3d36232..a7176ce 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,10 @@ backend: This setting does not control how you get the long-running `oz-agent-worker` image (see [Image releases and pinning](#image-releases-and-pinning)). It controls only the task and sidecar images the worker uses to run tasks. -`backend.docker.sidecar_image` overrides the warp-agent sidecar image reference sent by the server (e.g. `docker.io/warpdotdev/warp-agent:latest`); set this when the worker host cannot pull directly from Docker Hub and must use an internal registry mirror or pull-through cache instead. This only affects the warp-agent sidecar (mounted at `/agent`), not any additional sidecars. When using this override, you are responsible for keeping your mirror in sync with `docker.io/warpdotdev/warp-agent` — the server normally sends the correct version-matched image per task, so a stale mirror may cause version incompatibility. +`backend.docker.sidecar_image` is a legacy shorthand for mapping the official +`docker.io/warpdotdev/warp-agent` repository to a custom image. Prefer +`sidecar_image_mappings` for new configurations. Do not combine the two options. +The [remapping version rules](#remapping-sidecar-images) apply to both forms. ```yaml worker_id: "my-worker" @@ -58,6 +61,61 @@ backend: sidecar_image: "my-registry.io/warpdotdev/warp-agent:latest" ``` +### Remapping sidecar images + +Docker and Kubernetes both support `sidecar_image_mappings` under their backend +configuration. Keys are source **repositories without a tag or digest**; values +are destination image references. The mapping applies to every assigned sidecar: +the main agent, coding-agent CLI/ACP images, computer use, and any other additional +sidecar. It does not remap the task's base image, Kubernetes preflight image, or +service containers configured in `pod_template`. + +```yaml +worker_id: "my-worker" +backend: + docker: # Use kubernetes here for the same remapping behavior. + sidecar_image_mappings: + docker.io/warpdotdev/warp-agent: "registry.internal/warp-agent" + docker.io/warpdotdev/warp-claude-cli-sidecar: "registry.internal/claude:approved" + docker.io/warpdotdev/warp-xvfb-sidecar: "registry.internal/computer-use@sha256:" +``` + +- When the destination has no tag or digest, the assigned tag/digest is preserved: + `docker.io/warpdotdev/warp-agent:v123` becomes + `registry.internal/warp-agent:v123`. +- When the destination includes a tag and/or digest, that destination version + replaces the assigned version entirely. For example, any version of the Claude + image above becomes `registry.internal/claude:approved`. Replace `` with + the full digest of your mirrored computer-use image before using this config. +- Matching is by normalized repository, not prefix or pattern: + `warpdotdev/warp-agent` and `docker.io/warpdotdev/warp-agent` identify the same + source. Duplicate normalized sources, versioned source keys and malformed + references are rejected at startup. +- Mappings act once on the assigned repositories. A mapped destination is not + remapped again. Unmatched images and mount options are unchanged; no sidecars + are injected. +- Legacy `sidecar_image` and Kubernetes `coding_cli_sidecars` settings are + converted into mappings at startup. `sidecar_image` maps + `docker.io/warpdotdev/warp-agent`; each `coding_cli_sidecars` harness maps both + `docker.io/warpdotdev/warp-{harness}-cli-sidecar` and + `docker.io/warpdotdev/warp-{harness}-acp-sidecar` to its configured image. + Non-empty `sidecar_image_mappings` cannot be combined with either legacy option. + These aliases use the same version rules as explicit mappings. +- **Compatibility change:** legacy options now match only official Warp + repositories. They no longer override arbitrary source images by mount path or + inject coding-CLI sidecars missing from the assignment. Use explicit mappings + for non-Warp repositories; the server must assign a sidecar for it to be used. + +For Helm installations, configure the equivalent +`kubernetesBackend.sidecarImageMappings` map in your values file. + +Docker sidecar pulls use credentials from the worker's Docker configuration +(including credential helpers), just like task-image pulls. Make that +configuration available to the worker process when using private mirrors. +Kubernetes uses `imagePullSecrets` from `backend.kubernetes.pod_template`. +Operators remain responsible for mirroring the selected versions and ensuring +that pinned custom images are compatible with the server and agent. + ### Image releases and pinning Production self-hosted workers should pin an immutable image version instead of relying on `latest`. @@ -215,8 +273,8 @@ Notes: - `unschedulable_timeout` controls the Pod-age threshold for failing an unschedulable task; it defaults to `10m` to allow node autoscaling, and `0s` disables that fail-fast behavior - `image_pull_policy` defaults to `IfNotPresent` - the worker owns a set of names in the task container's environment and writes them last, so an entry with one of these names in `task_env` or `pod_template` is overwritten rather than honoured: `OZ_RUN_ID`, `OZ_WORKER_BACKEND`, `OZ_WORKSPACE_ROOT`, `OZ_ENVIRONMENT_FILE`, and the `WARP_`-prefixed name of each (`WARP_RUN_ID` and so on). Pick different names for operator-supplied variables. Warp reserves the `WARP_` names against managed secrets too, but a worker's own config is yours, so nothing stops you setting them — they just will not survive -- `sidecar_image` overrides the warp-agent sidecar image reference sent by the server (e.g. `docker.io/warpdotdev/warp-agent:latest`); set this when cluster nodes cannot pull directly from Docker Hub and must use an internal registry mirror or pull-through cache instead. This only affects the warp-agent sidecar (mounted at `/agent`), not any additional sidecars. When using this override, you are responsible for keeping your mirror in sync with `docker.io/warpdotdev/warp-agent` — the server normally sends the correct version-matched image per task, so a stale mirror may cause version incompatibility -- `coding_cli_sidecars` maps a harness config name (e.g. `claude`, `codex`) to a custom Docker image that will be mounted as the coding CLI sidecar for runs using that harness. When set, the worker replaces the server-provided sidecar image (or injects a new entry if the server did not send one) at the standard mount path `/mnt/{harness}-cli-sidecar`. Use this when your cluster uses a custom or internal Claude Code binary wrapper instead of the Warp-provided image. Example: +- `sidecar_image` is a legacy shorthand for mapping the official warp-agent repository to a custom image, with the same [remapping rules](#remapping-sidecar-images) as Docker +- `coding_cli_sidecars` is a legacy shorthand mapping a harness config name (e.g. `claude`, `codex`) to a custom image for its official Warp CLI and ACP repositories. It affects only assigned sidecars; it does not inject missing sidecars or match by mount path. Neither legacy option can be combined with non-empty `sidecar_image_mappings`. Example: ```yaml backend: diff --git a/charts/oz-agent-worker/templates/configmap.yaml b/charts/oz-agent-worker/templates/configmap.yaml index c686570..2d5a3d5 100644 --- a/charts/oz-agent-worker/templates/configmap.yaml +++ b/charts/oz-agent-worker/templates/configmap.yaml @@ -30,6 +30,10 @@ data: {{- end }} {{- if .Values.kubernetesBackend.sidecarImage }} sidecar_image: {{ .Values.kubernetesBackend.sidecarImage | quote }} + {{- end }} + {{- with .Values.kubernetesBackend.sidecarImageMappings }} + sidecar_image_mappings: +{{ toYaml . | indent 10 }} {{- end }} {{- if .Values.kubernetesBackend.setupCommand }} setup_command: |- diff --git a/charts/oz-agent-worker/values.yaml b/charts/oz-agent-worker/values.yaml index d51a37f..ce66e2f 100644 --- a/charts/oz-agent-worker/values.yaml +++ b/charts/oz-agent-worker/values.yaml @@ -85,6 +85,10 @@ kubernetesBackend: useImageVolumes: null preflightImage: "" sidecarImage: "" + # Mapping from Warp-provided sidecar images to mirrored repositories. If the destination + # repository includes a tag or digest, that takes precedence over any server-provided + # version. + sidecarImageMappings: {} setupCommand: "" teardownCommand: "" extraLabels: {} diff --git a/internal/common/image_remapping.go b/internal/common/image_remapping.go new file mode 100644 index 0000000..07e1a7d --- /dev/null +++ b/internal/common/image_remapping.go @@ -0,0 +1,65 @@ +package common + +import ( + // Register the OCI digest algorithms used by reference parsing. + _ "crypto/sha256" + _ "crypto/sha512" + "fmt" + "sort" + "strings" + + "github.com/distribution/reference" +) + +// ImageRemapper matches normalized repositories, independently of tags and digests. +type ImageRemapper struct { + repositories map[string]reference.Named +} + +func NewImageRemapper(mappings map[string]string) (*ImageRemapper, error) { + remapper := &ImageRemapper{repositories: make(map[string]reference.Named, len(mappings))} + sources := make([]string, 0, len(mappings)) + for source := range mappings { + sources = append(sources, source) + } + sort.Strings(sources) + for _, source := range sources { + from, err := reference.ParseNormalizedNamed(source) + if err != nil { + return nil, fmt.Errorf("invalid source repository %q: %w", source, err) + } + if !reference.IsNameOnly(from) { + return nil, fmt.Errorf("source repository %q must not include a tag or digest", source) + } + if _, exists := remapper.repositories[from.String()]; exists { + return nil, fmt.Errorf("duplicate source repository %q after normalization", source) + } + to, err := reference.ParseNormalizedNamed(mappings[source]) + if err != nil { + return nil, fmt.Errorf("invalid destination for repository %q: %w", source, err) + } + remapper.repositories[from.String()] = to + } + return remapper, nil +} + +// Remap applies one mapping. A pinned destination replaces the source version; +// otherwise the source's tag and digest are carried over to the new repository. +func (r *ImageRemapper) Remap(image string) string { + if r == nil || len(r.repositories) == 0 { + return image + } + from, err := reference.ParseNormalizedNamed(image) + if err != nil { + return image + } + repository := reference.TrimNamed(from).String() + to, ok := r.repositories[repository] + if !ok { + return image + } + if !reference.IsNameOnly(to) { + return to.String() + } + return to.String() + strings.TrimPrefix(from.String(), repository) +} diff --git a/internal/common/image_remapping_test.go b/internal/common/image_remapping_test.go new file mode 100644 index 0000000..da39e72 --- /dev/null +++ b/internal/common/image_remapping_test.go @@ -0,0 +1,63 @@ +package common + +import ( + "strings" + "testing" +) + +func TestImageRemapper(t *testing.T) { + digest := "sha256:" + strings.Repeat("a", 64) + otherDigest := "sha256:" + strings.Repeat("b", 64) + for _, tc := range []struct { + name, source, destination, image, want string + }{ + {"digest preserved", "warpdotdev/warp-agent", "mirror.example/agent", "warpdotdev/warp-agent@" + digest, "mirror.example/agent@" + digest}, + {"tag and digest preserved", "warpdotdev/warp-agent", "mirror.example/agent", "warpdotdev/warp-agent:v123@" + digest, "mirror.example/agent:v123@" + digest}, + {"tag pin replaces source digest", "warpdotdev/warp-agent", "mirror.example/agent:approved", "warpdotdev/warp-agent:v123@" + digest, "mirror.example/agent:approved"}, + {"tag and digest pin", "warpdotdev/warp-agent", "mirror.example/agent:approved@" + otherDigest, "warpdotdev/warp-agent@" + digest, "mirror.example/agent:approved@" + otherDigest}, + {"implicit latest", "warpdotdev/warp-agent", "mirror.example/agent", "warpdotdev/warp-agent", "mirror.example/agent"}, + {"registry port", "source.example:5000/agent", "mirror.example:5001/agent", "source.example:5000/agent:v1", "mirror.example:5001/agent:v1"}, + {"default library repository", "busybox", "mirror.example/busybox", "docker.io/library/busybox:1.36", "mirror.example/busybox:1.36"}, + {"exact repository match", "warpdotdev/warp-agent", "mirror.example/agent", "warpdotdev/warp-agent-extra:v123", "warpdotdev/warp-agent-extra:v123"}, + {"invalid assignment left for backend validation", "warpdotdev/warp-agent", "mirror.example/agent", "invalid image", "invalid image"}, + } { + t.Run(tc.name, func(t *testing.T) { + remapper, err := NewImageRemapper(map[string]string{tc.source: tc.destination}) + if err != nil { + t.Fatal(err) + } + if got := remapper.Remap(tc.image); got != tc.want { + t.Fatalf("Remap(%q) = %q, want %q", tc.image, got, tc.want) + } + }) + } +} + +func TestImageRemapperRejectsInvalidMappings(t *testing.T) { + for _, mappings := range []map[string]string{ + {"": "mirror.example/agent"}, + {"bad source": "mirror.example/agent"}, + {"warpdotdev/warp-agent:latest": "mirror.example/agent"}, + {"warpdotdev/warp-agent@sha256:" + strings.Repeat("a", 64): "mirror.example/agent"}, + {"warpdotdev/warp-agent": ""}, + {"warpdotdev/warp-agent": "https://mirror.example/agent"}, + {"warpdotdev/warp-agent": "mirror.example/agent@sha256:bad"}, + {"warpdotdev/warp-agent": "mirror.example/agent", "docker.io/warpdotdev/warp-agent": "other.example/agent"}, + } { + if _, err := NewImageRemapper(mappings); err == nil { + t.Fatalf("expected invalid mapping error for %v", mappings) + } + } +} + +func TestImageRemapperDoesNotChainMappings(t *testing.T) { + mappings := map[string]string{"source.example/agent": "mirror.example/agent", "mirror.example/agent": "other.example/agent"} + remapper, err := NewImageRemapper(mappings) + if err != nil { + t.Fatal(err) + } + mappings["source.example/agent"] = "mutated.example/agent" + if got := remapper.Remap("source.example/agent:v1"); got != "mirror.example/agent:v1" { + t.Fatalf("Remap = %q, want single-pass immutable mapping", got) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index d2a7aab..e4276ad 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,9 +57,10 @@ type DockerConfig struct { // Kubernetes backend's image_pull_policy: Always, IfNotPresent, Never. Unlike the // Kubernetes backend, an omitted value defaults to Always here, preserving the Docker // backend's original unconditional-pull behavior for existing installations. - ImagePullPolicy string `yaml:"image_pull_policy" validate:"omitempty,oneof=Always Never IfNotPresent"` - SidecarImage string `yaml:"sidecar_image" validate:"omitempty,no_whitespace"` - Environment []EnvEntry `yaml:"environment" validate:"dive"` + ImagePullPolicy string `yaml:"image_pull_policy" validate:"omitempty,oneof=Always Never IfNotPresent"` + SidecarImage string `yaml:"sidecar_image" validate:"omitempty,no_whitespace"` + SidecarImageMappings map[string]string `yaml:"sidecar_image_mappings"` + Environment []EnvEntry `yaml:"environment" validate:"dive"` } // DirectConfig holds direct-backend-specific configuration. @@ -82,6 +83,7 @@ type KubernetesConfig struct { UseImageVolumes *bool `yaml:"use_image_volumes"` PreflightImage string `yaml:"preflight_image" validate:"omitempty,no_whitespace"` SidecarImage string `yaml:"sidecar_image" validate:"omitempty,no_whitespace"` + SidecarImageMappings map[string]string `yaml:"sidecar_image_mappings"` SetupCommand string `yaml:"setup_command"` TeardownCommand string `yaml:"teardown_command"` ExtraLabels map[string]string `yaml:"extra_labels"` @@ -90,13 +92,9 @@ type KubernetesConfig struct { TTLSecondsAfterFinish *int32 `yaml:"ttl_seconds_after_finished"` WorkspaceSizeLimit string `yaml:"workspace_size_limit"` UnschedulableTimeout *string `yaml:"unschedulable_timeout"` - // CodingCLISidecars maps harness config name (e.g. "claude", "codex") to a custom - // Docker image. When set, the worker overrides the server-provided sidecar image for - // that harness (or injects a new sidecar entry if the server did not send one), - // mounting it at /mnt/{harness}-cli-sidecar. Use this to bring your own Claude Code - // wrapper or custom harness binary instead of the Warp-provided sidecar image. - // The custom image must place the harness binary in the path where the Warp agent - // entrypoint expects it (e.g. /usr/local/bin for most sidecar layouts). + // CodingCLISidecars is a compatibility alias for mappings from the official + // Warp CLI and ACP repositories for each harness to a custom image. + // It cannot be combined with SidecarImageMappings and does not inject sidecars. CodingCLISidecars map[string]string `yaml:"coding_cli_sidecars"` // PodTemplate holds a raw Kubernetes PodSpec that is merged with the worker's // required fields at runtime. Declarative task Job configuration such as diff --git a/internal/worker/docker.go b/internal/worker/docker.go index 65361f9..aea3ac6 100644 --- a/internal/worker/docker.go +++ b/internal/worker/docker.go @@ -47,7 +47,6 @@ type DockerBackendConfig struct { // empty value defaults to PullPolicyAlways, preserving the backend's original // unconditional-pull behavior for installations that don't set it. ImagePullPolicy string - SidecarImage string } type sidecarVolumeState struct { @@ -676,8 +675,8 @@ func (b *DockerBackend) prepareSidecars(ctx context.Context, dockerClient *clien log.Infof(ctx, "Preparing additional sidecar: image=%s, mount=%s", sidecar.Image, sidecar.MountPath) - // Additional sidecar images are public, so no auth is needed when a pull is required. - if err := b.prepareImage(ctx, sidecar.Image, "", nil); err != nil { + authStr := b.getRegistryAuth(ctx, sidecar.Image) + if err := b.prepareImage(ctx, sidecar.Image, authStr, nil); err != nil { return nil, fmt.Errorf("failed to prepare additional sidecar image %s: %w", sidecar.Image, err) } diff --git a/internal/worker/kubernetes.go b/internal/worker/kubernetes.go index 70b5e95..c9a3685 100644 --- a/internal/worker/kubernetes.go +++ b/internal/worker/kubernetes.go @@ -97,7 +97,6 @@ type KubernetesBackendConfig struct { ImagePullPolicy string UseImageVolumes *bool PreflightImage string - SidecarImage string SetupCommand string TeardownCommand string NoCleanup bool @@ -107,10 +106,6 @@ type KubernetesBackendConfig struct { TTLSecondsAfterFinish *int32 WorkspaceSizeLimit *resource.Quantity UnschedulableTimeout *time.Duration - // CodingCLISidecars maps harness config name (e.g. "claude", "codex") to a custom - // Docker image for the coding CLI sidecar. When non-empty for a task's harness, - // the worker overrides or injects the sidecar at /mnt/{harness}-cli-sidecar. - CodingCLISidecars map[string]string // TaskEnv contains runtime-only env overrides from CLI -e/--env. Declarative // task container env in file or Helm config should be set in PodTemplate. TaskEnv map[string]string diff --git a/internal/worker/sidecar_image_mappings_test.go b/internal/worker/sidecar_image_mappings_test.go new file mode 100644 index 0000000..80cfc84 --- /dev/null +++ b/internal/worker/sidecar_image_mappings_test.go @@ -0,0 +1,179 @@ +package worker + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "net/http" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + cliconfig "github.com/docker/cli/cli/config" + "github.com/moby/moby/api/pkg/authconfig" + "github.com/warpdotdev/oz-agent-worker/internal/common" + "github.com/warpdotdev/oz-agent-worker/internal/types" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func TestSidecarImageMappingsBackends(t *testing.T) { + digest := "sha256:" + strings.Repeat("b", 64) + remapper, err := common.NewImageRemapper(map[string]string{ + "warpdotdev/warp-agent": "mirror.example/agent", + "warpdotdev/warp-claude-cli-sidecar": "mirror.example/claude:approved", + "warpdotdev/warp-xvfb-sidecar": "mirror.example/computer-use@" + digest, + "ubuntu": "mirror.example/not-a-sidecar", + }) + if err != nil { + t.Fatal(err) + } + for _, mode := range []string{"docker", "kubernetes-copy", "kubernetes-image-volume"} { + t.Run(mode, func(t *testing.T) { + assignment := &types.TaskAssignmentMessage{ + TaskID: "task-1", + Task: &types.Task{ID: "task-1"}, + DockerImage: "ubuntu:22.04", + SidecarImage: "docker.io/warpdotdev/warp-agent:v123", + AdditionalSidecars: []types.SidecarMount{ + {Image: "warpdotdev/warp-claude-cli-sidecar:v2", MountPath: "/mnt/claude-cli-sidecar"}, + {Image: "warpdotdev/warp-xvfb-sidecar:v3", MountPath: "/mnt/xvfb-sidecar"}, + {Image: "other.example/extra:v1", MountPath: "/mnt/extra"}, + }, + } + original := append([]types.SidecarMount(nil), assignment.AdditionalSidecars...) + w := &Worker{ctx: context.Background(), config: Config{SidecarImageMappings: remapper}} + params := w.prepareTaskParams(assignment) + want := append([]types.SidecarMount{{Image: "mirror.example/agent:v123", MountPath: "/agent"}}, original...) + want[1].Image = "mirror.example/claude:approved" + want[2].Image = "mirror.example/computer-use@" + digest + if !reflect.DeepEqual(params.Sidecars, want) || !reflect.DeepEqual(assignment.AdditionalSidecars, original) { + t.Fatalf("sidecars = %+v; assignment = %+v; want %+v without mutating assignment", params.Sidecars, assignment.AdditionalSidecars, want) + } + if params.DockerImage != assignment.DockerImage || assignment.SidecarImage != "docker.io/warpdotdev/warp-agent:v123" { + t.Fatal("mapping changed the task image or main sidecar assignment") + } + + if mode == "docker" { + configDir := t.TempDir() + oldDir := cliconfig.Dir() + cliconfig.SetDir(configDir) + t.Cleanup(func() { cliconfig.SetDir(oldDir) }) + config := fmt.Sprintf(`{"auths":{"mirror.example":{"auth":%q}}}`, base64.StdEncoding.EncodeToString([]byte("mirror-test:fixture"))) + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(config), 0600); err != nil { + t.Fatal(err) + } + var pulled []string + engine := &fakeDockerEngine{ + t: t, volumeFound: true, + inspectResponses: []func(*http.Request) *http.Response{ + func(req *http.Request) *http.Response { + return mockImageInspectResponse(t, req, "linux", "amd64", "sha256:fixture") + }, + }, + pullResponse: func(req *http.Request) *http.Response { + ref := req.URL.Query().Get("fromImage") + if tag := req.URL.Query().Get("tag"); tag != "" { + if strings.HasPrefix(tag, "sha256:") { + ref += "@" + tag + } else { + ref += ":" + tag + } + } + pulled = append(pulled, ref) + if strings.HasPrefix(ref, "mirror.example/") { + auth, err := authconfig.Decode(req.Header.Get("X-Registry-Auth")) + if err != nil || auth.Username != "mirror-test" { + t.Fatal("sidecar pull did not use destination registry credentials") + } + } + return mockEngineResponse(req, http.StatusOK, "application/json", "{}\n") + }, + } + backend := newTestDockerBackend(t, engine, PullPolicyAlways) + binds, err := backend.prepareSidecars(context.Background(), backend.dockerClient, params.Sidecars, nil) + if err != nil { + t.Fatal(err) + } + for i, sidecar := range want { + if i >= len(pulled) || pulled[i] != sidecar.Image { + t.Fatalf("Docker pulled %v, want sidecar %q at index %d", pulled, sidecar.Image, i) + } + if i >= len(binds) || !strings.HasSuffix(binds[i], ":"+sidecar.MountPath+":ro") { + t.Fatalf("Docker binds = %v, missing original mount %q", binds, sidecar.MountPath) + } + } + return + } + + client := fake.NewSimpleClientset() + stop := errors.New("captured task Job") + var pod *corev1.PodSpec + client.PrependReactor("create", "jobs", func(action k8stesting.Action) (bool, runtime.Object, error) { + pod = &action.(k8stesting.CreateAction).GetObject().(*batchv1.Job).Spec.Template.Spec + return true, nil, stop + }) + backend := &KubernetesBackend{ + config: KubernetesBackendConfig{Namespace: "agents"}, + clientset: client, + useImageVolumes: mode == "kubernetes-image-volume", + } + if result := backend.ExecuteTask(context.Background(), params); !errors.Is(result.Error, stop) { + t.Fatalf("ExecuteTask error = %v, want captured Job", result.Error) + } + for i, sidecar := range want { + if mode == "kubernetes-copy" { + if pod.InitContainers[i].Image != sidecar.Image { + t.Fatalf("init container image = %q, want %q", pod.InitContainers[i].Image, sidecar.Image) + } + } else if image := pod.Volumes[i+1].Image; image == nil || image.Reference != sidecar.Image { + t.Fatalf("image volume = %+v, want %q", image, sidecar.Image) + } + mount := pod.Containers[0].VolumeMounts[i+1] + if mount.MountPath != sidecar.MountPath || !mount.ReadOnly { + t.Fatalf("Kubernetes mount = %+v, want read-only %q", mount, sidecar.MountPath) + } + } + }) + } +} + +func TestSidecarImageMappingsPreserveAssignedSidecars(t *testing.T) { + remapper, err := common.NewImageRemapper(map[string]string{ + "warpdotdev/warp-agent": "mirror.example/agent", + "warpdotdev/warp-claude-cli-sidecar": "mirror.example/claude:approved", + }) + if err != nil { + t.Fatal(err) + } + harness := "claude" + w := &Worker{ctx: context.Background(), config: Config{SidecarImageMappings: remapper}} + assignment := &types.TaskAssignmentMessage{ + TaskID: "task-1", + Task: &types.Task{ID: "task-1", AgentConfigSnapshot: &types.AmbientAgentConfig{Harness: &types.Harness{Type: &harness}}}, + SidecarImage: "custom.example/agent:v1", + AdditionalSidecars: []types.SidecarMount{ + {Image: "custom.example/claude:v2", MountPath: "/mnt/claude-cli-sidecar"}, + {Image: "warpdotdev/warp-claude-cli-sidecar:v3", MountPath: "/mnt/other", ReadWrite: true}, + }, + } + want := []types.SidecarMount{ + {Image: "custom.example/agent:v1", MountPath: "/agent"}, + assignment.AdditionalSidecars[0], + {Image: "mirror.example/claude:approved", MountPath: "/mnt/other", ReadWrite: true}, + } + if got := w.prepareTaskParams(assignment).Sidecars; !reflect.DeepEqual(got, want) { + t.Fatalf("sidecars = %+v, want %+v", got, want) + } + assignment.SidecarImage = "" + assignment.AdditionalSidecars = nil + if got := w.prepareTaskParams(assignment).Sidecars; len(got) != 0 { + t.Fatalf("empty assignment injected sidecars: %+v", got) + } +} diff --git a/internal/worker/worker.go b/internal/worker/worker.go index 4976dd5..3012519 100644 --- a/internal/worker/worker.go +++ b/internal/worker/worker.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "net/url" - "strings" "sync" "time" @@ -63,6 +62,7 @@ type Config struct { SessionSharingServerURL string CollectTaskLogs bool Tracing *tracing.Handle + SidecarImageMappings *common.ImageRemapper // Backend-specific configs. Only the one matching BackendType should be set. Docker *DockerBackendConfig @@ -811,47 +811,15 @@ func (w *Worker) prepareTaskParams(assignment *types.TaskAssignmentMessage) *Tas // entrypoint.sh lives) comes first, followed by any additional sidecars. var sidecars []types.SidecarMount if assignment.SidecarImage != "" { - sidecarImage := assignment.SidecarImage - if override := w.configuredWarpAgentSidecarImage(); override != "" { - log.Infof(w.ctx, "Overriding server sidecar image %s with configured sidecar image %s", assignment.SidecarImage, override) - sidecarImage = override - } sidecars = append(sidecars, types.SidecarMount{ - Image: sidecarImage, + Image: assignment.SidecarImage, MountPath: "/agent", }) } sidecars = append(sidecars, assignment.AdditionalSidecars...) - // Apply worker-configured coding CLI sidecar overrides. - // For each harness entry in the worker's coding_cli_sidecars config, replace the - // server-provided sidecar image at /mnt/{harness}-cli-sidecar or inject a new entry - // when the server did not send one (e.g. because no Warp-side image is configured). - if w.config.Kubernetes != nil && len(w.config.Kubernetes.CodingCLISidecars) > 0 { - if task != nil && task.AgentConfigSnapshot != nil && - task.AgentConfigSnapshot.Harness != nil && - task.AgentConfigSnapshot.Harness.Type != nil { - harnessType := strings.TrimSpace(*task.AgentConfigSnapshot.Harness.Type) - if customImage, ok := w.config.Kubernetes.CodingCLISidecars[harnessType]; ok && customImage != "" { - mountPath := fmt.Sprintf("/mnt/%s-cli-sidecar", harnessType) - overridden := false - for i, s := range sidecars { - if s.MountPath == mountPath { - log.Infof(w.ctx, "Overriding server coding CLI sidecar %s with configured image %s for harness %s", s.Image, customImage, harnessType) - sidecars[i].Image = customImage - overridden = true - break - } - } - if !overridden { - log.Infof(w.ctx, "Injecting configured coding CLI sidecar %s for harness %s at %s", customImage, harnessType, mountPath) - sidecars = append(sidecars, types.SidecarMount{ - Image: customImage, - MountPath: mountPath, - }) - } - } - } + for i := range sidecars { + sidecars[i].Image = w.config.SidecarImageMappings.Remap(sidecars[i].Image) } setupEvents := newSetupEventReporter(w.config.ServerRootURL, assignment) @@ -879,19 +847,6 @@ func (w *Worker) prepareTaskParams(assignment *types.TaskAssignmentMessage) *Tas } } -// configuredWarpAgentSidecarImage returns the operator-configured warp-agent sidecar -// image, or empty if neither backend set one. Only one backend config is populated -// at runtime. -func (w *Worker) configuredWarpAgentSidecarImage() string { - if w.config.Kubernetes != nil && w.config.Kubernetes.SidecarImage != "" { - return w.config.Kubernetes.SidecarImage - } - if w.config.Docker != nil && w.config.Docker.SidecarImage != "" { - return w.config.Docker.SidecarImage - } - return "" -} - // defaultImageForTask returns the Docker image to use for a task, applying the // precedence: server-provided > worker config default_image > hardcoded fallback. func (w *Worker) defaultImageForTask(assignmentImage string, task *types.Task) string { diff --git a/internal/worker/worker_test.go b/internal/worker/worker_test.go index c5985b7..70b53b7 100644 --- a/internal/worker/worker_test.go +++ b/internal/worker/worker_test.go @@ -1131,234 +1131,6 @@ func TestDefaultImageForTask(t *testing.T) { }) } -func TestPrepareTaskParamsSidecarImageOverride(t *testing.T) { - const ( - serverImage = "docker.io/warpdotdev/warp-agent:latest" - overrideImage = "my-registry.io/warpdotdev/warp-agent:latest" - ) - - newK8sWorker := func(sidecarImage string) *Worker { - return &Worker{ - ctx: context.Background(), - config: Config{ - Kubernetes: &KubernetesBackendConfig{SidecarImage: sidecarImage}, - }, - } - } - newDockerWorker := func(sidecarImage string) *Worker { - return &Worker{ - ctx: context.Background(), - config: Config{ - Docker: &DockerBackendConfig{SidecarImage: sidecarImage}, - }, - } - } - assignment := func(sidecarImage string, extra ...types.SidecarMount) *types.TaskAssignmentMessage { - return &types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: &types.Task{ID: "task-1"}, - SidecarImage: sidecarImage, - AdditionalSidecars: extra, - } - } - - assertAgentSidecar := func(t *testing.T, params *TaskParams, wantImage string) { - t.Helper() - if len(params.Sidecars) == 0 { - t.Fatal("expected at least one sidecar") - } - if params.Sidecars[0].Image != wantImage { - t.Errorf("sidecar image = %q, want %q", params.Sidecars[0].Image, wantImage) - } - if params.Sidecars[0].MountPath != "/agent" { - t.Errorf("sidecar mount path = %q, want /agent", params.Sidecars[0].MountPath) - } - } - - t.Run("kubernetes config sidecar_image overrides server-provided image", func(t *testing.T) { - params := newK8sWorker(overrideImage).prepareTaskParams(assignment(serverImage)) - assertAgentSidecar(t, params, overrideImage) - }) - - t.Run("docker config sidecar_image overrides server-provided image", func(t *testing.T) { - params := newDockerWorker(overrideImage).prepareTaskParams(assignment(serverImage)) - assertAgentSidecar(t, params, overrideImage) - }) - - t.Run("server-provided image used when kubernetes config sidecar_image empty", func(t *testing.T) { - params := newK8sWorker("").prepareTaskParams(assignment(serverImage)) - assertAgentSidecar(t, params, serverImage) - }) - - t.Run("server-provided image used when docker config sidecar_image empty", func(t *testing.T) { - params := newDockerWorker("").prepareTaskParams(assignment(serverImage)) - assertAgentSidecar(t, params, serverImage) - }) - - t.Run("server-provided image used when docker config is nil", func(t *testing.T) { - w := &Worker{ctx: context.Background(), config: Config{}} - params := w.prepareTaskParams(assignment(serverImage)) - assertAgentSidecar(t, params, serverImage) - }) - - t.Run("no sidecar when server provides empty sidecar image with kubernetes override", func(t *testing.T) { - params := newK8sWorker(overrideImage).prepareTaskParams(assignment("")) - if len(params.Sidecars) != 0 { - t.Errorf("expected no sidecars when server sidecar image is empty, got %d", len(params.Sidecars)) - } - }) - - t.Run("no sidecar when server provides empty sidecar image with docker override", func(t *testing.T) { - params := newDockerWorker(overrideImage).prepareTaskParams(assignment("")) - if len(params.Sidecars) != 0 { - t.Errorf("expected no sidecars when server sidecar image is empty, got %d", len(params.Sidecars)) - } - }) - - t.Run("docker override does not rewrite additional sidecars", func(t *testing.T) { - extra := types.SidecarMount{Image: "docker.io/warpdotdev/extra:latest", MountPath: "/mnt/extra"} - params := newDockerWorker(overrideImage).prepareTaskParams(assignment(serverImage, extra)) - if len(params.Sidecars) != 2 { - t.Fatalf("sidecar count = %d, want 2: %+v", len(params.Sidecars), params.Sidecars) - } - assertAgentSidecar(t, params, overrideImage) - if params.Sidecars[1] != extra { - t.Errorf("additional sidecar = %+v, want %+v", params.Sidecars[1], extra) - } - }) -} - -func TestPrepareTaskParamsCodingCLISidecarOverride(t *testing.T) { - newWorker := func(codingCLISidecars map[string]string) *Worker { - return &Worker{ - ctx: context.Background(), - config: Config{ - Kubernetes: &KubernetesBackendConfig{ - CodingCLISidecars: codingCLISidecars, - }, - }, - } - } - - strPtr := func(s string) *string { return &s } - - // harnessTask builds a task whose agent config snapshot advertises the given - // harness type. A nil harnessType produces a snapshot with a harness whose - // Type pointer is nil. - harnessTask := func(harnessType *string) *types.Task { - return &types.Task{ - ID: "task-1", - AgentConfigSnapshot: &types.AmbientAgentConfig{ - Harness: &types.Harness{Type: harnessType}, - }, - } - } - - findSidecar := func(sidecars []types.SidecarMount, mountPath string) (types.SidecarMount, bool) { - for _, s := range sidecars { - if s.MountPath == mountPath { - return s, true - } - } - return types.SidecarMount{}, false - } - - t.Run("overrides server-provided coding CLI sidecar at mount path", func(t *testing.T) { - w := newWorker(map[string]string{"claude": "registry.internal/my-claude:v1"}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: harnessTask(strPtr("claude")), - SidecarImage: "docker.io/warpdotdev/warp-agent:latest", - AdditionalSidecars: []types.SidecarMount{ - {Image: "docker.io/warpdotdev/claude-cli:latest", MountPath: "/mnt/claude-cli-sidecar", ReadWrite: true}, - }, - }) - - got, ok := findSidecar(params.Sidecars, "/mnt/claude-cli-sidecar") - if !ok { - t.Fatalf("expected coding CLI sidecar at /mnt/claude-cli-sidecar, got %+v", params.Sidecars) - } - if got.Image != "registry.internal/my-claude:v1" { - t.Errorf("image = %q, want %q", got.Image, "registry.internal/my-claude:v1") - } - // Overriding only replaces the image; other mount options are preserved. - if !got.ReadWrite { - t.Error("expected ReadWrite to be preserved on overridden sidecar") - } - // No new sidecar should be injected: the /agent + claude sidecars remain. - if len(params.Sidecars) != 2 { - t.Fatalf("sidecar count = %d, want 2: %+v", len(params.Sidecars), params.Sidecars) - } - // The warp-agent sidecar at /agent must be untouched. - if agent, ok := findSidecar(params.Sidecars, "/agent"); !ok || agent.Image != "docker.io/warpdotdev/warp-agent:latest" { - t.Errorf("agent sidecar = %+v, want image docker.io/warpdotdev/warp-agent:latest", agent) - } - }) - - t.Run("injects coding CLI sidecar when server did not provide one", func(t *testing.T) { - w := newWorker(map[string]string{"claude": "registry.internal/my-claude:v1"}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: harnessTask(strPtr("claude")), - }) - - got, ok := findSidecar(params.Sidecars, "/mnt/claude-cli-sidecar") - if !ok { - t.Fatalf("expected injected coding CLI sidecar at /mnt/claude-cli-sidecar, got %+v", params.Sidecars) - } - if got.Image != "registry.internal/my-claude:v1" { - t.Errorf("image = %q, want %q", got.Image, "registry.internal/my-claude:v1") - } - }) - - t.Run("trims whitespace from harness type", func(t *testing.T) { - w := newWorker(map[string]string{"claude": "registry.internal/my-claude:v1"}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: harnessTask(strPtr(" claude ")), - }) - if _, ok := findSidecar(params.Sidecars, "/mnt/claude-cli-sidecar"); !ok { - t.Fatalf("expected coding CLI sidecar at /mnt/claude-cli-sidecar, got %+v", params.Sidecars) - } - }) - - t.Run("no override when harness type not configured", func(t *testing.T) { - w := newWorker(map[string]string{"claude": "registry.internal/my-claude:v1"}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: harnessTask(strPtr("codex")), - }) - if _, ok := findSidecar(params.Sidecars, "/mnt/codex-cli-sidecar"); ok { - t.Error("did not expect coding CLI sidecar for unconfigured harness") - } - if _, ok := findSidecar(params.Sidecars, "/mnt/claude-cli-sidecar"); ok { - t.Error("did not expect claude coding CLI sidecar for codex harness") - } - }) - - t.Run("no override when configured image is empty", func(t *testing.T) { - w := newWorker(map[string]string{"claude": ""}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: harnessTask(strPtr("claude")), - }) - if _, ok := findSidecar(params.Sidecars, "/mnt/claude-cli-sidecar"); ok { - t.Error("did not expect coding CLI sidecar when configured image is empty") - } - }) - - t.Run("no override when task has no harness snapshot", func(t *testing.T) { - w := newWorker(map[string]string{"claude": "registry.internal/my-claude:v1"}) - params := w.prepareTaskParams(&types.TaskAssignmentMessage{ - TaskID: "task-1", - Task: &types.Task{ID: "task-1"}, - }) - if len(params.Sidecars) != 0 { - t.Errorf("expected no sidecars when task has no harness, got %d: %+v", len(params.Sidecars), params.Sidecars) - } - }) -} - func TestPrepareTaskParamsTeamShareConditional(t *testing.T) { newWorker := func() *Worker { return &Worker{ diff --git a/main.go b/main.go index 0c48933..0ca4660 100644 --- a/main.go +++ b/main.go @@ -10,6 +10,7 @@ import ( "time" "github.com/alecthomas/kong" + "github.com/warpdotdev/oz-agent-worker/internal/common" "github.com/warpdotdev/oz-agent-worker/internal/config" "github.com/warpdotdev/oz-agent-worker/internal/log" "github.com/warpdotdev/oz-agent-worker/internal/metrics" @@ -195,6 +196,11 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { SessionSharingServerURL: CLI.SessionSharingServerURL, } + var ( + sidecarImageMappings map[string]string + legacySidecarImage string + legacyCodingCLISidecars map[string]string + ) switch backendType { case "kubernetes": var ( @@ -204,8 +210,6 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { imagePullPolicy string useImageVolumes *bool preflightImage string - sidecarImage string - codingCLISidecars map[string]string setupCmd string teardownCmd string extraLabels map[string]string @@ -226,8 +230,9 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { imagePullPolicy = kc.ImagePullPolicy useImageVolumes = kc.UseImageVolumes preflightImage = kc.PreflightImage - sidecarImage = kc.SidecarImage - codingCLISidecars = copyStringMap(kc.CodingCLISidecars) + legacySidecarImage = kc.SidecarImage + sidecarImageMappings = kc.SidecarImageMappings + legacyCodingCLISidecars = kc.CodingCLISidecars setupCmd = kc.SetupCommand teardownCmd = kc.TeardownCommand extraLabels = copyStringMap(kc.ExtraLabels) @@ -280,8 +285,6 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { ImagePullPolicy: imagePullPolicy, UseImageVolumes: useImageVolumes, PreflightImage: preflightImage, - SidecarImage: sidecarImage, - CodingCLISidecars: codingCLISidecars, SetupCommand: setupCmd, TeardownCommand: teardownCmd, NoCleanup: noCleanup, @@ -374,11 +377,12 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { // Merge volumes: config file + CLI (concatenated). var volumes []string - var imagePullPolicy, sidecarImage string + var imagePullPolicy string if fileConfig != nil && fileConfig.Backend.Docker != nil { volumes = append(volumes, fileConfig.Backend.Docker.Volumes...) imagePullPolicy = fileConfig.Backend.Docker.ImagePullPolicy - sidecarImage = fileConfig.Backend.Docker.SidecarImage + legacySidecarImage = fileConfig.Backend.Docker.SidecarImage + sidecarImageMappings = fileConfig.Backend.Docker.SidecarImageMappings } volumes = append(volumes, CLI.Volumes...) @@ -387,10 +391,33 @@ func mergeConfig(fileConfig *config.FileConfig) (worker.Config, error) { Volumes: volumes, Env: mergedEnv, ImagePullPolicy: imagePullPolicy, - SidecarImage: sidecarImage, } } + if len(sidecarImageMappings) > 0 { + if legacySidecarImage != "" { + return worker.Config{}, fmt.Errorf("backend.%s.sidecar_image_mappings cannot be combined with sidecar_image", backendType) + } + if len(legacyCodingCLISidecars) > 0 { + return worker.Config{}, fmt.Errorf("backend.%s.sidecar_image_mappings cannot be combined with coding_cli_sidecars", backendType) + } + } else { + sidecarImageMappings = make(map[string]string) + if legacySidecarImage != "" { + sidecarImageMappings["warpdotdev/warp-agent"] = legacySidecarImage + } + for harness, image := range legacyCodingCLISidecars { + if image == "" { + continue + } + sidecarImageMappings[fmt.Sprintf("warpdotdev/warp-%s-cli-sidecar", harness)] = image + sidecarImageMappings[fmt.Sprintf("warpdotdev/warp-%s-acp-sidecar", harness)] = image + } + } + wc.SidecarImageMappings, err = common.NewImageRemapper(sidecarImageMappings) + if err != nil { + return worker.Config{}, fmt.Errorf("invalid backend.%s.sidecar_image_mappings: %w", backendType, err) + } return wc, nil } diff --git a/main_test.go b/main_test.go index b277a51..e8f16e5 100644 --- a/main_test.go +++ b/main_test.go @@ -1,8 +1,10 @@ package main import ( + "fmt" "os" "path/filepath" + "strings" "testing" "time" @@ -433,58 +435,6 @@ func TestMergeConfigDockerImagePullPolicyFromFile(t *testing.T) { } } -func TestMergeConfigDockerSidecarImageFromFile(t *testing.T) { - resetCLIForTest() - t.Cleanup(resetCLIForTest) - - fileConfig := &config.FileConfig{ - WorkerID: "docker-worker", - Backend: config.BackendConfig{ - Docker: &config.DockerConfig{ - SidecarImage: "my-registry.io/warpdotdev/warp-agent:latest", - }, - }, - } - - wc, err := mergeConfig(fileConfig) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if wc.BackendType != "docker" { - t.Fatalf("BackendType = %q, want %q", wc.BackendType, "docker") - } - if wc.Docker == nil { - t.Fatal("expected docker backend config") - } - if wc.Docker.SidecarImage != "my-registry.io/warpdotdev/warp-agent:latest" { - t.Errorf("SidecarImage = %q, want %q", wc.Docker.SidecarImage, "my-registry.io/warpdotdev/warp-agent:latest") - } -} - -func TestMergeConfigDockerSidecarImageOmitted(t *testing.T) { - resetCLIForTest() - t.Cleanup(resetCLIForTest) - - fileConfig := &config.FileConfig{ - WorkerID: "docker-worker", - Backend: config.BackendConfig{ - Docker: &config.DockerConfig{}, - }, - } - - wc, err := mergeConfig(fileConfig) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if wc.Docker == nil { - t.Fatal("expected docker backend config") - } - if wc.Docker.SidecarImage != "" { - t.Errorf("SidecarImage = %q, want empty", wc.Docker.SidecarImage) - } -} - func TestMergeConfigDockerImagePullPolicyOmitted(t *testing.T) { resetCLIForTest() t.Cleanup(resetCLIForTest) @@ -508,36 +458,103 @@ func TestMergeConfigDockerImagePullPolicyOmitted(t *testing.T) { } } -func TestMergeConfigKubernetesCodingCLISidecars(t *testing.T) { - resetCLIForTest() - t.Cleanup(resetCLIForTest) - - fileConfig := &config.FileConfig{ - WorkerID: "worker-123", - Backend: config.BackendConfig{ - Kubernetes: &config.KubernetesConfig{ - CodingCLISidecars: map[string]string{ - "claude": "registry.internal/my-claude:v1", - }, - }, - }, +func TestMergeConfigLegacySidecarMappings(t *testing.T) { + for _, backend := range []string{"docker", "kubernetes"} { + t.Run(backend, func(t *testing.T) { + resetCLIForTest() + t.Cleanup(resetCLIForTest) + content := fmt.Sprintf("worker_id: test-worker\nbackend:\n %s:\n sidecar_image: mirror.example/agent:approved\n", backend) + if backend == "kubernetes" { + content += " coding_cli_sidecars:\n claude: mirror.example/claude\n codex: \"\"\n" + } + path := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + cfg, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + wc, err := mergeConfig(cfg) + if err != nil { + t.Fatal(err) + } + if got := wc.SidecarImageMappings.Remap("docker.io/warpdotdev/warp-agent:v1"); got != "mirror.example/agent:approved" { + t.Fatalf("main-agent mapping = %q", got) + } + if backend == "kubernetes" { + for _, transport := range []string{"cli", "acp"} { + source := fmt.Sprintf("docker.io/warpdotdev/warp-claude-%s-sidecar:v2", transport) + if got := wc.SidecarImageMappings.Remap(source); got != "mirror.example/claude:v2" { + t.Fatalf("%s mapping = %q", transport, got) + } + } + source := "warpdotdev/warp-codex-cli-sidecar:v3" + if got := wc.SidecarImageMappings.Remap(source); got != source { + t.Fatalf("empty legacy image remapped %q to %q", source, got) + } + } + }) } +} - wc, err := mergeConfig(fileConfig) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if wc.Kubernetes == nil { - t.Fatal("expected kubernetes backend config") - } - if got := wc.Kubernetes.CodingCLISidecars["claude"]; got != "registry.internal/my-claude:v1" { - t.Errorf("CodingCLISidecars[claude] = %q, want %q", got, "registry.internal/my-claude:v1") +func TestMergeConfigSidecarMappingsRejectLegacyOptions(t *testing.T) { + for _, tc := range []struct { + backend, legacyOption, value string + }{ + {"docker", "sidecar_image", "mirror.example/agent:approved"}, + {"kubernetes", "sidecar_image", "mirror.example/agent:approved"}, + {"kubernetes", "coding_cli_sidecars", "{claude: mirror.example/claude:approved}"}, + } { + t.Run(tc.backend+"/"+tc.legacyOption, func(t *testing.T) { + resetCLIForTest() + t.Cleanup(resetCLIForTest) + path := filepath.Join(t.TempDir(), "config.yaml") + content := fmt.Sprintf("worker_id: test-worker\nbackend:\n %s:\n %s: %s\n sidecar_image_mappings:\n warpdotdev/warp-xvfb-sidecar: mirror.example/computer-use\n", tc.backend, tc.legacyOption, tc.value) + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + cfg, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + _, err = mergeConfig(cfg) + if err == nil || !strings.Contains(err.Error(), "backend."+tc.backend+".sidecar_image_mappings") || !strings.Contains(err.Error(), tc.legacyOption) { + t.Fatalf("startup error = %v, want mutually exclusive config error", err) + } + }) } +} - // mergeConfig must copy the map rather than alias the file config's map, so a - // later mutation of the file config does not leak into the merged worker config. - fileConfig.Backend.Kubernetes.CodingCLISidecars["claude"] = "mutated" - if got := wc.Kubernetes.CodingCLISidecars["claude"]; got != "registry.internal/my-claude:v1" { - t.Errorf("expected merged config to be insulated from file config mutation, got %q", got) +func TestSidecarImageMappingsFromYAML(t *testing.T) { + for _, backend := range []string{"docker", "kubernetes"} { + t.Run(backend, func(t *testing.T) { + resetCLIForTest() + t.Cleanup(resetCLIForTest) + path := filepath.Join(t.TempDir(), "config.yaml") + content := fmt.Sprintf("worker_id: test-worker\nbackend:\n %s:\n sidecar_image_mappings:\n warpdotdev/warp-agent: mirror.example/agent:approved\n", backend) + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + cfg, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + wc, err := mergeConfig(cfg) + if err != nil { + t.Fatal(err) + } + if got := wc.SidecarImageMappings.Remap("docker.io/warpdotdev/warp-agent:v1"); got != "mirror.example/agent:approved" { + t.Fatalf("merged mapping = %q", got) + } + if backend == "docker" { + cfg.Backend.Docker.SidecarImageMappings["warpdotdev/warp-agent"] = "" + } else { + cfg.Backend.Kubernetes.SidecarImageMappings["warpdotdev/warp-agent"] = "" + } + if _, err := mergeConfig(cfg); err == nil || !strings.Contains(err.Error(), "backend."+backend+".sidecar_image_mappings") { + t.Fatalf("startup error = %v, want contextual mapping validation error", err) + } + }) } }