fix(ui): source can in fact be undefined

[blakepettersson]

The assumption that a source is always there is not always true. To repro, create an app-of-apps containing a single app without any source present (an example can be found at https://github.com/blakepettersson/broken-app-of-apps). In the UI this will crash, horribly. This PR fixes that so that instead of crashing the user will get useful info indicating what is wrong with the app.

Presumably fixes #15545.

Cherry-pick for at least 2.12 and 2.13, perhaps even lower depending how easy this is to port.

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).
• The title of the PR conforms to the Toolchain Guide
• 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.
• 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).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[gdsoumya]

I agree that by mistake there can be cases where an app is deployed without source but the UI shouldn't crash, and should throw appropriate errors.

[crenshaw-dev]

I'm not sure I understand the target experience(s) here. There are a lot of new nil checks which will presumably replace a bad error message with some other behavior, but it's not clear what the other behavior will be. Seems like we should probably be detecting misconfigurations at a high level, showing a tailored error message, and short-circuiting the field access code paths entirely.

[blakepettersson]

I'm not sure I understand the target experience(s) here.

There are a lot of new nil checks which will presumably replace a bad error message with some other behavior, but it's not clear what the other behavior will be.

The target experience is to display what is in the screenshot. When the source is not present, the App Health is Unknown, The Sync Status is also Unknown and the error message which is rendered in this case is spec.source.repoURL and either spec.source.path or spec.source.chart are required, coming from the application controller.

For a bit more context, @34fathombelow got paged by a customer due to this issue (they having misconfigured an application in this manner), which is the driver of this PR.

Granted that I've sprinkled the coalescing operator a bit too liberally here and can do some amendments for that...

Seems like we should probably be detecting misconfigurations at a high level, showing a tailored error message, and short-circuiting the field access code paths entirely.

I agree in principle, but the issue here is that we would need to figure out a whole new flow(s) for that. UX wise this would present a completely different flow than what users usually expect when their apps are misconfigured. Or am I misunderstanding something here?

[blakepettersson]

I amended this to disable the details, sync and diff buttons in case the source is not present.

[crenshaw-dev]

Will review again! Makes sense, gotta be pragmatic about the cost of special error handling vs. just having reasonable fallback behavior.

Stale

[crenshaw-dev]

Merging since we're aligned on design, and @gdsoumya approved. Support has been brutal this week, won't have time to review myself.

[blakepettersson]

Support has been brutal this week

We feel you, it's been the same for us at Akuity as well 😞

[blakepettersson]

/cherry-pick release-2.13

[blakepettersson]

/cherry-pick release-2.12

[blakepettersson]

/cherry-pick release-2.11

[gcp-cherry-pick-bot]

Cherry-pick failed with Merge error ed4c0ee7138da579a4392824b0632a7056a1d106 into temp-cherry-pick-82e9b5-release-2.12

[gcp-cherry-pick-bot]

Cherry-pick failed with Merge error ed4c0ee7138da579a4392824b0632a7056a1d106 into temp-cherry-pick-82e9b5-release-2.11