From 61f48d56b1962d188ee8ff9c30f84583e087afd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20Cr=C3=A9gut?= Date: Tue, 31 May 2022 22:19:54 +0200 Subject: [PATCH] feat: Add plugin call variables to sidecar plugin discovery (#9273) (#9319) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: do not export repo-server environment to sidecar (#9393) getPluginEnvs is both used for local plugins and sidecar plugins. For the later do not include the environement variables of the repo-server in the supplied variables. Fixes: #9393 Signed-off-by: Pierre Crégut * feat: Add plugin call variables to sidecar plugin discovery (#9273) Gives access to variables declared in the call of the plugin in the application manifest to the discover command run on the CMP server. Variables are prefixed with ARGOCD_ENV_ to avoid security issues (plugin call overiding important variables). Fixes #9273 Signed-off-by: Pierre Crégut --- cmpserver/plugin/plugin.go | 10 +++-- cmpserver/plugin/plugin_test.go | 53 ++++++++++++++++++++---- reposerver/repository/repository.go | 30 +++++++++----- reposerver/repository/repository_test.go | 2 +- test/e2e/custom_tool_test.go | 4 +- util/app/discovery/discovery.go | 10 ++--- 6 files changed, 79 insertions(+), 30 deletions(-) diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go index 717fea0250cd6..899312f8a81f1 100644 --- a/cmpserver/plugin/plugin.go +++ b/cmpserver/plugin/plugin.go @@ -233,12 +233,12 @@ func (s *Service) MatchRepository(stream apiclient.ConfigManagementPluginService } }() - _, err = cmp.ReceiveRepoStream(bufferedCtx, stream, workDir) + metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir) if err != nil { return fmt.Errorf("match repository error receiving stream: %s", err) } - isSupported, err := s.matchRepository(bufferedCtx, workDir) + isSupported, err := s.matchRepository(bufferedCtx, workDir, metadata.GetEnv()) if err != nil { return fmt.Errorf("match repository error: %s", err) } @@ -251,7 +251,7 @@ func (s *Service) MatchRepository(stream apiclient.ConfigManagementPluginService return nil } -func (s *Service) matchRepository(ctx context.Context, workdir string) (bool, error) { +func (s *Service) matchRepository(ctx context.Context, workdir string, envEntries []*apiclient.EnvEntry) (bool, error) { config := s.initConstants.PluginConfig if config.Spec.Discover.FileName != "" { log.Debugf("config.Spec.Discover.FileName is provided") @@ -284,7 +284,9 @@ func (s *Service) matchRepository(ctx context.Context, workdir string) (bool, er } log.Debugf("Going to try runCommand.") - find, err := runCommand(ctx, config.Spec.Discover.Find.Command, workdir, os.Environ()) + env := append(os.Environ(), environ(envEntries)...) + + find, err := runCommand(ctx, config.Spec.Discover.Find.Command, workdir, env) if err != nil { return false, fmt.Errorf("error running find command: %s", err) } diff --git a/cmpserver/plugin/plugin_test.go b/cmpserver/plugin/plugin_test.go index cb00510385fb5..ab117c1d5083c 100644 --- a/cmpserver/plugin/plugin_test.go +++ b/cmpserver/plugin/plugin_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/argoproj/argo-cd/v2/cmpserver/apiclient" "github.com/argoproj/argo-cd/v2/test" ) @@ -62,6 +63,7 @@ func TestMatchRepository(t *testing.T) { type fixture struct { service *Service path string + env []*apiclient.EnvEntry } setup := func(t *testing.T, opts ...pluginOpt) *fixture { t.Helper() @@ -71,6 +73,7 @@ func TestMatchRepository(t *testing.T) { return &fixture{ service: s, path: path, + env: []*apiclient.EnvEntry{{Name: "ENV_VAR", Value: "1"}}, } } t.Run("will match plugin by filename", func(t *testing.T) { @@ -81,7 +84,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -95,7 +98,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -111,7 +114,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -127,7 +130,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -145,7 +148,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -163,7 +166,43 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) + + // then + assert.NoError(t, err) + assert.False(t, match) + }) + t.Run("will match plugin because env var defined", func(t *testing.T) { + // given + d := Discover{ + Find: Find{ + Command: Command{ + Command: []string{"sh", "-c", "echo -n $ENV_VAR"}, + }, + }, + } + f := setup(t, withDiscover(d)) + + // when + match, err := f.service.matchRepository(context.Background(), f.path, f.env) + + // then + assert.NoError(t, err) + assert.True(t, match) + }) + t.Run("will not match plugin because no env var defined", func(t *testing.T) { + // given + d := Discover{ + Find: Find{ + Command: Command{ + Command: []string{"sh", "-c", "echo -n $ENV_NO_VAR"}, + }, + }, + } + f := setup(t, withDiscover(d)) + + // when + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.NoError(t, err) @@ -181,7 +220,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, err := f.service.matchRepository(context.Background(), f.path) + match, err := f.service.matchRepository(context.Background(), f.path, f.env) // then assert.Error(t, err) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 46521fd111810..a8b1d76db802c 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1271,7 +1271,7 @@ func runConfigManagementPlugin(appPath, repoRoot string, envVars *v1alpha1.Env, } } - env, err := getPluginEnvs(envVars, q, creds) + env, err := getPluginEnvs(envVars, q, creds, false) if err != nil { return nil, err } @@ -1289,8 +1289,14 @@ func runConfigManagementPlugin(appPath, repoRoot string, envVars *v1alpha1.Env, return kube.SplitYAML([]byte(out)) } -func getPluginEnvs(envVars *v1alpha1.Env, q *apiclient.ManifestRequest, creds git.Creds) ([]string, error) { - env := append(os.Environ(), envVars.Environ()...) +func getPluginEnvs(envVars *v1alpha1.Env, q *apiclient.ManifestRequest, creds git.Creds, remote bool) ([]string, error) { + env := envVars.Environ() + // Local plugins need also to have access to the local environment variables. + // Remote side car plugins will use the environment in the side car + // container. + if !remote { + env = append(os.Environ(), env...) + } if creds != nil { closer, environ, err := creds.Environ() if err != nil { @@ -1313,27 +1319,29 @@ func getPluginEnvs(envVars *v1alpha1.Env, q *apiclient.ManifestRequest, creds gi if q.ApplicationSource.Plugin != nil { pluginEnv := q.ApplicationSource.Plugin.Env - for i, j := range pluginEnv { - pluginEnv[i].Value = parsedEnv.Envsubst(j.Value) + for _, entry := range pluginEnv { + newValue := parsedEnv.Envsubst(entry.Value) + env = append(env, fmt.Sprintf("ARGOCD_ENV_%s=%s", entry.Name, newValue)) } - env = append(env, pluginEnv.Environ()...) } return env, nil } func runConfigManagementPluginSidecars(ctx context.Context, appPath, repoPath string, envVars *v1alpha1.Env, q *apiclient.ManifestRequest, creds git.Creds, tarDoneCh chan<- bool) ([]*unstructured.Unstructured, error) { - // detect config management plugin server (sidecar) - conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath) + // compute variables. + env, err := getPluginEnvs(envVars, q, creds, true) if err != nil { return nil, err } - defer io.Close(conn) - // generate manifests using commands provided in plugin config file in detected cmp-server sidecar - env, err := getPluginEnvs(envVars, q, creds) + // detect config management plugin server (sidecar) + conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, env) if err != nil { return nil, err } + defer io.Close(conn) + + // generate manifests using commands provided in plugin config file in detected cmp-server sidecar cmpManifests, err := generateManifestsCMP(ctx, appPath, repoPath, env, cmpClient, tarDoneCh) if err != nil { return nil, fmt.Errorf("error generating manifests in cmp: %s", err) diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index aeba9b8dd1df7..f326b031620ba 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -1103,7 +1103,7 @@ func TestRunCustomTool(t *testing.T) { Name: "test", Generate: argoappv1.Command{ Command: []string{"sh", "-c"}, - Args: []string{`echo "{\"kind\": \"FakeObject\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"GIT_ASKPASS\": \"$GIT_ASKPASS\", \"GIT_USERNAME\": \"$GIT_USERNAME\", \"GIT_PASSWORD\": \"$GIT_PASSWORD\"}, \"labels\": {\"revision\": \"$TEST_REVISION\"}}}"`}, + Args: []string{`echo "{\"kind\": \"FakeObject\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"GIT_ASKPASS\": \"$GIT_ASKPASS\", \"GIT_USERNAME\": \"$GIT_USERNAME\", \"GIT_PASSWORD\": \"$GIT_PASSWORD\"}, \"labels\": {\"revision\": \"$ARGOCD_ENV_TEST_REVISION\"}}}"`}, }, }}, Repo: &argoappv1.Repository{ diff --git a/test/e2e/custom_tool_test.go b/test/e2e/custom_tool_test.go index 26adcbc4eac1e..2afc469fc6ab6 100644 --- a/test/e2e/custom_tool_test.go +++ b/test/e2e/custom_tool_test.go @@ -102,7 +102,7 @@ func TestCustomToolWithEnv(t *testing.T) { Name: Name(), Generate: Command{ Command: []string{"sh", "-c"}, - Args: []string{`echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"`}, + Args: []string{`echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$ARGOCD_ENV_FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"`}, }, }, ). @@ -162,7 +162,7 @@ func TestCustomToolSyncAndDiffLocal(t *testing.T) { Name: Name(), Generate: Command{ Command: []string{"sh", "-c"}, - Args: []string{`echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"`}, + Args: []string{`echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$ARGOCD_ENV_FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"`}, }, }, ). diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go index 7f83d7eae1e4e..bda4f942ae68f 100644 --- a/util/app/discovery/discovery.go +++ b/util/app/discovery/discovery.go @@ -33,7 +33,7 @@ func Discover(ctx context.Context, repoPath string, enableGenerateManifests map[ apps := make(map[string]string) // Check if it is CMP - conn, _, err := DetectConfigManagementPlugin(ctx, repoPath) + conn, _, err := DetectConfigManagementPlugin(ctx, repoPath, []string{}) if err == nil { // Found CMP io.Close(conn) @@ -82,7 +82,7 @@ func AppType(ctx context.Context, path string, enableGenerateManifests map[strin // 3. check isSupported(path)? // 4.a if no then close connection // 4.b if yes then return conn for detected plugin -func DetectConfigManagementPlugin(ctx context.Context, repoPath string) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, error) { +func DetectConfigManagementPlugin(ctx context.Context, repoPath string, env []string) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, error) { var conn io.Closer var cmpClient pluginclient.ConfigManagementPluginServiceClient @@ -106,7 +106,7 @@ func DetectConfigManagementPlugin(ctx context.Context, repoPath string) (io.Clos continue } - isSupported, err := matchRepositoryCMP(ctx, repoPath, cmpClient) + isSupported, err := matchRepositoryCMP(ctx, repoPath, cmpClient, env) if err != nil { log.Errorf("repository %s is not the match because %v", repoPath, err) continue @@ -131,13 +131,13 @@ func DetectConfigManagementPlugin(ctx context.Context, repoPath string) (io.Clos // matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will // inspect the files and return true if the repo is supported for manifest generation. // Will return false otherwise. -func matchRepositoryCMP(ctx context.Context, repoPath string, client pluginclient.ConfigManagementPluginServiceClient) (bool, error) { +func matchRepositoryCMP(ctx context.Context, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env []string) (bool, error) { matchRepoStream, err := client.MatchRepository(ctx, grpc_retry.Disable()) if err != nil { return false, fmt.Errorf("error getting stream client: %s", err) } - err = cmp.SendRepoStream(ctx, repoPath, repoPath, matchRepoStream, []string{}) + err = cmp.SendRepoStream(ctx, repoPath, repoPath, matchRepoStream, env) if err != nil { return false, fmt.Errorf("error sending stream: %s", err) }