Conversation
|
Merge status: PR is mergeable by metadata, but merge attempts were blocked by the tool safety layer in this session. Please retry from another GitHub client or manually squash-merge after verifying required checks. Head SHA observed: |
There was a problem hiding this comment.
Pull request overview
This PR introduces an explicitly gated “live Tekton adapter” path for the SourceOS delegated runner, allowing controlled kubectl get/apply execution only when --execute-live is provided, while preserving the default fail-closed posture for side effects.
Changes:
- Add
--execute-livesupport, validation, and evidence capture for live Tekton observe/submit paths. - Record live adapter results under
artifacts/.../live-tekton/and embed aliveTektonsection insourceos-delegated-execution-request.json. - Extend CI to assert fail-closed behavior and exercise the live adapter via a fake
kubectl.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/run_sourceos_delegated.py | Adds live adapter validation/execution (kubectl get/apply) guarded by --execute-live, plus evidence/result recording. |
| .github/workflows/ci.yml | Adds CI coverage for live adapter guards and a fake kubectl-based observe/submit validation path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| require_non_empty(args.kubeconfig_ref, "--kubeconfig-ref is required for live Tekton modes") | ||
| require_non_empty(args.kubeconfig_env, "--kubeconfig-env is required for live Tekton modes") | ||
| credential_env(args) | ||
| if args.mode == "tekton-observe": | ||
| require_non_empty(args.pipeline_run_name, "--pipeline-run-name is required for live tekton-observe") | ||
| require_non_empty(args.tekton_namespace, "--tekton-namespace is required for live tekton-observe") | ||
| if args.mode == "tekton-submit": | ||
| require_non_empty(args.pipeline_run_manifest, "--pipeline-run-manifest is required for live tekton-submit") |
There was a problem hiding this comment.
require_non_empty() builds its own "{name} is required" message, but here it’s being passed a full sentence (e.g. "--kubeconfig-ref is required for live Tekton modes"). This will produce awkward duplicated errors like "--kubeconfig-ref is required for live Tekton modes is required". Pass only the flag name (e.g. "--kubeconfig-ref") or change require_non_empty() to accept a full error message.
| require_non_empty(args.kubeconfig_ref, "--kubeconfig-ref is required for live Tekton modes") | |
| require_non_empty(args.kubeconfig_env, "--kubeconfig-env is required for live Tekton modes") | |
| credential_env(args) | |
| if args.mode == "tekton-observe": | |
| require_non_empty(args.pipeline_run_name, "--pipeline-run-name is required for live tekton-observe") | |
| require_non_empty(args.tekton_namespace, "--tekton-namespace is required for live tekton-observe") | |
| if args.mode == "tekton-submit": | |
| require_non_empty(args.pipeline_run_manifest, "--pipeline-run-manifest is required for live tekton-submit") | |
| require_non_empty(args.kubeconfig_ref, "--kubeconfig-ref") | |
| require_non_empty(args.kubeconfig_env, "--kubeconfig-env") | |
| credential_env(args) | |
| if args.mode == "tekton-observe": | |
| require_non_empty(args.pipeline_run_name, "--pipeline-run-name") | |
| require_non_empty(args.tekton_namespace, "--tekton-namespace") | |
| if args.mode == "tekton-submit": | |
| require_non_empty(args.pipeline_run_manifest, "--pipeline-run-manifest") |
| if not args.execute_live: | ||
| require_non_empty(args.pipeline_run_ref, "--pipeline-run-ref is required for tekton-observe mode") | ||
| mode_gate["requirement"] = "observe_existing_pipeline_run" |
There was a problem hiding this comment.
Same issue with require_non_empty() usage: passing a full sentence as the name parameter will lead to duplicated error text ("... is required is required"). Use just "--pipeline-run-ref" (or adjust the helper to accept a custom message) so the user-facing error is clean.
| elif args.mode == "tekton-submit": | ||
| observed_ref = args.pipeline_run_ref or f"tekton://pipelinerun/{args.tekton_namespace}/{Path(args.pipeline_run_manifest).stem}" | ||
|
|
There was a problem hiding this comment.
For live tekton-submit, the fallback pipelineRunRef is derived from Path(pipeline_run_manifest).stem, which isn’t guaranteed to match metadata.name inside the manifest (or the actual created object name). This can record an incorrect tektonPipelineRunRef in artifacts. Consider extracting the name/namespace from the manifest (parse YAML) and/or capturing the created resource name from kubectl apply output (-o name/-o json) instead of using the filename.
| stdout = exc.stdout or "" if isinstance(exc.stdout, str) else "" | ||
| stderr = exc.stderr or "" if isinstance(exc.stderr, str) else "" |
There was a problem hiding this comment.
The TimeoutExpired stdout/stderr assignments rely on or + conditional-expression precedence, which is hard to read and easy to get wrong during future edits. Consider rewriting these lines with explicit parentheses or a small helper that safely normalizes exc.stdout/exc.stderr to strings.
| stdout = exc.stdout or "" if isinstance(exc.stdout, str) else "" | |
| stderr = exc.stderr or "" if isinstance(exc.stderr, str) else "" | |
| stdout = exc.stdout if isinstance(exc.stdout, str) and exc.stdout else "" | |
| stderr = exc.stderr if isinstance(exc.stderr, str) and exc.stderr else "" |
Program objective
Move the SourceOS delegated runner from guarded intent toward a safe live Tekton adapter surface without changing the default fail-closed posture.
Workstream advanced
agentplane_sourceos_image_production_laneThis PR adds explicit
--execute-livebehavior for Tekton observe/submit modes while keeping all external mutation guarded.Files changed
scripts/run_sourceos_delegated.py.github/workflows/ci.ymlEvidence produced
The runner now supports:
record-only: no external mutation;--execute-liveis rejected.tekton-observe: can observe an existing PipelineRun via guardedkubectl getwhen--execute-liveis supplied.tekton-submit: can execute a guardedkubectl applypath when all of these are present:--allow-side-effects--execute-live--pipeline-run-manifest--kubeconfig-ref--kubeconfig-env--tekton-service-account-ref--tekton-namespaceThe runner writes live adapter evidence under
artifacts/.../live-tekton/and records aliveTektonsection insourceos-delegated-execution-request.json.Current score change
Agentplane SourceOS image-production lane:
Risks reduced
kubectl, so no real cluster is touched.Risks still open
SociOS-Linux/sociosTekton substrate.Validation
CI now exercises:
record-only --execute-liverejectiontekton-observetekton-submit