kapp icon indicating copy to clipboard operation
kapp copied to clipboard

Add resource namespaces saved during deploy to fallbackAllowedNamespaces

Open praveenrewar opened this issue 2 years ago • 1 comments

What this PR does / why we need it:

Add resource namespaces saved during deploy to fallbackAllowedNamespaces. These resource namespaces are stored as app change meta during deploy

TODO:

  • [ ] Think of a better way to pass on the fallbackAllowedNamespaces.
  • [ ] More manual testing

Which issue(s) this PR fixes:

Fixes #479

Does this PR introduce a user-facing change?

NONE

Additional Notes for your reviewer:

I am still thinking if there's a better place to make this change. Another option that I thought of was to call the FactoryClients separately and a little late (after Exists function is called) and populate the fallbackAllowedNamespaces with the resource namespaces there itself, but that would require a lot of repetition as App.Find requires a lot of things from FactoryClients.

Review Checklist:
  • [ ] Follows the developer guidelines
  • [ ] Relevant tests are added or updated
  • [ ] Relevant docs in this repo added or updated
  • [ ] Relevant carvel.dev docs added or updated in a separate PR and there's a link to that PR
  • [ ] Code is at least as readable and maintainable as it was before this change

Additional documentation e.g., Proposal, usage docs, etc.:


praveenrewar avatar Jun 19 '22 06:06 praveenrewar

How are we updating the LastChange.Namespaces. Do we update it after every redeploy? If yes, then it will be interesting to see how things will behave when we use filter

rohitagg2020 avatar Jun 23 '22 11:06 rohitagg2020

Code looks good to me, gonna take a closer look at the new test!

100mik avatar Sep 15 '22 20:09 100mik