Be a bit more defensive in the error checks

Author: mbaldessari

internal/controller/argo.go

@@ -411,7 +411,10 @@ func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interf
 
 	if !haveArgo(client, name, namespace) {
 		// create it
-		obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
+		obj, errConvert := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
+		if errConvert != nil {
+			return fmt.Errorf("failed to convert ArgoCD to unstructured for create: %v", errConvert)
+		}
 		newArgo := &unstructured.Unstructured{Object: obj}
 		_, err = client.Resource(gvr).Namespace(namespace).Create(context.TODO(), newArgo, metav1.CreateOptions{})
 	} else { // update it
@@ -420,7 +423,10 @@ func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interf
 			return fmt.Errorf("failed to get existing ArgoCD %s/%s: %v", namespace, name, err)
 		}
 		argo.SetResourceVersion(oldArgo.GetResourceVersion())
-		obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
+		obj, errConvert := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
+		if errConvert != nil {
+			return fmt.Errorf("failed to convert ArgoCD to unstructured for update: %v", errConvert)
+		}
 		newArgo := &unstructured.Unstructured{Object: obj}
 
 		_, err = client.Resource(gvr).Namespace(namespace).Update(context.TODO(), newArgo, metav1.UpdateOptions{})
@@ -708,9 +714,10 @@ func commonApplicationSourceHelm(p *api.Pattern, prefix string) *argoapi.Applica
 	valueFiles := newApplicationValueFiles(p, prefix)
 	sharedValueFiles, err := getSharedValueFiles(p, prefix)
 	if err != nil {
-		fmt.Printf("Could not fetch sharedValueFiles: %s", err)
+		log.Printf("Could not fetch sharedValueFiles: %s", err)
+	} else {
+		valueFiles = append(valueFiles, sharedValueFiles...)
 	}
-	valueFiles = append(valueFiles, sharedValueFiles...)
 
 	return &argoapi.ApplicationSourceHelm{
 		ValueFiles: valueFiles,
@@ -913,9 +920,12 @@ func getApplication(client argoclient.Interface, name, namespace string) (*argoa
 
 func createApplication(client argoclient.Interface, app *argoapi.Application, namespace string) error {
 	saved, err := client.ArgoprojV1alpha1().Applications(namespace).Create(context.Background(), app, metav1.CreateOptions{})
+	if err != nil {
+		return err
+	}
 	yamlOutput, _ := objectYaml(saved)
 	log.Printf("Created: %s\n", yamlOutput)
-	return err
+	return nil
 }
 
 func updateApplication(client argoclient.Interface, target, current *argoapi.Application, namespace string) (bool, error) {

internal/controller/pattern_controller.go

@@ -599,7 +599,11 @@ func (r *PatternReconciler) deleteSpokeApps(targetApp, app *argoapi.Application,
 	log.Printf("Deletion phase: %s - checking if all child applications are gone from spoke", api.DeleteSpokeChildApps)
 
 	// Update application with deletePattern=DeleteSpokeChildApps to trigger spoke child deletion
-	if changed, _ := updateApplication(r.argoClient, targetApp, app, namespace); changed {
+	changed, errUpdate := updateApplication(r.argoClient, targetApp, app, namespace)
+	if errUpdate != nil {
+		return fmt.Errorf("failed to update application %q for spoke child deletion: %v", app.Name, errUpdate)
+	}
+	if changed {
 		return fmt.Errorf("updated application %q for spoke child deletion", app.Name)
 	}
 
@@ -660,7 +664,11 @@ func (r *PatternReconciler) deleteHubApps(targetApp, app *argoapi.Application, n
 	}
 
 	// Update application with deletePattern=DeleteHubChildApps to trigger hub child app deletion
-	if changed, _ := updateApplication(r.argoClient, targetApp, app, namespace); changed {
+	changed, errUpdate := updateApplication(r.argoClient, targetApp, app, namespace)
+	if errUpdate != nil {
+		return fmt.Errorf("failed to update application %q for hub deletion: %v", app.Name, errUpdate)
+	}
+	if changed {
 		return fmt.Errorf("updated application %q for hub deletion", app.Name)
 	}
 
@@ -731,7 +739,11 @@ func (r *PatternReconciler) finalizeObject(instance *api.Pattern) error {
 
 		// Phase 2: Delete app of apps from spoke
 		if qualifiedInstance.Status.DeletionPhase == api.DeleteSpoke {
-			if changed, _ := updateApplication(r.argoClient, targetApp, app, ns); changed {
+			changed, errUpdate := updateApplication(r.argoClient, targetApp, app, ns)
+			if errUpdate != nil {
+				return fmt.Errorf("failed to update application %q for spoke app of apps deletion: %v", app.Name, errUpdate)
+			}
+			if changed {
 				return fmt.Errorf("updated application %q for spoke app of apps deletion", app.Name)
 			}