[Jaana Dogan (rakyll)]

This removes a set of features that widened what ate-env-api
and the guest can be made to do, without review in proportion
to that reach. Our goal for this project is to keep the data
plane APIs to a minimum and see if we can provide the guest
services from Substrate actors only via config in the long-term.

Removed:

• Runtime layers and sidecars: ate-env manifest template --layer,
  --sidecar and --sidecar-readyz, and the guest's -sidecar and
  -sidecar-readyz support (cmd/ate-env-guest/sidecar.go). /readyz
  answers once the guest is listening.
• Port proxy (internal/portproxy) and its ate-env-api flags
  --atenet-connect, --port-proxy-trusted-origins and
  --port-proxy-response-header-timeout, plus the CONNECT support in
  the fake router.
• On-demand rootfs images: CreateEnvironmentRequest.rootfs_image
  (field 4 and its name are now reserved), the server-side derivation
  of <base>-<digest> ActorTemplates, ate-env create --rootfs-image,
  and rootfs_image= in the Python client. ate-env manifest template --rootfs-image stays: operators can still register image templates
  themselves.
• The NeMo Gym integration (integrations/nemo-gym), which depended on
  the port proxy and on runtime layers.
• SWE-bench benchmarking (cmd/benchmarking, internal/benchmarking,
  benchmarking/), which depended on on-demand rootfs images.
• Docs for all of the above: docs/rootfs-images, examples/runtime-layers,
  and the related README sections.
• CreateEnvironment now returns InvalidArgument when the request has
  unknown fields. The message names the replacement: register an image
  template with ate-env manifest template --rootfs-image and pass its
  name in template.name.

Process:

Changes of this size to a project like this, which adds new network
surface, new privileges for the API service, new wire-protocol fields,
and new integrations, need substantial design discussion and security
review before they are merged. They should not land incrementally
through feature PRs. Any of these capabilities can come back through a
design proposal with a threat model and explicit maintainer sign-off.

Compatibility:

• Clients that set rootfs_image get an environment created from the
  base template, because the server drops the unknown field. Update
  them.
• ate-env-api deployments that pass the removed port-proxy flags will
  fail to start; the generated manifest never set them.
• Templates derived by earlier versions (<base>-<12 hex>) are unused
  and can be deleted with kubectl-ate delete actor-template.

[Tomer Glottmann (tomergee)]

Verdict: approve after one fix. The removal is complete and consistent. No dangling reference to a removed feature remains in code, CI, the Makefile, dependabot or the docs. The proto reserves field 4 and the name rootfs_image, and the generated Go and Python code agree with it. One item must change before merge: an old client that still sends rootfs_image gets a wrong environment and no error.

Before merge

1. internal/apiservice/server.go (CreateEnvironment) with proto/ateenv/v1alpha/env.proto:116-117: proto3 keeps a reserved field as unknown bytes and does not reject it. I marshaled a CreateEnvironmentRequest with field 4 set, as an old client sends it. proto.Unmarshal returns no error and keeps 38 unknown bytes. CreateEnvironment does not look at them. Thus the server creates the environment from the base template, which runs the guest image, not the requested image. The caller gets no error, and its commands then fail for an unrelated reason. Old Python clients (Client.create(rootfs_image=...)) and older NeMo Gym providers send this field. Do these changes:
   • Reject a request that has unknown fields with InvalidArgument, for example if len(req.ProtoReflect().GetUnknown()) > 0.
   • Name the replacement in the message: register an image template with ate-env manifest template --rootfs-image and pass its name.
   • Add a test that sends field 4 and expects InvalidArgument.

Nits

2. PR description, "Security", sidecars: a sidecar comes from the template, and only a template author can set it. That author already controls the container image and its command, thus a sidecar adds no privilege for that author. The sidecar process also runs inside the CPU and memory limits of the actor. The exposure was the port proxy, which made the sidecar ports reachable. Consider stating it this way, because the reason for a later reintroduction depends on it.
3. PR description: "There may be more changes to clean up". I found none. go mod tidy changes nothing, staticcheck reports nothing, and the code that created ActorTemplates in ate-env-api is gone. GetActorTemplate stays, because server.go still uses it.

Verified

• Head f36dcf5 merges clean with main fccd920. GitHub CI is green: Go build, vet, test and license headers; Python client tests; Wiz and zizmor scans.
• Local: go build ./..., go vet ./..., go test ./... pass; go mod tidy makes no change; staticcheck ./... is clean.
• Local Python (the ci.yaml steps): 116 passed, 26 skipped; ruff check clients/python is clean.
• git grep for rootfs_image, portproxy, sidecar, nemo-gym, swebench, benchmarking/, atenet-connect: the only hits are the kept ate-env manifest template --rootfs-image path and its tests.
• No Markdown link points to a removed path. The README keeps the --rootfs-image template flow and tells operators to delete the <base>-<12 hex digits> templates that older versions derived.
• Probe for item 1: a Go test that unmarshals a request with field 4 (removed after the run).

[Jaana Dogan (rakyll)]

Tomer Glottmann (@tomergee) Addressed the comments, PTAL.

[Tomer Glottmann (tomergee)]

Verdict: approve. Head 6ee7e08 addresses the one before-merge item: CreateEnvironment rejects a request with unknown fields with InvalidArgument (internal/apiservice/server.go:105-107), and TestCreateRejectsUnknownFields sends field 4 over gRPC and expects that code.

Verified on 6ee7e08

• GitHub CI is green. No conflict with main at de462e9.
• Local: go build ./..., go vet ./..., go test ./... pass. go mod tidy changes nothing. staticcheck is clean.
• Python 3.12: 116 passed, 26 skipped. ruff check clients/python is clean.
• buf generate with protoc-gen-go v1.36.11 and protoc-gen-go-grpc v1.5.1 leaves the Go stubs unchanged. gen-protos.sh reproduces the Python stubs, apart from the license header.
• grpc-go v1.84.0 keeps unknown fields on unmarshal, thus the server sees field 4 from an old client.
• No reference to a removed feature remains, apart from the kept ate-env manifest template --rootfs-image path and the reserved proto entry.

Nits, no action needed for merge

1. internal/apiservice/server.go:106: the message names rootfs_image for all unknown fields. Consider reading the field number with protowire.ConsumeTag and naming rootfs_image only for field 4.
2. PR description, "Compatibility", first bullet: it says the server drops the field. The server returns InvalidArgument. Update the bullet before the squash merge.
3. README.md:79-83: the paragraph keeps the instruction to register templates again, but the stated failure went away with the derivation. State what fails, or make the instruction optional.

Merge requirement

The new org ruleset enforce-DCO requires the DCO status check. The head commit has no Signed-off-by trailer, and the check has not run on this PR. Amend with git commit --amend -s --no-edit and force-push. This approval stays valid after the push.

[Jaana Dogan (rakyll)]

See #116 instead.

[Jaana Dogan (rakyll)]

We cannot force push to branches that are not from a dev fork anymore. We are migrating the changes to another PR to be able to sign the commit to pass the DCO check.

[Jaana Dogan (rakyll)]

Closing in favor of #116.