Parameterized cmps docs (#12)

* use printf instead of echo Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * docs Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * test for temp dir cleanup Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * handle empty params Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * handle empty values Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * consolidate types Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix tests Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * docs Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * docs Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * remove duplicate info Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * add warning about param announcements vs param values Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
argoproj · Apr 28, 2022 · 4f363f0 · 4f363f0
1 parent 624ca31
commit 4f363f0
Show file tree

Hide file tree

Showing 11 changed files with 368 additions and 998 deletions.
diff --git a/cmpserver/apiclient/plugin.pb.go b/cmpserver/apiclient/plugin.pb.go
diff --git a/cmpserver/plugin/config.go b/cmpserver/plugin/config.go
@@ -7,6 +7,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/argoproj/argo-cd/v2/common"
+	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	configUtil "github.com/argoproj/argo-cd/v2/util/config"
 )
 
@@ -50,21 +51,8 @@ type Find struct {
 
 // Parameters holds static and dynamic configurations
 type Parameters struct {
-	Static  []Static `yaml:"static"`
-	Dynamic Command  `yaml:"dynamic"`
-}
-
-// Static hold the static announcements for CMP's
-type Static struct {
-	Name           string            `yaml:"name,omitempty"`
-	Title          string            `yaml:"title,omitempty"`
-	Tooltip        string            `yaml:"tooltip,omitempty"`
-	Required       bool              `yaml:"required,omitempty"`
-	ItemType       string            `yaml:"itemType,omitempty"`
-	CollectionType string            `yaml:"collectionType,omitempty"`
-	String         string            `yaml:"string,omitempty"`
-	Array          []string          `yaml:"array,omitempty"`
-	Map            map[string]string `yaml:"map,omitempty"`
+	Static  []*apiclient.ParameterAnnouncement `yaml:"static"`
+	Dynamic Command                            `yaml:"dynamic"`
 }
 
 // Dynamic hold the dynamic announcements for CMP's

diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
 	"github.com/argoproj/argo-cd/v2/common"
+	repoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	"github.com/argoproj/argo-cd/v2/util/buffered_context"
 	"github.com/argoproj/argo-cd/v2/util/cmp"
 	"github.com/argoproj/argo-cd/v2/util/io/files"
@@ -143,20 +144,31 @@ func environ(envVars []*apiclient.EnvEntry) []string {
 	return environ
 }
 
+// getTempDirMustCleanup creates a temporary directory and returns a cleanup function. The cleanup function panics if
+// cleanup fails. Use this function when failing to clean up the temporary directory is a security risk.
+func getTempDirMustCleanup(baseDir string) (workDir string, cleanup func(), err error) {
+	workDir, err = files.CreateTempDir(baseDir)
+	if err != nil {
+		return "", nil, fmt.Errorf("error creating temp dir: %s", err)
+	}
+	cleanup = func() {
+		if err := os.RemoveAll(workDir); err != nil {
+			// we panic here as the workDir may contain sensitive information
+			panic(fmt.Sprintf("error removing plugin workdir: %s", err))
+		}
+	}
+	return workDir, cleanup, nil
+}
+
 // GenerateManifest runs generate command from plugin config file and returns generated manifest files
 func (s *Service) GenerateManifest(stream apiclient.ConfigManagementPluginService_GenerateManifestServer) error {
 	ctx, cancel := buffered_context.WithEarlierDeadline(stream.Context(), cmpTimeoutBuffer)
 	defer cancel()
-	workDir, err := files.CreateTempDir(common.GetCMPWorkDir())
+	workDir, cleanup, err := getTempDirMustCleanup(common.GetCMPWorkDir())
 	if err != nil {
-		return fmt.Errorf("error creating temp dir: %s", err)
+		return fmt.Errorf("error creating workdir for manifest generation: %s", err)
 	}
-	defer func() {
-		if err := os.RemoveAll(workDir); err != nil {
-			// we panic here as the workDir may contain sensitive information
-			panic(fmt.Sprintf("error removing generate manifest workdir: %s", err))
-		}
-	}()
+	defer cleanup()
 
 	metadata, err := cmp.ReceiveRepoStream(ctx, stream, workDir)
 	if err != nil {
@@ -222,16 +234,11 @@ func (s *Service) MatchRepository(stream apiclient.ConfigManagementPluginService
 	bufferedCtx, cancel := buffered_context.WithEarlierDeadline(stream.Context(), cmpTimeoutBuffer)
 	defer cancel()
 
-	workDir, err := files.CreateTempDir(common.GetCMPWorkDir())
+	workDir, cleanup, err := getTempDirMustCleanup(common.GetCMPWorkDir())
 	if err != nil {
-		return fmt.Errorf("error creating match repository workdir: %s", err)
+		return fmt.Errorf("error creating workdir for repository matching: %s", err)
 	}
-	defer func() {
-		if err := os.RemoveAll(workDir); err != nil {
-			// we panic here as the workDir may contain sensitive information
-			panic(fmt.Sprintf("error removing match repository workdir: %s", err))
-		}
-	}()
+	defer cleanup()
 
 	_, err = cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
 	if err != nil {
@@ -299,16 +306,11 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu
 	bufferedCtx, cancel := buffered_context.WithEarlierDeadline(stream.Context(), cmpTimeoutBuffer)
 	defer cancel()
 
-	workDir, err := files.CreateTempDir(common.GetCMPWorkDir())
+	workDir, cleanup, err := getTempDirMustCleanup(common.GetCMPWorkDir())
 	if err != nil {
-		return fmt.Errorf("error creating parameters announcement workdir: %s", err)
+		return fmt.Errorf("error creating workdir for generating parameter announcements: %s", err)
 	}
-	defer func() {
-		if err := os.RemoveAll(workDir); err != nil {
-			// we panic here as the workDir may contain sensitive information
-			panic(fmt.Sprintf("error removing parameters announcement repository workdir: %s", err))
-		}
-	}()
+	defer cleanup()
 
 	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
 	if err != nil {
@@ -331,29 +333,14 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu
 	return nil
 }
 
-func getParametersAnnouncement(ctx context.Context, appDir string, staticAnnouncements []Static, command Command) (*apiclient.ParametersAnnouncementResponse, error) {
-	var announcements []*apiclient.ParameterAnnouncement
-	for _, static := range staticAnnouncements {
-		announcements = append(announcements, &apiclient.ParameterAnnouncement{
-			Name:           static.Name,
-			Title:          static.Title,
-			Tooltip:        static.Tooltip,
-			Required:       static.Required,
-			ItemType:       static.ItemType,
-			CollectionType: static.CollectionType,
-			String_:        static.String,
-			Array:          static.Array,
-			Map:            static.Map,
-		})
-	}
-
+func getParametersAnnouncement(ctx context.Context, appDir string, announcements []*repoclient.ParameterAnnouncement, command Command) (*apiclient.ParametersAnnouncementResponse, error) {
 	if len(command.Command) > 0 {
 		stdout, err := runCommand(ctx, command, appDir, os.Environ())
 		if err != nil {
 			return nil, fmt.Errorf("error executing dynamic parameter output command: %s", err)
 		}
 
-		var dynamicParamAnnouncements []*apiclient.ParameterAnnouncement
+		var dynamicParamAnnouncements []*repoclient.ParameterAnnouncement
 		err = json.Unmarshal([]byte(stdout), &dynamicParamAnnouncements)
 		if err != nil {
 			return nil, fmt.Errorf("error unmarshaling dynamic parameter output into ParametersAnnouncementResponse: %s", err)

diff --git a/cmpserver/plugin/plugin.proto b/cmpserver/plugin/plugin.proto
@@ -3,6 +3,8 @@ option go_package = "github.com/argoproj/argo-cd/v2/cmpserver/apiclient";
 
 package plugin;
 
+import "github.com/argoproj/argo-cd/v2/reposerver/repository/repository.proto";
+
 // AppStreamRequest is the request object used to send the application's
 // files over a stream.
 message AppStreamRequest {
@@ -44,34 +46,10 @@ message RepositoryResponse {
     bool isSupported = 1;
 }
 
-message ParameterAnnouncement {
-    // name is the name identifying a parameter.
-    string name = 1;
-    // title is a human-readable text of the parameter name.
-    string title = 2;
-    // tooltip is a human-readable description of the parameter.
-    string tooltip = 3;
-    // required defines if this given parameter is mandatory.
-    bool required = 4;
-    // itemType determines the primitive data type represented by the parameter. Parameters are always encoded as
-    // strings, but this field lets them be interpreted as other primitive types.
-    string itemType = 5;
-    // collectionType is the type of value this parameter holds - either a single value (a string) or a collection
-    // (array or map). If collectionType is set, only the field with that type will be used. If collectionType is not
-    // set, `string` is the default. If collectionType is set to an invalid value, a validation error is thrown.
-    string collectionType = 6;
-    // string is the default value of the parameter if the parameter is a string.
-    string string = 7;
-    // array is the default value of the parameter if the parameter is an array.
-    repeated string array = 8;
-    // map is the default value of the parameter if the parameter is a map.
-    map<string, string> map = 9;
-}
-
 // ParametersAnnouncementResponse contains a list of announcements. This list represents all the parameters which a CMP
 // is able to accept.
 message ParametersAnnouncementResponse {
-    repeated ParameterAnnouncement parameterAnnouncements = 1;
+    repeated repository.ParameterAnnouncement parameterAnnouncements = 1;
 }
 
 message File {

diff --git a/cmpserver/plugin/plugin_test.go b/cmpserver/plugin/plugin_test.go
@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"context"
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -11,7 +12,7 @@ import (
 	"gopkg.in/yaml.v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+	repoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	"github.com/argoproj/argo-cd/v2/test"
 )
 
@@ -234,7 +235,7 @@ func Test_getParametersAnnouncement_empty_command(t *testing.T) {
 - name: static-a
 - name: static-b
 `
-	static := &[]Static{}
+	static := &[]*repoclient.ParameterAnnouncement{}
 	err := yaml.Unmarshal([]byte(staticYAML), static)
 	require.NoError(t, err)
 	command := Command{
@@ -243,29 +244,29 @@ func Test_getParametersAnnouncement_empty_command(t *testing.T) {
 	}
 	res, err := getParametersAnnouncement(context.Background(), "", *static, command)
 	require.NoError(t, err)
-	assert.Equal(t, []*apiclient.ParameterAnnouncement{{Name: "static-a"}, {Name: "static-b"}}, res.ParameterAnnouncements)
+	assert.Equal(t, []*repoclient.ParameterAnnouncement{{Name: "static-a"}, {Name: "static-b"}}, res.ParameterAnnouncements)
 }
 
 func Test_getParametersAnnouncement_no_command(t *testing.T) {
 	staticYAML := `
 - name: static-a
 - name: static-b
 `
-	static := &[]Static{}
+	static := &[]*repoclient.ParameterAnnouncement{}
 	err := yaml.Unmarshal([]byte(staticYAML), static)
 	require.NoError(t, err)
 	command := Command{}
 	res, err := getParametersAnnouncement(context.Background(), "", *static, command)
 	require.NoError(t, err)
-	assert.Equal(t, []*apiclient.ParameterAnnouncement{{Name: "static-a"}, {Name: "static-b"}}, res.ParameterAnnouncements)
+	assert.Equal(t, []*repoclient.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{}
+	static := &[]*repoclient.ParameterAnnouncement{}
 	err := yaml.Unmarshal([]byte(staticYAML), static)
 	require.NoError(t, err)
 	command := Command{
@@ -274,7 +275,7 @@ func Test_getParametersAnnouncement_static_and_dynamic(t *testing.T) {
 	}
 	res, err := getParametersAnnouncement(context.Background(), "", *static, command)
 	require.NoError(t, err)
-	expected := []*apiclient.ParameterAnnouncement{
+	expected := []*repoclient.ParameterAnnouncement{
 		{Name: "dynamic-a"},
 		{Name: "dynamic-b"},
 		{Name: "static-a"},
@@ -288,7 +289,7 @@ func Test_getParametersAnnouncement_invalid_json(t *testing.T) {
 		Command: []string{"echo"},
 		Args:    []string{`[`},
 	}
-	_, err := getParametersAnnouncement(context.Background(), "", []Static{}, command)
+	_, err := getParametersAnnouncement(context.Background(), "", []*repoclient.ParameterAnnouncement{}, command)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "unexpected end of JSON input")
 }
@@ -298,7 +299,26 @@ func Test_getParametersAnnouncement_bad_command(t *testing.T) {
 		Command: []string{"exit"},
 		Args:    []string{"1"},
 	}
-	_, err := getParametersAnnouncement(context.Background(), "", []Static{}, command)
+	_, err := getParametersAnnouncement(context.Background(), "", []*repoclient.ParameterAnnouncement{}, command)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "error executing dynamic parameter output command")
 }
+
+func Test_getTempDirMustCleanup(t *testing.T) {
+	tempDir := t.TempDir()
+	workDir, cleanup, err := getTempDirMustCleanup(tempDir)
+	require.NoError(t, err)
+	require.DirExists(t, workDir)
+
+	// Induce a cleanup error to verify panic behavior.
+	err = os.Chmod(tempDir, 0000)
+	require.NoError(t, err)
+	assert.Panics(t, func() {
+		cleanup()
+	}, "cleanup must panic to protect from directory traversal vulnerabilities")
+
+	err = os.Chmod(tempDir, 0700)
+	require.NoError(t, err)
+	cleanup()
+	assert.NoDirExists(t, workDir)
+}
diff --git a/docs/operator-manual/application.yaml b/docs/operator-manual/application.yaml
@@ -95,6 +95,7 @@ spec:
 
     # plugin specific config
     plugin:
+      # NOTE: this field is deprecated in v2.4 and must be removed to use sidecar-based plugins.
       # Only set the plugin name if the plugin is defined in argocd-cm.
       # If the plugin is defined as a sidecar, omit the name. The plugin will be automatically matched with the
       # Application according to the plugin's discovery rules.
@@ -103,6 +104,15 @@ spec:
       env:
         - name: FOO
           value: bar
+      # Plugin parameters are new in v2.4.
+      parameters:
+        - name: string-param
+          string: example-string
+        - name: array-param
+          array: [item1, item2]
+        - name: map-param
+          map:
+            param-name: param-value
 
   # Destination cluster and namespace to deploy the application
   destination: