From 3acc0b3af299b6fc27e512edd071e9ca97523aa9 Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Date: Wed, 6 Mar 2019 00:02:27 -0800
Subject: [PATCH] Issue #1229 - App creation failed for public repository
 (#1230)

---
 controller/appcontroller.go       |  2 +-
 server/application/application.go |  7 +------
 util/argo/argo.go                 | 35 +++++++------------------------
 3 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/controller/appcontroller.go b/controller/appcontroller.go
index ac1945e9ce..0f2e948084 100644
--- a/controller/appcontroller.go
+++ b/controller/appcontroller.go
@@ -659,7 +659,7 @@ func (ctrl *ApplicationController) refreshAppConditions(app *appv1.Application)
 			})
 		}
 	} else {
-		specConditions, err := argo.GetSpecErrors(context.Background(), &app.Spec, proj, ctrl.repoClientset, ctrl.db)
+		specConditions, _, err := argo.GetSpecErrors(context.Background(), &app.Spec, proj, ctrl.repoClientset, ctrl.db)
 		if err != nil {
 			conditions = append(conditions, appv1.ApplicationCondition{
 				Type:    appv1.ApplicationConditionUnknownError,
diff --git a/server/application/application.go b/server/application/application.go
index 06ac2f329f..9a0daa7630 100644
--- a/server/application/application.go
+++ b/server/application/application.go
@@ -492,18 +492,13 @@ func (s *Server) validateAndNormalizeApp(ctx context.Context, app *appv1.Applica
 		}
 	}
 
-	conditions, err := argo.GetSpecErrors(ctx, &app.Spec, proj, s.repoClientset, s.db)
+	conditions, appSourceType, err := argo.GetSpecErrors(ctx, &app.Spec, proj, s.repoClientset, s.db)
 	if err != nil {
 		return err
 	}
 	if len(conditions) > 0 {
 		return status.Errorf(codes.InvalidArgument, "application spec is invalid: %s", argo.FormatAppConditions(conditions))
 	}
-
-	appSourceType, err := argo.QueryAppSourceType(ctx, app, s.repoClientset, s.db)
-	if err != nil {
-		return err
-	}
 	app.Spec = *argo.NormalizeApplicationSpec(&app.Spec, appSourceType)
 	return nil
 }
diff --git a/util/argo/argo.go b/util/argo/argo.go
index b642496c8b..5b06ab8824 100644
--- a/util/argo/argo.go
+++ b/util/argo/argo.go
@@ -140,20 +140,20 @@ func GetSpecErrors(
 	proj *argoappv1.AppProject,
 	repoClientset reposerver.Clientset,
 	db db.ArgoDB,
-) ([]argoappv1.ApplicationCondition, error) {
+) ([]argoappv1.ApplicationCondition, argoappv1.ApplicationSourceType, error) {
 	conditions := make([]argoappv1.ApplicationCondition, 0)
 	if spec.Source.RepoURL == "" || spec.Source.Path == "" {
 		conditions = append(conditions, argoappv1.ApplicationCondition{
 			Type:    argoappv1.ApplicationConditionInvalidSpecError,
 			Message: "spec.source.repoURL and spec.source.path are required",
 		})
-		return conditions, nil
+		return conditions, "", nil
 	}
 
 	// Test the repo
 	conn, repoClient, err := repoClientset.NewRepoServerClient()
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 	defer util.Close(conn)
 	repoAccessable := false
@@ -174,12 +174,13 @@ func GetSpecErrors(
 				repoAccessable = true
 			}
 		} else {
-			return nil, err
+			return nil, "", err
 		}
 	} else {
 		repoAccessable = true
 	}
 
+	var appSourceType argoappv1.ApplicationSourceType
 	// Verify only one source type is defined
 	explicitSourceType, err := spec.Source.ExplicitType()
 	if err != nil {
@@ -190,7 +191,6 @@ func GetSpecErrors(
 	}
 
 	if repoAccessable {
-		var appSourceType argoappv1.ApplicationSourceType
 		if explicitSourceType != nil {
 			appSourceType = *explicitSourceType
 		} else {
@@ -249,11 +249,11 @@ func GetSpecErrors(
 					Message: fmt.Sprintf("cluster '%s' has not been configured", spec.Destination.Server),
 				})
 			} else {
-				return nil, err
+				return nil, "", err
 			}
 		}
 	}
-	return conditions, nil
+	return conditions, appSourceType, nil
 }
 
 // GetAppProject returns a project from an application
@@ -261,27 +261,6 @@ func GetAppProject(spec *argoappv1.ApplicationSpec, projLister applicationsv1.Ap
 	return projLister.AppProjects(ns).Get(spec.GetProject())
 }
 
-// QueryAppSourceType queries repo server for yaml files in a directory, and determines its
-// application source type based on the files in the directory.
-// This code is redundant to the logic in argo.GetSpecErrors, but since it's is hard to
-// extract out of there. We will be throwing away this code when we remove
-// componentParameterOverrides.
-func QueryAppSourceType(ctx context.Context, app *argoappv1.Application, repoClientset reposerver.Clientset, db db.ArgoDB) (argoappv1.ApplicationSourceType, error) {
-	if t, _ := app.Spec.Source.ExplicitType(); t != nil {
-		return *t, nil
-	}
-	repoRes, err := db.GetRepository(ctx, app.Spec.Source.RepoURL)
-	if err != nil {
-		return "", err
-	}
-	conn, repoClient, err := repoClientset.NewRepoServerClient()
-	if err != nil {
-		return "", err
-	}
-	defer util.Close(conn)
-	return queryAppSourceType(ctx, &app.Spec, repoRes, repoClient)
-}
-
 func queryAppSourceType(ctx context.Context, spec *argoappv1.ApplicationSpec, repoRes *argoappv1.Repository, repoClient repository.RepoServerServiceClient) (argoappv1.ApplicationSourceType, error) {
 	req := repository.ListDirRequest{
 		Repo: &argoappv1.Repository{