Cleanup: preflight checks
- Dominant language
- Go
- Stars
- 1.1k
- Forks
- 134
- PR merge metrics
- No merged PRs in 30d
Description
Recently we have introduced the preflight flag in kapp and also added a couple of checks. This issue tracks a few of the items that we didn't cover during the reviews and it would be good to do some clean up.
- [ ] Package name for test files should be *_test ([registry_test.go](https://github.com/carvel-dev/kapp/blob/develop/pkg/kapp/preflight/registry_test.go))
- [ ] Remove the usages of `RemoveClusterResource` in tests that are no longer required ([example](https://github.com/carvel-dev/kapp/blob/0e94ff15321f67ba737df346bd29e82cd62ee057/test/e2e/preflight_permission_validation_escalation_test.go#L84))
- [ ] In Carvel, we capitalise first character of error messages along with colons, this is used in the ui library to help with printing [multiline error messages](https://github.com/carvel-dev/kapp/blob/0e94ff15321f67ba737df346bd29e82cd62ee057/cmd/kapp/kapp.go#L45). We should aim to keep the error format consistent ([example](https://github.com/carvel-dev/kapp/blob/0e94ff15321f67ba737df346bd29e82cd62ee057/test/e2e/apply_wait_error_test.go#L208) of how the error looks like with indentation)
- [ ] In some scenarios, there might be more than one errors that we want to report, we use [semi structured error](https://github.com/carvel-dev/kapp/blob/0e94ff15321f67ba737df346bd29e82cd62ee057/pkg/kapp/clusterapply/waiting_changes.go#L143) to ensure that each individual error gets it's own line making the overall error more clear. ([example](https://github.com/carvel-dev/kapp/blob/0e94ff15321f67ba737df346bd29e82cd62ee057/test/e2e/formatted_error_test.go#L53) of how such an error message looks like)
- [ ] We seem to be adding a lot of e2e tests for preflight checks (which is good). Currently all these tests are in the test/e2e directory and most of them have a dedicated file (which is also good as # of lines per file are less). Should we move them to a dedicated directory like `test/e2e/preflights` so that the other e2e tests are not overshadowed by preflight checks?
- [ ] Fix handled() function for some of the validators ([details](https://github.com/carvel-dev/kapp/pull/950#discussion_r1588263642))
Contributor guide
Assessment
This issue has not been assessed yet.