feat: run classic Harbor on stock 0.20.0 + OpenShift custom env

[ikrispin]

Summary

• Migrate classic Harbor A/B (eval_engine: harbor) off the skills_eval_corrections fork onto pinned upstream Harbor (harbor==0.20.0) plus the AEH-style custom OpenShift environment (abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment).
• Prebuilt CI now hands off trial images via task.toml docker_image and selects the env with environment.import_path / --environment-import-path (not environment.type: openshift or fork kwargs.image_ref).
• Update eval-base (Dockerfile.base + build_base_image.sh) to install stock Harbor + pinned AEH k8s env; remove harbor-fork-url / harbor-fork-revision pipeline params; refresh docs and unit tests.

Closes / tracks: APPENG-5914 (parent APPENG-4901).

Motivation

Classic Harbor CI depended on a Harbor fork for OpenShift. AEH already runs stock Harbor with a custom env plugin. This aligns classic A/B with that model so we can track upstream Harbor without maintaining the fork or adopting upstream’s oc/SCC OpenShift backend.

What changed

Area	Change
Config	generate_eval_config.py: prebuilt → import_path + docker_image; local-build → type: docker
Contract	Shared OPENSHIFT_ENVIRONMENT_IMPORT_PATH in abevalflow/harbor_extensions
Images	templates/Dockerfile.base: harbor==0.20.0, AEH SHA ff8b8301…, bake abevalflow
Pipeline	evaluate / harbor-eval: PyPI Harbor fallback + --environment-import-path; drop fork params
Docs	New Docs/harbor_custom_env.md; fork/OpenShift docs redirected
Tests	Updated test_generate_eval_config.py; added test_openshift_environment.py

Out of scope (per ticket): upstream type: openshift (oc/SCC), harbor run -d, dropping DB column harbor_fork_revision.

Test plan

• Unit tests for config generation + OpenShiftEnvironment import path
• Rebuild/push eval-base:latest and eval-base:local-env with Harbor 0.20.0
• Unbaked cluster smoke (pipeline clones feature branch; no baked sources image):
  • Namespace: itay-evalflow
  • Submission: hello-world / Harbor / prebuilt
  • Branch: feat/harbor-custom-env from https://github.com/ikrispin/ABEvalFlow.git
  • Result: Succeeded (prepare → evaluate → analyze → store → cleanup)
  • Verified in logs: Harbor 0.20.0, docker_image set, --environment-import-path, no skills_eval_corrections, treatment/control mean reward 1.0
  • Run: https://console-openshift-console.apps.cn-ai-lab.2vn8.p1.openshiftapps.com/k8s/ns/itay-evalflow/tekton.dev~v1~PipelineRun/harbor-unbaked-smoke-59p2g/
• After merge: apply updated Tasks/Pipelines in shared ab-eval-flow and confirm monitoring CI still healthy
• Optional follow-up: replace deprecated --environment-import-path with Harbor’s --env flag

Notes for reviewers

• Feature branch is based on origin/main (Harbor-only commit d6cb164).
• Smoke used namespace-local apply tweaks (litellm URL, image refs, scheduling) that are not part of this PR.

Successful run:

[GuyZivRH]

PR #67 Consolidated Review: Stock Harbor 0.20.0 + OpenShift Custom Env

PR: #67
Title: feat: run classic Harbor on stock 0.20.0 + OpenShift custom env
Author: ikrispin
Branch: feat/harbor-custom-env → main
JIRA: APPENG-5914
Reviewed: 2026-08-12
Reviews Consolidated: 4

Executive Summary

This PR migrates classic Harbor A/B evaluation (eval_engine: harbor) off the skills_eval_corrections fork onto stock PyPI Harbor (harbor==0.20.0), reusing the existing abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment plugin via Harbor's environment.import_path mechanism. Prebuilt trial images are now passed via task.toml docker_image field instead of fork-specific kwargs.image_ref.
Consensus verdict: Approve with fixes — The strategic direction is sound and reduces maintenance burden significantly. However, several technical issues need attention before merge.

What Changed

Area	Files	Key Changes
Config generation	scripts/generate_eval_config.py	+393/-334 — Rewrote for import_path + docker_image
OpenShift env	abevalflow/harbor_extensions/	Custom env plugin + shared constant
Base image	templates/Dockerfile.base	Stock Harbor + pinned AEH SHA + baked abevalflow
Pipeline	harbor-eval.yaml, evaluate.yaml	Removed fork params, added --environment-import-path
Docs	harbor_custom_env.md (new)	Integration contract documentation
Tests	test_openshift_environment.py (new)	91 lines of unit tests
Total: +994/-893 lines across 22 files

---

Architecture Change

Before (Fork-based)

skills_eval_corrections fork
  └── Built-in OpenShiftEnvironment (type: openshift)
  └── kwargs.image_ref for prebuilt images

After (Stock + Plugin)

harbor==0.20.0 (PyPI)
  └── AEH KubernetesEnvironment (via PYTHONPATH)
      └── ABEvalFlow OpenShiftEnvironment (import_path)
          └── docker_image in task.toml for prebuilt

---

Must-Fix Issues

1. Rebase Required (All Reviews)

