Skip to content

fix(install): cosign version grep fails silently due to pipefail#384

Merged
lockwobr merged 2 commits intomainfrom
fix/install-cosign-bug
Mar 12, 2026
Merged

fix(install): cosign version grep fails silently due to pipefail#384
lockwobr merged 2 commits intomainfrom
fix/install-cosign-bug

Conversation

@lockwobr
Copy link
Copy Markdown
Contributor

Summary

  • Fixed silent install failure when cosign is present on the system
  • The cosign version --json grep pattern ("gitVersion":"[^"]*") didn't account for whitespace in the JSON output ("gitVersion": "v3.0.4"), causing grep to exit 1
  • With set -euo pipefail, the failed grep killed the script before reaching the install step — leaving the old binary in place with no error message
  • Added [[:space:]]* to the pattern and || true as a safety net so the existing fallback path (line 266) can actually execute

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/api, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: ____________

Implementation Notes

Testing

# Commands run (prefer `make qualify` for non-trivial changes)
make qualify

Risk Assessment

  • Low — Isolated change, well-tested, easy to revert
  • Medium — Touches multiple components or has broader impact
  • High — Breaking change, affects critical paths, or complex rollout

Rollout notes:

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint)
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality
  • I updated docs if user-facing behavior changed
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S) — GPG signing info

@lockwobr lockwobr self-assigned this Mar 12, 2026
@lockwobr lockwobr requested a review from a team as a code owner March 12, 2026 20:48
@github-actions
Copy link
Copy Markdown

Welcome to AICR, @lockwobr! Thanks for your first pull request.

Before review, please ensure:

  • All commits are signed off per the DCO
  • CI checks pass (tests, lint, security scan)
  • The PR description explains the why behind your changes

A maintainer will review this soon.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 12, 2026

Coverage Report ✅

Metric Value
Coverage 73.3%
Threshold 70%
Status Pass
Coverage Badge
![Coverage](https://img.shields.io/badge/coverage-73.3%25-green)

No Go source files changed in this PR.

mchmarny
mchmarny previously approved these changes Mar 12, 2026
@mchmarny mchmarny self-requested a review March 12, 2026 21:59
The JSON grep pattern for extracting cosign's gitVersion lacked
whitespace handling, causing grep to exit 1. Combined with
set -euo pipefail, this killed the script before reaching the
install step — leaving the old binary in place with no error shown.
@lockwobr lockwobr force-pushed the fix/install-cosign-bug branch from 9af0764 to e345905 Compare March 12, 2026 22:20
@lockwobr lockwobr requested a review from a team as a code owner March 12, 2026 22:20
@lockwobr lockwobr enabled auto-merge (squash) March 12, 2026 22:26
@lockwobr lockwobr merged commit 8550939 into main Mar 12, 2026
18 checks passed
@lockwobr lockwobr deleted the fix/install-cosign-bug branch March 12, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants