Skip to content

Changes to remove ttl.sh dependency - #95

Merged
osmman merged 3 commits into
mainfrom
tdalton/ttlRemoval
Aug 4, 2026
Merged

Changes to remove ttl.sh dependency#95
osmman merged 3 commits into
mainfrom
tdalton/ttlRemoval

Conversation

@tommyd450

Copy link
Copy Markdown
Contributor

No description provided.

@qodo-for-securesign

Copy link
Copy Markdown

PR Summary by Qodo

Remove ttl.sh usage by pushing expiring test images to Quay

🧪 Tests ✨ Enhancement 🕐 40+ Minutes

Grey Divider

AI Description

• Replace ttl.sh-based test image publishing with a Quay-backed helper.
• Refactor cosign and TSA e2e tests to use shared image push logic.
• Add an auto-expiration label to keep the test image repository clean.
Diagram

graph TD
T["Cosign E2E tests"] --> D{"Manual image setup?"} -->|"no"| H["PushTestImage()"] --> Q[("Quay test repo")] --> C["cosign sign/verify"]
D -->|"yes"| E["ENV TARGET_IMAGE_NAME"] --> C
H --> S[("Base image registry")]

subgraph Legend
direction LR
_p["Process"] ~~~ _d{Decision} ~~~ _r[(Registry)]
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep Docker client flow, push to Quay directly
  • ➕ Avoids pulling/mutating/pushing via go-containerregistry APIs if Docker is already required in CI
  • ➖ Still requires Docker daemon access in the test environment
  • ➖ More verbose and harder to reuse across suites than a single helper
2. Run a local ephemeral registry for tests (kind/registry container)
  • ➕ No reliance on external registry availability or credentials
  • ➕ Fast push/pull loops and deterministic cleanup
  • ➖ More moving parts in CI and local runs
  • ➖ Still must ensure cosign can access the registry from where tests execute
3. Use a different hosted registry with built-in TTL/retention (e.g., GHCR)
  • ➕ Potentially simpler auth story in GitHub Actions
  • ➕ Centralized retention controls
  • ➖ Retention/TTL semantics can vary and may require org-level policy changes
  • ➖ May not align with existing project registry ownership/preferences

Recommendation: The current approach (shared PushTestImage() using go-containerregistry + Quay auto-expiration label) is a good fit: it removes ttl.sh, avoids Docker-daemon coupling, and centralizes behavior for multiple suites. The main operational prerequisite is ensuring Quay credentials are available via Docker config in the environments that run these tests.

Files changed (3) +73 / -59

Tests (3) +73 / -59
cosign_sign_verify_test.goSwitch cosign sign/verify test image setup to PushTestImage helper +5/-30

Switch cosign sign/verify test image setup to PushTestImage helper

• Removes ttl.sh + Docker-client-based pull/tag/push logic and replaces it with a single call to testsupport.PushTestImage. Drops the related Docker/uuid imports and variables, keeping the manual image setup escape hatch intact.

test/cosign/cosign_sign_verify_test.go

cosign_sign_verify_tsa_test.goSwitch TSA cosign test image setup to PushTestImage helper +3/-29

Switch TSA cosign test image setup to PushTestImage helper

• Eliminates ttl.sh + Docker-client-based image publishing and uses testsupport.PushTestImage instead. Cleans up imports/variables accordingly while preserving MANUAL_IMAGE_SETUP behavior.

test/cosign/cosign_sign_verify_tsa_test.go

image.goAdd PushTestImage helper to publish expiring images to Quay +65/-0

Add PushTestImage helper to publish expiring images to Quay

• Introduces a shared helper that pulls a base image, adds the quay.expires-after label, and pushes to quay.io/securesign/e2e-test with a unique tag. Uses Docker config keychain auth via go-containerregistry and returns the pushed image reference for tests to sign/verify.

test/testsupport/image.go

@tommyd450 tommyd450 changed the title Changes to remove ttl.sh dependenc Changes to remove ttl.sh dependency Aug 4, 2026
@qodo-for-securesign

qodo-for-securesign Bot commented Aug 4, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Unauthenticated Quay push ✓ Resolved 🐞 Bug ☼ Reliability
Description
testsupport.PushTestImage hard-codes pushing to quay.io/securesign/e2e-test and uses
authn.DefaultKeychain, so test setup fails anywhere without valid Quay push credentials. The repo’s
e2e workflow runs go test -v ./test/... without Quay login and without setting MANUAL_IMAGE_SETUP,
so the cosign suites will fail at image push during BeforeAll.
Code

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

