Fix mergeCredentials namespace reference

[denysvitali]

On certain setups, the following error appears (and the deploy is not successful?) even though the manifest is correct:

ts=2019-11-20T08:56:10.472306948Z caller=images.go:78 resource=:deployment/dep-product-backend err="getting secret \"regcred\" from namespace \"\": an empty namespace may not be set when a resource name is provided"

This issue is caused by the fact that the ns variable may be set to "" (meaning "all namespaces", see meta_v1.NamespaceAll), therefore in mergeCredentials(...) the "" namespace is forwarded and used for looking at the imagePullSecrets.

Related commit: 1dbeff3 .

Please do not merge yet, I'm testing it (nanosapp/flux:feature-namespace-regcred-fix-e1276641-wip)

Seems to be fixed, ready to be merged 🚀

[2opremio]

Nice catch!!! Thanks!

For extra points, could you add a test? (it's OK if you don't have time or simply don't want to)

[2opremio]

I have re-checked all the invocations of getAllowedAndExistingNamespaces and I haven't found any other leftovers from #2539 but, to be sure, @squaremo could you also check?

We need more coverage in the e2e tests

[2opremio]

(@denysvitali sorry, I initially selected the request changes option)

[squaremo]

On certain setups

On the face of it, this looks like it would break most setups (because by default all namespaces are allowed, therefore the sentinel value will usually be used). For the benefit of my understanding, what are the factors that make it only certain setups?

[squaremo]

Looks correct to me!

[denysvitali]

By certain I meant my setup 😅
I discovered this problem by looking at FluxCD's logs, then I did a little bit of troubleshooting and found out that it is not an intended behaviour. Hopefully this PR won't break anybody's workflow (relevant XKCD).

[squaremo]

Bonus points for finding a relevant xkcd (there's one for every situation! well, every computer situation) 👍