feat: support image pull secret

pkg/provider/aws/aws.go

@@ -682,6 +682,13 @@ func (p *Provider) ClassifyProvisionError(err error, accelerator, region string)
 		tier = nebulav1alpha1.CapacitySpot
 	}
 	scope := provider.ClassifyError(err, tier, accelerator)
+	// The zero scope means BLOCK NOTHING — a rejection of this request that says nothing
+	// about the candidate, such as an image pull credential this adapter cannot honour.
+	// Stamping a region onto it would make it non-empty, and recordBlock would then install a
+	// block covering every accelerator and tier in that region.
+	if scope == (provider.BlockScope{}) {
+		return scope
+	}
 	// Confine an accelerator/capacity/quota block to the region that failed. A
 	// DenyAll (auth) fails in every region, so it stays region-wide (Region left nil).
 	// An empty region (should not happen — every request carries one) maps to &"" =
@@ -722,6 +729,18 @@ func (p *Provider) instanceSpecFromPod(
 	}
 	c := pod.Spec.Containers[0]
 
+	// Refused, not ignored: buildUserData's `docker pull` is anonymous, so honouring this
+	// needs a bootstrap that logs in first (`aws ecr get-login-password` for a role — the
+	// instance profile is already there). Until then, silently pulling without the credential
+	// would either 401 or fetch a PUBLIC image of the same name.
+	//
+	// TODO: implement the role path in buildUserData. A basic credential needs more care —
+	// user-data is readable via DescribeInstanceAttribute (see buildSpec's env caveat).
+	if req.RegistryAuth != nil {
+		// No kind is wired here yet, so the shared refusal covers every one of them.
+		return InstanceSpec{}, req.RegistryAuth.Unsupported("aws")
+	}
+
 	// Accelerator type comes from the AcceleratorTypeLabel; the count rides on the
 	// nvidia.com/gpu resource. On EC2 both are lookup keys: the instance type is the
 	// one whose (accelerator_type, gpu_count) pair matches, since the GPU count is

pkg/provider/aws/aws_test.go

@@ -572,6 +572,13 @@ func TestClassifyProvisionError(t *testing.T) {
 			&smithy.GenericAPIError{Code: "InvalidFleetConfiguration", Message: "not supported in AZ"},
 			onDemandRegional},
 		{"nil", nil, provider.BlockScope{}},
