From 304a7428d625e7115a0a42b0baba73f464fdeac1 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev <AMatyushentsev@gmail.com> Date: Wed, 22 Mar 2023 09:15:19 -0700 Subject: [PATCH] feat: allow CMP plugins to preserve repo files mode (#12940) * feat: allow CMP plugins to preserve repo files mode Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * implement missing e2e test Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --------- Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --- cmpserver/plugin/config.go | 11 ++++--- cmpserver/plugin/plugin.go | 8 ++--- .../config-management-plugins.md | 30 ++++++++++++++++++ test/e2e/custom_tool_test.go | 31 ++++++++++++++++--- .../cmp-preserve-file-mode/plugin.yaml | 11 +++++++ .../testdata/cmp-preserve-file-mode/script.sh | 8 +++++ util/cmp/stream.go | 7 +++-- util/cmp/stream_test.go | 2 +- util/io/files/tar.go | 11 +++++-- util/io/files/tar_test.go | 25 +++++++++++++-- util/io/files/testdata/executable/script.sh | 1 + util/manifeststream/stream.go | 5 +-- 12 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 test/e2e/testdata/cmp-preserve-file-mode/plugin.yaml create mode 100755 test/e2e/testdata/cmp-preserve-file-mode/script.sh create mode 100755 util/io/files/testdata/executable/script.sh diff --git a/cmpserver/plugin/config.go b/cmpserver/plugin/config.go index 4dc564821257a..c772abb7f7f30 100644 --- a/cmpserver/plugin/config.go +++ b/cmpserver/plugin/config.go @@ -22,11 +22,12 @@ type PluginConfig struct { } type PluginConfigSpec struct { - Version string `json:"version"` - Init Command `json:"init,omitempty"` - Generate Command `json:"generate"` - Discover Discover `json:"discover"` - Parameters Parameters `yaml:"parameters"` + Version string `json:"version"` + Init Command `json:"init,omitempty"` + Generate Command `json:"generate"` + Discover Discover `json:"discover"` + Parameters Parameters `yaml:"parameters"` + PreserveFileMode bool `json:"preserveFileMode,omitempty"` } // Discover holds find and fileName diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go index bc12e189de8f8..0f78121ea6c1b 100644 --- a/cmpserver/plugin/plugin.go +++ b/cmpserver/plugin/plugin.go @@ -113,7 +113,7 @@ func runCommand(ctx context.Context, command Command, path string, env []string) } if len(output) == 0 { log.WithFields(log.Fields{ - "stderr": stderr, + "stderr": stderr, "command": command, }).Warn("Plugin command returned zero output") } @@ -213,7 +213,7 @@ func (s *Service) generateManifestGeneric(stream GenerateManifestStream) error { } defer cleanup() - metadata, err := cmp.ReceiveRepoStream(ctx, stream, workDir) + metadata, err := cmp.ReceiveRepoStream(ctx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode) if err != nil { return fmt.Errorf("generate manifest error receiving stream: %w", err) } @@ -297,7 +297,7 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error { } defer cleanup() - metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir) + metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode) if err != nil { return fmt.Errorf("match repository error receiving stream: %w", err) } @@ -375,7 +375,7 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu } defer cleanup() - metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir) + metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode) if err != nil { return fmt.Errorf("parameters announcement error receiving stream: %w", err) } diff --git a/docs/operator-manual/config-management-plugins.md b/docs/operator-manual/config-management-plugins.md index 9ec31540ce87a..0334b2c168faa 100644 --- a/docs/operator-manual/config-management-plugins.md +++ b/docs/operator-manual/config-management-plugins.md @@ -117,6 +117,10 @@ spec: # The command is run in an Application's source directory. Standard output must be JSON matching the schema of the # static parameter announcements list. command: [echo, '[{"name": "example-param", "string": "default-string-value"}]'] + + # If set to then the plugin receives repository files with original file mode. Dangerous since the repository + # might have executable files. Set to true only if you trust the CMP plugin authors. + preserveFileMode: false ``` !!! note @@ -484,3 +488,29 @@ After installing the plugin as a sidecar [according to the directions above](#in test it out on a few Applications before migrating all of them to the sidecar plugin. Once tests have checked out, remove the plugin entry from your argocd-cm ConfigMap. + +### Additional Settings + +#### Preserve repository files mode + +By default, config management plugin receives source repository files with reset file mode. This is done for security +reasons. If you want to preserve original file mode, you can set `preserveFileMode` to `true` in the plugin spec: + +!!! warning + Make sure you trust the plugin you are using. If you set `preserveFileMode` to `true` then the plugin might receive + files with executable permissions which can be a security risk. + +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: ConfigManagementPlugin +metadata: + name: pluginName +spec: + init: + command: ["sample command"] + args: ["sample args"] + generate: + command: ["sample command"] + args: ["sample args"] + preserveFileMode: true +``` diff --git a/test/e2e/custom_tool_test.go b/test/e2e/custom_tool_test.go index cd587f42693b6..28ee8b52a5336 100644 --- a/test/e2e/custom_tool_test.go +++ b/test/e2e/custom_tool_test.go @@ -10,6 +10,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/health" . "github.com/argoproj/gitops-engine/pkg/sync/common" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" . "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" . "github.com/argoproj/argo-cd/v2/test/e2e/fixture" @@ -156,7 +157,7 @@ func TestCustomToolWithEnv(t *testing.T) { }) } -//make sure we can sync and diff with --local +// make sure we can sync and diff with --local func TestCustomToolSyncAndDiffLocal(t *testing.T) { ctx := Given(t) ctx. @@ -203,7 +204,7 @@ func startCMPServer(configFile string) { FailOnErr(RunWithStdin("", "", "../../dist/argocd", "--config-dir-path", configFile)) } -//Discover by fileName +// Discover by fileName func TestCMPDiscoverWithFileName(t *testing.T) { pluginName := "cmp-fileName" Given(t). @@ -222,7 +223,7 @@ func TestCMPDiscoverWithFileName(t *testing.T) { Expect(HealthIs(health.HealthStatusHealthy)) } -//Discover by Find glob +// Discover by Find glob func TestCMPDiscoverWithFindGlob(t *testing.T) { Given(t). And(func() { @@ -240,7 +241,7 @@ func TestCMPDiscoverWithFindGlob(t *testing.T) { Expect(HealthIs(health.HealthStatusHealthy)) } -//Discover by Plugin Name +// Discover by Plugin Name func TestCMPDiscoverWithPluginName(t *testing.T) { Given(t). And(func() { @@ -261,7 +262,7 @@ func TestCMPDiscoverWithPluginName(t *testing.T) { Expect(HealthIs(health.HealthStatusHealthy)) } -//Discover by Find command +// Discover by Find command func TestCMPDiscoverWithFindCommandWithEnv(t *testing.T) { pluginName := "cmp-find-command" ctx := Given(t) @@ -329,3 +330,23 @@ func TestPruneResourceFromCMP(t *testing.T) { assert.Error(t, err) }) } + +func TestPreserveFileModeForCMP(t *testing.T) { + Given(t). + And(func() { + go startCMPServer("./testdata/cmp-preserve-file-mode") + time.Sleep(1 * time.Second) + os.Setenv("ARGOCD_BINARY_NAME", "argocd") + }). + Path("cmp-preserve-file-mode"). + When(). + CreateFromFile(func(app *Application) { + app.Spec.Source.Plugin = &ApplicationSourcePlugin{Name: "cmp-preserve-file-mode-v1.0"} + }). + Refresh(RefreshTypeNormal). + Then(). + And(func(app *Application) { + require.Len(t, app.Status.Resources, 1) + assert.Equal(t, "ConfigMap", app.Status.Resources[0].Kind) + }) +} diff --git a/test/e2e/testdata/cmp-preserve-file-mode/plugin.yaml b/test/e2e/testdata/cmp-preserve-file-mode/plugin.yaml new file mode 100644 index 0000000000000..c522649234e88 --- /dev/null +++ b/test/e2e/testdata/cmp-preserve-file-mode/plugin.yaml @@ -0,0 +1,11 @@ +apiVersion: argoproj.io/v1alpha1 +kind: ConfigManagementPlugin +metadata: + name: cmp-preserve-file-mode +spec: + version: v1.0 + generate: + command: [sh, -c , "./script.sh"] + discovery: + - glob: "**/*" + preserveFileMode: true diff --git a/test/e2e/testdata/cmp-preserve-file-mode/script.sh b/test/e2e/testdata/cmp-preserve-file-mode/script.sh new file mode 100755 index 0000000000000..1fbb8fde0c14d --- /dev/null +++ b/test/e2e/testdata/cmp-preserve-file-mode/script.sh @@ -0,0 +1,8 @@ +cat << EOF +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + foo: bar +EOF \ No newline at end of file diff --git a/util/cmp/stream.go b/util/cmp/stream.go index 81fc1d0179e74..2b4f798dce493 100644 --- a/util/cmp/stream.go +++ b/util/cmp/stream.go @@ -12,11 +12,12 @@ import ( "path/filepath" "strings" + log "github.com/sirupsen/logrus" + pluginclient "github.com/argoproj/argo-cd/v2/cmpserver/apiclient" "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/util/io/files" "github.com/argoproj/argo-cd/v2/util/tgzstream" - log "github.com/sirupsen/logrus" ) // StreamSender defines the contract to send App files over stream @@ -33,7 +34,7 @@ type StreamReceiver interface { // ReceiveRepoStream will receive the repository files and save them // in destDir. Will return the stream metadata if no error. Metadata // will be nil in case of errors. -func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir string) (*pluginclient.ManifestRequestMetadata, error) { +func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir string, preserveFileMode bool) (*pluginclient.ManifestRequestMetadata, error) { header, err := receiver.Recv() if err != nil { return nil, fmt.Errorf("error receiving stream header: %w", err) @@ -47,7 +48,7 @@ func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir str if err != nil { return nil, fmt.Errorf("error receiving tgz file: %w", err) } - err = files.Untgz(destDir, tgzFile, math.MaxInt64) + err = files.Untgz(destDir, tgzFile, math.MaxInt64, preserveFileMode) if err != nil { return nil, fmt.Errorf("error decompressing tgz file: %w", err) } diff --git a/util/cmp/stream_test.go b/util/cmp/stream_test.go index 7033890a18f3d..93a76cf10e673 100644 --- a/util/cmp/stream_test.go +++ b/util/cmp/stream_test.go @@ -62,7 +62,7 @@ func TestReceiveApplicationStream(t *testing.T) { go streamMock.sendFile(context.Background(), t, appDir, streamMock, []string{"env1", "env2"}, []string{"DUMMY.md", "dum*"}) // when - env, err := cmp.ReceiveRepoStream(context.Background(), streamMock, workdir) + env, err := cmp.ReceiveRepoStream(context.Background(), streamMock, workdir, false) // then require.NoError(t, err) diff --git a/util/io/files/tar.go b/util/io/files/tar.go index f665ded3ed4b0..91b743e0c4704 100644 --- a/util/io/files/tar.go +++ b/util/io/files/tar.go @@ -58,7 +58,7 @@ func Tgz(srcPath string, inclusions []string, exclusions []string, writers ...io // - a full path // - points to an empty directory or // - points to a non existing directory -func Untgz(dstPath string, r io.Reader, maxSize int64) error { +func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) error { if !filepath.IsAbs(dstPath) { return fmt.Errorf("dstPath points to a relative path: %s", dstPath) } @@ -90,9 +90,14 @@ func Untgz(dstPath string, r io.Reader, maxSize int64) error { return fmt.Errorf("illegal filepath in archive: %s", target) } + var mode os.FileMode = 0755 + if preserveFileMode { + mode = os.FileMode(header.Mode) + } + switch header.Typeflag { case tar.TypeDir: - err := os.MkdirAll(target, 0755) + err := os.MkdirAll(target, mode) if err != nil { return fmt.Errorf("error creating nested folders: %w", err) } @@ -118,7 +123,7 @@ func Untgz(dstPath string, r io.Reader, maxSize int64) error { return fmt.Errorf("error creating nested folders: %w", err) } - f, err := os.Create(target) + f, err := os.OpenFile(target, os.O_RDWR|os.O_CREATE|os.O_TRUNC, mode) if err != nil { return fmt.Errorf("error creating file %q: %w", target, err) } diff --git a/util/io/files/tar_test.go b/util/io/files/tar_test.go index fac9cf7d7d252..7c08246c72962 100644 --- a/util/io/files/tar_test.go +++ b/util/io/files/tar_test.go @@ -7,6 +7,7 @@ import ( "io" "math" "os" + "path" "path/filepath" "testing" @@ -168,7 +169,7 @@ func TestUntgz(t *testing.T) { destDir := filepath.Join(tmpDir, "untgz1") // when - err := files.Untgz(destDir, tgzFile, math.MaxInt64) + err := files.Untgz(destDir, tgzFile, math.MaxInt64, false) // then require.NoError(t, err) @@ -191,12 +192,32 @@ func TestUntgz(t *testing.T) { destDir := filepath.Join(tmpDir, "untgz2") // when - err := files.Untgz(destDir, tgzFile, math.MaxInt64) + err := files.Untgz(destDir, tgzFile, math.MaxInt64, false) // then assert.Error(t, err) assert.Contains(t, err.Error(), "illegal filepath in symlink") }) + + t.Run("preserves file mode", func(t *testing.T) { + // given + tmpDir := createTmpDir(t) + defer deleteTmpDir(t, tmpDir) + tgzFile := createTgz(t, filepath.Join(getTestDataDir(t), "executable"), tmpDir) + defer tgzFile.Close() + + destDir := filepath.Join(tmpDir, "untgz1") + + // when + err := files.Untgz(destDir, tgzFile, math.MaxInt64, false) + require.NoError(t, err) + + // then + + scriptFileInfo, err := os.Stat(path.Join(destDir, "script.sh")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0755), scriptFileInfo.Mode()) + }) } // read returns a map with the filename as key. In case diff --git a/util/io/files/testdata/executable/script.sh b/util/io/files/testdata/executable/script.sh new file mode 100755 index 0000000000000..e78c829c1730d --- /dev/null +++ b/util/io/files/testdata/executable/script.sh @@ -0,0 +1 @@ +echo hello world \ No newline at end of file diff --git a/util/manifeststream/stream.go b/util/manifeststream/stream.go index 6776344199848..729e564e7950c 100644 --- a/util/manifeststream/stream.go +++ b/util/manifeststream/stream.go @@ -9,11 +9,12 @@ import ( "io" "os" + log "github.com/sirupsen/logrus" + applicationpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" "github.com/argoproj/argo-cd/v2/util/io/files" "github.com/argoproj/argo-cd/v2/util/tgzstream" - log "github.com/sirupsen/logrus" ) // Defines the contract for the application sender, i.e. the CLI @@ -183,7 +184,7 @@ func ReceiveManifestFileStream(ctx context.Context, receiver RepoStreamReceiver, if err != nil { return nil, nil, fmt.Errorf("error receiving tgz file: %w", err) } - err = files.Untgz(destDir, tgzFile, maxExtractedSize) + err = files.Untgz(destDir, tgzFile, maxExtractedSize, false) if err != nil { return nil, nil, fmt.Errorf("error decompressing tgz file: %w", err) }