OCM-20815 | chore: adding debug logs for proxy creation error

[l-urbini]

Changed log format and extended logs whenever missing.

[coderabbitai]

Walkthrough

Error logging in the LaunchProxyInstanceWithAuth function is standardized from %s to %v format. An explicit log entry is added for FindProxyLaunchImage() failures. Error messages for subnet preparation, security group creation, key pair operations, instance tagging, and network setup are updated to use consistent verbose formatting.

Changes

Cohort / File(s)

Summary

Error logging standardization pkg/test/vpc_client/proxy.go

Replace error logging format from %s to %v across multiple error paths in LaunchProxyInstanceWithAuth function. Add explicit log entry for FindProxyLaunchImage() failure. Standardize error log messages for subnet preparation, security group creation, key pair creation and tagging, private key file writing, proxy instance launching, instance tagging, EIP allocation, MITM proxy setup, and remote command execution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

• Homogeneous changes (consistent format replacement pattern) across multiple error logging statements reduces cognitive load per review pass
• No logic modifications or control-flow changes to verify
• Primary focus should be validating that all affected error paths are correctly converted and that the new FindProxyLaunchImage() log statement placement is appropriate

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a pull request description explaining the rationale for standardizing error log formatting and the benefits of enhanced logging.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding debug logs for proxy creation errors through standardized error formatting.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/test/vpc_client/proxy.go (1)

189-242: Debug logging currently leaks proxy credentials in plain text.

In setupMITMProxyServer, when authentication is enabled you build authScript with the literal username and password, then pass that full script as cmd to Exec_CMD. The subsequent log.LogDebug("Run the cmd successfully: %s", cmd) (Line 241) will log the entire multi-line shell/Python script, including the raw credentials, contradicting the security note at the top of the file and creating a serious secret-exposure risk.

I recommend not logging the full command content here. For example, you can log only a step index or a short label instead of cmd:

- for _, cmd := range setupProxyCMDs {
-     _, err = Exec_CMD(CON.AWSInstanceUser, sshKey, hostname, cmd)
-     if err != nil {
-         return err
-     }
-     log.LogDebug("Run the cmd successfully: %s", cmd)
- }
+ for i, cmd := range setupProxyCMDs {
+     _, err = Exec_CMD(CON.AWSInstanceUser, sshKey, hostname, cmd)
+     if err != nil {
+         return err
+     }
+     log.LogDebug("Run proxy setup command #%d successfully", i+1)
+ }

This preserves useful debug information (progress through setup steps) without ever logging the actual command text or embedded credentials.

🧹 Nitpick comments (1)

pkg/test/vpc_client/proxy.go (1)

112-185: Logging changes in LaunchProxyInstanceWithAuth look good; consider minor polish.

Switching the error formatting from %s to %v on all the log.LogError calls correctly matches Go’s fmt-style handling of error values and will produce more useful messages. The added log when FindProxyLaunchImage fails also makes this failure mode easier to diagnose from the caller’s perspective.

One minor nit: the message Add tag for instance %s failed (Line 161) still contains a double space before %s; you may want to normalize that while you’re touching the log line, but it’s purely cosmetic.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between dbfc861 and b5f2871.

📒 Files selected for processing (1)

• pkg/test/vpc_client/proxy.go (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Lint
• GitHub Check: Test (ubuntu-latest)
• GitHub Check: Test (windows-latest)
• GitHub Check: Test (macos-latest)

[lucasponce]

LGTM some linter issues

[l-urbini]

Hi @gdbranco would you be able to merge this?