🐛ClusterExtension configurations using watchNamespace: "" (empty string) no longer fail with a confusing validation error.

[camilamacedo86]

Motivation

https://issues.redhat.com//browse/OCPBUGS-63612

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	1eb4e4a
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/691d77473cd2270008622297
😎 Deploy Preview	https://deploy-preview-2348--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[Copilot]

Pull Request Overview

This PR fixes a bug where an empty string value for watchNamespace is now treated as "not provided," defaulting to AllNamespaces install mode instead of causing validation errors.

Key Changes:

• Modified GetWatchNamespace() to return nil for empty string values, treating them as AllNamespaces mode
• Updated validation logic to skip validation for empty string watchNamespace, preventing spurious errors
• Added comprehensive test coverage for empty string handling across different install modes (OwnNamespace, SingleNamespace, AllNamespaces)

Reviewed Changes

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

File	Description
internal/operator-controller/config/config.go	Added empty string checks in GetWatchNamespace() and custom format validators to treat empty strings as AllNamespaces mode
internal/operator-controller/config/config_test.go	Added three test cases verifying empty string watchNamespace is accepted and treated as nil for different install mode combinations
test/e2e/single_namespace_support_test.go	Added new e2e test TestClusterExtensionEmptyWatchNamespace that verifies empty watchNamespace configuration results in successful AllNamespaces mode installation

---

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

[pedjak]

/lgtm

[perdasilva]

This is the desired behavior. OLM should not know about the semantics of a bundle configuration. A bundle that only support AllNamespaces does not provide a watchNamespace configuration. Therefore, watchNamespace should be treated as any other non-existing configuration key.

[rashmigottipati]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pedjak, perdasilva, rashmigottipati

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 [perdasilva]

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

[tmshort]

This is the desired behavior. OLM should not know about the semantics of a bundle configuration. A bundle that only support AllNamespaces does not provide a watchNamespace configuration. Therefore, watchNamespace should be treated as any other non-existing configuration key.

@perdasilva do you not approve (although you gave it LGTM), or are you asking for someone else to tag it approved?

[perdasilva]

/hold I don't think this is a bug

[perdasilva]

This is the desired behavior. OLM should not know about the semantics of a bundle configuration. A bundle that only support AllNamespaces does not provide a watchNamespace configuration. Therefore, watchNamespace should be treated as any other non-existing configuration key.

@perdasilva do you not approve (although you gave it LGTM), or are you asking for someone else to tag it approved?

I did originally approve. But then it occurred to me it's not right and I took the label away. I've put it on hold now until I can clarify everything with Camila

[camilamacedo86]

I’m closing this one.
I agree with @perdasilva.

I get up today, and start to look on it and I don’t think it’s maintainable for us to support that. It would require normalizing every option and flag, which leads to a worse UX overall. Some options treat an empty string as nil, others don’t, and we don’t have a reliable or consistent way to handle that across the board.

Close it. We should not do it.