Merge pull request #10 from crenshaw-dev/parameterized-cmps-rename

Parameterized cmps rename

cmpserver/plugin/plugin.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/argoproj/pkg/rand"
 
+	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/util/buffered_context"
 	"github.com/argoproj/argo-cd/v2/util/cmp"
@@ -22,8 +23,6 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
 	"github.com/mattn/go-zglob"
 	log "github.com/sirupsen/logrus"
-
-	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
 )
 
 // cmpTimeoutBuffer is the amount of time before the request deadline to timeout server-side work. It makes sure there's
@@ -311,12 +310,16 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu
 		}
 	}()
 
-	_, err = cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
+	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
 	if err != nil {
 		return fmt.Errorf("parameters announcement error receiving stream: %s", err)
 	}
+	appPath := filepath.Clean(filepath.Join(workDir, metadata.AppRelPath))
+	if !strings.HasPrefix(appPath, workDir) {
+		return fmt.Errorf("illegal appPath: out of workDir bound")
+	}
 
-	repoResponse, err := s.getParametersAnnouncement(bufferedCtx, workDir)
+	repoResponse, err := getParametersAnnouncement(bufferedCtx, appPath, s.initConstants.PluginConfig.Spec.Parameters.Static, s.initConstants.PluginConfig.Spec.Parameters.Dynamic)
 	if err != nil {
 		return fmt.Errorf("get parameters announcement error: %s", err)
 	}
@@ -328,11 +331,9 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu
 	return nil
 }
 
-func (s *Service) getParametersAnnouncement(ctx context.Context, workDir string) (*apiclient.ParametersAnnouncementResponse, error) {
-	config := s.initConstants.PluginConfig
-
+func getParametersAnnouncement(ctx context.Context, appDir string, staticAnnouncements []Static, command Command) (*apiclient.ParametersAnnouncementResponse, error) {
 	var staticParamAnnouncements []*apiclient.ParameterAnnouncement
-	for _, static := range config.Spec.Parameters.Static {
+	for _, static := range staticAnnouncements {
 		staticParamAnnouncements = append(staticParamAnnouncements, &apiclient.ParameterAnnouncement{
 			Name:           static.Name,
 			Title:          static.Title,
@@ -346,7 +347,7 @@ func (s *Service) getParametersAnnouncement(ctx context.Context, workDir string)
 		})
 	}
 
-	stdout, err := runCommand(ctx, config.Spec.Parameters.Dynamic, workDir, os.Environ())
+	stdout, err := runCommand(ctx, command, appDir, os.Environ())
 	if err != nil {
 		return nil, fmt.Errorf("error executing dynamic parameter output command: %s", err)
 	}

cmpserver/plugin/plugin_test.go

@@ -8,8 +8,10 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
 	"github.com/argoproj/argo-cd/v2/test"
 )
 
@@ -226,3 +228,53 @@ func TestRunCommandContextTimeout(t *testing.T) {
 	assert.Error(t, err) // The command should time out, causing an error.
 	assert.Less(t, after.Sub(before), 1*time.Second)
 }
+
+func Test_getParametersAnnouncement_empty_command(t *testing.T) {
+	staticYAML := `
+- name: static-a
+- name: static-b
+`
+	static := &[]Static{}
+	err := yaml.Unmarshal([]byte(staticYAML), static)
+	require.NoError(t, err)
+	command := Command{
+		Command: []string{"echo"},
+		Args:    []string{`[]`},
+	}
+	res, err := getParametersAnnouncement(context.Background(), "", *static, command)
+	require.NoError(t, err)
+	assert.Equal(t, []*apiclient.ParameterAnnouncement{{Name: "static-a"}, {Name: "static-b"}}, res.ParameterAnnouncements)
+}
+
+func Test_getParametersAnnouncement_static_and_dynamic(t *testing.T) {
+	staticYAML := `
+- name: static-a
+- name: static-b
+`
+	static := &[]Static{}
+	err := yaml.Unmarshal([]byte(staticYAML), static)
+	require.NoError(t, err)
+	command := Command{
+		Command: []string{"echo"},
+		Args:    []string{`[{"name": "dynamic-a"}, {"name": "dynamic-b"}]`},
+	}
+	res, err := getParametersAnnouncement(context.Background(), "", *static, command)
+	require.NoError(t, err)
+	expected := []*apiclient.ParameterAnnouncement{
+		{Name: "static-a"},
+		{Name: "static-b"},
+		{Name: "dynamic-a"},
+		{Name: "dynamic-b"},
+	}
+	assert.Equal(t, expected, res.ParameterAnnouncements)
+}
+
+func Test_getParametersAnnouncement_invalid_json(t *testing.T) {
+	command := Command{
+		Command: []string{"echo"},
+		Args:    []string{`[`},
+	}
+	_, err := getParametersAnnouncement(context.Background(), "", []Static{}, command)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "unexpected end of JSON input")
+}

pkg/apis/application/v1alpha1/types_test.go

@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	"encoding/json"
 	fmt "fmt"
 	"io/ioutil"
 	"os"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	argocdcommon "github.com/argoproj/argo-cd/v2/common"
+	"github.com/stretchr/testify/require"
 	"k8s.io/utils/pointer"
 
 	"github.com/argoproj/gitops-engine/pkg/sync/common"
@@ -2635,3 +2637,83 @@ func TestGetCAPath(t *testing.T) {
 		assert.Empty(t, path)
 	}
 }
