fix: Adding namespace when converting bundle from registryv1 to plain

[varshaprasad96]

Currently, if the objects being passed through the bundle do not have their namespaces set, they were being created on the "rukpak-system" namespace. This PR fixes this issue for only "registryV1" bundles. It sets the namespace to installNs if not provided by default.

[codecov]

Codecov Report

Merging #678 (ad5b5d8) into main (5791e52) will decrease coverage by 2.03%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
- Coverage   21.24%   19.21%   -2.03%     
==========================================
  Files          12       13       +1     
  Lines         772     1046     +274     
==========================================
+ Hits          164      201      +37     
- Misses        583      801     +218     
- Partials       25       44      +19     

Files Changed	Coverage Δ
internal/convert/registryv1.go	13.50% <100.00%> (ø)

[awgreene]

Note: this PR updates the registry v1 provisioner to mirror OLM V0 behavior, in which the namespace is set to the installNamespace if the resource does not specify a namespace.
/approve

[joelanford]

This makes sense to me because its an implicit feature of the registry+v1 bundle format that objects without metadata.namespace defined will be created in the install namespace.

[joelanford]

@awgreene, if a bundle object "A" hardcodes its namespace to "foo" and then someone creates a subscription resulting in the installation of that bundle in namespace "bar", where does "A" end up? "foo" or "bar"?

[awgreene]

@joelanford my impression was that the namespace is always set to the installNamespace. Let me double check.

[awgreene]

This is interesting @joelanford. Based on the ensure methods defined in step_ensurer.go, some objects end up in foo and some end up in bar:

• EnsureServiceAccount creates the object in bar. This applies for services, serviceAccounts, CSVs, subscriptions, secrets, services, roles, roleBindings
• EnsureUnstructuredObject creates the object in foo. This applies to objects not defined in the previous bullet but allowed in bundles.

[awgreene]

NVM, @varshaprasad96 pointed out that the namespace for unstructured objects are set here, I thought that the ensure methods were responsible for setting the namespace as all ensure methods do other than EnsureUnstructureObject.

All namespaced resources would be created in the bar namespace @joelanford.

[joelanford]

@varshaprasad96 for both of your questions, for better or worse, I think we should make registry+v1 bundles in OLMv1 behave as closely as possible to OLMv0. Reasons being:

1. There are years of expectations, assumptions, and muscle memory built up around the behavior of registry+v1 + OLMv0, to the point that registry+v1 is pretty much synonymous with OLMv0.
2. Any deviation from OLMv0 behavior with respect to registry+v1 bundles has the potential to cause a breaking change in migration scenarios.
3. Any new features or improvements we put in registry+v1 is one less reason for people to migrate to a different format. If we think registry+v1 is not what we want folks using long-term, it is perhaps better to avoid adding features to it that make it more appealing.

[ncdc]

@joelanford does your answer change at all (maybe only from an implementation perspective) if/when we switch away from provisioners that are tied to bundle format? i.e. unpack-process-apply?

[joelanford]

I was thinking about that last night. The idea that came to mind first was to expose something like renderRegistryV1 as a processor type, which would essentially call this same Convert function.

We probably have to stew on it more, but my instinct is that we should:

1. stick to our registry+v1 bundle validations.
2. make sure to support a simple unpack-process-apply setup that implements OLMv0 behavior to make migrations easy
3. allow BD clients to do more complex post-processing of registry+v1 bundles using more complex unpack-process-apply setups.