[TESTING- do not merge] Add agentic SDLC conformance check pipeline (ROSA-730) - #337
[TESTING- do not merge] Add agentic SDLC conformance check pipeline (ROSA-730)#337tnierman wants to merge 7 commits into
Conversation
Adds an informing (non-blocking) Konflux pipeline that validates ROSA-730 agentic SDLC standards on every PR. Checks for: - File existence: CLAUDE.md, .pre-commit-config.yaml, .codecov.yml, CONTRIBUTING.md, DEVELOPMENT.md, TESTING.md, .claude/settings.json, golangci.yml - Content validation: CLAUDE.md required sections, codecov patch coverage >= 50%, pre-commit hook requirements (gitleaks, golangci-lint, file hygiene) - Documentation substance: docs must be > 10 lines (not stubs) Uses onError: continue so the pipeline always succeeds — failures appear as informing status in the PR checks tab without blocking merge. ROSA-730 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tnierman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThis PR adds a Tekton PipelineRun for pull-request SDLC checks. It triggers on PRs to ChangesTekton Agentic SDLC Conformance Pipeline
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml (2)
52-52: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the
ubi-minimalimage tag instead of:latest.The git-clone task is pinned by digest (Line 34), and this repo's own
.pre-commit-config.yamlenforces "pinned immutable tag" for all tool versions. Usingubi9/ubi-minimal:latesthere breaks that convention and risks non-reproducible conformance results if the base image changes underneath the pipeline.♻️ Pin image by digest
- image: registry.access.redhat.com/ubi9/ubi-minimal:latest + image: registry.access.redhat.com/ubi9/ubi-minimal@sha256:<pinned-digest>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml at line 52, The pipeline step is using the mutable ubi9/ubi-minimal:latest image, which should be pinned like the git-clone task to keep runs reproducible. Update the image reference in the Tekton task so this step uses an immutable tag or digest instead of latest, keeping the change localized to the step that currently references registry.access.redhat.com/ubi9/ubi-minimal.
168-179: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFragile grep-based parsing of
.codecov.ymlpatch target.
grep -A5 'patch:' | grep 'target:' | head -1 | grep -oE '[0-9]+'is order/indentation-dependent and can silently misparse if the YAML structure changes (e.g., extra keys inserted betweenpatch:andtarget:, orproject.default.targetappearing within the 5-line window). Since this drives a PASS/FAIL determination, a structural YAML parser would be more robust.♻️ Suggested alternative using yq (requires yq available in image or an install step)
- patch_target=$(grep -A5 'patch:' .codecov.yml | grep 'target:' | head -1 | grep -oE '[0-9]+' | head -1) + patch_target=$(yq -r '.coverage.status.patch.default.target // ""' .codecov.yml | grep -oE '[0-9]+' | head -1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml around lines 168 - 179, The `.codecov.yml` patch coverage check is using fragile grep-based parsing in the shell block that reads `patch_target`, which can misread the wrong `target` value if the YAML layout changes. Replace that parsing logic with a structural YAML lookup in the same check step, using a real parser such as `yq`, and keep the PASS/FAIL behavior in the surrounding script (`pass`, `fail`, `warn`) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml:
- Line 52: The pipeline step is using the mutable ubi9/ubi-minimal:latest image,
which should be pinned like the git-clone task to keep runs reproducible. Update
the image reference in the Tekton task so this step uses an immutable tag or
digest instead of latest, keeping the change localized to the step that
currently references registry.access.redhat.com/ubi9/ubi-minimal.
- Around line 168-179: The `.codecov.yml` patch coverage check is using fragile
grep-based parsing in the shell block that reads `patch_target`, which can
misread the wrong `target` value if the YAML layout changes. Replace that
parsing logic with a structural YAML lookup in the same check step, using a real
parser such as `yq`, and keep the PASS/FAIL behavior in the surrounding script
(`pass`, `fail`, `warn`) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 732cc02f-d11a-4508-89a6-5d89d0aa4660
📒 Files selected for processing (1)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml
Replace the stale git-clone Tekton bundle reference (SHA no longer exists in Quay) with an inline clone step. Uses git-auth workspace credentials for authenticated clone to support private repos. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous approach failed because: 1. ubi-minimal can't install packages (permission denied for microdnf) 2. Two-step approach skips check-conformance when clone step fails, since onError:continue only prevents pipeline failure, not intra-task step skipping Fix by combining into a single step using alpine/git (has git pre-installed) so there's no step dependency and no package install. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/hold |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml (3)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStderr suppression hides install failures.
microdnf install -y git-core 2>/dev/nulldiscards error output. If the install fails,set -ewill abort the step but the log will show no reason why, complicating debugging.♻️ Proposed fix
- microdnf install -y git-core 2>/dev/null + microdnf install -y git-core🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml at line 41, The microdnf install step is suppressing stderr, which hides the failure reason if git-core cannot be installed. Update the install command in the deadmanssnitch operator agentic SDLC check step to stop redirecting stderr to /dev/null so failures are visible in logs, while keeping the existing set -e behavior. Focus on the microdnf install invocation in the YAML snippet and preserve its current package install logic.
21-254: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding step/task timeouts.
Neither the
clonenorcheck-conformancesteps, nor thePipelineRun/task, declare atimeout. A stalled network clone (e.g., due to auth/DNS issues) could hang indefinitely, tying up the pod. Adding atimeoutfield improves resiliency for an informing-only check that shouldn't be allowed to block resources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml around lines 21 - 254, The Tekton pipeline has no timeout guardrails, so a stalled clone or conformance check can hang indefinitely. Add explicit timeout settings to the `agentic-sdlc-check` task/run and to the `clone` and `check-conformance` steps so the job fails fast instead of blocking resources. Update the `pipelineSpec.tasks` definition and the `taskSpec.steps` for `clone` and `check-conformance` to use appropriate timeout values.
168-179: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile grep-based YAML parsing of
.codecov.ymlpatch target.
grep -A5 'patch:' .codecov.yml | grep 'target:' | head -1 | grep -oE '[0-9]+'works for the current.codecov.ymllayout but breaks silently (falls into thewarnbranch) if the file is reformatted or thepatch:block exceeds 5 lines beforetarget:. Since this is a non-blocking, informing check this is low risk, but a YAML-aware tool likeyqwould be more robust if extended later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml around lines 168 - 179, The `.codecov.yml` patch target check is using fragile grep-based parsing in the PR validation script, so it can misread or miss the value when the YAML layout changes. Update the parsing logic in the Dead Man’s Snitch operator agentic SDLC check step to use a YAML-aware approach instead of the current `grep -A5 ... target:` pipeline, keeping the same pass/fail/warn behavior but making the `.codecov.yml` patch coverage target extraction robust to formatting changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml:
- Line 41: The microdnf install step is suppressing stderr, which hides the
failure reason if git-core cannot be installed. Update the install command in
the deadmanssnitch operator agentic SDLC check step to stop redirecting stderr
to /dev/null so failures are visible in logs, while keeping the existing set -e
behavior. Focus on the microdnf install invocation in the YAML snippet and
preserve its current package install logic.
- Around line 21-254: The Tekton pipeline has no timeout guardrails, so a
stalled clone or conformance check can hang indefinitely. Add explicit timeout
settings to the `agentic-sdlc-check` task/run and to the `clone` and
`check-conformance` steps so the job fails fast instead of blocking resources.
Update the `pipelineSpec.tasks` definition and the `taskSpec.steps` for `clone`
and `check-conformance` to use appropriate timeout values.
- Around line 168-179: The `.codecov.yml` patch target check is using fragile
grep-based parsing in the PR validation script, so it can misread or miss the
value when the YAML layout changes. Update the parsing logic in the Dead Man’s
Snitch operator agentic SDLC check step to use a YAML-aware approach instead of
the current `grep -A5 ... target:` pipeline, keeping the same pass/fail/warn
behavior but making the `.codecov.yml` patch coverage target extraction robust
to formatting changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ef5d9b96-23af-414f-92ec-da5439df64b4
📒 Files selected for processing (1)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml
Remove onError:continue so non-conformant repos show a red X on the PR checks tab. The check is not in required_status_checks (prow config in openshift/release), so it won't block /lgtm + /approve merges — but the visible failure ensures repo owners notice non-compliance instead of silently passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #337 +/- ##
=======================================
Coverage 43.28% 43.28%
=======================================
Files 11 11
Lines 834 834
=======================================
Hits 361 361
Misses 424 424
Partials 49 49 🚀 New features to boost your workflow:
|
- Use Konflux catalog git-clone:0.1 bundle (correct SHA) for cloning - Use quay.io/konflux-ci/git-clone image for check steps (Red Hat owned, has git+bash, same image the catalog task uses) - Split into three pipeline tasks: clone-repository, check-file-existence, check-content-validation - File existence and content validation run in parallel after clone Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml (1)
146-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile YAML parsing via
grep/regex for.codecov.ymlpatch target.
grep -A5 'patch:' .codecov.yml | grep 'target:' | ...is brittle against reordering, comments, or additional nested keys in the YAML. Given the file is informing-only and low frequency of change, this is low priority, but a proper YAML tool (yq) would be more robust if this pipeline gets promoted to boilerplate for other repos.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml at line 146, The `.codecov.yml` patch target extraction in the pipeline is using fragile `grep`/regex parsing, so update the logic in this step to use a real YAML parser instead. Replace the current `.codecov.yml` lookup around the `patch_target` assignment with a `yq`-based read of the nested `patch.target` value, keeping the surrounding script behavior the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml:
- Around line 43-93: The conformance checks are currently hard-failing the
PipelineRun instead of behaving as informing-only checks. Add onError: continue
to the PipelineTask definitions for check-file-existence and
check-content-validation so failures are recorded without stopping the pipeline,
and keep the scripts’ exit 1 behavior only for the task status. Use the existing
task names and their script blocks to locate the two PipelineTasks in the Tekton
manifest.
- Line 50: The check steps are using a workingDir under
$(workspaces.source.path)/source, but task-git-clone:0.1 clones into the
workspace root by default, so update the pipeline/task wiring to make the paths
match. Either change the workingDir in the relevant check steps to the cloned
workspace root, or configure git-clone with subdirectory: source so the source
directory actually exists; use the existing workingDir settings and git-clone
task references to locate the two affected steps.
---
Nitpick comments:
In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml:
- Line 146: The `.codecov.yml` patch target extraction in the pipeline is using
fragile `grep`/regex parsing, so update the logic in this step to use a real
YAML parser instead. Replace the current `.codecov.yml` lookup around the
`patch_target` assignment with a `yq`-based read of the nested `patch.target`
value, keeping the surrounding script behavior the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e1d50110-3f45-4668-a039-6cf90c89e4d3
📒 Files selected for processing (1)
.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml
| - name: check-file-existence | ||
| runAfter: | ||
| - clone-repository | ||
| taskSpec: | ||
| steps: | ||
| - name: check-files | ||
| image: quay.io/konflux-ci/git-clone@sha256:09ac9c14392b5c2b8057f66cc4abfb8ce5d7214706318959d00908923a754434 | ||
| workingDir: $(workspaces.source.path)/source | ||
| script: | | ||
| #!/usr/bin/env sh | ||
| set -eu | ||
|
|
||
| PASS=0 | ||
| FAIL=0 | ||
|
|
||
| pass() { echo "PASS: $1"; PASS=$((PASS + 1)); } | ||
| fail() { echo "FAIL: $1"; FAIL=$((FAIL + 1)); } | ||
|
|
||
| echo "=== Agentic SDLC Conformance: File Existence (ROSA-730) ===" | ||
| echo "Repository: $(basename $(pwd))" | ||
| echo "Date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| echo "" | ||
|
|
||
| if [ -f "CLAUDE.md" ]; then pass "CLAUDE.md exists"; else fail "CLAUDE.md missing"; fi | ||
| if [ -f ".pre-commit-config.yaml" ]; then pass ".pre-commit-config.yaml exists"; else fail ".pre-commit-config.yaml missing"; fi | ||
| if [ -f ".codecov.yml" ]; then pass ".codecov.yml exists"; else fail ".codecov.yml missing"; fi | ||
| if [ -f "CONTRIBUTING.md" ]; then pass "CONTRIBUTING.md exists"; else fail "CONTRIBUTING.md missing"; fi | ||
| if [ -f "DEVELOPMENT.md" ]; then pass "DEVELOPMENT.md exists"; else fail "DEVELOPMENT.md missing"; fi | ||
| if [ -f "TESTING.md" ]; then pass "TESTING.md exists"; else fail "TESTING.md missing"; fi | ||
|
|
||
| if [ -f "CLAUDE.md" ]; then | ||
| if [ -f ".claude/settings.json" ]; then pass ".claude/settings.json exists"; else fail ".claude/settings.json missing (CLAUDE.md present but no Claude hooks)"; fi | ||
| fi | ||
|
|
||
| if [ -d "boilerplate/openshift/golang-osd-operator" ]; then | ||
| if [ -f "boilerplate/openshift/golang-osd-operator/golangci.yml" ]; then pass "golangci.yml exists (boilerplate convention)"; else fail "golangci.yml missing from boilerplate/openshift/golang-osd-operator/"; fi | ||
| elif [ -d "boilerplate/openshift/golang-lint" ]; then | ||
| if [ -f "boilerplate/openshift/golang-lint/golangci.yml" ]; then pass "golangci.yml exists (golang-lint convention)"; else fail "golangci.yml missing from boilerplate/openshift/golang-lint/"; fi | ||
| else | ||
| if [ -f ".golangci.yml" ] || [ -f ".golangci.yaml" ]; then pass "golangci config exists at repo root"; else fail "golangci config missing (no boilerplate dir and no root .golangci.yml/.yaml)"; fi | ||
| fi | ||
|
|
||
| echo "" | ||
| TOTAL=$((PASS + FAIL)) | ||
| echo "File existence: $PASS / $TOTAL passed" | ||
| if [ "$FAIL" -gt 0 ]; then exit 1; fi | ||
| workspaces: | ||
| - name: source | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Missing onError: continue — pipeline will actually block/fail on non-conformance, contradicting the stated design.
The PR objective states this pipeline is meant to be informing-only via onError: continue, "so the pipeline always succeeds while reporting failures as PR check statuses without blocking merge." Neither check-file-existence nor check-content-validation sets onError: continue at the PipelineTask level, and both scripts exit 1 on failures (Line 88, Line 199). Without onError: continue on the PipelineTask, a failing check causes the whole PipelineRun to be marked failed (default stopAndFail), which is the opposite of "non-blocking."
🔧 Proposed fix
- name: check-file-existence
runAfter:
- clone-repository
+ onError: continue
taskSpec:
steps:
...
- name: check-content-validation
runAfter:
- clone-repository
+ onError: continue
taskSpec:
steps:As per PR objectives: "It uses onError: continue, so the pipeline always succeeds while reporting failures as PR check statuses without blocking merge."
Also applies to: 94-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.tekton/deadmanssnitch-operator-agentic-sdlc-check-pull-request.yaml around
lines 43 - 93, The conformance checks are currently hard-failing the PipelineRun
instead of behaving as informing-only checks. Add onError: continue to the
PipelineTask definitions for check-file-existence and check-content-validation
so failures are recorded without stopping the pipeline, and keep the scripts’
exit 1 behavior only for the task status. Use the existing task names and their
script blocks to locate the two PipelineTasks in the Tekton manifest.
The catalog git-clone task needs the build service account to resolve ConfigMaps (trusted-ca) and access git-auth secrets in the namespace. Match the existing build pipeline's taskRunTemplate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The git-clone task defaults subdirectory to "source", which our check steps rely on. Make it explicit to avoid breakage if the default changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@tnierman: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Summary
required_status_checks(prow config), so/lgtm+/approvestill merge via tideWhat it checks
File existence (parallel task)
CLAUDE.md,.pre-commit-config.yaml,.codecov.ymlCONTRIBUTING.md,DEVELOPMENT.md,TESTING.md.claude/settings.json(ifCLAUDE.mdexists)golangci.yml(in boilerplate convention dir or repo root)Content validation (parallel task)
CLAUDE.mdhas required sections (build commands, architecture/overview).codecov.ymlpatch coverage target >= 50%.pre-commit-config.yamlincludes gitleaks, golangci-lint, file hygiene hooksExpected behavior on this repo
This repo is currently partially conformant — the check reports failures for missing
CONTRIBUTING.md,DEVELOPMENT.md,TESTING.md, and.claude/settings.json. The pipeline fails (red X) but does not block merge.Test plan
ROSA-730
🤖 Generated with Claude Code