From 4a12b9ab3245749a9f97682aeec72b458639ec9d Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Thu, 24 Feb 2022 11:45:26 -0800 Subject: [PATCH] Unique repo path and permissions (#8517) Unique repo path and permissions (#8517) Signed-off-by: Alexander Matyushentsev Signed-off-by: wojtekidd --- reposerver/repository/lock.go | 13 ++- reposerver/repository/lock_test.go | 14 +-- reposerver/repository/repository.go | 130 ++++++++++++++++++----- reposerver/repository/repository_test.go | 78 +++++++++++++- reposerver/server.go | 17 ++- util/grpc/sanitizer.go | 82 ++++++++++++++ util/grpc/sanitizer_test.go | 51 +++++++++ util/helm/client.go | 81 ++++++++------ util/io/paths.go | 44 ++++++++ util/io/paths_test.go | 37 +++++++ 10 files changed, 468 insertions(+), 79 deletions(-) create mode 100644 util/grpc/sanitizer.go create mode 100644 util/grpc/sanitizer_test.go create mode 100644 util/io/paths.go create mode 100644 util/io/paths_test.go diff --git a/reposerver/repository/lock.go b/reposerver/repository/lock.go index 14a2e6f07a422..06c4aa4acedeb 100644 --- a/reposerver/repository/lock.go +++ b/reposerver/repository/lock.go @@ -17,7 +17,7 @@ type repositoryLock struct { } // Lock acquires lock unless lock is already acquired with the same commit and allowConcurrent is set to true -func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool, init func() error) (io.Closer, error) { +func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool, init func() (io.Closer, error)) (io.Closer, error) { r.lock.Lock() state, ok := r.stateByKey[path] if !ok { @@ -30,26 +30,30 @@ func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool state.cond.L.Lock() notify := false state.processCount-- + var err error if state.processCount == 0 { notify = true state.revision = "" + err = state.initCloser.Close() } + state.cond.L.Unlock() if notify { state.cond.Broadcast() } - return nil + return err }) for { state.cond.L.Lock() if state.revision == "" { // no in progress operation for that repo. Go ahead. - if err := init(); err != nil { + initCloser, err := init() + if err != nil { state.cond.L.Unlock() return nil, err } - + state.initCloser = initCloser state.revision = revision state.processCount = 1 state.allowConcurrent = allowConcurrent @@ -71,6 +75,7 @@ func (r *repositoryLock) Lock(path string, revision string, allowConcurrent bool type repositoryState struct { cond *sync.Cond revision string + initCloser io.Closer processCount int allowConcurrent bool } diff --git a/reposerver/repository/lock_test.go b/reposerver/repository/lock_test.go index e3000128c41c4..db2bf2558a2b9 100644 --- a/reposerver/repository/lock_test.go +++ b/reposerver/repository/lock_test.go @@ -26,10 +26,10 @@ func lockQuickly(action func() (io.Closer, error)) (io.Closer, bool) { } } -func numberOfInits(initializedTimes *int) func() error { - return func() error { +func numberOfInits(initializedTimes *int) func() (io.Closer, error) { + return func() (io.Closer, error) { *initializedTimes++ - return nil + return util.NopCloser, nil } } @@ -120,8 +120,8 @@ func TestLock_FailedInitialization(t *testing.T) { lock := NewRepositoryLock() closer1, done := lockQuickly(func() (io.Closer, error) { - return lock.Lock("myRepo", "1", true, func() error { - return errors.New("failed") + return lock.Lock("myRepo", "1", true, func() (io.Closer, error) { + return util.NopCloser, errors.New("failed") }) }) @@ -132,8 +132,8 @@ func TestLock_FailedInitialization(t *testing.T) { assert.Nil(t, closer1) closer2, done := lockQuickly(func() (io.Closer, error) { - return lock.Lock("myRepo", "1", true, func() error { - return nil + return lock.Lock("myRepo", "1", true, func() (io.Closer, error) { + return util.NopCloser, nil }) }) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 82cb9fa18df7b..39c7178299a23 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -6,6 +6,8 @@ import ( "encoding/json" "errors" "fmt" + goio "io" + "io/fs" "io/ioutil" "net/url" "os" @@ -16,10 +18,6 @@ import ( "strings" "time" - "github.com/google/go-jsonnet" - - "github.com/argoproj/argo-cd/v2/util/argo" - "github.com/Masterminds/semver/v3" "github.com/TomOnTime/utfutil" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -27,6 +25,8 @@ import ( "github.com/argoproj/pkg/sync" jsonpatch "github.com/evanphx/json-patch" "github.com/ghodss/yaml" + gogit "github.com/go-git/go-git/v5" + "github.com/google/go-jsonnet" log "github.com/sirupsen/logrus" "golang.org/x/sync/semaphore" "google.golang.org/grpc/codes" @@ -44,10 +44,12 @@ import ( "github.com/argoproj/argo-cd/v2/reposerver/metrics" "github.com/argoproj/argo-cd/v2/util/app/discovery" argopath "github.com/argoproj/argo-cd/v2/util/app/path" + "github.com/argoproj/argo-cd/v2/util/argo" executil "github.com/argoproj/argo-cd/v2/util/exec" "github.com/argoproj/argo-cd/v2/util/git" "github.com/argoproj/argo-cd/v2/util/glob" "github.com/argoproj/argo-cd/v2/util/gpg" + "github.com/argoproj/argo-cd/v2/util/grpc" "github.com/argoproj/argo-cd/v2/util/helm" "github.com/argoproj/argo-cd/v2/util/io" "github.com/argoproj/argo-cd/v2/util/ksonnet" @@ -68,12 +70,16 @@ const ( // Service implements ManifestService interface type Service struct { gitCredsStore git.CredsStore + rootDir string + gitRepoPaths *io.TempPaths + chartPaths *io.TempPaths + gitRepoInitializer func(rootPath string) goio.Closer repoLock *repositoryLock cache *reposervercache.Cache parallelismLimitSemaphore *semaphore.Weighted metricsServer *metrics.MetricsServer resourceTracking argo.ResourceTracking - newGitClient func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, proxy string, opts ...git.ClientOpts) (git.Client, error) + newGitClient func(rawRepoURL string, root string, creds git.Creds, insecure bool, enableLfs bool, proxy string, opts ...git.ClientOpts) (git.Client, error) newHelmClient func(repoURL string, creds helm.Creds, enableOci bool, proxy string, opts ...helm.ClientOpts) helm.Client initConstants RepoServerInitConstants // now is usually just time.Now, but may be replaced by unit tests for testing purposes @@ -89,7 +95,7 @@ type RepoServerInitConstants struct { } // NewService returns a new instance of the Manifest service -func NewService(metricsServer *metrics.MetricsServer, cache *reposervercache.Cache, initConstants RepoServerInitConstants, resourceTracking argo.ResourceTracking, gitCredsStore git.CredsStore) *Service { +func NewService(metricsServer *metrics.MetricsServer, cache *reposervercache.Cache, initConstants RepoServerInitConstants, resourceTracking argo.ResourceTracking, gitCredsStore git.CredsStore, rootDir string) *Service { var parallelismLimitSemaphore *semaphore.Weighted if initConstants.ParallelismLimit > 0 { parallelismLimitSemaphore = semaphore.NewWeighted(initConstants.ParallelismLimit) @@ -100,15 +106,54 @@ func NewService(metricsServer *metrics.MetricsServer, cache *reposervercache.Cac repoLock: repoLock, cache: cache, metricsServer: metricsServer, - newGitClient: git.NewClient, + newGitClient: git.NewClientExt, resourceTracking: resourceTracking, newHelmClient: func(repoURL string, creds helm.Creds, enableOci bool, proxy string, opts ...helm.ClientOpts) helm.Client { return helm.NewClientWithLock(repoURL, creds, sync.NewKeyLock(), enableOci, proxy, opts...) }, - initConstants: initConstants, - now: time.Now, - gitCredsStore: gitCredsStore, + initConstants: initConstants, + now: time.Now, + gitCredsStore: gitCredsStore, + gitRepoPaths: io.NewTempPaths(rootDir), + chartPaths: io.NewTempPaths(rootDir), + gitRepoInitializer: directoryPermissionInitializer, + rootDir: rootDir, + } +} + +func (s *Service) Init() error { + _, err := os.Stat(s.rootDir) + if os.IsNotExist(err) { + return os.MkdirAll(s.rootDir, 0300) + } + if err == nil { + // give itself read permissions to list previously written directories + err = os.Chmod(s.rootDir, 0700) + } + var files []fs.FileInfo + if err == nil { + files, err = ioutil.ReadDir(s.rootDir) + } + if err != nil { + log.Warnf("Failed to restore cloned repositories paths: %v", err) + return nil + } + + for _, file := range files { + if !file.IsDir() { + continue + } + fullPath := filepath.Join(s.rootDir, file.Name()) + closer := s.gitRepoInitializer(fullPath) + if repo, err := gogit.PlainOpen(fullPath); err == nil { + if remotes, err := repo.Remotes(); err == nil && len(remotes) > 0 && len(remotes[0].Config().URLs) > 0 { + s.gitRepoPaths.Add(git.NormalizeGitURL(remotes[0].Config().URLs[0]), fullPath) + } + } + io.Close(closer) } + // remove read permissions since no-one should be able to list the directories + return os.Chmod(s.rootDir, 0300) } // List a subset of the refs (currently, branches and tags) of a git repo @@ -148,8 +193,8 @@ func (s *Service) ListApps(ctx context.Context, q *apiclient.ListAppsRequest) (* s.metricsServer.IncPendingRepoRequest(q.Repo.Repo) defer s.metricsServer.DecPendingRepoRequest(q.Repo.Repo) - closer, err := s.repoLock.Lock(gitClient.Root(), commitSHA, true, func() error { - return checkoutRevision(gitClient, commitSHA, s.initConstants.SubmoduleEnabled) + closer, err := s.repoLock.Lock(gitClient.Root(), commitSHA, true, func() (goio.Closer, error) { + return s.checkoutRevision(gitClient, commitSHA, s.initConstants.SubmoduleEnabled) }) if err != nil { @@ -207,6 +252,11 @@ func (s *Service) runRepoOperation( operation func(repoRoot, commitSHA, cacheKey string, ctxSrc operationContextSrc) error, settings operationSettings) error { + if sanitizer, ok := grpc.SanitizerFromContext(ctx); ok { + // make sure randomized path replaced with '.' in the error message + sanitizer.AddRegexReplacement(regexp.MustCompile(`(`+s.rootDir+`/.*?)/`), ".") + } + var gitClient git.Client var helmClient helm.Client var err error @@ -260,8 +310,8 @@ func (s *Service) runRepoOperation( return &operationContext{chartPath, ""}, nil }) } else { - closer, err := s.repoLock.Lock(gitClient.Root(), revision, settings.allowConcurrent, func() error { - return checkoutRevision(gitClient, revision, s.initConstants.SubmoduleEnabled) + closer, err := s.repoLock.Lock(gitClient.Root(), revision, settings.allowConcurrent, func() (goio.Closer, error) { + return s.checkoutRevision(gitClient, revision, s.initConstants.SubmoduleEnabled) }) if err != nil { @@ -1626,8 +1676,8 @@ func (s *Service) GetRevisionMetadata(ctx context.Context, q *apiclient.RepoServ s.metricsServer.IncPendingRepoRequest(q.Repo.Repo) defer s.metricsServer.DecPendingRepoRequest(q.Repo.Repo) - closer, err := s.repoLock.Lock(gitClient.Root(), q.Revision, true, func() error { - return checkoutRevision(gitClient, q.Revision, s.initConstants.SubmoduleEnabled) + closer, err := s.repoLock.Lock(gitClient.Root(), q.Revision, true, func() (goio.Closer, error) { + return s.checkoutRevision(gitClient, q.Revision, s.initConstants.SubmoduleEnabled) }) if err != nil { @@ -1675,8 +1725,12 @@ func fileParameters(q *apiclient.RepoServerAppDetailsQuery) []v1alpha1.HelmFileP } func (s *Service) newClient(repo *v1alpha1.Repository, opts ...git.ClientOpts) (git.Client, error) { + repoPath, err := s.gitRepoPaths.GetPath(git.NormalizeGitURL(repo.Repo)) + if err != nil { + return nil, err + } opts = append(opts, git.WithEventHandlers(metrics.NewGitClientEventHandlers(s.metricsServer))) - return s.newGitClient(repo.Repo, repo.GetGitCreds(s.gitCredsStore), repo.IsInsecure(), repo.EnableLFS, repo.Proxy, opts...) + return s.newGitClient(repo.Repo, repoPath, repo.GetGitCreds(s.gitCredsStore), repo.IsInsecure(), repo.EnableLFS, repo.Proxy, opts...) } // newClientResolveRevision is a helper to perform the common task of instantiating a git client @@ -1695,7 +1749,7 @@ func (s *Service) newClientResolveRevision(repo *v1alpha1.Repository, revision s func (s *Service) newHelmClientResolveRevision(repo *v1alpha1.Repository, revision string, chart string, noRevisionCache bool) (helm.Client, string, error) { enableOCI := repo.EnableOCI || helm.IsHelmOciRepo(repo.Repo) - helmClient := s.newHelmClient(repo.Repo, repo.GetHelmCreds(), enableOCI, repo.Proxy, helm.WithIndexCache(s.cache)) + helmClient := s.newHelmClient(repo.Repo, repo.GetHelmCreds(), enableOCI, repo.Proxy, helm.WithIndexCache(s.cache), helm.WithChartPaths(s.chartPaths)) // OCI helm registers don't support semver ranges. Assuming that given revision is exact version if helm.IsVersion(revision) || enableOCI { return helmClient, revision, nil @@ -1719,13 +1773,35 @@ func (s *Service) newHelmClientResolveRevision(repo *v1alpha1.Repository, revisi return helmClient, version.String(), nil } +// directoryPermissionInitializer ensures the directory has read/write/execute permissions and returns +// a function that can be used to remove all permissions. +func directoryPermissionInitializer(rootPath string) goio.Closer { + if _, err := os.Stat(rootPath); err == nil { + if err := os.Chmod(rootPath, 0700); err != nil { + log.Warnf("Failed to restore read/write/execute permissions on %s: %v", rootPath, err) + } else { + log.Debugf("Successfully restored read/write/execute permissions on %s", rootPath) + } + } + + return io.NewCloser(func() error { + if err := os.Chmod(rootPath, 0000); err != nil { + log.Warnf("Failed to remove permissions on %s: %v", rootPath, err) + } else { + log.Debugf("Successfully removed permissions on %s", rootPath) + } + return nil + }) +} + // checkoutRevision is a convenience function to initialize a repo, fetch, and checkout a revision // Returns the 40 character commit SHA after the checkout has been performed // nolint:unparam -func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool) error { +func (s *Service) checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool) (goio.Closer, error) { + closer := s.gitRepoInitializer(gitClient.Root()) err := gitClient.Init() if err != nil { - return status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err) + return closer, status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err) } err = gitClient.Fetch(revision) @@ -1735,25 +1811,25 @@ func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bo log.Infof("Fallback to fetch default") err = gitClient.Fetch("") if err != nil { - return status.Errorf(codes.Internal, "Failed to fetch default: %v", err) + return closer, status.Errorf(codes.Internal, "Failed to fetch default: %v", err) } err = gitClient.Checkout(revision, submoduleEnabled) if err != nil { - return status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err) + return closer, status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err) } - return err + return closer, err } err = gitClient.Checkout("FETCH_HEAD", submoduleEnabled) if err != nil { - return status.Errorf(codes.Internal, "Failed to checkout FETCH_HEAD: %v", err) + return closer, status.Errorf(codes.Internal, "Failed to checkout FETCH_HEAD: %v", err) } - return err + return closer, err } func (s *Service) GetHelmCharts(ctx context.Context, q *apiclient.HelmChartsRequest) (*apiclient.HelmChartsResponse, error) { - index, err := s.newHelmClient(q.Repo.Repo, q.Repo.GetHelmCreds(), q.Repo.EnableOCI, q.Repo.Proxy).GetIndex(true) + index, err := s.newHelmClient(q.Repo.Repo, q.Repo.GetHelmCreds(), q.Repo.EnableOCI, q.Repo.Proxy, helm.WithChartPaths(s.chartPaths)).GetIndex(true) if err != nil { return nil, err } @@ -1814,7 +1890,7 @@ func (s *Service) ResolveRevision(ctx context.Context, q *apiclient.ResolveRevis if helm.IsVersion(ambiguousRevision) { return &apiclient.ResolveRevisionResponse{Revision: ambiguousRevision, AmbiguousRevision: ambiguousRevision}, nil } - client := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI || app.Spec.Source.IsHelmOci(), repo.Proxy) + client := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI || app.Spec.Source.IsHelmOci(), repo.Proxy, helm.WithChartPaths(s.chartPaths)) index, err := client.GetIndex(false) if err != nil { return &apiclient.ResolveRevisionResponse{Revision: "", AmbiguousRevision: ""}, err diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 6c8162f4cdb6e..62d66d51d3a3a 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -5,17 +5,17 @@ import ( "encoding/json" "errors" "fmt" + goio "io" "io/ioutil" "os" "os/exec" + "path" "path/filepath" "regexp" "strings" "testing" "time" - "github.com/argoproj/argo-cd/v2/util/argo" - "github.com/ghodss/yaml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -29,6 +29,7 @@ import ( "github.com/argoproj/argo-cd/v2/reposerver/cache" "github.com/argoproj/argo-cd/v2/reposerver/metrics" fileutil "github.com/argoproj/argo-cd/v2/test/fixture/path" + "github.com/argoproj/argo-cd/v2/util/argo" cacheutil "github.com/argoproj/argo-cd/v2/util/cache" "github.com/argoproj/argo-cd/v2/util/git" gitmocks "github.com/argoproj/argo-cd/v2/util/git/mocks" @@ -72,7 +73,7 @@ func newServiceWithOpt(cf clientFunc) (*Service, *gitmocks.Client) { cacheutil.NewCache(cacheutil.NewInMemoryCache(1*time.Minute)), 1*time.Minute, 1*time.Minute, - ), RepoServerInitConstants{ParallelismLimit: 1}, argo.NewResourceTracking(), &git.NoopCredsStore{}) + ), RepoServerInitConstants{ParallelismLimit: 1}, argo.NewResourceTracking(), &git.NoopCredsStore{}, os.TempDir()) chart := "my-chart" version := "1.1.0" @@ -82,12 +83,15 @@ func newServiceWithOpt(cf clientFunc) (*Service, *gitmocks.Client) { helmClient.On("ExtractChart", chart, version).Return("./testdata/my-chart", io.NopCloser, nil) helmClient.On("CleanChartCache", chart, version).Return(nil) - service.newGitClient = func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, prosy string, opts ...git.ClientOpts) (client git.Client, e error) { + service.newGitClient = func(rawRepoURL string, root string, creds git.Creds, insecure bool, enableLfs bool, prosy string, opts ...git.ClientOpts) (client git.Client, e error) { return gitClient, nil } service.newHelmClient = func(repoURL string, creds helm.Creds, enableOci bool, proxy string, opts ...helm.ClientOpts) helm.Client { return helmClient } + service.gitRepoInitializer = func(rootPath string) goio.Closer { + return io.NopCloser + } return service, gitClient } @@ -118,7 +122,7 @@ func newServiceWithCommitSHA(root, revision string) *Service { gitClient.On("Root").Return(root) }) - service.newGitClient = func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, proxy string, opts ...git.ClientOpts) (client git.Client, e error) { + service.newGitClient = func(rawRepoURL string, root string, creds git.Creds, insecure bool, enableLfs bool, proxy string, opts ...git.ClientOpts) (client git.Client, e error) { return gitClient, nil } @@ -1904,3 +1908,67 @@ func Test_resolveHelmValueFilePath(t *testing.T) { assert.Equal(t, "", p) }) } + +func TestDirectoryPermissionInitializer(t *testing.T) { + dir, err := ioutil.TempDir("", "") + require.NoError(t, err) + defer func() { + _ = os.RemoveAll(dir) + }() + + file, err := ioutil.TempFile(dir, "") + require.NoError(t, err) + io.Close(file) + + // remove read permissions + assert.NoError(t, os.Chmod(dir, 0000)) + closer := directoryPermissionInitializer(dir) + + // make sure permission are restored + _, err = ioutil.ReadFile(file.Name()) + require.NoError(t, err) + + // make sure permission are removed by closer + io.Close(closer) + _, err = ioutil.ReadFile(file.Name()) + require.Error(t, err) +} + +func initGitRepo(repoPath string, remote string) error { + if err := os.Mkdir(repoPath, 0755); err != nil { + return err + } + + cmd := exec.Command("git", "init", repoPath) + cmd.Dir = repoPath + if err := cmd.Run(); err != nil { + return err + } + cmd = exec.Command("git", "remote", "add", "origin", remote) + cmd.Dir = repoPath + return cmd.Run() +} + +func TestInit(t *testing.T) { + dir, err := ioutil.TempDir("", "") + require.NoError(t, err) + defer func() { + _ = os.RemoveAll(dir) + }() + + repoPath := path.Join(dir, "repo1") + require.NoError(t, initGitRepo(repoPath, "https://github.com/argo-cd/test-repo1")) + + service := newService(".") + service.rootDir = dir + + require.NoError(t, service.Init()) + + repo1Path, err := service.gitRepoPaths.GetPath(git.NormalizeGitURL("https://github.com/argo-cd/test-repo1")) + assert.NoError(t, err) + assert.Equal(t, repoPath, repo1Path) + + _, err = ioutil.ReadDir(dir) + require.Error(t, err) + require.NoError(t, initGitRepo(path.Join(dir, "repo2"), "https://github.com/argo-cd/test-repo2")) +} diff --git a/reposerver/server.go b/reposerver/server.go index fb2e861c3dc39..75ddbfd2c0605 100644 --- a/reposerver/server.go +++ b/reposerver/server.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "fmt" "os" + "path/filepath" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" @@ -32,6 +33,7 @@ import ( // ArgoCDRepoServer is the repo server implementation type ArgoCDRepoServer struct { log *log.Entry + repoService *repository.Service metricsServer *metrics.MetricsServer gitCredsStore git.CredsStore cache *reposervercache.Cache @@ -65,7 +67,12 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach serverLog := log.NewEntry(log.StandardLogger()) streamInterceptors := []grpc.StreamServerInterceptor{grpc_logrus.StreamServerInterceptor(serverLog), grpc_prometheus.StreamServerInterceptor, grpc_util.PanicLoggerStreamServerInterceptor(serverLog)} - unaryInterceptors := []grpc.UnaryServerInterceptor{grpc_logrus.UnaryServerInterceptor(serverLog), grpc_prometheus.UnaryServerInterceptor, grpc_util.PanicLoggerUnaryServerInterceptor(serverLog)} + unaryInterceptors := []grpc.UnaryServerInterceptor{ + grpc_logrus.UnaryServerInterceptor(serverLog), + grpc_prometheus.UnaryServerInterceptor, + grpc_util.PanicLoggerUnaryServerInterceptor(serverLog), + grpc_util.ErrorSanitizerUnaryServerInterceptor(), + } serverOpts := []grpc.ServerOption{ grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(unaryInterceptors...)), @@ -79,6 +86,10 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach if tlsConfig != nil { serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(tlsConfig))) } + repoService := repository.NewService(metricsServer, cache, initConstants, argo.NewResourceTracking(), gitCredsStore, filepath.Join(os.TempDir(), "_argocd-repo")) + if err := repoService.Init(); err != nil { + return nil, err + } return &ArgoCDRepoServer{ log: serverLog, @@ -87,6 +98,7 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach initConstants: initConstants, opts: serverOpts, gitCredsStore: gitCredsStore, + repoService: repoService, }, nil } @@ -96,8 +108,7 @@ func (a *ArgoCDRepoServer) CreateGRPC() *grpc.Server { versionpkg.RegisterVersionServiceServer(server, version.NewServer(nil, func() (bool, error) { return true, nil })) - manifestService := repository.NewService(a.metricsServer, a.cache, a.initConstants, argo.NewResourceTracking(), a.gitCredsStore) - apiclient.RegisterRepoServerServiceServer(server, manifestService) + apiclient.RegisterRepoServerServiceServer(server, a.repoService) healthService := health.NewServer() grpc_health_v1.RegisterHealthServer(server, healthService) diff --git a/util/grpc/sanitizer.go b/util/grpc/sanitizer.go new file mode 100644 index 0000000000000..90eafd62f5aac --- /dev/null +++ b/util/grpc/sanitizer.go @@ -0,0 +1,82 @@ +package grpc + +import ( + "errors" + "regexp" + "strings" + + "golang.org/x/net/context" + "google.golang.org/grpc" + "google.golang.org/grpc/status" +) + +const ( + contextKey = "sanitizer" +) + +// ErrorSanitizerUnaryServerInterceptor returns a new unary server interceptor that sanitizes error messages +// and provides Sanitizer to define replacements. +func ErrorSanitizerUnaryServerInterceptor() grpc.UnaryServerInterceptor { + return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) { + sanitizer := NewSanitizer() + resp, err = handler(ContextWithSanitizer(ctx, sanitizer), req) + if err == nil { + return resp, nil + } + + if se, ok := err.(interface{ GRPCStatus() *status.Status }); ok { + return resp, status.Error(se.GRPCStatus().Code(), sanitizer.Replace(se.GRPCStatus().Message())) + } + + return resp, errors.New(sanitizer.Replace(err.Error())) + } +} + +// ContextWithSanitizer returns a new context with sanitizer set. +func ContextWithSanitizer(ctx context.Context, sanitizer Sanitizer) context.Context { + return context.WithValue(ctx, contextKey, sanitizer) +} + +// SanitizerFromContext returns sanitizer from context. +func SanitizerFromContext(ctx context.Context) (Sanitizer, bool) { + res, ok := ctx.Value(contextKey).(Sanitizer) + return res, ok +} + +// Sanitizer provides methods to define list of strings and replacements +type Sanitizer interface { + Replace(s string) string + AddReplacement(val string, replacement string) + AddRegexReplacement(regex *regexp.Regexp, replacement string) +} + +type sanitizer struct { + replacers []func(in string) string +} + +// NewSanitizer returns a new Sanitizer instance +func NewSanitizer() *sanitizer { + return &sanitizer{} +} + +// AddReplacement adds a replacement to the Sanitizer +func (s *sanitizer) AddReplacement(val string, replacement string) { + s.replacers = append(s.replacers, func(in string) string { + return strings.Replace(in, val, replacement, -1) + }) +} + +// AddRegexReplacement adds a replacement to the sanitizer using regexp +func (s *sanitizer) AddRegexReplacement(regex *regexp.Regexp, replacement string) { + s.replacers = append(s.replacers, func(in string) string { + return regex.ReplaceAllString(in, replacement) + }) +} + +// Replace replaces all occurrences of the configured values in the sanitizer with the replacements +func (s *sanitizer) Replace(val string) string { + for _, replacer := range s.replacers { + val = replacer(val) + } + return val +} diff --git a/util/grpc/sanitizer_test.go b/util/grpc/sanitizer_test.go new file mode 100644 index 0000000000000..bd7108b6d0085 --- /dev/null +++ b/util/grpc/sanitizer_test.go @@ -0,0 +1,51 @@ +package grpc + +import ( + "context" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestSanitizer(t *testing.T) { + s := NewSanitizer() + + ctx := ContextWithSanitizer(context.TODO(), s) + + sanitizer, ok := SanitizerFromContext(ctx) + require.True(t, ok) + sanitizer.AddReplacement("/my-random/path", ".") + + res := s.Replace("error at /my-random/path/sub-dir: something went wrong") + assert.Equal(t, "error at ./sub-dir: something went wrong", res) +} + +func TestSanitizer_RegexReplacement(t *testing.T) { + s := NewSanitizer() + + ctx := ContextWithSanitizer(context.TODO(), s) + + sanitizer, ok := SanitizerFromContext(ctx) + require.True(t, ok) + + sanitizer.AddRegexReplacement(regexp.MustCompile("(/my-random/path)"), ".") + res := s.Replace("error at /my-random/path/something: something went wrong") + assert.Equal(t, "error at ./something: something went wrong", res) +} + +func TestErrorSanitizerUnaryServerInterceptor(t *testing.T) { + interceptor := ErrorSanitizerUnaryServerInterceptor() + + _, err := interceptor(context.Background(), nil, nil, func(ctx context.Context, req interface{}) (interface{}, error) { + sanitizer, ok := SanitizerFromContext(ctx) + require.True(t, ok) + sanitizer.AddReplacement("/my-random/path", ".") + return nil, status.Error(codes.Internal, "error at /my-random/path/sub-dir: something went wrong") + }) + + assert.Equal(t, "rpc error: code = Internal desc = error at ./sub-dir: something went wrong", err.Error()) +} diff --git a/util/helm/client.go b/util/helm/client.go index 2911c68cb8ae2..c3c9ed7e7e11b 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/json" "errors" "fmt" "io/ioutil" @@ -17,6 +18,7 @@ import ( "time" "github.com/argoproj/pkg/sync" + "github.com/google/uuid" log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" @@ -60,18 +62,24 @@ func WithIndexCache(indexCache indexCache) ClientOpts { } } +func WithChartPaths(chartPaths *io.TempPaths) ClientOpts { + return func(c *nativeHelmChart) { + c.chartCachePaths = chartPaths + } +} + func NewClient(repoURL string, creds Creds, enableOci bool, proxy string, opts ...ClientOpts) Client { return NewClientWithLock(repoURL, creds, globalLock, enableOci, proxy, opts...) } func NewClientWithLock(repoURL string, creds Creds, repoLock sync.KeyLock, enableOci bool, proxy string, opts ...ClientOpts) Client { c := &nativeHelmChart{ - repoURL: repoURL, - creds: creds, - repoPath: filepath.Join(os.TempDir(), strings.Replace(repoURL, "/", "_", -1)), - repoLock: repoLock, - enableOci: enableOci, - proxy: proxy, + repoURL: repoURL, + creds: creds, + repoLock: repoLock, + enableOci: enableOci, + proxy: proxy, + chartCachePaths: io.NewTempPaths(os.TempDir()), } for i := range opts { opts[i](c) @@ -82,13 +90,13 @@ func NewClientWithLock(repoURL string, creds Creds, repoLock sync.KeyLock, enabl var _ Client = &nativeHelmChart{} type nativeHelmChart struct { - repoPath string - repoURL string - creds Creds - repoLock sync.KeyLock - enableOci bool - indexCache indexCache - proxy string + chartCachePaths *io.TempPaths + repoURL string + creds Creds + repoLock sync.KeyLock + enableOci bool + indexCache indexCache + proxy string } func fileExist(filePath string) (bool, error) { @@ -102,29 +110,29 @@ func fileExist(filePath string) (bool, error) { return true, nil } -func (c *nativeHelmChart) ensureHelmChartRepoPath() error { - c.repoLock.Lock(c.repoPath) - defer c.repoLock.Unlock(c.repoPath) - - err := os.Mkdir(c.repoPath, 0700) - if err != nil && !os.IsExist(err) { +func (c *nativeHelmChart) CleanChartCache(chart string, version string) error { + cachePath, err := c.getCachedChartPath(chart, version) + if err != nil { return err } - return nil -} - -func (c *nativeHelmChart) CleanChartCache(chart string, version string) error { - return os.RemoveAll(c.getCachedChartPath(chart, version)) + return os.RemoveAll(cachePath) } -func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool) (string, io.Closer, error) { - err := c.ensureHelmChartRepoPath() +func createTempDir() (string, error) { + newUUID, err := uuid.NewRandom() if err != nil { - return "", nil, err + return "", err + } + tempDir := path.Join(os.TempDir(), newUUID.String()) + if err := os.Mkdir(tempDir, 0755); err != nil { + return "", err } + return tempDir, nil +} +func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool) (string, io.Closer, error) { // always use Helm V3 since we don't have chart content to determine correct Helm version - helmCmd, err := NewCmdWithVersion(c.repoPath, HelmV3, c.enableOci, c.proxy) + helmCmd, err := NewCmdWithVersion("", HelmV3, c.enableOci, c.proxy) if err != nil { return "", nil, err @@ -137,12 +145,15 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredent } // throw away temp directory that stores extracted chart and should be deleted as soon as no longer needed by returned closer - tempDir, err := ioutil.TempDir("", "helm") + tempDir, err := createTempDir() if err != nil { return "", nil, err } - cachedChartPath := c.getCachedChartPath(chart, version) + cachedChartPath, err := c.getCachedChartPath(chart, version) + if err != nil { + return "", nil, err + } c.repoLock.Lock(cachedChartPath) defer c.repoLock.Unlock(cachedChartPath) @@ -155,7 +166,7 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredent if !exists { // create empty temp directory to extract chart from the registry - tempDest, err := ioutil.TempDir("", "helm") + tempDest, err := createTempDir() if err != nil { return "", nil, err } @@ -355,8 +366,12 @@ func normalizeChartName(chart string) string { return nc } -func (c *nativeHelmChart) getCachedChartPath(chart string, version string) string { - return path.Join(c.repoPath, fmt.Sprintf("%s-%s.tgz", strings.ReplaceAll(chart, "/", "_"), version)) +func (c *nativeHelmChart) getCachedChartPath(chart string, version string) (string, error) { + keyData, err := json.Marshal(map[string]string{"url": c.repoURL, "chart": chart, "version": version}) + if err != nil { + return "", err + } + return c.chartCachePaths.GetPath(string(keyData)) } // Ensures that given OCI registries URL does not have protocol diff --git a/util/io/paths.go b/util/io/paths.go new file mode 100644 index 0000000000000..b9c20283bcdcd --- /dev/null +++ b/util/io/paths.go @@ -0,0 +1,44 @@ +package io + +import ( + "path/filepath" + "sync" + + "github.com/google/uuid" +) + +// TempPaths allows generating and memoizing random paths, each path being mapped to a specific key. +type TempPaths struct { + root string + paths map[string]string + lock sync.Mutex +} + +func NewTempPaths(root string) *TempPaths { + return &TempPaths{ + root: root, + paths: map[string]string{}, + } +} + +func (p *TempPaths) Add(key string, value string) { + p.lock.Lock() + defer p.lock.Unlock() + p.paths[key] = value +} + +// GetPath generates a path for the given key or returns previously generated one. +func (p *TempPaths) GetPath(key string) (string, error) { + p.lock.Lock() + defer p.lock.Unlock() + if val, ok := p.paths[key]; ok { + return val, nil + } + uniqueId, err := uuid.NewRandom() + if err != nil { + return "", err + } + repoPath := filepath.Join(p.root, uniqueId.String()) + p.paths[key] = repoPath + return repoPath, nil +} diff --git a/util/io/paths_test.go b/util/io/paths_test.go new file mode 100644 index 0000000000000..ea98dbaafb878 --- /dev/null +++ b/util/io/paths_test.go @@ -0,0 +1,37 @@ +package io + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetPath_SameURLs(t *testing.T) { + paths := NewTempPaths(os.TempDir()) + res1, err := paths.GetPath("https://localhost/test.txt") + require.NoError(t, err) + res2, err := paths.GetPath("https://localhost/test.txt") + require.NoError(t, err) + assert.Equal(t, res1, res2) +} + +func TestGetPath_DifferentURLs(t *testing.T) { + paths := NewTempPaths(os.TempDir()) + res1, err := paths.GetPath("https://localhost/test1.txt") + require.NoError(t, err) + res2, err := paths.GetPath("https://localhost/test2.txt") + require.NoError(t, err) + assert.NotEqual(t, res1, res2) +} + +func TestGetPath_SameURLsDifferentInstances(t *testing.T) { + paths1 := NewTempPaths(os.TempDir()) + res1, err := paths1.GetPath("https://localhost/test.txt") + require.NoError(t, err) + paths2 := NewTempPaths(os.TempDir()) + res2, err := paths2.GetPath("https://localhost/test.txt") + require.NoError(t, err) + assert.NotEqual(t, res1, res2) +}