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 
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 <pierre.cregut@orange.com>
  • Loading branch information
@pierrecregut
pierrecregut committed May 13, 2022
1 parent e28ca33 commit ce0ad30
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 27 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
18 changes: 10 additions & 8 deletions reposerver/repository/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1300,27 +1300,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, true)
// 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 @@ -1031,7 +1031,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 ce0ad30

Please sign in to comment.