Skip to content

Commit

Permalink
feat: Add plugin call variables to sidecar plugin discovery (#9273) (#…
Browse files Browse the repository at this point in the history 
…9319)

* 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 <[email protected]>

* 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 <[email protected]>
  • Loading branch information
@pierrecregut
pierrecregut authored May 31, 2022
1 parent e33f968 commit 61f48d5
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 30 deletions.
10 changes: 6 additions & 4 deletions cmpserver/plugin/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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")
Expand Down Expand Up @@ -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)
}
Expand Down
53 changes: 46 additions & 7 deletions cmpserver/plugin/plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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()
Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
30 changes: 19 additions & 11 deletions reposerver/repository/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion reposerver/repository/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/custom_tool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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\"}}}"`},
},
},
).
Expand Down Expand Up @@ -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\"}}}"`},
},
},
).
Expand Down
10 changes: 5 additions & 5 deletions util/app/discovery/discovery.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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)
}
Expand Down

0 comments on commit 61f48d5

Please sign in to comment.