diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter.go b/pkg/app/piped/appconfigreporter/appconfigreporter.go index 57d541d264..fda579dae5 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter.go +++ b/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 } diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go b/pkg/app/piped/appconfigreporter/appconfigreporter_test.go index 4228f1a7e1..14c517fbb8 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go +++ b/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", }, }, diff --git a/pkg/model/common.proto b/pkg/model/common.proto index ea15666aec..cc435acbaf 100644 --- a/pkg/model/common.proto +++ b/pkg/model/common.proto @@ -61,5 +61,5 @@ message ApplicationInfo { ApplicationKind kind = 2 [(validate.rules).enum.defined_only = true]; map labels = 3; string path = 4 [(validate.rules).string.pattern = "^[^/].+$"]; - string config_filename = 5; + string config_filename = 5 [(validate.rules).string.min_len = 1]; }