added --set flag to install-cni plugin (#10633)

This PR added support for --set flag to linkerd cni-plugin installation command.
Also made changes to test file for cni-plugin install.
Fixed a bug at pkg/chart/charts.go for resources template.
fixes #9917

* Allow supporting all flags and values

This leverages `chartutil.CoalesceValues` in order to merge the values provided through regular flags, and the ones provided via `--set` flags. As that function consumes maps, I introduced the `ToMap` method function on the cni `Values` struct (a copy of the same function from the core linkerd `Values` struct) to convert the struct backing the regular flags into a map.

And for the `RenderCNI` method to be able to deal with value maps instead of yaml, the `charts.Chart` struct now distinguishes between `Values` (a map) and `RawValues` (YAML).

This allowed removing the `parseYAMLValue` function and avoid having to deal with individual entries in `buildValues()`, and we no longer need the `valuesOverrides` field in the `cniPluginOptions` struct.

## Tests

```bash
# Testing regular flag
$ bin/go-run cli install-cni --use-wait-flag | grep use.wait.flag
        "use-wait-flag": true

# Testing using --set
$ bin/go-run cli install-cni --set useWaitFlag=true | grep use.wait.flag
        "use-wait-flag": true

# Testing using --set on a setting that has no regular flag
$ bin/go-run cli install-cni --set enablePSP=true | grep PodSecurityPolicy
kind: PodSecurityPolicy
```

---------

Signed-off-by: amit-62 <kramit6662@gmail.com>
Co-authored-by: Alejandro Pedraza <alejandro.pedraza@gmail.com>
Co-authored-by: Matei David <matei.david.35@gmail.com>

charts/linkerd2-cni/README.md

@@ -32,7 +32,7 @@ Kubernetes: `>=1.21.0-0`
 | image.name | string | `"cr.l5d.io/linkerd/cni-plugin"` | Docker image for the CNI plugin |
 | image.pullPolicy | string | `"IfNotPresent"` | Pull policy for the linkerd-cni container |
 | image.version | string | `"v1.1.0"` | Tag for the CNI container Docker image |
-| imagePullSecrets | string | `nil` |  |
+| imagePullSecrets | list | `[]` |  |
 | inboundProxyPort | int | `4143` | Inbound port for the proxy container |
 | logLevel | string | `"info"` | Log level for the CNI plugin |
 | outboundProxyPort | int | `4140` | Outbound port for the proxy container |

charts/linkerd2-cni/values.yaml

@@ -67,7 +67,7 @@ image:
 #
 # The pull secrets are applied to the respective service accounts
 # which will further orchestrate the deployments.
-imagePullSecrets:
+imagePullSecrets: []
 
 # -- Add additional initContainers to the daemonset
 extraInitContainers: []

cli/cmd/install-cni-plugin.go

@@ -14,9 +14,10 @@ import (
 	"github.com/linkerd/linkerd2/pkg/version"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
-	"sigs.k8s.io/yaml"
+	"helm.sh/helm/v3/pkg/cli/values"
 )
 
 const (
@@ -96,6 +97,7 @@ func newCmdInstallCNIPlugin() *cobra.Command {
 		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
+	var valOpts values.Options
 
 	cmd := &cobra.Command{
 		Use:   "install-cni [flags]",
@@ -106,9 +108,11 @@ This command installs a DaemonSet into the Linkerd control plane. The DaemonSet
 copies the necessary linkerd-cni plugin binaries and configs onto the host. It
 assumes that the 'linkerd install' command will be executed with the
 '--linkerd-cni-enabled' flag. This command needs to be executed before the
-'linkerd install --linkerd-cni-enabled' command.`,
+'linkerd install --linkerd-cni-enabled' command. 
+
+The installation can be configured by using the --set, --values, --set-string and --set-file flags. A full list of configurable values can be found at https://artifacthub.io/packages/helm/linkerd2/linkerd2-cni#values`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return renderCNIPlugin(os.Stdout, options)
+			return renderCNIPlugin(os.Stdout, valOpts, options)
 		},
 	}
 
@@ -135,6 +139,8 @@ assumes that the 'linkerd install' command will be executed with the
 		options.useWaitFlag,
 		"Configures the CNI plugin to use the \"-w\" flag for the iptables command. (default false)")
 
+	flags.AddValueOptionsFlags(cmd.Flags(), &valOpts)
+
 	return cmd
 }
 
@@ -204,19 +210,32 @@ func (options *cniPluginOptions) buildValues() (*cnicharts.Values, error) {
 	return installValues, nil
 }
 
-func renderCNIPlugin(w io.Writer, config *cniPluginOptions) error {
+func renderCNIPlugin(w io.Writer, valOpts values.Options, config *cniPluginOptions) error {
 
 	if err := config.validate(); err != nil {
 		return err
 	}
 
+	valuesOverrides, err := valOpts.MergeValues(nil)
+	if err != nil {
+		return err
+	}
+
 	values, err := config.buildValues()
 	if err != nil {
 		return err
 	}
 
-	// Render raw values and create chart config
-	rawValues, err := yaml.Marshal(values)
+	mapValues, err := values.ToMap()
+	if err != nil {
+		return err
+	}
+
+	valuesWrapper := &chart.Chart{
+		Metadata: &chart.Metadata{Name: ""},
+		Values:   mapValues,
+	}
+	mergedValues, err := chartutil.CoalesceValues(valuesWrapper, valuesOverrides)
 	if err != nil {
 		return err
 	}
@@ -226,15 +245,16 @@ func renderCNIPlugin(w io.Writer, config *cniPluginOptions) error {
 		{Name: "templates/cni-plugin.yaml"},
 	}
 
-	chart := &charts.Chart{
+	ch := &charts.Chart{
 		Name:      helmCNIDefaultChartName,
 		Dir:       helmCNIDefaultChartDir,
 		Namespace: defaultCNINamespace,
-		RawValues: rawValues,
+		Values:    mergedValues,
 		Files:     files,
 		Fs:        static.Templates,
 	}
-	buf, err := chart.RenderCNI()
+
+	buf, err := ch.RenderCNI()
 	if err != nil {
 		return err
 	}

cli/cmd/install-cni-plugin_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"fmt"
 	"testing"
+
+	"helm.sh/helm/v3/pkg/cli/values"
 )
 
 func TestRenderCNIPlugin(t *testing.T) {
@@ -76,22 +78,27 @@ func TestRenderCNIPlugin(t *testing.T) {
 	defaultOptionsWithSkipPorts.ignoreInboundPorts = append(defaultOptionsWithSkipPorts.ignoreInboundPorts, []string{"80", "8080"}...)
 	defaultOptionsWithSkipPorts.ignoreOutboundPorts = append(defaultOptionsWithSkipPorts.ignoreOutboundPorts, []string{"443", "1000"}...)
 
+	valOpts := values.Options{
+		Values: []string{"resources.cpu.limit=1m"},
+	}
+
 	testCases := []struct {
 		*cniPluginOptions
+		valOpts        values.Options
 		goldenFileName string
 	}{
-		{defaultOptions, "install-cni-plugin_default.golden"},
-		{fullyConfiguredOptions, "install-cni-plugin_fully_configured.golden"},
-		{fullyConfiguredOptionsEqualDsts, "install-cni-plugin_fully_configured_equal_dsts.golden"},
-		{fullyConfiguredOptionsNoNamespace, "install-cni-plugin_fully_configured_no_namespace.golden"},
-		{defaultOptionsWithSkipPorts, "install-cni-plugin_skip_ports.golden"},
+		{defaultOptions, valOpts, "install-cni-plugin_default.golden"},
+		{fullyConfiguredOptions, values.Options{}, "install-cni-plugin_fully_configured.golden"},
+		{fullyConfiguredOptionsEqualDsts, values.Options{}, "install-cni-plugin_fully_configured_equal_dsts.golden"},
+		{fullyConfiguredOptionsNoNamespace, values.Options{}, "install-cni-plugin_fully_configured_no_namespace.golden"},
+		{defaultOptionsWithSkipPorts, values.Options{}, "install-cni-plugin_skip_ports.golden"},
 	}
 
 	for i, tc := range testCases {
 		tc := tc // pin
 		t.Run(fmt.Sprintf("%d: %s", i, tc.goldenFileName), func(t *testing.T) {
 			var buf bytes.Buffer
-			err := renderCNIPlugin(&buf, tc.cniPluginOptions)
+			err := renderCNIPlugin(&buf, tc.valOpts, tc.cniPluginOptions)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}

cli/cmd/install_cni_helm_test.go

@@ -110,6 +110,7 @@ func chartCniPlugin(t *testing.T) *chart.Chart {
 		"templates/_helpers.tpl",
 		"templates/_metadata.tpl",
 		"templates/_tolerations.tpl",
+		"templates/_resources.tpl",
 	})
 
 	cniChart := &chart.Chart{

cli/cmd/testdata/install-cni-plugin_default.golden

@@ -157,7 +157,10 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
+          limits:
+            cpu: "1m"
       volumes:
       - name: cni-bin-dir
         hostPath:

cli/cmd/testdata/install-cni-plugin_fully_configured.golden

@@ -158,7 +158,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-bin-dir
         hostPath:

cli/cmd/testdata/install-cni-plugin_fully_configured_equal_dsts.golden

@@ -156,7 +156,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-net-dir
         hostPath:

cli/cmd/testdata/install-cni-plugin_fully_configured_no_namespace.golden

@@ -158,7 +158,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-bin-dir
         hostPath:

cli/cmd/testdata/install-cni-plugin_skip_ports.golden

@@ -158,7 +158,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-bin-dir
         hostPath:

cli/cmd/testdata/install_cni_helm_default_output.golden

@@ -150,7 +150,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-bin-dir
         hostPath:

cli/cmd/testdata/install_cni_helm_override_output.golden

@@ -151,7 +151,8 @@ spec:
           name: linkerd-tmp-dir
         securityContext:
           readOnlyRootFilesystem: true
-          privileged: 
+          privileged: false
+        resources:
       volumes:
       - name: cni-bin-dir
         hostPath:

pkg/charts/charts.go

@@ -2,6 +2,7 @@ package charts
 
 import (
 	"bytes"
+	"errors"
 	"net/http"
 	"path"
 	"strings"
@@ -45,9 +46,17 @@ type Chart struct {
 	Name      string
 	Dir       string
 	Namespace string
+
+	// RawValues are yaml-formatted values entries. Either this or Values
+	// should be set, but not both
 	RawValues []byte
-	Files     []*loader.BufferedFile
-	Fs        http.FileSystem
+
+	// Values are the config key-value entries. Either this or RawValues should
+	// be set, but not both
+	Values map[string]any
+
+	Files []*loader.BufferedFile
+	Fs    http.FileSystem
 }
 
 func (c *Chart) render(partialsFiles []*loader.BufferedFile) (bytes.Buffer, error) {
@@ -73,13 +82,17 @@ func (c *Chart) render(partialsFiles []*loader.BufferedFile) (bytes.Buffer, erro
 		Namespace: c.Namespace,
 	}
 
-	var rawMapValues map[string]interface{}
-	err = yaml.Unmarshal(c.RawValues, &rawMapValues)
-	if err != nil {
-		return bytes.Buffer{}, err
+	if len(c.RawValues) > 0 {
+		if c.Values != nil {
+			return bytes.Buffer{}, errors.New("either RawValues or Values should be set, but not both")
+		}
+		err = yaml.Unmarshal(c.RawValues, &c.Values)
+		if err != nil {
+			return bytes.Buffer{}, err
+		}
 	}
 
-	valuesToRender, err := chartutil.ToRenderValues(chart, rawMapValues, releaseOptions, nil)
+	valuesToRender, err := chartutil.ToRenderValues(chart, c.Values, releaseOptions, nil)
 	if err != nil {
 		return bytes.Buffer{}, err
 	}
@@ -124,6 +137,7 @@ func (c *Chart) RenderCNI() (bytes.Buffer, error) {
 		{Name: "charts/partials/templates/_metadata.tpl"},
 		{Name: "charts/partials/templates/_pull-secrets.tpl"},
 		{Name: "charts/partials/templates/_tolerations.tpl"},
+		{Name: "charts/partials/templates/_resources.tpl"},
 	}
 	return c.render(cniPartials)
 }

pkg/charts/cni/values.go

@@ -22,24 +22,44 @@ type Image struct {
 	PullPolicy interface{} `json:"pullPolicy"`
 }
 
+// Constraints wraps the Limit and Request settings for computational resources
+type Constraints struct {
+	Limit   string `json:"limit"`
+	Request string `json:"request"`
+}
+
+// Resources represents the computational resources setup for a given container
+type Resources struct {
+	CPU              Constraints `json:"cpu"`
+	Memory           Constraints `json:"memory"`
+	EphemeralStorage Constraints `json:"ephemeral-storage"`
+}
+
 // Values contains the top-level elements in the cni Helm chart
 type Values struct {
-	InboundProxyPort    uint          `json:"inboundProxyPort"`
-	OutboundProxyPort   uint          `json:"outboundProxyPort"`
-	IgnoreInboundPorts  string        `json:"ignoreInboundPorts"`
-	IgnoreOutboundPorts string        `json:"ignoreOutboundPorts"`
-	CliVersion          string        `json:"cliVersion"`
-	Image               Image         `json:"image"`
-	LogLevel            string        `json:"logLevel"`
-	PortsToRedirect     string        `json:"portsToRedirect"`
-	ProxyUID            int64         `json:"proxyUID"`
-	DestCNINetDir       string        `json:"destCNINetDir"`
-	DestCNIBinDir       string        `json:"destCNIBinDir"`
-	UseWaitFlag         bool          `json:"useWaitFlag"`
-	PriorityClassName   string        `json:"priorityClassName"`
-	ProxyAdminPort      string        `json:"proxyAdminPort"`
-	ProxyControlPort    string        `json:"proxyControlPort"`
-	Tolerations         []interface{} `json:"tolerations"`
+	InboundProxyPort    uint                `json:"inboundProxyPort"`
+	OutboundProxyPort   uint                `json:"outboundProxyPort"`
+	IgnoreInboundPorts  string              `json:"ignoreInboundPorts"`
+	IgnoreOutboundPorts string              `json:"ignoreOutboundPorts"`
+	CliVersion          string              `json:"cliVersion"`
+	Image               Image               `json:"image"`
+	LogLevel            string              `json:"logLevel"`
+	PortsToRedirect     string              `json:"portsToRedirect"`
+	ProxyUID            int64               `json:"proxyUID"`
+	DestCNINetDir       string              `json:"destCNINetDir"`
+	DestCNIBinDir       string              `json:"destCNIBinDir"`
+	UseWaitFlag         bool                `json:"useWaitFlag"`
+	PriorityClassName   string              `json:"priorityClassName"`
+	ProxyAdminPort      string              `json:"proxyAdminPort"`
+	ProxyControlPort    string              `json:"proxyControlPort"`
+	Tolerations         []interface{}       `json:"tolerations"`
+	PodLabels           map[string]string   `json:"podLabels"`
+	CommonLabels        map[string]string   `json:"commonLabels"`
+	ImagePullSecrets    []map[string]string `json:"imagePullSecrets"`
+	ExtraInitContainers []interface{}       `json:"extraInitContainers"`
+	EnablePSP           bool                `json:"enablePSP"`
+	Privileged          bool                `json:"privileged"`
+	Resources           Resources           `json:"resources"`
 }
 
 // NewValues returns a new instance of the Values type.
@@ -54,6 +74,22 @@ func NewValues() (*Values, error) {
 	return v, nil
 }
 
+// ToMap converts Values into a map[string]interface{}
+func (v *Values) ToMap() (map[string]interface{}, error) {
+	var valuesMap map[string]interface{}
+	rawValues, err := yaml.Marshal(v)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal the values struct: %w", err)
+	}
+
+	err = yaml.Unmarshal(rawValues, &valuesMap)
+	if err != nil {
+		return nil, fmt.Errorf("failed to Unmarshal Values into a map: %w", err)
+	}
+
+	return valuesMap, nil
+}
+
 // readDefaults reads all the default variables from the values.yaml file.
 // chartDir is the root directory of the Helm chart where values.yaml is.
 func readDefaults(chartDir string) (*Values, error) {