fix(appset): regard selector in nested matrix generator's generators

[Roshick]

Fixes #11982

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Patch coverage: 27.58% and project coverage change: -0.03 ⚠️

Comparison is base (150a184) 49.64% compared to head (c6098e8) 49.62%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11984      +/-   ##
==========================================
- Coverage   49.64%   49.62%   -0.03%     
==========================================
  Files         256      256              
  Lines       43914    43943      +29     
==========================================
+ Hits        21800    21805       +5     
- Misses      19958    19976      +18     
- Partials     2156     2162       +6     

Impacted Files	Coverage Δ
applicationset/generators/matrix.go	71.96% <0.00%> (-5.90%)	⬇️
.../apis/application/v1alpha1/applicationset_types.go	31.11% <0.00%> (-0.35%)	⬇️
applicationset/generators/merge.go	52.40% <40.00%> (-0.80%)	⬇️
...licationset/generators/generator_spec_processor.go	59.63% <50.00%> (-0.77%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[blakepettersson]

Does this work as intended? If so, could you write some e2e tests to exercise that functionality?

[Roshick]

The fix works, tried with my local cluster. I'll have a look into the e2e testing setup this project uses! Need to check why my branch is failing after the rebase anyway.

[Roshick]

@blakepettersson I've added an e2e test that covers the fix.

[Roshick]

@crenshaw-dev as discussed I've added a flag and log some warnings if someone is using terminal generators with selectors without having the flag enabled. I'm not sure if this is the cleanest solution (i.e. adding the terminal selector when unmarshalling and removing it afterwards) instead of not adding it in the first place when going from terminal to nested, but the latter would need access to the application set (1. for the flag, 2. for some meaningful log warnings).

if this solution is okay, I would write 3 more e2e tests for the remaining branches (matrix->merge, merge->matrix, merge->merge).

[Roshick]

@crenshaw-dev pull request is ready for review now

[ishitasequeira]

@Roshick would you mind resolving conflicts? I will give the PR a review today.

[Roshick]

@Roshick would you mind resolving conflicts? I will give the PR a review today.

Sure, I can take care of it this evening!

[Roshick]

@ishitasequeira done, but for some reason I don't understand the docs build failed

[ishitasequeira]

LGTM!! @crenshaw-dev could you take another look?

[Roshick]

@crenshaw-dev would appreciate another review :)

[Roshick]

I will rebase and fix up the pull request next week and hope we can get it merged soon.

[crenshaw-dev]

Thanks, @Roshick! Apologies for the delayed review. Looking forward to giving it a final pass next week.

[Roshick]

@crenshaw-dev all fixed up now and ready to be reviewed! :)

[crenshaw-dev]

Thanks, @Roshick!

[crenshaw-dev]

Drat, I broke auto-merge. I'll solve merge conflicts and push, then I'll merge after checks pass.