Code Review

Code Review

tektoncd/catalog follows Tekton community review standards and TEP-0003 Catalog Organization.

Review Checklist

Catalog Organization (TEP-0003)

• File path follows <kind>/<name>/<version>/name.yaml (kind = task, stepaction, or pipeline)
• Resource YAML file name matches the resource name
• README.md exists at <kind>/<name>/<version>/README.md
• OWNERS file exists at <kind>/<name>/OWNERS (or version-level)
• Version directory name is a valid semver-like format (e.g. 0.1, 0.2)

Mandatory Metadata

• Label app.kubernetes.io/version matches the directory version
• Annotation tekton.dev/pipelines.minVersion is present and accurate
• Annotation tekton.dev/categories is present (e.g. Git, Build, Deploy, CLI)
• Annotation tekton.dev/tags is present with comma-separated tags
• Annotation tekton.dev/displayName is present (optional but recommended)
• Annotation tekton.dev/platforms lists supported platforms (optional)
• spec.description follows the convention: one-line summary followed by detailed paragraph(s)

Task Authoring Quality

• No $(params.*) interpolation inside script blocks — this is a security vulnerability (code injection) and reliability issue. Parameters must be passed via env or args and referenced as $ENV_VAR or $1. See recommendations.md
• Images are referenced by digest (image@sha256:...) where possible, not by mutable tags (:latest)
• Steps run as non-root and non-privileged unless explicitly required. If root is needed, use securityContext.runAsUser: 0 explicitly
• Parameters have default values where reasonable
• Workspaces have clear descriptions. Tasks should use at most one writeable workspace (Tekton recommendation)
• Script steps use #!/usr/bin/env bash (or appropriate shebang) with set -e for error handling
• No hardcoded values that should be parameters
• Results are documented and used only for small data (commit SHAs, branch names). Large data should use a Workspace instead
• Tasks are idempotent — safe to re-execute
• tekton.dev/pipelines.minVersion is set to the lowest version that supports the features used (portability across Pipeline versions)

YAML Quality

• yamllint passes with the project's .yamllint config
• No trailing whitespace
• Consistent indentation (2 spaces)
• Proper quoting of strings where needed

Testing

• New tasks or changed functionality include tests in tests/ directory
• Test directory contains run.yaml (TaskRun or PipelineRun)
• Pre-apply hook scripts (pre-apply-task-hook.sh, pre-apply-taskrun-hook.sh) are used where setup is needed
• For tasks calling external APIs, fixtures are provided in tests/fixtures/ for go-rest-api-test
• Tests do not depend on external services without fixture mocking

New Version Workflow

• If bumping a version (e.g. 0.1 to 0.2), the old version was copied first in a separate commit, then modified in a subsequent commit
• Changes from the previous version are clearly visible in the diff (not buried in a full copy)

Documentation

• README.md documents all parameters with types and defaults
• README.md documents all workspaces
• README.md documents all results (if any)
• README.md includes usage example(s)
• samples/ directory contains example TaskRun/PipelineRun YAMLs

Security

• No $(params.*) interpolation in scripts — critical injection risk (repeat check — this is the most common security issue in catalog tasks)
• No secrets or credentials hardcoded in YAML
• Sensitive data passed via Kubernetes Secrets or workspaces
• Container images come from trusted registries
• No use of privileged: true without justification

What to Approve

Approve when:

• All checklist items pass
• Tests cover the changed behavior
• The change is focused (one concern per PR)
• Commit messages follow Tekton commit conventions
• New version workflow is followed (copy + modify in separate commits)

What to Block

Block (request changes) when:

• $(params.*) is used inside script blocks (injection vulnerability)
• TEP-0003 structure is not followed
• Mandatory metadata labels/annotations are missing
• Images use mutable tags without justification
• Tests are missing for new or changed tasks
• Security issues (hardcoded secrets, unnecessary privileges)
• yamllint fails
• Version copy and modifications are squashed into a single commit