carvel-dev / carvel-dev/kapp-controller
App CR should require one of serviceAccountName or cluster.
- Dominant language
- Go
- Stars
- 323
- Forks
- 127
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 6
Description
**Describe the problem/challenge you have**
Both `serviceAccountName` and `cluster` properties of the `App` CR are marked as optional. Anyone not understanding that one or the other is required may not even add either. When neither is added, then the `App` CR is still accepted and you end up with an error which you have to look in the `App` CR status of:
```
status:
conditions:
- message: 'Preparing kapp: Expected service account or cluster specified'
status: "True"
type: ReconcileFailed
```
Further, when trying to delete the `App` in this situation it fails with it eventually exiting with the error:
```
kapp: Error: waiting on reconcile app/educates-training-platform (kappctrl.k14s.io/v1alpha1) namespace: default:
Finished unsuccessfully (Reconcile failed: (message: Preparing kapp: Expected service account or cluster specified))
```
resulting in you needing to manually delete the finalizer from the `App` resource.
The latter problem relates to issue https://github.com/vmware-tanzu/carvel-kapp-controller/issues/416.
**Describe the solution you'd like**
The CR definition should enforce that one or the other of `serviceAccountName` and `cluster` is required so that attempting to create an `App` CR with neither is rejected immediately.
My understanding is that this can be achieved in the CR schema by using:
```
anyOf:
- required:
- serviceAccountName
- required:
- cluster
```
**Anything else you would like to add:**
Having to look into the status of the `App` CR is non obvious so anything that avoids getting into that situation in the first place is a bonus. So failing early at the point of creating the `App` is better.
---
Vote on this request
This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.
👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"
We are also happy to receive and review Pull Requests if you want to help working on this issue.
Contributor guide
Research direction
Start with the App CR definition and its schema validation, focusing on serviceAccountName and cluster. Check the existing validation tests or add a focused test showing that an App with neither field is rejected, while either field alone is accepted.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- devops, infrastructure
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100