From 02172d44ebc3b1d90c00b2ccf866c62465394545 Mon Sep 17 00:00:00 2001 From: nakabonne Date: Wed, 24 Nov 2021 12:26:37 +0900 Subject: [PATCH 1/3] Add an rpc to update registered apps that have been changed --- pkg/app/api/grpcapi/piped_api.go | 71 ++++++++++++++++++- .../appconfigreporter/appconfigreporter.go | 7 +- .../appconfigreporter_test.go | 4 ++ pkg/model/common.proto | 5 +- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/pkg/app/api/grpcapi/piped_api.go b/pkg/app/api/grpcapi/piped_api.go index 3da696e40e..0490f0f435 100644 --- a/pkg/app/api/grpcapi/piped_api.go +++ b/pkg/app/api/grpcapi/piped_api.go @@ -957,8 +957,75 @@ func (a *PipedAPI) GetDesiredVersion(ctx context.Context, _ *pipedservice.GetDes } func (a *PipedAPI) UpdateApplicationConfigurations(ctx context.Context, req *pipedservice.UpdateApplicationConfigurationsRequest) (*pipedservice.UpdateApplicationConfigurationsResponse, error) { - // TODO: Update the given application configurations - return nil, status.Errorf(codes.Unimplemented, "UpdateApplicationConfigurations is not implemented yet") + projectID, pipedID, _, err := rpcauth.ExtractPipedToken(ctx) + if err != nil { + return nil, err + } + // TODO: Consider bulk-updating multiple apps + for _, appInfo := range req.Applications { + opts := datastore.ListOptions{ + // NOTE: Assume that no more than one application is referring to a single configuration file. + Limit: 1, + Filters: []datastore.ListFilter{ + { + Field: "ProjectId", + Operator: datastore.OperatorEqual, + Value: projectID, + }, + { + Field: "PipedId", + Operator: datastore.OperatorEqual, + Value: pipedID, + }, + { + Field: "GitPath.Repo.Id", + Operator: datastore.OperatorEqual, + Value: appInfo.RepoId, + }, + { + Field: "GitPath.Path", + Operator: datastore.OperatorEqual, + Value: appInfo.Path, + }, + { + Field: "GitPath.ConfigFilename", + Operator: datastore.OperatorEqual, + Value: appInfo.ConfigFilename, + }, + { + Field: "Disabled", + Operator: datastore.OperatorEqual, + Value: false, + }, + }, + } + // TODO: Enable to update multiple apps that match the given condition instead of accessing it twice + // like: UPDATE Application SET Name = 'foo' WHERE GitPath.Repo.Id = '?' AND GitPath.Path = '?' + apps, _, err := a.applicationStore.ListApplications(ctx, opts) + if err != nil { + a.logger.Error("failed to fetch applications", zap.Error(err)) + return nil, status.Error(codes.Internal, "failed to fetch applications") + } + if len(apps) == 0 { + a.logger.Error("not found any enabled apps that match the terms", + zap.String("repo-id", appInfo.RepoId), + zap.String("path", appInfo.Path), + zap.String("config-filename", appInfo.ConfigFilename), + ) + return nil, status.Error(codes.NotFound, "not found any enabled apps that match the terms") + } + updater := func(app *model.Application) error { + app.Name = appInfo.Name + app.Kind = appInfo.Kind + app.Labels = appInfo.Labels + return nil + } + if err := a.applicationStore.UpdateApplication(ctx, apps[0].Id, updater); err != nil { + a.logger.Error("failed to update application", zap.Error(err)) + return nil, status.Error(codes.Internal, "failed to update application") + } + } + return &pipedservice.UpdateApplicationConfigurationsResponse{}, nil } func (a *PipedAPI) ReportUnregisteredApplicationConfigurations(ctx context.Context, req *pipedservice.ReportUnregisteredApplicationConfigurationsRequest) (*pipedservice.ReportUnregisteredApplicationConfigurationsResponse, error) { diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter.go b/pkg/app/piped/appconfigreporter/appconfigreporter.go index dc78c75739..a1ccba4f66 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter.go +++ b/pkg/app/piped/appconfigreporter/appconfigreporter.go @@ -232,7 +232,7 @@ func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo g if _, registered := registeredAppPaths[gitPathKey]; !registered { continue } - appInfo, err := r.readApplicationInfo(repo.GetPath(), filepath.Dir(path), filepath.Base(path)) + appInfo, err := r.readApplicationInfo(repo.GetPath(), repoID, filepath.Dir(path), filepath.Base(path)) if err != nil { r.logger.Error("failed to read application info", zap.String("repo-id", repoID), @@ -313,7 +313,7 @@ func (r *Reporter) scanAllFiles(repoRoot, repoID string, shouldSkip func(string) return nil } - appInfo, err := r.readApplicationInfo(repoRoot, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath)) + appInfo, err := r.readApplicationInfo(repoRoot, repoID, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath)) if err != nil { r.logger.Error("failed to read application info", zap.String("repo-id", repoID), @@ -338,7 +338,7 @@ func makeGitPathKey(repoID, cfgFilePath string) string { return fmt.Sprintf("%s:%s", repoID, cfgFilePath) } -func (r *Reporter) readApplicationInfo(repoDir, appDirRelPath, cfgFilename string) (*model.ApplicationInfo, error) { +func (r *Reporter) readApplicationInfo(repoDir, repoID, 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) @@ -361,6 +361,7 @@ func (r *Reporter) readApplicationInfo(repoDir, appDirRelPath, cfgFilename strin // TODO: Convert Kind string into dedicated type //Kind: cfg.Kind, Labels: spec.Labels, + RepoId: repoID, Path: appDirRelPath, ConfigFilename: cfgFilename, }, nil diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go b/pkg/app/piped/appconfigreporter/appconfigreporter_test.go index 388c39a0fb..00452e0c5c 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go +++ b/pkg/app/piped/appconfigreporter/appconfigreporter_test.go @@ -110,6 +110,7 @@ spec: { Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, + RepoId: "repo-1", Path: "app-1", ConfigFilename: ".pipe.yaml", }, @@ -139,6 +140,7 @@ spec: { Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, + RepoId: "repo-1", Path: "app-1", ConfigFilename: "dev.pipecd.yaml", }, @@ -260,6 +262,7 @@ spec: { Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, + RepoId: "repo-1", Path: "app-1", ConfigFilename: ".pipe.yaml", }, @@ -292,6 +295,7 @@ spec: { Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, + RepoId: "repo-1", Path: "app-1", ConfigFilename: ".pipe.yaml", }, diff --git a/pkg/model/common.proto b/pkg/model/common.proto index cc435acbaf..cef5c5b215 100644 --- a/pkg/model/common.proto +++ b/pkg/model/common.proto @@ -60,6 +60,7 @@ message ApplicationInfo { string name = 1 [(validate.rules).string.min_len = 1]; ApplicationKind kind = 2 [(validate.rules).enum.defined_only = true]; map labels = 3; - string path = 4 [(validate.rules).string.pattern = "^[^/].+$"]; - string config_filename = 5 [(validate.rules).string.min_len = 1]; + string repo_id = 4 [(validate.rules).string.min_len = 1]; + string path = 5 [(validate.rules).string.pattern = "^[^/].+$"]; + string config_filename = 6; } From d645a943cccddb29f8ae4e602fb56f373d2335de Mon Sep 17 00:00:00 2001 From: nakabonne Date: Wed, 24 Nov 2021 17:10:24 +0900 Subject: [PATCH 2/3] Use app id to specify what application to update --- pkg/app/api/grpcapi/piped_api.go | 55 +--------- .../appconfigreporter/appconfigreporter.go | 101 +++++++++++------- .../appconfigreporter_test.go | 30 +++--- pkg/model/common.proto | 13 +-- 4 files changed, 86 insertions(+), 113 deletions(-) diff --git a/pkg/app/api/grpcapi/piped_api.go b/pkg/app/api/grpcapi/piped_api.go index 0490f0f435..411b095ce3 100644 --- a/pkg/app/api/grpcapi/piped_api.go +++ b/pkg/app/api/grpcapi/piped_api.go @@ -957,70 +957,19 @@ func (a *PipedAPI) GetDesiredVersion(ctx context.Context, _ *pipedservice.GetDes } func (a *PipedAPI) UpdateApplicationConfigurations(ctx context.Context, req *pipedservice.UpdateApplicationConfigurationsRequest) (*pipedservice.UpdateApplicationConfigurationsResponse, error) { - projectID, pipedID, _, err := rpcauth.ExtractPipedToken(ctx) + _, _, _, err := rpcauth.ExtractPipedToken(ctx) if err != nil { return nil, err } // TODO: Consider bulk-updating multiple apps for _, appInfo := range req.Applications { - opts := datastore.ListOptions{ - // NOTE: Assume that no more than one application is referring to a single configuration file. - Limit: 1, - Filters: []datastore.ListFilter{ - { - Field: "ProjectId", - Operator: datastore.OperatorEqual, - Value: projectID, - }, - { - Field: "PipedId", - Operator: datastore.OperatorEqual, - Value: pipedID, - }, - { - Field: "GitPath.Repo.Id", - Operator: datastore.OperatorEqual, - Value: appInfo.RepoId, - }, - { - Field: "GitPath.Path", - Operator: datastore.OperatorEqual, - Value: appInfo.Path, - }, - { - Field: "GitPath.ConfigFilename", - Operator: datastore.OperatorEqual, - Value: appInfo.ConfigFilename, - }, - { - Field: "Disabled", - Operator: datastore.OperatorEqual, - Value: false, - }, - }, - } - // TODO: Enable to update multiple apps that match the given condition instead of accessing it twice - // like: UPDATE Application SET Name = 'foo' WHERE GitPath.Repo.Id = '?' AND GitPath.Path = '?' - apps, _, err := a.applicationStore.ListApplications(ctx, opts) - if err != nil { - a.logger.Error("failed to fetch applications", zap.Error(err)) - return nil, status.Error(codes.Internal, "failed to fetch applications") - } - if len(apps) == 0 { - a.logger.Error("not found any enabled apps that match the terms", - zap.String("repo-id", appInfo.RepoId), - zap.String("path", appInfo.Path), - zap.String("config-filename", appInfo.ConfigFilename), - ) - return nil, status.Error(codes.NotFound, "not found any enabled apps that match the terms") - } updater := func(app *model.Application) error { app.Name = appInfo.Name app.Kind = appInfo.Kind app.Labels = appInfo.Labels return nil } - if err := a.applicationStore.UpdateApplication(ctx, apps[0].Id, updater); err != nil { + if err := a.applicationStore.UpdateApplication(ctx, appInfo.Id, updater); err != nil { a.logger.Error("failed to update application", zap.Error(err)) return nil, status.Error(codes.Internal, "failed to update application") } diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter.go b/pkg/app/piped/appconfigreporter/appconfigreporter.go index a1ccba4f66..6b69868437 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter.go +++ b/pkg/app/piped/appconfigreporter/appconfigreporter.go @@ -142,12 +142,12 @@ func (r *Reporter) scanAppConfigs(ctx context.Context) error { } } - // Create a map to determine from GitPath if the application is registered. apps := r.applicationLister.List() - registeredAppPaths := make(map[string]struct{}, len(apps)) + // Create a map from Git path key to app id, to determine if the application is registered. + registeredAppPaths := make(map[string]string, len(apps)) for _, app := range apps { key := makeGitPathKey(app.GitPath.Repo.Id, filepath.Join(app.GitPath.Path, app.GitPath.ConfigFilename)) - registeredAppPaths[key] = struct{}{} + registeredAppPaths[key] = app.Id } if err := r.updateRegisteredApps(ctx, registeredAppPaths); err != nil { @@ -161,7 +161,7 @@ func (r *Reporter) scanAppConfigs(ctx context.Context) error { } // updateRegisteredApps sends application configurations that have changed since the last time to the control-plane. -func (r *Reporter) updateRegisteredApps(ctx context.Context, registeredAppPaths map[string]struct{}) (err error) { +func (r *Reporter) updateRegisteredApps(ctx context.Context, registeredAppPaths map[string]string) (err error) { apps := make([]*model.ApplicationInfo, 0) headCommits := make(map[string]string, len(r.gitRepos)) for repoID, repo := range r.gitRepos { @@ -207,15 +207,45 @@ 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) { +func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo gitRepo, lastScannedCommit, headCommitHash string, registeredAppPaths map[string]string) ([]*model.ApplicationInfo, error) { if lastScannedCommit == "" { - return r.scanAllFiles(repo.GetPath(), repoID, func(fileRelPath string) bool { - gitPathKey := makeGitPathKey(repoID, fileRelPath) - if _, registered := registeredAppPaths[gitPathKey]; !registered { - return true + // Scan all files because it seems to be the first commit since Piped starts. + apps := make([]*model.ApplicationInfo, 0) + err := fs.WalkDir(r.fileSystem, repo.GetPath(), func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err } - return false + if d.IsDir() { + return nil + } + cfgRelPath, err := filepath.Rel(repo.GetPath(), path) + if err != nil { + return err + } + gitPathKey := makeGitPathKey(repoID, cfgRelPath) + appID, registered := registeredAppPaths[gitPathKey] + if !registered { + return nil + } + + appInfo, err := r.readApplicationInfo(repo.GetPath(), repoID, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath)) + if err != nil { + r.logger.Error("failed to read application info", + zap.String("repo-id", repoID), + zap.String("config-file-path", cfgRelPath), + zap.Error(err), + ) + return nil + } + appInfo.Id = appID + apps = append(apps, appInfo) + // Continue reading so that it can return apps as much as possible. + return nil }) + if err != nil { + return nil, fmt.Errorf("failed to inspect files under %s: %w", repo.GetPath(), err) + } + return apps, nil } filePaths, err := repo.ChangedFiles(ctx, lastScannedCommit, headCommitHash) @@ -229,7 +259,8 @@ func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo g apps := make([]*model.ApplicationInfo, 0) for _, path := range filePaths { gitPathKey := makeGitPathKey(repoID, path) - if _, registered := registeredAppPaths[gitPathKey]; !registered { + appID, registered := registeredAppPaths[gitPathKey] + if !registered { continue } appInfo, err := r.readApplicationInfo(repo.GetPath(), repoID, filepath.Dir(path), filepath.Base(path)) @@ -241,13 +272,14 @@ func (r *Reporter) findRegisteredApps(ctx context.Context, repoID string, repo g ) continue } + appInfo.Id = appID apps = append(apps, appInfo) } return apps, nil } // updateUnregisteredApps sends all unregistered application configurations to the control-plane. -func (r *Reporter) updateUnregisteredApps(ctx context.Context, registeredAppPaths map[string]struct{}) error { +func (r *Reporter) updateUnregisteredApps(ctx context.Context, registeredAppPaths map[string]string) error { apps := make([]*model.ApplicationInfo, 0) for repoID, repo := range r.gitRepos { as, err := r.findUnregisteredApps(repo.GetPath(), repoID, registeredAppPaths) @@ -280,40 +312,29 @@ func (r *Reporter) updateUnregisteredApps(ctx context.Context, registeredAppPath // findUnregisteredApps finds out unregistered application info in the given git repository. // The file name must be default name in order to be recognized as an Application config. -func (r *Reporter) findUnregisteredApps(repoPath, repoID string, registeredAppPaths map[string]struct{}) ([]*model.ApplicationInfo, error) { - return r.scanAllFiles(repoPath, repoID, func(fileRelPath string) bool { - if !model.IsApplicationConfigFile(filepath.Base(fileRelPath)) { - return true - } - - gitPathKey := makeGitPathKey(repoID, fileRelPath) - if _, registered := registeredAppPaths[gitPathKey]; registered { - return true - } - return 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(repoRoot, repoID string, shouldSkip func(string) bool) ([]*model.ApplicationInfo, error) { +func (r *Reporter) findUnregisteredApps(repoPath, repoID string, registeredAppPaths map[string]string) ([]*model.ApplicationInfo, error) { apps := make([]*model.ApplicationInfo, 0) - err := fs.WalkDir(r.fileSystem, repoRoot, func(path string, d fs.DirEntry, err error) error { + // Scan all files under the repository. + err := fs.WalkDir(r.fileSystem, repoPath, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } if d.IsDir() { return nil } - cfgRelPath, err := filepath.Rel(repoRoot, path) + cfgRelPath, err := filepath.Rel(repoPath, path) if err != nil { return err } - if shouldSkip(cfgRelPath) { + if !model.IsApplicationConfigFile(filepath.Base(cfgRelPath)) { + return nil + } + gitPathKey := makeGitPathKey(repoID, cfgRelPath) + if _, registered := registeredAppPaths[gitPathKey]; registered { return nil } - appInfo, err := r.readApplicationInfo(repoRoot, repoID, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath)) + appInfo, err := r.readApplicationInfo(repoPath, repoID, filepath.Dir(cfgRelPath), filepath.Base(cfgRelPath)) if err != nil { r.logger.Error("failed to read application info", zap.String("repo-id", repoID), @@ -327,17 +348,11 @@ func (r *Reporter) scanAllFiles(repoRoot, repoID string, shouldSkip func(string) return nil }) if err != nil { - return nil, fmt.Errorf("failed to inspect files under %s: %w", repoRoot, err) + return nil, fmt.Errorf("failed to inspect files under %s: %w", repoPath, err) } return apps, nil } -// makeGitPathKey builds a unique path between repositories. -// cfgFilePath is a relative path from the repo root. -func makeGitPathKey(repoID, cfgFilePath string) string { - return fmt.Sprintf("%s:%s", repoID, cfgFilePath) -} - func (r *Reporter) readApplicationInfo(repoDir, repoID, appDirRelPath, cfgFilename string) (*model.ApplicationInfo, error) { b, err := fs.ReadFile(r.fileSystem, filepath.Join(repoDir, appDirRelPath, cfgFilename)) if err != nil { @@ -366,3 +381,9 @@ func (r *Reporter) readApplicationInfo(repoDir, repoID, appDirRelPath, cfgFilena ConfigFilename: cfgFilename, }, nil } + +// makeGitPathKey builds a unique path between repositories. +// cfgFilePath is a relative path from the repo root. +func makeGitPathKey(repoID, cfgFilePath string) string { + return fmt.Sprintf("%s:%s", repoID, cfgFilePath) +} diff --git a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go b/pkg/app/piped/appconfigreporter/appconfigreporter_test.go index 00452e0c5c..7ba8dcb183 100644 --- a/pkg/app/piped/appconfigreporter/appconfigreporter_test.go +++ b/pkg/app/piped/appconfigreporter/appconfigreporter_test.go @@ -27,7 +27,7 @@ import ( func TestReporter_findUnregisteredApps(t *testing.T) { type args struct { - registeredAppPaths map[string]struct{} + registeredAppPaths map[string]string repoPath, repoID string } testcases := []struct { @@ -48,7 +48,7 @@ func TestReporter_findUnregisteredApps(t *testing.T) { args: args{ repoPath: "invalid", repoID: "repo-1", - registeredAppPaths: map[string]struct{}{}, + registeredAppPaths: map[string]string{}, }, want: nil, wantErr: true, @@ -64,8 +64,8 @@ func TestReporter_findUnregisteredApps(t *testing.T) { args: args{ repoPath: "path/to/repo-1", repoID: "repo-1", - registeredAppPaths: map[string]struct{}{ - "repo-1:app-1/.pipe.yaml": {}, + registeredAppPaths: map[string]string{ + "repo-1:app-1/.pipe.yaml": "id-1", }, }, want: []*model.ApplicationInfo{}, @@ -82,7 +82,7 @@ func TestReporter_findUnregisteredApps(t *testing.T) { args: args{ repoPath: "path/to/repo-1", repoID: "repo-1", - registeredAppPaths: map[string]struct{}{}, + registeredAppPaths: map[string]string{}, }, want: []*model.ApplicationInfo{}, wantErr: false, @@ -104,7 +104,7 @@ spec: args: args{ repoPath: "path/to/repo-1", repoID: "repo-1", - registeredAppPaths: map[string]struct{}{}, + registeredAppPaths: map[string]string{}, }, want: []*model.ApplicationInfo{ { @@ -134,7 +134,7 @@ spec: args: args{ repoPath: "path/to/repo-1", repoID: "repo-1", - registeredAppPaths: map[string]struct{}{}, + registeredAppPaths: map[string]string{}, }, want: []*model.ApplicationInfo{ { @@ -176,7 +176,7 @@ func TestReporter_findRegisteredApps(t *testing.T) { repoID string repo gitRepo lastScannedCommit string - registeredAppPaths map[string]struct{} + registeredAppPaths map[string]string } testcases := []struct { name string @@ -229,8 +229,8 @@ func TestReporter_findRegisteredApps(t *testing.T) { repoID: "repo-1", repo: &fakeGitRepo{path: "path/to/repo-1", changedFiles: []string{"app-1/.pipe.yaml"}}, lastScannedCommit: "xxx", - registeredAppPaths: map[string]struct{}{ - "repo-1:app-1/.pipe.yaml": {}, + registeredAppPaths: map[string]string{ + "repo-1:app-1/.pipe.yaml": "id-1", }, }, want: []*model.ApplicationInfo{}, @@ -254,12 +254,13 @@ spec: repoID: "repo-1", repo: &fakeGitRepo{path: "path/to/repo-1", changedFiles: []string{"app-1/.pipe.yaml"}}, lastScannedCommit: "xxx", - registeredAppPaths: map[string]struct{}{ - "repo-1:app-1/.pipe.yaml": {}, + registeredAppPaths: map[string]string{ + "repo-1:app-1/.pipe.yaml": "id-1", }, }, want: []*model.ApplicationInfo{ { + Id: "id-1", Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, RepoId: "repo-1", @@ -287,12 +288,13 @@ spec: repoID: "repo-1", repo: &fakeGitRepo{path: "path/to/repo-1"}, lastScannedCommit: "", - registeredAppPaths: map[string]struct{}{ - "repo-1:app-1/.pipe.yaml": {}, + registeredAppPaths: map[string]string{ + "repo-1:app-1/.pipe.yaml": "id-1", }, }, want: []*model.ApplicationInfo{ { + Id: "id-1", Name: "app-1", Labels: map[string]string{"key-1": "value-1"}, RepoId: "repo-1", diff --git a/pkg/model/common.proto b/pkg/model/common.proto index cef5c5b215..022727c96a 100644 --- a/pkg/model/common.proto +++ b/pkg/model/common.proto @@ -57,10 +57,11 @@ enum SyncStrategy { } message ApplicationInfo { - string name = 1 [(validate.rules).string.min_len = 1]; - ApplicationKind kind = 2 [(validate.rules).enum.defined_only = true]; - map labels = 3; - string repo_id = 4 [(validate.rules).string.min_len = 1]; - string path = 5 [(validate.rules).string.pattern = "^[^/].+$"]; - string config_filename = 6; + string id = 1; + string name = 2 [(validate.rules).string.min_len = 1]; + ApplicationKind kind = 3 [(validate.rules).enum.defined_only = true]; + map labels = 4; + string repo_id = 5 [(validate.rules).string.min_len = 1]; + string path = 6 [(validate.rules).string.pattern = "^[^/].+$"]; + string config_filename = 7; } From d22b018e3240d3c39afa1deaf32973199664b845 Mon Sep 17 00:00:00 2001 From: nakabonne Date: Wed, 24 Nov 2021 17:49:49 +0900 Subject: [PATCH 3/3] Add validation for app --- pkg/app/api/grpcapi/piped_api.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/app/api/grpcapi/piped_api.go b/pkg/app/api/grpcapi/piped_api.go index 411b095ce3..7a16189b4b 100644 --- a/pkg/app/api/grpcapi/piped_api.go +++ b/pkg/app/api/grpcapi/piped_api.go @@ -957,10 +957,16 @@ func (a *PipedAPI) GetDesiredVersion(ctx context.Context, _ *pipedservice.GetDes } func (a *PipedAPI) UpdateApplicationConfigurations(ctx context.Context, req *pipedservice.UpdateApplicationConfigurationsRequest) (*pipedservice.UpdateApplicationConfigurationsResponse, error) { - _, _, _, err := rpcauth.ExtractPipedToken(ctx) + _, pipedID, _, err := rpcauth.ExtractPipedToken(ctx) if err != nil { return nil, err } + // Scan all of them to guarantee in advance that there is no invalid request. + for _, appInfo := range req.Applications { + if err := a.validateAppBelongsToPiped(ctx, appInfo.Id, pipedID); err != nil { + return nil, err + } + } // TODO: Consider bulk-updating multiple apps for _, appInfo := range req.Applications { updater := func(app *model.Application) error {