NO-JIRA: Downloads: Ensure cleanup of the artifact folder on each startup

[hongkailiu]

Downloads: Ensure cleanup of the artifact folder on each startup

Follow up openshift/console-operator#1176

The GoLang implementation uses a fixed folder name defaultArtifactsDir to store the files and cleans the folder up [1]. So it should not suffer the issue of accumulating the files over startups.

An existing unit test ensures already the same defaultArtifactsDir is used [2].

Another existing unit test checks folders with the prefix defaultArtifactsDir are removed. We add the logic in the pull to this unit test to ensure that the files in defaultArtifactsDir from previous container are removed too in the new container after startup.

[1].

console/cmd/downloads/config/downloads_config.go

Lines 119 to 122 in 71f7e03

	matches, _ := filepath.Glob("/tmp/artifacts*")
	for _, match := range matches {
	os.RemoveAll(match)
	}

[2].

console/cmd/downloads/config/downloads_config_test.go

Lines 266 to 268 in 28f9ca4

	if cfg.TempDir != defaultArtifactsDir {
	t.Errorf("TempDir = %q, want %q", cfg.TempDir, defaultArtifactsDir)
	}

Analysis / Root cause:

Solution description:

Screenshots / screen recording:

Test setup:

Test cases:

Browser conformance:

• Chrome
• Firefox
• Safari (or Epiphany on Linux)

Additional info:

Reviewers and assignees:

Summary by CodeRabbit

• Tests
  • Improved the downloads configuration test to ensure all previously created temporary directories are cleaned up during server config creation, including the default artifacts directory.
  • Added a shared “leftover” sentinel file written into each cleanup target to reliably validate directory removal, with special-case assertions for the default artifacts directory.
  • Expanded cleanup expectations to also remove additional /tmp/artifacts*test* artifact-related temporary directories left by the test run.

[openshift-ci-robot]

@hongkailiu: This pull request explicitly references no jira issue.

Details

In response to this:

Follow up openshift/console-operator#1176

The GoLang implementation uses a fixed folder name defaultArtifactsDir to store the files and cleans the folder up [1]. So it should not suffer the issue of accumulating the files over startups.

The current unit test ensures already the same defaultArtifactsDir is used [2], and we add the logic in the pull to ensure that the file from previous container is removed in the new container after startup.

[1].

console/cmd/downloads/config/downloads_config.go

Lines 119 to 122 in 71f7e03

	matches, _ := filepath.Glob("/tmp/artifacts*")
	for _, match := range matches {
	os.RemoveAll(match)
	}

[2].

console/cmd/downloads/config/downloads_config_test.go

Lines 266 to 268 in 28f9ca4

	if cfg.TempDir != defaultArtifactsDir {
	t.Errorf("TempDir = %q, want %q", cfg.TempDir, defaultArtifactsDir)
	}

Analysis / Root cause:

Solution description:

Screenshots / screen recording:

Test setup:

Test cases:

Browser conformance:

• Chrome
• Firefox
• Safari (or Epiphany on Linux)

Additional info:

Reviewers and assignees:

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4e60801d-ff9a-4938-81f6-e7896c3cde4b

📥 Commits

Reviewing files that changed from the base of the PR and between b2a248c and a95b53c.

📒 Files selected for processing (1)

• cmd/downloads/config/downloads_config_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/downloads/config/downloads_config_test.go

---

Walkthrough

The downloads config test now includes defaultArtifactsDir in the stale directory setup and verifies its sentinel file is removed with a special-case assertion.

Changes

Downloads config cleanup test

Layer / File(s)	Summary
Cleanup setup cmd/downloads/config/downloads_config_test.go	The test adds defaultArtifactsDir to oldDirs and writes a shared leftover file into each prepared directory.
Cleanup assertions cmd/downloads/config/downloads_config_test.go	The verification step checks that the leftover file under defaultArtifactsDir is removed and skips the generic removed-directory assertion for that path.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title matches the change, but it violates the repository rule requiring a Jira issue key prefix instead of NO-JIRA.	Rename it to a Jira-backed title like OCPBUGS-XXXX: Downloads: Ensure cleanup of the artifact folder on each startup.
Description check	⚠️ Warning	The PR description is mostly template placeholders and lacks the required Analysis, Solution, Test setup, Test cases, and other filled sections.	Fill in the required template sections with concrete details, validation steps, browser conformance, and reviewer/assignee information.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	Touched tests use only static names; the new name is deterministic and no Ginkgo titles or runtime-derived values were added.
Test Structure And Quality	✅ Passed	The changed test is isolated, uses t.Cleanup for cleanup, has explicit failure messages, and contains no cluster waits or resource leaks.
Microshift Test Compatibility	✅ Passed	PASS: The changed file is a standard Go unit test (testing only), with no Ginkgo e2e constructs or MicroShift-unsupported OpenShift APIs.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The change only updates a Go unit test in downloads_config_test.go; no new Ginkgo e2e tests or SNO-relevant multi-node assumptions were added.
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The PR only edits a unit test in cmd/downloads/config; no manifests, controllers, or scheduling constraints were added or modified.
Ote Binary Stdout Contract	✅ Passed	The PR only changes a unit test; no main/init/TestMain/RunSpecs or stdout writes were added in process-level code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Changed test is a Go unit test, not Ginkgo e2e, and it contains no IPv4-only or external connectivity assumptions.
No-Weak-Crypto	✅ Passed	The only changed file is a test; the diff adds cleanup assertions and no MD5/SHA1/DES/RC4/3DES/ECB/custom crypto or secret comparisons.
Container-Privileges	✅ Passed	PASS: The PR only changes a Go unit test; no manifests or container specs were modified, and no privilege-related fields exist under cmd/downloads.
No-Sensitive-Data-In-Logs	✅ Passed	Only test cleanup assertions changed; no new logs were added, and existing logs are generic path/cleanup messages.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hongkailiu
Once this PR has been reviewed and has the lgtm label, please assign therealjon for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• cmd/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@cmd/downloads/config/downloads_config_test.go`:
- Around line 321-326: The test setup in the oldDirs loop ignores the error from
os.WriteFile, which can make the fixture creation silently fail. Update the
downloads_config_test setup to check and handle the os.WriteFile return value,
using the existing test failure pattern in the same block so the cleanup
assertions only run after the leftover file is definitely created.

🪄 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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 27f6f2d0-9bad-4a4d-a24d-19acd2cf77cb

📥 Commits

Reviewing files that changed from the base of the PR and between efe9dc8 and c8d0b21.

📒 Files selected for processing (1)

• cmd/downloads/config/downloads_config_test.go

[openshift-ci]

@hongkailiu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-playwright	a95b53c	link	false	/test e2e-playwright

Full PR test history. Your PR dashboard.

Details

Instructions 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.