cleanup sample app namespaces

[kaovilai]

cleanup: remove persistent namespaces after e2e tests

Adds deletion of mongo-persistent and mysql-persistent namespaces in the test-e2e-cleanup Makefile target. These sample app namespaces were left behind after e2e test runs, causing namespace pollution in the cluster.

Why the changes were made

E2E tests create mongo-persistent and mysql-persistent namespaces for sample application deployments but the cleanup target did not remove them. This left stale namespaces in the cluster after test runs.

How to test the changes made

• Run make test-e2e-cleanup after e2e tests and verify mongo-persistent and mysql-persistent namespaces are deleted
• --ignore-not-found=true ensures the command succeeds even if the namespaces don't exist

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e2cec95-911e-42e7-adbb-9b4c6bc3fb4c

📥 Commits

Reviewing files that changed from the base of the PR and between e3061bf and b364837.

📒 Files selected for processing (1)

• Makefile

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

• Makefile

---

Walkthrough

The test-e2e-cleanup target in the Makefile is updated to additionally delete the mongo-persistent and mysql-persistent namespaces using oc delete ns with the --ignore-not-found=true flag before removing temporary settings.

Changes

Cohort / File(s)	Summary
Makefile Updates Makefile	Added namespace deletion commands for mongo-persistent and mysql-persistent in the test-e2e-cleanup target, executed before temporary settings cleanup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

internal/controller/registry.go (1)

345-367: Unused variable hasStsFields can be removed.

The variable hasStsFields is set to true on line 361 but the function returns immediately on line 365, so it's never actually used. This appears to be leftover from a refactoring.

♻️ Suggested simplification

-				// Check if this profile contains STS fields
-				hasStsFields := false
 				for _, profLine := range splitString[index+1:] {
 					if profLine == "" {
 						continue
 					}
 					if keyNameRegex.MatchString(profLine) {
 						// We've reached the next profile
 						break
 					}
 					
 					// Check for STS-specific fields
 					if roleArnRegex.MatchString(profLine) || 
 					   webIdentityTokenFileRegex.MatchString(profLine) || 
 					   stsRegionalEndpointsRegex.MatchString(profLine) {
-						hasStsFields = true
 						r.Log.Info(fmt.Sprintf("Detected STS authentication in profile %s", matchProfile))
 						// For STS profiles, we return empty strings for access key and secret key
 						// but don't error out
 						return "", "", nil
 					}
 				}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/controller/registry.go` around lines 345 - 367, The local boolean
hasStsFields is unused and should be removed: in the STS-detection loop inside
the profile-parsing block (the loop that iterates splitString[index+1:] and
checks roleArnRegex, webIdentityTokenFileRegex, stsRegionalEndpointsRegex for
matchProfile), delete the declaration "hasStsFields := false" and the assignment
"hasStsFields = true"; keep the existing r.Log.Info(...) and the early return
("return \"\", \"\", nil") when any of roleArnRegex, webIdentityTokenFileRegex,
or stsRegionalEndpointsRegex matches, so the logic is preserved without the dead
variable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/controller/registry.go`:
- Around line 345-367: The local boolean hasStsFields is unused and should be
removed: in the STS-detection loop inside the profile-parsing block (the loop
that iterates splitString[index+1:] and checks roleArnRegex,
webIdentityTokenFileRegex, stsRegionalEndpointsRegex for matchProfile), delete
the declaration "hasStsFields := false" and the assignment "hasStsFields =
true"; keep the existing r.Log.Info(...) and the early return ("return \"\",
\"\", nil") when any of roleArnRegex, webIdentityTokenFileRegex, or
stsRegionalEndpointsRegex matches, so the logic is preserved without the dead
variable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b82a774-6ac7-467c-9472-550a8ea14416

📥 Commits

Reviewing files that changed from the base of the PR and between 661b69a and e3061bf.

📒 Files selected for processing (3)

• Makefile
• internal/controller/registry.go
• internal/controller/registry_test.go

[Copilot]

Pull request overview

Updates AWS credential parsing to tolerate STS/web-identity style profiles and improves e2e cleanup by removing leftover sample app namespaces.

Changes:

• Extend parseAWSSecret to detect STS-related fields and avoid failing when static access keys are absent.
• Add unit tests for parseAWSSecret, including an STS profile scenario, and extend existing secret fixtures.
• Update test-e2e-cleanup to delete mongo-persistent and mysql-persistent namespaces.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

File	Description
internal/controller/registry.go	Adds STS field detection to AWS secret parsing logic.
internal/controller/registry_test.go	Adds new table-driven tests for parseAWSSecret and updates AWS secret test data with an STS profile.
Makefile	Cleans up persistent sample namespaces as part of e2e cleanup target.

Comments suppressed due to low confidence (1)

Makefile:615

• oc delete ns can block for a long time (default --wait=true), and namespaces often get stuck terminating due to finalizers. Consider adding --wait=false (and/or a finalizer cleanup) here so test-e2e-cleanup doesn’t hang the pipeline when these namespaces are slow/stuck to delete.

		if [ -n "$$INSTALL_PLAN" ]; then \
			echo "InstallPlan $$INSTALL_PLAN found"; \

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mpryc]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, mpryc, weshayutin

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [kaovilai,mpryc]

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

[openshift-ci]

@kaovilai: all tests passed!

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.