PR was CONFLICTING (now may be resolved). Multiple PRs have merged since this was opened (#65, #66, #70, #74). Rebase is required to:

• Pick up CI workflow fix (split jobs)
• Incorporate repo rename URL changes from PR #74
• Ensure no merge conflicts

git fetch origin main
git rebase origin/main

2. components/harbor-eval.yaml Missing AEH Dependency (Critical)

OpenShiftEnvironment imports AEH at module load:

from agent_eval.harbor.kubernetes import KubernetesEnvironment

But harbor-eval.yaml runs on bare ubi9/python-312 and only installs Harbor — never AEH. When --environment-import-path triggers the import, it will crash with ModuleNotFoundError: No module named 'agent_eval'.
Note: This task may be orphaned (not referenced by any Pipeline via taskRef), but it's documented in pipeline_deployment_commands.md as a deployed resource. Either fix it to work or mark it deprecated.

3. set_task_docker_image() Silent Failure Edge Cases (Critical)

The regex-based TOML editing has verified failure modes:
(a) Trailing comment on docker_image line → silent no-op:

# Input: docker_image = "old@sha256:x" # pinned digest
# Result: unchanged (regex doesn't match) — stale image used silently

(b) Trailing comment on [environment] header → duplicate section:

# Input: [environment] # config
# Result: appends second [environment] table — TOML parse error

Neither triggers with current task.toml.j2 template, but this is fragile for a function that determines which container image gets evaluated. Consider using tomlkit for round-trip-preserving edits.

Should-Fix Issues

4. --environment-import-path Is Deprecated and Redundant

Verified against harbor==0.20.0: the flag is hidden=True and emits a deprecation warning. Since generate_eval_config.py already sets environment.import_path in the YAML config, the CLI flag is 100% redundant. Drop it entirely rather than switching to --env.

5. phases/evaluate.yaml Fallback Doesn't Install AEH

The "Harbor not pre-installed" fallback branch installs Harbor but not AEH:

pip install "harbor==0.20.0" "kubernetes>=32.0.0" pydantic pyyaml

If this branch ever runs (stale base image, local testing), the same ModuleNotFoundError occurs. Either fail fast with a clear message or also install AEH.

6. local-build Mode Constraints on OpenShift

local-build sets environment to stock Harbor docker + force_build. This won't work on OpenShift without privileged docker-in-docker. If not intended for cluster CI:

• Document as "local only"
• Reject in cluster pipelines (fail-fast)

7. Import Path String Duplication

Tasks hardcode the import path string:

OPENSHIFT_IMPORT_PATH="abevalflow.harbor_extensions.openshift_environment:OpenShiftEnvironment"

Use the shared constant or centralize to avoid drift/typos.

8. Bash Style Inconsistency

components/harbor-eval.yaml uses plain string interpolation; phases/evaluate.yaml uses proper bash array. Align for safety.

Strengths (All Reviews Agree)

1. Low-risk core design — Reuses already-tested OpenShiftEnvironment plugin instead of writing new backend
2. Good doc hygiene — Fork docs redirected to new harbor_custom_env.md with migration guidance
3. Centralized constant — OPENSHIFT_ENVIRONMENT_IMPORT_PATH in single source of truth
4. New test coverage — test_openshift_environment.py covers pod manifest, start(), error paths
5. Smoke tested — Real cluster run in itay-evalflow namespace with full pipeline success
6. Pinned versions — harbor==0.20.0 and AEH SHA for reproducibility

---

CI Status

Check	Status
test	✅ Pass (51s)
test (observability)	✅ Pass (57s)

---

Questions for Author

1. Is pipeline/tasks/components/harbor-eval.yaml still a live resource or superseded by phases/evaluate.yaml? If dead, delete it.
2. Was the smoke run through phases/evaluate.yaml or components/harbor-eval.yaml? (Only the former would succeed given finding #2)

---

Final Verdict

Reviewer	Verdict
Review 1 (detailed)	Request changes (2 critical bugs)
Review 2 (quick)	Approved
Review 3 (medium)	Request changes (rebase needed)
Review 4 (recent)	Approve pending rebase
Consolidated: Request Changes
The migration strategy is excellent and will eliminate significant technical debt. However:

• Blocking: Rebase required, set_task_docker_image() silent failures
• High priority: components/harbor-eval.yaml AEH dependency, deprecated CLI flag
  After addressing the must-fix items (especially the silent-failure TOML bug), this is ready to merge.

---

Checklist for Author

• Rebase on latest main
• Fix set_task_docker_image() edge cases (use tomlkit or validate writes)
• Remove redundant --environment-import-path CLI flag
• Either fix components/harbor-eval.yaml to install AEH or delete/deprecate it
• Verify no ABEvalFlow URLs remain after rebase
• Re-run CI after fixes

[ikrispin]

@GuyZivRH Thanks for the review, fixed on latest (ba11e08):

• set_task_docker_image() now uses tomlkit (preserves comments, no silent no-ops)
• Dropped classic --environment-import-path (YAML import_path only, kept for AEH)
• Deleted unused components/harbor-eval.yaml (pipelines use phases/evaluate, that’s what we smoked)
• Fail-fast if Harbor/AEH aren’t in eval-base
• Removed smoke-only abevalflow-src, noted local-build as local-only