+
+func TestApplicationSourcePluginParameters_Environ_string(t *testing.T) {
+	params := ApplicationSourcePluginParameters{
+		{
+			Name: "version",
+			String_: pointer.String("1.2.3"),
+		},
+	}
+	environ, err := params.Environ()
+	require.NoError(t, err)
+	assert.Len(t, environ, 2)
+	assert.Contains(t, environ, "PARAM_VERSION=1.2.3")
+	paramsJson, err := json.Marshal(params)
+	require.NoError(t, err)
+	assert.Contains(t, environ, fmt.Sprintf("ARGOCD_APP_PARAMETERS=%s", paramsJson))
+}
+
+func TestApplicationSourcePluginParameters_Environ_array(t *testing.T) {
+	params := ApplicationSourcePluginParameters{
+		{
+			Name: "dependencies",
+			Array: []string{"redis", "minio"},
+		},
+	}
+	environ, err := params.Environ()
+	require.NoError(t, err)
+	assert.Len(t, environ, 3)
+	assert.Contains(t, environ, "PARAM_DEPENDENCIES_0=redis")
+	assert.Contains(t, environ, "PARAM_DEPENDENCIES_1=minio")
+	paramsJson, err := json.Marshal(params)
+	require.NoError(t, err)
+	assert.Contains(t, environ, fmt.Sprintf("ARGOCD_APP_PARAMETERS=%s", paramsJson))
+}
+
+func TestApplicationSourcePluginParameters_Environ_map(t *testing.T) {
+	params := ApplicationSourcePluginParameters{
+		{
+			Name: "helm-parameters",
+			Map: map[string]string{
+				"image.repo": "quay.io/argoproj/argo-cd",
+				"image.tag": "v2.4.0",
+			},
+		},
+	}
+	environ, err := params.Environ()
+	require.NoError(t, err)
+	assert.Len(t, environ, 3)
+	assert.Contains(t, environ, "PARAM_HELM_PARAMETERS_IMAGE_REPO=quay.io/argoproj/argo-cd")
+	assert.Contains(t, environ, "PARAM_HELM_PARAMETERS_IMAGE_TAG=v2.4.0")
+	paramsJson, err := json.Marshal(params)
+	require.NoError(t, err)
+	assert.Contains(t, environ, fmt.Sprintf("ARGOCD_APP_PARAMETERS=%s", paramsJson))
+}
+
+func TestApplicationSourcePluginParameters_Environ_all(t *testing.T) {
+	// Technically there's no rule against specifying multiple types as values. It's up to the CMP how to handle them.
+	// Name collisions can happen for the convenience env vars. When in doubt, CMP authors should use the JSON env var.
+	params := ApplicationSourcePluginParameters{
+		{
+			Name: "some-name",
+			String_: pointer.String("1.2.3"),
+			Array: []string{"redis", "minio"},
+			Map: map[string]string{
+				"image.repo": "quay.io/argoproj/argo-cd",
+				"image.tag": "v2.4.0",
+			},
+		},
+	}
+	environ, err := params.Environ()
+	require.NoError(t, err)
+	assert.Len(t, environ, 6)
+	assert.Contains(t, environ, "PARAM_SOME_NAME=1.2.3")
+	assert.Contains(t, environ, "PARAM_SOME_NAME_0=redis")
+	assert.Contains(t, environ, "PARAM_SOME_NAME_1=minio")
+	assert.Contains(t, environ, "PARAM_SOME_NAME_IMAGE_REPO=quay.io/argoproj/argo-cd")
+	assert.Contains(t, environ, "PARAM_SOME_NAME_IMAGE_TAG=v2.4.0")
+	paramsJson, err := json.Marshal(params)
+	require.NoError(t, err)
+	assert.Contains(t, environ, fmt.Sprintf("ARGOCD_APP_PARAMETERS=%s", paramsJson))
+}