App CR should require one of serviceAccountName or cluster.

[GrahamDumpleton]

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 #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.

[hemakshis]

Hi team, I would like to take up this issue.

[neil-hickey]

hi @hemakshis ! Thanks for showing interest in taking this on.

This might be a helpful start:

https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/pkg/apis/kappctrl/v1alpha1/types.go#L51-L55

1. We use https://github.com/kubernetes-sigs/controller-tools to generate our crds, the script we use can be found here - https://github.com/vmware-tanzu/carvel-kapp-controller/blob/develop/hack/gen-crds.sh
2. This uses kubebuilder underneath as a way to specify constraints (such as optional, nullable, int max etc ) on the structure of our custom resources.

Let us know how we can support you! You might also find our dev docs a good place to start on just getting this up and running.

[sarthaksarthak9]

@GrahamDumpleton could you pls provide the file where I need to commit changes

[GrahamDumpleton]

@sarthaksarthak9 I have no idea as I don't know how kubebuilder is configured to specify such a constraint.