+		// An image pull credential this adapter cannot honour is a fact about the POD, so it
+		// blocks nothing. Region must NOT be stamped on: a scope carrying only a region
+		// fences off every accelerator and tier there, on behalf of one Pod.
+		{"image pull blocks nothing", provider.ErrImagePull, provider.BlockScope{}},
+		{"wrapped image pull blocks nothing",
+			fmt.Errorf("aws: unsupported image pull credential: %w", provider.ErrImagePull),
+			provider.BlockScope{}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/provider/errors.go

@@ -48,18 +48,30 @@ var (
 	// nothing about the same request in another. The adapter confines it to the
 	// failing region (see aws.ClassifyProvisionError). Transient until quota frees up.
 	ErrQuota = errors.New("provider: quota exceeded")
+	// ErrImagePull: the image could not be pulled — a credential the provider cannot honour,
+	// one it was not given, or a registry that refused it.
+	//
+	// REQUEST-scoped, and the only sentinel that is: it describes the Pod, not the candidate.
+	// So it blocklists NOTHING (see ClassifyError). Both alternatives are wrong — ErrAuth
+	// widens to DenyAll, fencing off the whole provider because one Pod named a role it
+	// cannot assume, and a capacity scope evicts an accelerator/tier/region that is serving
+	// other Pods perfectly well, since the blocklist key carries no Pod, image or credential
+	// identity. A failure that belongs to one request cannot be recorded against a candidate.
+	ErrImagePull = errors.New("provider: cannot pull image")
 )
 
 // ClassifyError maps a provision error to the BlockScope it should be blocklisted at,
 // checking the shared sentinels first and falling back to string heuristics for raw API
 // messages. The rule it encodes: a narrow failure must not disqualify other accelerators, or
-// other regions. Only auth widens to the whole provider via DenyAll; everything else —
-// including an unrecognized error — is scoped to the failing accelerator/tier/region so
-// failover can route around it.
+// other regions. Only auth widens to the whole provider via DenyAll; a failure that belongs to
+// the REQUEST rather than the candidate blocks nothing at all; everything else — including an
+// unrecognized error — is scoped to the failing accelerator/tier/region so failover can route
+// around it.
 //
 // This is the single place the SHARED part of a scope is derived, so every adapter delegates
 // here and then only adds what is provider-specific (AWS adds its region). Nothing assembles
-// a scope elsewhere.
+// a scope elsewhere. An adapter that decorates the result must leave the ZERO scope alone —
+// it means "block nothing", and any field added to it becomes a block.
 //
 // capacityType is stamped onto accelerator-scoped blocks so a Spot failure does not block
 // OnDemand. accelerator is the request's POOL identity (type:count, "" for a CPU-only Pod),
@@ -86,6 +98,19 @@ func ClassifyError(err error, capacityType nebulav1alpha1.CapacityType, accelera
 		return BlockScope{DenyAll: true}
 	case catCapacity:
 		return capacityScope()
+	case catRequest:
+		// Nothing is blocklisted: the request was refused, the candidate is fine. A block
+		// here would be recorded against provider/accelerator/tier/region — a key with no
+		// Pod, image or credential in it — so one Pod's unusable pull credential would
+		// exclude that candidate for every OTHER Pod until the TTL lapsed.
+		//
+		// The zero scope makes recordBlock a no-op, which is the whole mechanism. An adapter
+		// must therefore not decorate this scope (see ClassifyProvisionError in each
+		// adapter): adding a region would make it non-empty and install a region-wide block.
+		//
+		// Still a rejection, so the Pod fails with the reason rather than retrying forever —
+		// the two questions are separate. See IsRejection.
+		return BlockScope{}
 	default:
 		// An unrecognized error is scoped like capacity (this accelerator + tier, and
 		// per region once the adapter confines it), NOT DenyAll. A DenyAll on an
@@ -113,9 +138,14 @@ const (
 	catUnattributable failureCategory = iota
 	// catAuth: credentials or authorization failed, so nothing on the provider works.
 	catAuth
-	// catCapacity: the provider refused THIS request — no capacity, quota exhausted,
-	// or an accelerator it does not offer.
+	// catCapacity: the provider refused this request because of something about the
+	// CANDIDATE — no capacity, quota exhausted, an accelerator it does not offer. Blocking
+	// it is meaningful, because the next Pod asking for the same candidate would fail too.
 	catCapacity
+	// catRequest: the provider refused this request because of something about the REQUEST
+	// itself — today only an image it cannot pull. A decision, so a rejection, but it says
+	// nothing about the candidate and must not blocklist one.
+	catRequest
 )
 
 // categorize buckets a provision error, sentinels first and string heuristics after.
@@ -125,7 +155,10 @@ func categorize(err error) failureCategory {
 	switch {
 	case errors.Is(err, ErrAuth):
 		return catAuth
-	case errors.Is(err, ErrNoCapacity), errors.Is(err, ErrUnsupportedAccelerator), errors.Is(err, ErrQuota):
+	case errors.Is(err, ErrImagePull):
+		return catRequest
+	case errors.Is(err, ErrNoCapacity), errors.Is(err, ErrUnsupportedAccelerator),
+		errors.Is(err, ErrQuota):
 		return catCapacity
 	}
 
@@ -167,9 +200,12 @@ func categorize(err error) failureCategory {
 // error, a timeout, a 503, an unparseable response.
 //
 // The distinction exists because the two call for opposite handling and the costs are
-// asymmetric. A rejection is authoritative, so the Pod is failed and the candidate
-// blocklisted, and failover routes around it — all correct, because the provider said
-// no. An unattributable failure is authoritative about nothing, so the same writes
+// asymmetric. A rejection is authoritative, so the Pod is failed with the reason — correct,
+// because the provider said no. Whether a CANDIDATE is also blocklisted is a separate
+// question, answered by ClassifyError: a capacity rejection blocks one and failover routes
+// around it, while a request-scoped one (an unusable image credential) blocks nothing, because
+// the candidate did nothing wrong. An unattributable failure is authoritative about nothing,
+// so the same writes
 // stamp a terminal status onto a request the provider may have accepted (leaving a
 // paid instance running behind a Pod that is about to be reaped) and fence off a
 // candidate that never misbehaved. Retrying costs a request and Provision is

pkg/provider/errors_test.go

@@ -45,6 +45,16 @@ func TestClassifyError(t *testing.T) {
 		{"quota sentinel", ErrQuota, capacityScope},
 		{"no-capacity sentinel", ErrNoCapacity, capacityScope},
 		{"unsupported sentinel", ErrUnsupportedAccelerator, capacityScope},
+		// An unusable image credential belongs to the POD. The blocklist key carries no Pod,
+		// image or credential identity, so ANY scope here would exclude the candidate for
+		// unrelated Pods that pull perfectly well — hence the zero scope, which blocks
+		// nothing. Still a rejection: see TestIsRejection.
+		{"image-pull sentinel blocks nothing", ErrImagePull, BlockScope{}},
+		{
+			"wrapped image-pull sentinel blocks nothing",
+			fmt.Errorf("modal: unsupported image pull credential: %w", ErrImagePull),
+			BlockScope{},
+		},
 		{"wrapped sentinel", fmt.Errorf("provision failed: %w", ErrNoCapacity), capacityScope},
 		{"string unauthorized", fmt.Errorf("HTTP 401 unauthorized"), BlockScope{DenyAll: true}},
 		{"string quota", fmt.Errorf("account limit exceeded"), capacityScope},
@@ -102,6 +112,10 @@ func TestIsRejection(t *testing.T) {
 		{"wrapped sentinel", fmt.Errorf("create sandbox: %w", ErrNoCapacity), true},
 		{"string capacity", errors.New("InsufficientInstanceCapacity"), true},
 		{"string auth", errors.New("HTTP 401 unauthorized"), true},
+		// Blocks nothing (see TestClassifyError), yet still a rejection: the provider DID
+		// decide, and a Pod whose credential it cannot use must fail with that reason rather
+		// than retry it forever. Blocking and rejecting are separate questions.
+		{"image-pull sentinel", ErrImagePull, true},
 
 		// The failures this predicate exists for.
 		{"deadline exceeded", context.DeadlineExceeded, false},

pkg/provider/modal/client.go

@@ -139,7 +139,10 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string
 	if spec.Image == "" {
 		return "", Credential{}, fmt.Errorf("modal: empty image in sandbox spec")
 	}
-	image := c.mc.Images.FromRegistry(spec.Image, nil)
+	image, err := c.imageFor(ctx, spec)
+	if err != nil {
+		return "", Credential{}, err
+	}
 
 	probe, err := modalProbe(spec.ReadinessProbe)
 	if err != nil {
@@ -161,6 +164,8 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string
 		GPU:            gpuReservation(spec.GPU, spec.GPUCount),
 		CPU:            spec.CPU,
 		MemoryMiB:      spec.MemoryMiB,
+		CPULimit:       spec.CPULimit,
+		MemoryLimitMiB: spec.MemoryLimitMiB,
 		EncryptedPorts: spec.Ports,
 		// Nil leaves Modal's SchedulerPlacement unset entirely (the SDK only builds one
 		// when Regions is non-empty), which is the unconstrained, un-multiplied case.
@@ -182,6 +187,57 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string
 	return sb.SandboxID, c.mintCredential(ctx, sb, spec), nil
 }
 
+// imageFor resolves the sandbox's image, attaching pull credentials when the spec carries
+// them. Modal takes them as a Secret, so this is where the spec's data becomes an SDK object.
+func (c *sdkClient) imageFor(ctx context.Context, spec SandboxSpec) (*modal.Image, error) {
+	a := spec.RegistryAuth
+	switch {
+	case a == nil:
+		return c.mc.Images.FromRegistry(spec.Image, nil), nil
+
+	case a.AWSRole != nil:
+		// REGISTRY_AUTH_TYPE_AWS: Modal assumes the role via its own OIDC identity, so the
+		// role's trust policy must trust this workspace — workspace-admin setup that a
+		// provision cannot arrange, hence a failure here is terminal for the Pod.
+		//
+		// Region is guaranteed non-empty by checkRegistryAuth; Modal always sends it.
+		secret, err := c.registrySecret(ctx, map[string]string{
+			"AWS_ROLE_ARN": a.AWSRole.RoleARN,
+			"AWS_REGION":   a.AWSRole.Region,
+		})
+		if err != nil {
+			return nil, fmt.Errorf("modal: ECR pull secret: %w", err)
+		}
+		return c.mc.Images.FromAwsEcr(spec.Image, secret, nil), nil
+
+	case a.Basic != nil:
+		secret, err := c.registrySecret(ctx, map[string]string{
+			"REGISTRY_USERNAME": a.Basic.Username,
+			"REGISTRY_PASSWORD": a.Basic.Password,
+		})
+		if err != nil {
+			return nil, fmt.Errorf("modal: registry pull secret: %w", err)
+		}
+		return c.mc.Images.FromRegistry(spec.Image, &modal.ImageFromRegistryParams{Secret: secret}), nil
+
+	default:
+		// Refuse rather than pull anonymously; see provider.RegistryAuth.
+		return nil, fmt.Errorf("modal: unsupported image pull credential: %w", provider.ErrImagePull)
+	}
+}
+
+// registrySecret builds a lazily-hydrated ephemeral Modal Secret for these values.
+//
+// Per sandbox, and cached nowhere: the credential belongs to the Pod, is re-read from the
+// Pod's Kubernetes Secret on every provision, and must not outlive the call. The cost is that
+// a distinct SecretID gives the image a distinct hash, so Modal may re-pull an image it has
+// already built — layer caching still applies, and a credential kept in process memory for
+// the manager's lifetime is the worse trade.
+func (c *sdkClient) registrySecret(ctx context.Context, kv map[string]string) (*modal.Secret, error) {
+	// The server-side create happens later, when the image build hydrates this.
+	return c.mc.Secrets.FromMap(ctx, kv, nil)
+}
+
 // mintCredential issues the sandbox's connect credential and RETURNS it, storing nothing.
 // Not in a tag, since Modal's tags are plaintext and bulk-listable — one ListSandboxes
 // would hand over every workload's token. Not in memory, which is not durable. A