Skip to content

Make ExtractResult sidecar-safe by reading from 'validator' container explicitly #795

Description

@xdu31

Problem

The current ExtractResult() implementation in pkg/validator/job/result.go is not sidecar-safe. It assumes the first container in the pod is the validator:

// Line 57 - assumes first container is validator
cs := jobPod.Status.ContainerStatuses[0]  // ❌ Breaks with sidecars!

// Line 82 - empty container name defaults to first container
logs, logErr := pod.GetPodLogs(ctx, d.clientset, d.namespace, jobPod.Name, "")  // ❌ Not explicit!

Impact: When external controllers inject sidecars for log streaming or result processing, ExtractResult() may read from the sidecar container instead of the validator, returning incorrect exit codes and logs.

Solution

Make ExtractResult() explicitly read from the "validator" container by name, and freeze "validator" as part of the public contract (ADR-002).

Changes Required

  1. Find container by name "validator" instead of using ContainerStatuses[0]
  2. Pass explicit container name to GetPodLogs(..., "validator")
  3. Update function documentation to document the "validator" contract
  4. Add helper function findContainerStatus() for name-based lookup
  5. Apply same fix to HandleTimeout() (line 122 has the same issue)

Implementation

// Add helper function
func findContainerStatus(statuses []corev1.ContainerStatus, name string) (corev1.ContainerStatus, bool) {
    for _, cs := range statuses {
        if cs.Name == name {
            return cs, true
        }
    }
    return corev1.ContainerStatus{}, false
}

// Update ExtractResult
func (d *Deployer) ExtractResult(ctx context.Context) *ctrf.ValidatorResult {
    // ...
    
    // BEFORE: cs := jobPod.Status.ContainerStatuses[0]
    // AFTER:
    cs, found := findContainerStatus(jobPod.Status.ContainerStatuses, "validator")
    if !found {
        result.ExitCode = -1
        result.TerminationMsg = "container 'validator' not found (ADR-002 violation)"
        return result
    }
    
    // ...
    
    // BEFORE: logs, logErr := pod.GetPodLogs(ctx, d.clientset, d.namespace, jobPod.Name, "")
    // AFTER:
    logs, logErr := pod.GetPodLogs(ctx, d.clientset, d.namespace, jobPod.Name, "validator")
}

Documentation Update

// ExtractResult reads the exit code, termination message, and stdout from the
// "validator" container in a completed validator pod.
//
// CONTRACT (ADR-002): The container name MUST be "validator". This is a frozen
// public contract to ensure sidecar-safety — ExtractResult will only read from
// the "validator" container, ignoring any sidecar containers that may be
// injected by external controllers (e.g., log streaming, result processing).
//
// This method must be called after WaitForCompletion returns, when the Job is
// in a terminal state (Complete or Failed).
func (d *Deployer) ExtractResult(ctx context.Context) *ctrf.ValidatorResult {

Files to Modify

  • pkg/validator/job/result.go (lines 29-35, 51-57, 82, 122)

Acceptance Criteria

  • ExtractResult() finds container by name "validator" instead of using index 0
  • GetPodLogs() is called with explicit container name "validator"
  • HandleTimeout() uses same sidecar-safe lookup
  • Function documentation explicitly states "validator" contract per ADR-002
  • Unit tests verify behavior with multiple containers (validator + sidecar)
  • Error message when "validator" container not found mentions ADR-002

Related

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions