Tdalton/rekor check fix

[tommyd450]

No description provided.

[qodo-for-securesign]

PR Summary by Qodo

Fix cosign e2e image setup by pushing test images to quay.io

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Replace ttl.sh-based test images with quay.io-pushed images for reliable Rekor checks.
• Add quay.io login and DOCKER_CONFIG wiring to the GitHub Actions e2e workflow.
• Centralize test image push logic in a shared testsupport helper with auto-expiration.

Diagram

graph TD
  GA["GitHub Actions runner"] --> WF["e2e.yml workflow"] --> LOGIN["podman-login quay.io"] --> TESTS["Go e2e tests"] --> PUSH["testsupport.PushTestImage"] --> QUAY[("quay.io test repo")]
  PUSH --> BASE[("registry.k8s.io base")]
  TESTS --> SIGSTORE["Rekor/Fulcio services"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a local in-cluster/Kind registry instead of quay.io

• ➕ No external registry dependency or secrets
• ➕ Faster and more hermetic CI runs
• ➖ Requires extra Kind/cluster wiring and image routing
• ➖ May diverge from real-world registry interactions used by cosign flows

2. Push to GHCR using GITHUB_TOKEN instead of quay.io secrets

• ➕ No additional secrets needed in the workflow
• ➕ Already logged in for package pulls
• ➖ No built-in auto-expiration; cleanup/retention needs handling
• ➖ Permissions/visibility can be tricky for forks and PR contexts

3. Keep ttl.sh ephemeral registry approach

• ➕ No credentials required
• ➕ Simple implementation
• ➖ External reliability/availability issues can break e2e runs
• ➖ TTL constraints can cause flaky behavior mid-test

Recommendation: The chosen quay.io approach is a good balance of realism and stability: it preserves a real registry push/pull flow while avoiding ttl.sh fragility via a controlled repo and short-lived images (quay.expires-after=1h). Ensure QUAY_USER/QUAY_PASSWORD are available in the CI context intended (especially for PRs from forks) and consider documenting required secrets/permissions for contributors.

Files changed (4) +82 / -59

Tests (2) +8 / -59

cosign_sign_verify_test.goReplace ttl.sh + Docker client image push with PushTestImage helper +5/-30

Replace ttl.sh + Docker client image push with PushTestImage helper

• Removes inline Docker client logic that pulled/tagged/pushed to ttl.sh. Uses testsupport.PushTestImage() to produce a unique, pushable image reference for the cosign signing/verification flow.

test/cosign/cosign_sign_verify_test.go

cosign_sign_verify_tsa_test.goUse shared PushTestImage helper for TSA test image setup +3/-29

Use shared PushTestImage helper for TSA test image setup

• Drops the ttl.sh-based Docker push flow and replaces it with testsupport.PushTestImage() for consistent test image provisioning. Keeps manual-image mode intact via TARGET_IMAGE_NAME when configured.

test/cosign/cosign_sign_verify_tsa_test.go

Other (2) +74 / -0

e2e.ymlAuthenticate to quay.io and pass Docker config to tests +9/-0

Authenticate to quay.io and pass Docker config to tests

• Adds a quay.io login step using podman-login and writes auth to /tmp/config.json. Exposes DOCKER_CONFIG=/tmp when running the e2e tests so go-containerregistry can resolve credentials from the generated config.

.github/workflows/e2e.yml

image.goAdd PushTestImage helper using go-containerregistry with quay expiry label +65/-0

Add PushTestImage helper using go-containerregistry with quay expiry label

• Introduces a helper that pulls a base image (registry.k8s.io/pause:3.9), injects quay.expires-after=1h into image labels, and pushes to quay.io/securesign/e2e-tests with a unique UUID tag. Auth is resolved via the default keychain, relying on DOCKER_CONFIG populated by the CI login step.

test/testsupport/image.go

[qodo-for-securesign]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Hard-coded Quay push dependency 🐞 Bug ☼ Reliability

Description

When MANUAL_IMAGE_SETUP is not "true" (default is "false"), the cosign E2E tests now always call
PushTestImage, which pushes to a hard-coded quay.io/securesign/e2e-tests using
authn.DefaultKeychain. Any environment without Quay write credentials in its Docker config will fail
before executing the actual cosign/rekor assertions.

Code

test/testsupport/image.go[R58-61]

+	logrus.Infof("Pushing test image: %s", targetRef)
+	if err := remote.Write(dstRef, img, remote.WithAuthFromKeychain(authn.DefaultKeychain), remote.WithContext(ctx)); err != nil {
+		return "", fmt.Errorf("pushing image: %w", err)
+	}

Relevance

●●● Strong

PR95 accepted concern that hard-coded Quay push + DefaultKeychain fails without creds; team likely
wants it configurable/guarded.

PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The tests take the non-manual path by default and invoke PushTestImage, which always pushes to a
fixed Quay repo and uses DefaultKeychain-based auth; therefore, missing Quay write creds/config
deterministically breaks test startup.

pkg/api/values.go[56-65]
test/cosign/cosign_sign_verify_test.go[66-73]
test/cosign/cosign_sign_verify_tsa_test.go[41-48]
test/testsupport/image.go[15-61]
PR-#95

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
E2E tests now depend on pushing an image to a hard-coded Quay repository via `PushTestImage()`. Because `MANUAL_IMAGE_SETUP` defaults to `false`, the default test path attempts this push and fails in any environment lacking Quay write credentials (or access to that repo), making the test suite non-portable and CI/environment fragile.

### Issue Context
- `MANUAL_IMAGE_SETUP` defaults to `false`, so the push path is used unless explicitly overridden.
- `PushTestImage()` hard-codes the destination repo and relies on `authn.DefaultKeychain`.

### Fix Focus Areas
- test/testsupport/image.go[15-61]
- test/cosign/cosign_sign_verify_test.go[66-73]
- test/cosign/cosign_sign_verify_tsa_test.go[41-48]
- pkg/api/values.go[56-65]

### Suggested fix approach
1. Add an env-configurable destination (e.g., `TEST_IMAGE_REPO` / `TEST_IMAGE_REGISTRY`) and default it to the current Quay repo.
2. Optionally add a fallback/escape hatch when creds are missing (e.g., fail with a clear message telling users to set `MANUAL_IMAGE_SETUP=true` and `TARGET_IMAGE_NAME`, or support an anonymous registry like `ttl.sh` for local/dev).
3. Update `.github/workflows/e2e.yml` to set the new env var explicitly (so CI remains deterministic).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Misleading Docker auth comment 🐞 Bug ⚙ Maintainability

Description

PushTestImage’s comment states auth comes from ~/.docker/config.json, but the workflow sets
DOCKER_CONFIG=/tmp and writes /tmp/config.json, and the code actually relies on DefaultKeychain
behavior. This mismatch can mislead maintainers when diagnosing auth failures.

Code

test/testsupport/image.go[R20-23]

+// PushTestImage pulls a base image, adds a quay.io auto-expiration label,
+// and pushes it to quay.io/securesign/e2e-tests with a unique tag.
+// Auth is resolved from the Docker config (~/.docker/config.json).
+func PushTestImage(ctx context.Context) (string, error) {

Relevance

●●● Strong

PR95 accepted fixing PushTestImage auth/config mismatch; misleading ~/.docker/config.json comment
should be corrected.

PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper’s comment asserts a fixed config location, but the workflow configures DOCKER_CONFIG to
point at /tmp and the helper uses DefaultKeychain rather than explicitly reading
~/.docker/config.json.

test/testsupport/image.go[20-23]
.github/workflows/e2e.yml[131-152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PushTestImage()` claims authentication is resolved from `~/.docker/config.json`, but CI config uses `DOCKER_CONFIG=/tmp` and the implementation uses `authn.DefaultKeychain` (which may honor `DOCKER_CONFIG` and other keychain sources). The comment is now incomplete/misleading.

### Issue Context
The E2E workflow explicitly sets `DOCKER_CONFIG=/tmp` while `podman-login` writes `/tmp/config.json`, so the documented path does not match the intended execution environment.

### Fix Focus Areas
- test/testsupport/image.go[20-23]
- .github/workflows/e2e.yml[131-152]

### Suggested fix approach
- Update the comment to describe credential resolution more accurately, e.g. “Auth is resolved from the Docker config (honors `DOCKER_CONFIG`, defaulting to `~/.docker/config.json`) via `authn.DefaultKeychain`.”

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}