[data] Improve error messages for empty PublicURL

[kkozoriz]

Description

When using --publish with DataExport commands (list, download, etc.), CLI showed uninformative "empty PublicURL" error if the public URL was not yet available or could not be created.

Now CLI provides clear, actionable messages:

• During wait: "waiting for public URL to be created by controller" with remaining timeout
• On timeout: suggests running d8 k -n <ns> get de <name> -o yaml to inspect conditions
• Condition messages from the controller (e.g. PublishFailed) are shown as-is instead of being overwritten

Also fixed a bug: podURL += "https://" → podURL = "https://" + podURL.

Changes

• internal/data/dataexport/util/util.go - reworked URL wait logic in GetDataExportWithRestart, removed redundant "empty PublicURL" check in getExportStatus
• internal/data/dataexport/util/util_test.go - added 7 test cases covering PublishFailed, Pending, timeout, and happy paths

Before

After

[szhem]

Code Review Summary

Severity	Count
Critical	0
High	0
Medium	1
Low	1
Nit	1

Positive Observations

• Core bug fix in GetDataExportWithRestart: the else if branch was incorrectly overwriting informative condition errors (e.g. PublishFailed: ingressPublicDomain not defined) with uninformative "empty PublicURL". The if returnErr == nil { switch {...} } pattern cleanly resolves this.
• Real bug fix: podURL += "https://" → podURL = "https://" + podURL — old code appended the scheme at the END of the URL.
• Quality tests: 7 test cases with regression comments explaining what each test prevents.
• The companion controller change (MR !100 in storage-volume-data-manager) writes Ready=False, Reason=PublishFailed condition — the end-to-end error chain is now complete.

[szhem]

Severity: Medium — Removed PublicURL == "" guard creates silent dependency on call order

The old guard if deObj.Status.PublicURL == "" { return err } was removed here. Currently safe because getExportStatus always calls GetDataExportWithRestart first, which guarantees PublicURL != "" on success.

However, if someone refactors getExportStatus to skip GetDataExportWithRestart, podURL will be "", and after the prefix check below it becomes "https://" — an invalid URL that will silently proceed to make HTTP requests.

Recommendation: Add a lightweight guard after assignment:

case public:
	podURL = deObj.Status.PublicURL
	if podURL == "" {
		return "", "", "", fmt.Errorf("DataExport %s/%s: publicURL is empty after waiting", namespace, deName)
	}
	if !strings.HasPrefix(podURL, "http") {
		podURL = "https://" + podURL
	}

[szhem]

Severity: Low — const → var for test override

Acceptable pattern for Go. The variable is unexported so mutation is limited to package util. Test correctly restores the original value via t.Cleanup.

If the retry logic grows more complex, consider accepting the count as a function parameter or struct field instead.

[szhem]

Severity: Nit — timeout_in field name is ambiguous

timeout_in could be read as "timeout input" rather than "timeout in X seconds". Consider remaining or time_remaining for clarity:

slog.String("remaining", remaining.String()),