✨ Promote Single Own Feature Gate AND Config spec in the CR to GA (OPRUN-4098)

[camilamacedo86]

Description

• Promote Single Own Feature Gate GA
• Promote the Config spec to the public/stable API. Without the spec.config.inline.watchNamespace field, there’s no place to declare which namespace the operator should watch, so the controller can only ever assume the default AllNamespaces behaviour. That’s why we need config available in the GA schema: it’s the user-facing knob that flips the controller into Single/Own namespace mode. Example:

  operator-controller/internal/operator-controller/applier/provider_test.go

  Lines 229 to 234 in 9eac616

  	Config: &ocv1.ClusterExtensionConfig{
  	ConfigType: ocv1.ClusterExtensionConfigTypeInline,
  	Inline: &apiextensionsv1.JSON{
  	Raw: []byte(`{"watchNamespace": "` + expectedWatchNamespace + `"}`),
  	},
  	},

Reviewer Checklist

• API Go Documentation
• Tests: Unit Tests (and E2E Tests, if appropriate)
• Comprehensive Commit Messages
• Links to related GitHub Issue(s)

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevinrizza for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	6b47340
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/68f332303ce12200087df04d
😎 Deploy Preview	https://deploy-preview-2268--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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.00%. Comparing base (292c0db) to head (6b47340).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2268   +/-   ##
=======================================
  Coverage   73.00%   73.00%           
=======================================
  Files          88       88           
  Lines        8743     8743           
=======================================
  Hits         6383     6383           
  Misses       1947     1947           
  Partials      413      413           

Flag	Coverage Δ
e2e	40.47% <ø> (+1.05%)	⬆️
experimental-e2e	46.79% <ø> (ø)
unit	58.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tmshort]

This seems to be conflated with #2267? i.e. this is changing the line for both the Webhook and Single/OwnNamespace features.

[camilamacedo86]

I thought the same when I was doing the PR. But let’s merge these in sequence: one first, then the other. The follow-up PR can be rebased and we’ll adjust the doc text—conflicts are only in the docs. I prefer keeping two PRs since they address different goals.

[camilamacedo86]

/hold

We might need to discuss more how to work with see: #2267 (comment)

So it si WIP

[camilamacedo86]

I think we can

/hold cancel

All comments are addressed
I am lookling for to see if we are satisfied with the default approach.

[tmshort]

Is there an order for this with respect to #2267?

[camilamacedo86]

Is there an order for this with respect to #2267?

No. I do think so.
But as we spoke, I think we will need to make changes here.
-> Do not enable feature flag by default
-> use the helm values to set those

/hold

until 🔨 meeting

[perdasilva]

@camilamacedo86 please extra hold this as I'd like to ensure that from the config side we are ready to GA. Please don't merge this without my /lgtm =D

[pedjak]

Promote the Config spec to the public/stable API. Without the spec.config.inline.watchNamespace field, there’s no place to declare which namespace the operator should watch, so the controller can only ever assume the default AllNamespaces behaviour. That’s why we need config available in the GA schema: it’s the user-facing knob that flips the controller into Single/Own namespace mode.

@camilamacedo86 Is it really needed to allow that .spec.config.inline is declared as Raw, i.e. can contain any json? This impact the discovery of API by users and CR validations later on. Would it be possible to declare .spec.config.watchNamespace actually with a proper type and validation rules?

[camilamacedo86]

Hi @pedjak

@camilamacedo86 Is it really needed to allow that .spec.config.inline is declared as Raw, i.e. can contain any json? This impact the discovery of API by users and CR validations later on. Would it be possible to declare .spec.config.watchNamespace actually with a proper type and validation rules?

We need to promote the SingleOwnNamespace feature.
This feature depends on the field crd.spec.config.

If we define .spec.config.watchNamespace in the spec, that field becomes fixed and can’t be changed later.
As I understand it, we want config to be a Raw field so we can store generic data there — for example, Helm values in the future when we support Helm. So, we do not know the options, and that is why we cannot define specific names for each config value. It can be anything.

We have an RFC in the Google Docs, about config spec and those changes in the API.

@perdasilva since you’re the author of this feature, did I miss anything here? 🙏

[camilamacedo86]

HI @perdasilva and @tmshort

It is good to get merged.
We just need to wait and ensure that downstream is prepared to receive this one.
So, I will keep the hold until you both give the green flag here.

[michaelryanpeter]

LMK if you have questions.

[michaelryanpeter]

Suggested change

	!!! note
	This feature is still in *alpha* the `SingleOwnNamespaceInstallSupport` feature-gate must be enabled to make use of it.
	See the instructions below on how to enable it.
	The `SingleOwnNamespaceInstallSupport` feature-gate is enabled by default. Use this guide to configure bundles that need Single or Own namespace install modes.
	You can deploy a cluster extension in a specific namespace by using the `OwnNamespace` or `SingleNamespace` install modes for `registry+v1` Operator bundles.

[camilamacedo86]

I think I prefer the original.

See either that in this PR we are promoting so we are just removing This feature is still in *alpha* the SingleOwnNamespaceInstallSupport feature-gate must be enabled to make use of it. See the instructions below on how to enable it. not changing anything

The suggestions make it sound like “Single” or “Own namespace” define where the CE content will be deployed, but it’s actually more about what the CE content can watch.

While it’s true that for Single, the CE content must be deployed in a specific namespace since the solution will only watch this ns, and for OwnNS, it means watching both the namespace where it’s deployed and the target namespace — the key point is that these options describe the CE’s content scope of watch, not its deployment location.

[michaelryanpeter]

If the feature gate is on by default, then I think this bullet should be cut.

[camilamacedo86]

100 %