Prevent local absolute paths from being entered in App config

pkg/app/piped/appconfigreporter/appconfigreporter.go

@@ -210,31 +210,31 @@ func (r *Reporter) updateRegisteredApps(ctx context.Context, registeredAppPaths
 // findRegisteredApps finds out registered application info in the given git repository.
 func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo gitRepo, lastScannedCommit, headCommitHash string, registeredAppPaths map[string]struct{}) ([]*model.ApplicationInfo, error) {
 	if lastScannedCommit == "" {
-		return r.scanAllFiles(ctx, repo.GetPath(), repoID, registeredAppPaths, true)
+		return r.scanAllFiles(repo.GetPath(), repoID, registeredAppPaths, true)
 	}
 
-	files, err := repo.ChangedFiles(ctx, lastScannedCommit, headCommitHash)
+	filePaths, err := repo.ChangedFiles(ctx, lastScannedCommit, headCommitHash)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get files those were touched between two commits: %w", err)
 	}
-	if len(files) == 0 {
+	if len(filePaths) == 0 {
 		// The case where all changes have been fully reverted.
 		return []*model.ApplicationInfo{}, nil
 	}
 	apps := make([]*model.ApplicationInfo, 0)
-	for _, filename := range files {
-		if !strings.HasSuffix(filename, model.DefaultDeploymentConfigFileExtension) {
+	for _, path := range filePaths {
+		if !strings.HasSuffix(path, model.DefaultDeploymentConfigFileExtension) {
 			continue
 		}
-		gitPathKey := makeGitPathKey(repoID, filename)
+		gitPathKey := makeGitPathKey(repoID, path)
 		if _, registered := registeredAppPaths[gitPathKey]; !registered {
 			continue
 		}
-		appInfo, err := r.readApplicationInfo(ctx, filepath.Join(repo.GetPath(), filename))
+		appInfo, err := r.readApplicationInfo(repo.GetPath(), filepath.Dir(path), filepath.Base(path))
 		if err != nil {
 			r.logger.Error("failed to read application info",
 				zap.String("repo-id", repoID),
-				zap.String("config-file-path", filename),
+				zap.String("config-file-path", path),
 				zap.Error(err),
 			)
 			continue
@@ -248,7 +248,7 @@ func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo g
 func (r *Reporter) updateUnregisteredApps(ctx context.Context, registeredAppPaths map[string]struct{}) error {
 	apps := make([]*model.ApplicationInfo, 0)
 	for repoID, repo := range r.gitRepos {
-		as, err := r.findUnregisteredApps(ctx, repo.GetPath(), repoID, registeredAppPaths)
+		as, err := r.findUnregisteredApps(repo.GetPath(), repoID, registeredAppPaths)
 		if err != nil {
 			return err
 		}
@@ -277,13 +277,13 @@ func (r *Reporter) updateUnregisteredApps(ctx context.Context, registeredAppPath
 }
 
 // findUnregisteredApps finds out unregistered application info in the given git repository.
-func (r *Reporter) findUnregisteredApps(ctx context.Context, repoPath, repoID string, registeredAppPaths map[string]struct{}) ([]*model.ApplicationInfo, error) {
-	return r.scanAllFiles(ctx, repoPath, repoID, registeredAppPaths, false)
+func (r *Reporter) findUnregisteredApps(repoPath, repoID string, registeredAppPaths map[string]struct{}) ([]*model.ApplicationInfo, error) {
+	return r.scanAllFiles(repoPath, repoID, registeredAppPaths, false)
 }
 
 // scanAllFiles inspects all files under the root or the given repository.
 // And gives back all application info as much as possible.
-func (r *Reporter) scanAllFiles(ctx context.Context, repoRoot, repoID string, registeredAppPaths map[string]struct{}, wantRegistered bool) ([]*model.ApplicationInfo, error) {
+func (r *Reporter) scanAllFiles(repoRoot, repoID string, registeredAppPaths map[string]struct{}, wantRegistered bool) ([]*model.ApplicationInfo, error) {
 	apps := make([]*model.ApplicationInfo, 0)
 	err := fs.WalkDir(r.fileSystem, repoRoot, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
@@ -306,7 +306,7 @@ func (r *Reporter) scanAllFiles(ctx context.Context, repoRoot, repoID string, re
 			return nil
 		}
 
-		appInfo, err := r.readApplicationInfo(ctx, path)
+		appInfo, err := r.readApplicationInfo(repoRoot, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath))
 		if err != nil {
 			r.logger.Error("failed to read application info",
 				zap.String("repo-id", repoID),
@@ -331,8 +331,8 @@ func makeGitPathKey(repoID, cfgFilePath string) string {
 	return fmt.Sprintf("%s:%s", repoID, cfgFilePath)
 }
 
-func (r *Reporter) readApplicationInfo(ctx context.Context, cfgFilePath string) (*model.ApplicationInfo, error) {
-	b, err := fs.ReadFile(r.fileSystem, cfgFilePath)
+func (r *Reporter) readApplicationInfo(repoDir, appDirRelPath, cfgFilename string) (*model.ApplicationInfo, error) {
+	b, err := fs.ReadFile(r.fileSystem, filepath.Join(repoDir, appDirRelPath, cfgFilename))
 	if err != nil {
 		return nil, fmt.Errorf("failed to open the configuration file: %w", err)
 	}
@@ -346,13 +346,15 @@ func (r *Reporter) readApplicationInfo(ctx context.Context, cfgFilePath string)
 		return nil, fmt.Errorf("unsupported application kind %q", cfg.Kind)
 	}
 
-	// TODO: Return an error if any one of required field of Application is empty
+	if spec.Name == "" {
+		return nil, fmt.Errorf("missing application name")
+	}
 	return &model.ApplicationInfo{
 		Name: spec.Name,
 		// TODO: Convert Kind string into dedicated type
 		//Kind:           cfg.Kind,
 		Labels:         spec.Labels,
-		Path:           filepath.Dir(cfgFilePath),
-		ConfigFilename: filepath.Base(cfgFilePath),
+		Path:           appDirRelPath,
+		ConfigFilename: cfgFilename,
 	}, nil
 }

pkg/app/piped/appconfigreporter/appconfigreporter_test.go

@@ -110,7 +110,7 @@ spec:
 				{
 					Name:           "app-1",
 					Labels:         map[string]string{"key-1": "value-1"},
-					Path:           "path/to/repo-1/app-1",
+					Path:           "app-1",
 					ConfigFilename: "app.pipecd.yaml",
 				},
 			},
@@ -119,7 +119,7 @@ spec:
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := tc.reporter.findUnregisteredApps(context.Background(), tc.args.repoPath, tc.args.repoID, tc.args.registeredAppPaths)
+			got, err := tc.reporter.findUnregisteredApps(tc.args.repoPath, tc.args.repoID, tc.args.registeredAppPaths)
 			assert.Equal(t, tc.wantErr, err != nil)
 			assert.Equal(t, tc.want, got)
 		})
@@ -231,7 +231,7 @@ spec:
 				{
 					Name:           "app-1",
 					Labels:         map[string]string{"key-1": "value-1"},
-					Path:           "path/to/repo-1/app-1",
+					Path:           "app-1",
 					ConfigFilename: "app.pipecd.yaml",
 				},
 			},
@@ -263,7 +263,7 @@ spec:
 				{
 					Name:           "app-1",
 					Labels:         map[string]string{"key-1": "value-1"},
-					Path:           "path/to/repo-1/app-1",
+					Path:           "app-1",
 					ConfigFilename: "app.pipecd.yaml",
 				},
 			},

pkg/model/common.proto

@@ -61,5 +61,5 @@ message ApplicationInfo {
     ApplicationKind kind = 2 [(validate.rules).enum.defined_only = true];
     map<string, string> labels = 3;
     string path = 4 [(validate.rules).string.pattern = "^[^/].+$"];
-    string config_filename = 5;
+    string config_filename = 5 [(validate.rules).string.min_len = 1];
 }