Merge pull request #179 from projectsyn/report-error-invalid-url

Report error if tenant registration contains an invalid git url

Author: bastjan

pkg/api/utils.go

@@ -131,7 +131,11 @@ func NewCRDFromAPITenant(apiTenant Tenant) (*synv1alpha1.Tenant, error) {
 	}
 
 	if apiTenant.GitRepo != nil {
-		tenant.Spec.GitRepoTemplate = newGitRepoTemplate(&apiTenant.GitRepo.GitRepo, string(apiTenant.Id))
+		tmpl, err := newGitRepoTemplate(&apiTenant.GitRepo.GitRepo, string(apiTenant.Id))
+		if err != nil {
+			return nil, fmt.Errorf("failed to create git repo template: %w", err)
+		}
+		tenant.Spec.GitRepoTemplate = tmpl
 	}
 
 	SyncCRDFromAPITenant(apiTenant.TenantProperties, tenant)
@@ -276,13 +280,17 @@ func NewCRDFromAPICluster(apiCluster Cluster) (*synv1alpha1.Cluster, error) {
 		},
 	}
 
-	cluster.Spec.GitRepoTemplate = newGitRepoTemplate(apiCluster.GitRepo, string(apiCluster.Id))
+	tmpl, err := newGitRepoTemplate(apiCluster.GitRepo, string(apiCluster.Id))
+	if err != nil {
+		return nil, fmt.Errorf("failed to create git repo template: %w", err)
+	}
+	cluster.Spec.GitRepoTemplate = tmpl
 
 	if apiCluster.GitRepo != nil && apiCluster.GitRepo.HostKeys != nil {
 		cluster.Spec.GitHostKeys = *apiCluster.GitRepo.HostKeys
 	}
 
-	err := SyncCRDFromAPICluster(apiCluster.ClusterProperties, cluster)
+	err = SyncCRDFromAPICluster(apiCluster.ClusterProperties, cluster)
 	return cluster, err
 }
 
@@ -366,23 +374,23 @@ func SyncCRDFromAPICluster(source ClusterProperties, target *synv1alpha1.Cluster
 	return nil
 }
 
-func newGitRepoTemplate(repo *GitRepo, name string) *synv1alpha1.GitRepoTemplate {
+func newGitRepoTemplate(repo *GitRepo, name string) (*synv1alpha1.GitRepoTemplate, error) {
 	if repo == nil {
 		// No git info was specified
-		return nil
+		return nil, nil
 	}
 
 	if repo.Type == nil || *repo.Type != string(synv1alpha1.UnmanagedRepoType) {
 		if repo.Url != nil && len(*repo.Url) > 0 {
 			// It's not unmanaged and the URL was specified, take it apart
 			url, err := url.Parse(*repo.Url)
 			if err != nil {
-				return nil
+				return nil, fmt.Errorf("failed to parse git repo URL: %w", err)
 			}
 			pathParts := strings.Split(url.Path, "/")
 			pathParts = pathParts[1:]
 			if len(pathParts) < 2 {
-				return nil
+				return nil, fmt.Errorf("failed to parse git repo URL, expected 2+ path elements in '%s'", url.Path)
 			}
 			// remove .git extension
 			repoName := strings.ReplaceAll(pathParts[len(pathParts)-1], ".git", "")
@@ -391,13 +399,13 @@ func newGitRepoTemplate(repo *GitRepo, name string) *synv1alpha1.GitRepoTemplate
 				RepoType: synv1alpha1.AutoRepoType,
 				Path:     repoPath,
 				RepoName: repoName,
-			}
+			}, nil
 		}
 	} else if repo.Type != nil {
 		// Repo is unmanaged, remove name and path
 		return &synv1alpha1.GitRepoTemplate{
 			RepoType: synv1alpha1.UnmanagedRepoType,
-		}
+		}, nil
 	}
-	return nil
+	return nil, nil
 }

pkg/api/utils_test.go

@@ -21,17 +21,18 @@ func TestRepoConversionDefaultAuto(t *testing.T) {
 		Url:  nil,
 	}
 	repoName := "c-dshfjuhrtu"
-	repoTemplate := newGitRepoTemplate(apiRepo, repoName)
+	repoTemplate, _ := newGitRepoTemplate(apiRepo, repoName)
+	assert.Nil(t, repoTemplate)
+	repoTemplate, _ = newGitRepoTemplate(apiRepo, repoName)
 	assert.Nil(t, repoTemplate)
-	assert.Nil(t, newGitRepoTemplate(nil, repoName))
 }
 
 func TestRepoConversionUnmanagedo(t *testing.T) {
 	apiRepo := &GitRepo{
 		Type: pointer.ToString("unmanaged"),
 		Url:  pointer.ToString("ssh://git@some.host/path/to/repo.git"),
 	}
-	repoTemplate := newGitRepoTemplate(apiRepo, "some-name")
+	repoTemplate, _ := newGitRepoTemplate(apiRepo, "some-name")
 	assert.Empty(t, repoTemplate.RepoName)
 	assert.Empty(t, repoTemplate.Path)
 }
@@ -43,7 +44,7 @@ func TestRepoConversionSpecSubGroupPath(t *testing.T) {
 		Type: pointer.ToString("auto"),
 		Url:  pointer.ToString("ssh://git@some.host/" + repoPath + "/" + repoName + ".git"),
 	}
-	repoTemplate := newGitRepoTemplate(apiRepo, "some-name")
+	repoTemplate, _ := newGitRepoTemplate(apiRepo, "some-name")
 	assert.Equal(t, repoName, repoTemplate.RepoName)
 	assert.Equal(t, repoPath, repoTemplate.Path)
 	assert.Empty(t, repoTemplate.APISecretRef.Name)
@@ -56,7 +57,7 @@ func TestRepoConversionSpecPath(t *testing.T) {
 		Type: pointer.ToString("auto"),
 		Url:  pointer.ToString("ssh://git@some.host/" + repoPath + "/" + repoName + ".git"),
 	}
-	repoTemplate := newGitRepoTemplate(apiRepo, "some-name")
+	repoTemplate, _ := newGitRepoTemplate(apiRepo, "some-name")
 	assert.Equal(t, repoName, repoTemplate.RepoName)
 	assert.Equal(t, repoPath, repoTemplate.Path)
 	assert.Empty(t, repoTemplate.APISecretRef.Name)
@@ -66,13 +67,13 @@ func TestRepoConversionFail(t *testing.T) {
 	apiRepo := &GitRepo{
 		Url: pointer.ToString("://git@some.host/group/example.git"),
 	}
-	repoTemplate := newGitRepoTemplate(apiRepo, "some-name")
-	assert.Nil(t, repoTemplate)
+	_, err := newGitRepoTemplate(apiRepo, "some-name")
+	assert.Error(t, err)
 
-	repoTemplate = newGitRepoTemplate(&GitRepo{
+	_, err = newGitRepoTemplate(&GitRepo{
 		Url: pointer.ToString("ssh://git@some.host/example.git"),
 	}, "test")
-	assert.Nil(t, repoTemplate)
+	assert.Error(t, err)
 }
 
 func TestGenerateClusterID(t *testing.T) {