+	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

Likely CI-breaking reliability issue; repo has accepted similar robustness fixes to avoid
flaky/failing runs.

PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Cosign tests now always call PushTestImage when MANUAL_IMAGE_SETUP is false;
MANUAL_IMAGE_SETUP defaults to false. PushTestImage pushes to a fixed Quay repo and requires
Docker keychain auth, while the e2e workflow neither sets MANUAL_IMAGE_SETUP=true nor logs into
quay.io before running go test -v ./test/....

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]
test/testsupport/image.go[15-61]
.github/workflows/e2e.yml[49-63]
.github/workflows/e2e.yml[123-143]

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

## Issue description
`testsupport.PushTestImage` always pushes to `quay.io/securesign/e2e-test` using `authn.DefaultKeychain`. In CI (and many dev environments) there are no Quay credentials available in the expected location, so the push fails and the cosign tests abort in `BeforeAll`.

## Issue Context
- E2E workflow logs into GHCR and registry.redhat.io, but not quay.io.
- The workflow uses an auth file at `/tmp/config.json`, while `PushTestImage` states it reads from `~/.docker/config.json`.
- `MANUAL_IMAGE_SETUP` defaults to `false`, so the auto-push path is taken.

## 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]
- .github/workflows/e2e.yml[49-63]
- .github/workflows/e2e.yml[123-143]

## Suggested fix direction
Implement one of these (pick one and align docs/workflow accordingly):
1) **Use a CI-available registry**: make the destination registry/repo configurable via env (e.g., `TEST_IMAGE_REPO`) and default it to something CI already authenticates to (or provide secrets + login step).
2) **Make auth discovery match CI**: add a Quay login step in `.github/workflows/e2e.yml` and ensure credentials are discoverable by `authn.DefaultKeychain` (e.g., place config at `~/.docker/config.json` or set the appropriate env so the keychain reads `/tmp/config.json`).
3) **Avoid external registry pushes**: start a local registry (e.g., via testcontainers or a Kind-registry pattern) and push the test image there, removing the need for Quay credentials entirely.

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



Remediation recommended

2. Docs still mention ttl.sh 🐞 Bug ⚙ Maintainability
Description
The README’s manual image setup example still directs users to use ttl.sh, even though the PR’s
stated goal is removing ttl.sh dependency and the default (non-manual) path now pushes to Quay. This
mismatch makes it unclear which registry users should use and what credentials are required after
this change.
Code

test/cosign/cosign_sign_verify_test.go[R68-72]

+			targetImageName, err = testsupport.PushTestImage(testsupport.TestContext)
			Expect(err).ToNot(HaveOccurred())
-			defer push.Close()
		} else {
			targetImageName = api.GetValueFor(api.TargetImageName)
			Expect(targetImageName).NotTo(BeEmpty(), "TARGET_IMAGE_NAME environment variable must be set when MANUAL_IMAGE_SETUP is true")
Relevance

●●● Strong

Docs mismatch with new default behavior; team typically accepts consistency/maintainability fixes.

PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
After this PR, the non-manual branch calls PushTestImage, which pushes to
quay.io/securesign/e2e-test; meanwhile README’s manual image example still uses ttl.sh and doesn’t
describe Quay/auth requirements implied by the new default behavior.

README.md[47-57]
test/cosign/cosign_sign_verify_test.go[66-73]
test/testsupport/image.go[15-25]

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

## Issue description
README guidance for manual image setup still uses `ttl.sh`, which conflicts with the PR intent (“remove ttl.sh dependency”) and doesn’t explain the new default auto-push behavior (Quay + credentials).

## Issue Context
- Manual mode still works with any registry reference set in `TARGET_IMAGE_NAME`, but the docs currently steer users to ttl.sh.
- Non-manual mode now pushes to quay.io and requires credentials.

## Fix Focus Areas
- README.md[47-57]
- test/testsupport/image.go[15-25]

## Suggested fix direction
- Clarify the two modes explicitly:
 - **Automatic mode** (MANUAL_IMAGE_SETUP=false): specify which registry is used, and document required auth.
 - **Manual mode** (MANUAL_IMAGE_SETUP=true): use a registry-agnostic example (or update to the intended replacement for ttl.sh) and mention any constraints (public accessibility, credentials).

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


Grey Divider

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

Qodo Logo

Comment thread test/testsupport/image.go
Comment thread test/testsupport/image.go Outdated
@osmman
osmman merged commit 4463a2b into main Aug 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants