fix: use %w for error wrapping and add missing bounds check

[Fortune-Ndlovu]

Summary

This PR solves several small but important error handling and correctness issues across the operator:

• Broken error wrapping (%s -> %w): 6 fmt.Errorf calls across pkg/model/runtime.go and internal/controller/spec_preprocessor.go used %s instead of %w, which dropped the error chain and broke errors.Is/errors.As for callers
• Missing bounds check in mergeDeployments: pkg/model/deployment.go accessed objs[0] without checking for an empty slice, which could panic and crash the controller. Added a guard consistent with the existing check in mergeDynamicPlugins, including source file paths in the error for debuggability
• Misleading error message for overlay reads: pkg/model/runtime.go reported "failed to read default value" when the error was actually from reading overlay config, sending operators looking in the wrong place during debugging

Test plan

• make test passes, all unit and integration tests green

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0)

1. Opaque empty deployment error ✓ Resolved 🐞 Bug ◔ Observability

Description

mergeDeployments now errors when parsing yields zero objects, but the error message omits which
deployment source file(s) were merged. This makes reconciliation failures difficult to diagnose when
base + flavour deployment.yaml inputs are involved.

Code

pkg/model/deployment.go[R499-501]

+	if len(objs) == 0 {
+		return nil, fmt.Errorf("no objects found in merged deployment")
+	}

Evidence

The new guard prevents an index-out-of-range panic, but the returned error is generic and does not
include the merged source paths. In contrast, similar merge logic for dynamic-plugins includes the
specific source path in its "no objects found" error, demonstrating an existing codebase pattern for
actionable error messages.

pkg/model/deployment.go[470-503]
pkg/model/default-config.go[87-130]

Agent prompt

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

### Issue description
`mergeDeployments` returns `fmt.Errorf("no objects found in merged deployment")` when `utils.ReadYamls(...)` returns an empty slice. When multiple deployment sources (base + flavours) are merged, this error lacks the key context needed to identify which input file(s) were empty/invalid.

### Issue Context
Other merge functions already include source file paths in similar errors (e.g., `mergeDynamicPlugins` returns `no objects found in %s`, `src.path`). Align `mergeDeployments` with that pattern.

### Fix Focus Areas
- pkg/model/deployment.go[499-501]

### Suggested change
Include the merged source paths (or at least `sources[0].path` plus flavour paths) in the error, e.g.:
- Build a `[]string` of `sources[i].path` and return `fmt.Errorf("no objects found after merging deployment sources: %v", paths)`
- Alternatively: `fmt.Errorf("no objects found in merged deployment (base: %s)", sources[0].path)` plus any flavour paths

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

---

Previous review results

Review updated until commit 368b1af

Results up to commit f627ae7

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Opaque empty deployment error ✓ Resolved 🐞 Bug ◔ Observability

Description

mergeDeployments now errors when parsing yields zero objects, but the error message omits which
deployment source file(s) were merged. This makes reconciliation failures difficult to diagnose when
base + flavour deployment.yaml inputs are involved.

Code

pkg/model/deployment.go[R499-501]

+	if len(objs) == 0 {
+		return nil, fmt.Errorf("no objects found in merged deployment")
+	}

Evidence

The new guard prevents an index-out-of-range panic, but the returned error is generic and does not
include the merged source paths. In contrast, similar merge logic for dynamic-plugins includes the
specific source path in its "no objects found" error, demonstrating an existing codebase pattern for
actionable error messages.

pkg/model/deployment.go[470-503]
pkg/model/default-config.go[87-130]

Agent prompt

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

### Issue description
`mergeDeployments` returns `fmt.Errorf("no objects found in merged deployment")` when `utils.ReadYamls(...)` returns an empty slice. When multiple deployment sources (base + flavours) are merged, this error lacks the key context needed to identify which input file(s) were empty/invalid.

### Issue Context
Other merge functions already include source file paths in similar errors (e.g., `mergeDynamicPlugins` returns `no objects found in %s`, `src.path`). Align `mergeDeployments` with that pattern.

### Fix Focus Areas
- pkg/model/deployment.go[499-501]

### Suggested change
Include the merged source paths (or at least `sources[0].path` plus flavour paths) in the error, e.g.:
- Build a `[]string` of `sources[i].path` and return `fmt.Errorf("no objects found after merging deployment sources: %v", paths)`
- Alternatively: `fmt.Errorf("no objects found in merged deployment (base: %s)", sources[0].path)` plus any flavour paths

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

---

[rhdh-qodo-merge]

Review Summary by Qodo

Fix error wrapping and add missing bounds check

🐞 Bug fix

Walkthroughs

Description

• Replace %s with %w in 6 fmt.Errorf calls to preserve error chains
• Add missing bounds check in mergeDeployments to prevent index panic
• Fix typo "reconsiled" to "reconciled" in controller comment

Diagram

flowchart LR
  A["Error Wrapping Issues"] -->|Replace %s with %w| B["Preserve Error Chains"]
  C["Missing Bounds Check"] -->|Add len check| D["Prevent Index Panic"]
  E["Typo in Comment"] -->|Fix reconsiled| F["Correct Documentation"]
  B --> G["Improved Error Handling"]
  D --> G
  F --> G

Loading

File Changes

1. internal/controller/backstage_controller.go 📝 Documentation +1/-1

Fix typo in reconciliation comment

• Fixed typo in comment: "reconsiled" → "reconciled"

internal/controller/backstage_controller.go

---

2. internal/controller/spec_preprocessor.go 🐞 Bug fix +1/-1

Use %w for proper error chain wrapping

• Changed fmt.Errorf format specifier from %s to %w for error wrapping
• Preserves error chain for errors.Is and errors.As compatibility

internal/controller/spec_preprocessor.go

---

3. pkg/model/deployment.go 🐞 Bug fix +3/-0

Add bounds check for empty deployment objects

• Added missing bounds check for empty slice before accessing objs[0]
• Prevents potential index-out-of-range panic
• Consistent with existing guard in mergeDynamicPlugins

pkg/model/deployment.go

---

View more (1)

4. pkg/model/runtime.go 🐞 Bug fix +5/-5

Use %w for error chain preservation in runtime

• Replaced %s with %w in 5 fmt.Errorf calls across InitObjects function
• Ensures error chains are preserved for proper error inspection
• Affects error messages for default config, overlay config, model initialization, validation, and
 external config

pkg/model/runtime.go

---

[github-actions]

PR images are available:

1. https://quay.io/rhdh-community/operator:0.10.0-pr-2803-f627ae7
2. https://quay.io/rhdh-community/operator-bundle:0.10.0-pr-2803-f627ae7
3. https://quay.io/rhdh-community/operator-catalog:0.10.0-pr-2803-f627ae7

[Fortune-Ndlovu]

/review

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removed after 2026-05-31).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error context The new empty-slice guard returns an error listing source paths; confirm this error is surfaced in a user-actionable way (e.g., includes enough context about why merging produced zero objects) and that returning only the first object remains the intended behavior when multiple objects are present. if len(objs) == 0 { paths := make([]string, len(sources)) for i, s := range sources { paths[i] = s.path } return nil, fmt.Errorf("no objects found after merging deployment sources: %v", paths) } return []client.Object{objs[0]}, nil Message accuracy The updated wrapped errors improve error chaining, but a couple of messages still say "failed to read default value" even when reading/decoding the overlay content; consider aligning wording with the actual source (default vs overlay) to reduce confusion during debugging. if objs, err := ReadDefaultConfig(conf, flavours, *scheme, platform.Extension); err != nil { if !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("failed to read default value for the key %s, reason: %w", conf.Key, err) } } else if len(objs) > 0 { if obj, err := adjustObject(conf, objs); err != nil { return nil, fmt.Errorf("failed to initialize object: %w", err) } else { backstageObject.setObject(obj) } } // read configuration defined in BackstageCR.Spec.RawConfigContent ConfigMap // if present, backstageObject's default configuration will be overridden overlay, overlayExist := externalConfig.RawConfig[conf.Key] if overlayExist { // new object to replace default, not merge if objs, err := utils.ReadYamls([]byte(overlay), nil, *scheme); err != nil { if !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("failed to read default value for the key %s, reason: %w", conf.Key, err) } } else { if obj, err := adjustObject(conf, objs); err != nil { return nil, fmt.Errorf("failed to initialize object: %w", err) } else { backstageObject.setObject(obj) } } } // apply spec and add the object to the model and list if added, err := backstageObject.addToModel(model, backstage); err != nil { return nil, fmt.Errorf("failed to initialize backstage, reason: %w", err) } else if added { backstageObject.setMetaInfo(backstage, scheme) } if ecc, ok := backstageObject.(ExternalConfigContributor); ok { // backstageObject is an instance of ExternalConfigContributor ecs = append(ecs, ecc) } } // set generic metainfo and updateAndValidate all for _, v := range model.RuntimeObjects { err := v.updateAndValidate(backstage) if err != nil { return nil, fmt.Errorf("failed object validation, reason: %w", err) } } // Add objects specified in Backstage CR for _, ecc := range ecs { if err := ecc.addExternalConfig(backstage.Spec); err != nil { return nil, fmt.Errorf("failed to contribute external config, reason: %w", err) }

📄 References redhat-developer/rhdh-operator/pkg/model/deployment_test.go [100-148] redhat-developer/rhdh-operator/pkg/model/deployment_test.go [150-160] redhat-developer/rhdh-operator/pkg/model/deployment_test.go [74-100] redhat-developer/rhdh-operator/internal/controller/watchers.go [130-176] redhat-developer/rhdh-operator/integration_tests/cr-config_test.go [305-356] redhat-developer/rhdh-operator/integration_tests/cr-config_test.go [225-265] redhat-developer/rhdh-operator/pkg/model/deployment_test.go [296-316] redhat-developer/rhdh-operator/pkg/model/deployment_test.go [1-54]

[github-actions]

PR images are available:

1. https://quay.io/rhdh-community/operator:0.10.0-pr-2803-e0c94d8
2. https://quay.io/rhdh-community/operator-bundle:0.10.0-pr-2803-e0c94d8
3. https://quay.io/rhdh-community/operator-catalog:0.10.0-pr-2803-e0c94d8

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Fortune-Ndlovu]

/agentic_review

[rhdh-qodo-merge]

Persistent review updated to latest commit 368b1af

[github-actions]

PR images are available:

1. https://quay.io/rhdh-community/operator:0.10.0-pr-2803-368b1af
2. https://quay.io/rhdh-community/operator-bundle:0.10.0-pr-2803-368b1af
3. https://quay.io/rhdh-community/operator-catalog:0.10.0-pr-2803-368b1af