Raise error for duplicate application names #69
Conversation
Signed-off-by: John Pitman <jpitman@redhat.com>
| @@ -78,6 +79,10 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err | |||
| if err != nil { | |||
| return ctrl.Result{}, err | |||
| } | |||
| err = validateDesiredApplications(desiredApplications) | |||
There was a problem hiding this comment.
I think this can be part of the generateApplications, if there is a duplicate name the generate will fail with an error. what do you think?
There was a problem hiding this comment.
I think it's cleaner if the validation is done separately from the generation, particularly in light of Jonathan's comments above about the reconcile being retried when there is an error. If validation is separate, an error from generateApplications will still result in the reconcile being retried, while a duplicate application name will not.
Also, if there is a problem generating one of the applications, generateApplications currently will return both an error and a slice containing the valid applications (I'm not sure why). I think weaving the return of an error for duplicate names into this logic would be unnecessarily complicated.
Signed-off-by: John Pitman <jpitman@redhat.com>
Co-authored-by: William Tam <wtam@redhat.com>
|
LGTM |
Signed-off-by: John Pitman jpitman@redhat.com
Fixes #68