feat(runners): implement Tekton Pipeline runner with native metadata discovery#2767
feat(runners): implement Tekton Pipeline runner with native metadata discovery#2767waveywaves wants to merge 2 commits intochainloop-dev:mainfrom
Conversation
waveywaves
force-pushed
the
feat/tekton-runner-native-discovery
branch
from
ef38b88 to
cbcf765
Compare
…discovery Implement the TektonPipeline runner with two-tier native metadata discovery and all SupportedRunner interface methods. Tier 1 (always available): reads HOSTNAME env var and ServiceAccount namespace file from the pod filesystem. Tier 2 (best-effort): queries the K8s API for pod labels with tekton.dev/* prefix, providing rich pipeline context. Gracefully degrades when RBAC is denied or SA token is missing. Interface methods: - RunURI: Tekton Dashboard URL when configured, tekton-pipeline:// identifier URI as fallback - Report: writes attestation summary to /tekton/results/ with 3500-byte truncation for Tekton Results size limits - Environment: detects GKE/EKS/AKS as Managed, plain K8s as SelfHosted - ResolveEnvVars: synthesizes discovered metadata as key-value entries - ListEnvVars: returns HOSTNAME as the only consumed env var Includes 26 unit tests covering discovery success, RBAC denial, missing SA token, all RunURI variants, Report truncation, Environment detection, and ResolveEnvVars with full and minimal label sets. Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
waveywaves
force-pushed
the
feat/tekton-runner-native-discovery
branch
from
cbcf765 to
229b0c9
Compare
|
@cubic-dev-ai review |
@migmartri I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/attestation/crafter/runners/tektonpipeline_test.go">
<violation number="1" location="pkg/attestation/crafter/runners/tektonpipeline_test.go:111">
P3: Unused `tmpDir` in test: `t.TempDir()` is called and immediately suppressed with `_ = tmpDir`. The test only exercises `taskRunNameFromHostname()` via a struct literal — no temp directory is needed. Remove both lines to avoid unnecessary filesystem allocations.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| t.Setenv("KUBERNETES_SERVICE_PORT", serverURL.Port()) | ||
|
|
||
| // Create temp directory for SA files | ||
| tmpDir := t.TempDir() |
There was a problem hiding this comment.
P3: Unused tmpDir in test: t.TempDir() is called and immediately suppressed with _ = tmpDir. The test only exercises taskRunNameFromHostname() via a struct literal — no temp directory is needed. Remove both lines to avoid unnecessary filesystem allocations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/attestation/crafter/runners/tektonpipeline_test.go, line 111:
<comment>Unused `tmpDir` in test: `t.TempDir()` is called and immediately suppressed with `_ = tmpDir`. The test only exercises `taskRunNameFromHostname()` via a struct literal — no temp directory is needed. Remove both lines to avoid unnecessary filesystem allocations.</comment>
<file context>
@@ -0,0 +1,650 @@
+ t.Setenv("KUBERNETES_SERVICE_PORT", serverURL.Port())
+
+ // Create temp directory for SA files
+ tmpDir := t.TempDir()
+
+ // Write SA token file
</file context>
…e:// Shorten the RunURI scheme from tekton-pipeline:// to tekton:// for consistency and brevity. Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
| url := fmt.Sprintf("https://%s:%s/api/v1/namespaces/%s/pods/%s", | ||
| apiHost, apiPort, r.namespace, r.podName) | ||
|
|
||
| req, err := http.NewRequest("GET", url, nil) |
There was a problem hiding this comment.
Wandering if we would like to use context, initialize it earlier and pass it here.
|
Thanks @waveywaves for the PR! Could you please provide an example of a completed chainloop attestation that has been processed on Tekton? |
javirln
left a comment
javirln
left a comment
There was a problem hiding this comment.
That's basically why I would to see this: #2767 (comment) 👀
| // All metadata is discovered from K8s API labels and filesystem. | ||
| // Return HOSTNAME as the only env var we consume (for traceability in attestation). | ||
| return []*EnvVarDefinition{ | ||
| {"HOSTNAME", true}, |
There was a problem hiding this comment.
Although the environment variables could not be auto resolved automatically, I think it would be good to have TEKTON_TASKRUN_NAME, TEKTON_PIPELINE_NAME, TEKTON_PIPELINERUN_NAME, TEKTON_TASK_NAME, TEKTON_PIPELINE_TASK_NAME, and TEKTON_NAMESPACE as we do with the rest of the runners.
In the Dagger client for example, we "promote" the underlying environment variables because of the encapsulation of Dagger
4 participants
Summary
TektonPipelinerunner with two-tier native metadata discovery: Tier 1 reads HOSTNAME env var and ServiceAccount namespace file (always available in K8s pods); Tier 2 makes a best-effort K8s API call to discovertekton.dev/*pod labels for rich pipeline contextSupportedRunnerinterface methods:RunURI(Tekton Dashboard URL withtekton-pipeline://fallback),Report(writes to/tekton/results/attestation-reportwith 3500-byte truncation),Environment(detects GKE/EKS/AKS as Managed vs self-hosted),ListEnvVars, andResolveEnvVars(synthesizes discovered metadata as key-value entries)runner.goto pass logger toNewTektonPipelineDesign decisions
$(context.*)variables as env varsWithHTTPClient,WithSATokenPath,WithNamespacePath,WithCACertPath,WithResultsDirallow test injection without polluting the production APITesting