remove Dir field and make Fs as the core path field

This branch removes dir field and makes users only specify the full path in `Fs` field. Helper functions are added to make this easy. Changes are also made in charts generation code to remove the passage of `Charts.yaml` for dependency charts i.e partials, etc as it confuses the generation. Only the top level charts will pass the `Chart.yaml` and for every other dep chart they only pass the templated files for rendering. Signed-off-by: Tarun Pothulapati <[email protected]>
linkerd · Nov 20, 2020 · a004d1f · a004d1f
1 parent b1dbb1c
commit a004d1f
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 69 deletions.
diff --git a/cli/cmd/install-cni-plugin.go b/cli/cmd/install-cni-plugin.go
@@ -215,11 +215,10 @@ func renderCNIPlugin(w io.Writer, config *cniPluginOptions) error {
 
 	chart := &charts.Chart{
 		Name:      helmCNIDefaultChartName,
-		Dir:       helmCNIDefaultChartDir,
 		Namespace: controlPlaneNamespace,
 		RawValues: rawValues,
 		Files:     files,
-		Fs:        static.Templates,
+		Fs:        static.WithDefaultChart(helmCNIDefaultChartDir),
 	}
 	buf, err := chart.RenderCNI()
 	if err != nil {

diff --git a/cli/cmd/install.go b/cli/cmd/install.go
@@ -315,7 +315,6 @@ func render(w io.Writer, values *l5dcharts.Values, stage string) error {
 	for _, addOn := range addOns {
 		addOnCharts[addOn.Name()] = &charts.Chart{
 			Name:      addOn.Name(),
-			Dir:       addOnChartsPath + "/" + addOn.Name(),
 			Namespace: controlPlaneNamespace,
 			RawValues: append(addOn.Values(), rawValues...),
 			Files: []*chartutil.BufferedFile{
@@ -326,7 +325,7 @@ func render(w io.Writer, values *l5dcharts.Values, stage string) error {
 					Name: chartutil.ValuesfileName,
 				},
 			},
-			Fs: static.Templates,
+			Fs: static.WithDefaultChart(addOnChartsPath + "/" + addOn.Name()),
 		}
 	}
 
@@ -360,11 +359,10 @@ func render(w io.Writer, values *l5dcharts.Values, stage string) error {
 	// TODO refactor to use l5dcharts.LoadChart()
 	chart := &charts.Chart{
 		Name:      helmDefaultChartName,
-		Dir:       helmDefaultChartDir,
 		Namespace: controlPlaneNamespace,
 		RawValues: rawValues,
 		Files:     files,
-		Fs:        static.Templates,
+		Fs:        static.WithDefaultChart(helmDefaultChartDir),
 	}
 	buf, err := chart.Render()
 	if err != nil {

diff --git a/cli/cmd/multicluster.go b/cli/cmd/multicluster.go
@@ -277,11 +277,10 @@ func newAllowCommand() *cobra.Command {
 
 			chart := &charts.Chart{
 				Name:      helmMulticlusterDefaultChartName,
-				Dir:       helmMulticlusterDefaultChartName,
 				Namespace: controlPlaneNamespace,
 				RawValues: rawValues,
 				Files:     files,
-				Fs:        static.Templates,
+				Fs:        static.WithDefaultChart(helmMulticlusterDefaultChartName),
 			}
 			buf, err := chart.RenderNoPartials()
 			if err != nil {
@@ -369,11 +368,10 @@ func newMulticlusterInstallCommand() *cobra.Command {
 
 			chart := &charts.Chart{
 				Name:      helmMulticlusterDefaultChartName,
-				Dir:       helmMulticlusterDefaultChartName,
 				Namespace: controlPlaneNamespace,
 				RawValues: rawValues,
 				Files:     files,
-				Fs:        static.Templates,
+				Fs:        static.WithDefaultChart(helmMulticlusterDefaultChartName),
 			}
 			buf, err := chart.RenderNoPartials()
 			if err != nil {
@@ -474,11 +472,10 @@ func newMulticlusterUninstallCommand() *cobra.Command {
 
 			chart := &charts.Chart{
 				Name:      helmMulticlusterDefaultChartName,
-				Dir:       helmMulticlusterDefaultChartName,
 				Namespace: controlPlaneNamespace,
 				RawValues: rawValues,
 				Files:     files,
-				Fs:        static.Templates,
+				Fs:        static.WithDefaultChart(helmMulticlusterDefaultChartName),
 			}
 			buf, err := chart.RenderNoPartials()
 			if err != nil {
@@ -694,11 +691,10 @@ func newLinkCommand() *cobra.Command {
 
 			chart := &charts.Chart{
 				Name:      helmMulticlusterLinkDefaultChartName,
-				Dir:       helmMulticlusterLinkDefaultChartName,
 				Namespace: controlPlaneNamespace,
 				RawValues: rawValues,
 				Files:     files,
-				Fs:        static.Templates,
+				Fs:        static.WithDefaultChart(helmMulticlusterLinkDefaultChartName),
 			}
 			serviceMirrorOut, err := chart.RenderNoPartials()
 			if err != nil {

diff --git a/jaeger/cmd/install.go b/jaeger/cmd/install.go
@@ -4,9 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http"
 	"os"
-	"path"
 
 	jaeger "github.com/linkerd/linkerd2/jaeger/values"
 	"github.com/linkerd/linkerd2/pkg/charts"
@@ -100,11 +98,10 @@ func render(w io.Writer, values *jaeger.Values) error {
 
 	chart := &charts.Chart{
 		Name:      "jaeger",
-		Dir:       "jaeger",
 		Namespace: values.Namespace,
 		RawValues: rawValues,
 		Files:     files,
-		Fs:        http.Dir(path.Join(static.GetRepoRoot(), "jaeger/charts")),
+		Fs:        static.WithPath("jaeger/charts/jaeger"),
 	}
 	buf, err := chart.Render()
 	if err != nil {

diff --git a/jaeger/values/values.go b/jaeger/values/values.go
@@ -2,8 +2,6 @@ package values
 
 import (
 	"fmt"
-	"net/http"
-	"path"
 
 	"github.com/linkerd/linkerd2/pkg/charts"
 	l5dcharts "github.com/linkerd/linkerd2/pkg/charts/linkerd2"
@@ -48,7 +46,7 @@ func readDefaults(chartDir string) (*Values, error) {
 		Name: chartutil.ValuesfileName,
 	}
 
-	if err := charts.ReadFile(http.Dir(path.Join(static.GetRepoRoot(), "jaeger/charts")), chartDir, valuesFile); err != nil {
+	if err := charts.ReadFile(static.WithPath("jaeger/charts/jaeger"), "", valuesFile); err != nil {
 		return nil, err
 	}
 

diff --git a/pkg/charts/charts.go b/pkg/charts/charts.go
@@ -19,20 +19,19 @@ const versionPlaceholder = "linkerdVersionValue"
 // Chart holds the necessary info to render a Helm chart
 type Chart struct {
 	Name      string
-	Dir       string
 	Namespace string
 	RawValues []byte
 	Files     []*chartutil.BufferedFile
 	Fs        http.FileSystem
 }
 
 func (c *Chart) render(partialsFiles []*chartutil.BufferedFile) (bytes.Buffer, error) {
-	if err := FilesReader(c.Fs, c.Dir+"/", c.Files); err != nil {
+	if err := FilesReader(c.Fs, "", c.Files); err != nil {
 		return bytes.Buffer{}, err
 	}
 
 	// partials are present only in the static.Templates FileSystem
-	if err := FilesReader(static.Templates, "", partialsFiles); err != nil {
+	if err := FilesReader(static.WithDefaultChart("partials"), "", partialsFiles); err != nil {
 		return bytes.Buffer{}, err
 	}
 
@@ -62,7 +61,7 @@ func (c *Chart) render(partialsFiles []*chartutil.BufferedFile) (bytes.Buffer, e
 	// Merge templates and inject
 	var buf bytes.Buffer
 	for _, tmpl := range c.Files {
-		t := path.Join(renderOpts.ReleaseOptions.Name, tmpl.Name)
+		t := path.Join(c.Name, tmpl.Name)
 		if _, err := buf.WriteString(renderedTemplates[t]); err != nil {
 			return bytes.Buffer{}, err
 		}
@@ -76,32 +75,30 @@ func (c *Chart) Render() (bytes.Buffer, error) {
 
 	// Keep this slice synced with the contents of /charts/partials
 	l5dPartials := []*chartutil.BufferedFile{
-		{Name: "charts/partials/" + chartutil.ChartfileName},
-		{Name: "charts/partials/templates/_proxy.tpl"},
-		{Name: "charts/partials/templates/_proxy-init.tpl"},
-		{Name: "charts/partials/templates/_volumes.tpl"},
-		{Name: "charts/partials/templates/_resources.tpl"},
-		{Name: "charts/partials/templates/_metadata.tpl"},
-		{Name: "charts/partials/templates/_helpers.tpl"},
-		{Name: "charts/partials/templates/_debug.tpl"},
-		{Name: "charts/partials/templates/_capabilities.tpl"},
-		{Name: "charts/partials/templates/_trace.tpl"},
-		{Name: "charts/partials/templates/_nodeselector.tpl"},
-		{Name: "charts/partials/templates/_tolerations.tpl"},
-		{Name: "charts/partials/templates/_affinity.tpl"},
-		{Name: "charts/partials/templates/_addons.tpl"},
-		{Name: "charts/partials/templates/_validate.tpl"},
-		{Name: "charts/partials/templates/_pull-secrets.tpl"},
+		{Name: "templates/_proxy.tpl"},
+		{Name: "templates/_proxy-init.tpl"},
+		{Name: "templates/_volumes.tpl"},
+		{Name: "templates/_resources.tpl"},
+		{Name: "templates/_metadata.tpl"},
+		{Name: "templates/_helpers.tpl"},
+		{Name: "templates/_debug.tpl"},
+		{Name: "templates/_capabilities.tpl"},
+		{Name: "templates/_trace.tpl"},
+		{Name: "templates/_nodeselector.tpl"},
+		{Name: "templates/_tolerations.tpl"},
+		{Name: "templates/_affinity.tpl"},
+		{Name: "templates/_addons.tpl"},
+		{Name: "templates/_validate.tpl"},
+		{Name: "templates/_pull-secrets.tpl"},
 	}
 	return c.render(l5dPartials)
 }
 
 // RenderCNI returns a bytes buffer with the result of rendering a Helm chart
 func (c *Chart) RenderCNI() (bytes.Buffer, error) {
 	cniPartials := []*chartutil.BufferedFile{
-		{Name: "charts/partials/" + chartutil.ChartfileName},
-		{Name: "charts/partials/templates/_helpers.tpl"},
-		{Name: "charts/partials/templates/_pull-secrets.tpl"},
+		{Name: "templates/_helpers.tpl"},
+		{Name: "templates/_pull-secrets.tpl"},
 	}
 	return c.render(cniPartials)
 }
@@ -114,9 +111,6 @@ func (c *Chart) RenderNoPartials() (bytes.Buffer, error) {
 // ReadFile updates the buffered file with the data read from disk
 func ReadFile(fs http.FileSystem, dir string, f *chartutil.BufferedFile) error {
 	filename := dir + f.Name
-	if dir == "" {
-		filename = filename[7:]
-	}
 	file, err := fs.Open(filename)
 	if err != nil {
 		return err

diff --git a/pkg/charts/static/helpers.go b/pkg/charts/static/helpers.go
@@ -0,0 +1,34 @@
+package static
+
+import (
+	"net/http"
+	"path"
+	"path/filepath"
+	"runtime"
+)
+
+// WithPath creates a FileSystem with the given path from the repo root path
+func WithPath(subPath string) http.FileSystem {
+	return http.Dir(path.Join(getRepoRoot(), subPath))
+}
+
+// WithDefaultChart creates a FileSystem with the given path under the charts path
+func WithDefaultChart(subPath string) http.FileSystem {
+	return http.Dir(path.Join(getRepoRoot(), "charts", subPath))
+}
+
+// getRepoRoot returns the full path to the root of the repo. We assume this
+// function is only called from the `Templates` var above, and that this source
+// file lives at `pkg/charts/static`, relative to the root of the repo.
+func getRepoRoot() string {
+	// /foo/bar/linkerd2/pkg/charts/static/templates.go
+	_, filename, _, _ := runtime.Caller(0)
+
+	// /foo/bar/linkerd2/pkg/charts/static
+	dir := filepath.Dir(filename)
+
+	// filepath.Dir returns the parent directory, so that combined with joining
+	// ".." walks 3 levels up the tree:
+	// /foo/bar/linkerd2
+	return filepath.Dir(path.Join(dir, "../.."))
+}
diff --git a/pkg/charts/static/templates.go b/pkg/charts/static/templates.go
@@ -6,26 +6,8 @@ package static
 import (
 	"net/http"
 	"path"
-	"path/filepath"
-	"runtime"
 )
 
 // Templates that will be rendered by `linkerd install`. This is only used on
 // dev builds.
-var Templates http.FileSystem = http.Dir(path.Join(GetRepoRoot(), "charts"))
-
-// GetRepoRoot returns the full path to the root of the repo. We assume this
-// function is only called from the `Templates` var above, and that this source
-// file lives at `pkg/charts/static`, relative to the root of the repo.
-func GetRepoRoot() string {
-	// /foo/bar/linkerd2/pkg/charts/static/templates.go
-	_, filename, _, _ := runtime.Caller(0)
-
-	// /foo/bar/linkerd2/pkg/charts/static
-	dir := filepath.Dir(filename)
-
-	// filepath.Dir returns the parent directory, so that combined with joining
-	// ".." walks 3 levels up the tree:
-	// /foo/bar/linkerd2
-	return filepath.Dir(path.Join(dir, "../.."))
-}
+var Templates http.FileSystem = http.Dir(path.Join(getRepoRoot(), "charts"))
diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go
@@ -282,17 +282,15 @@ func (conf *ResourceConfig) GetPatch(injectProxy bool) ([]byte, error) {
 
 	files := []*chartutil.BufferedFile{
 		{Name: chartutil.ChartfileName},
-		{Name: "requirements.yaml"},
 		{Name: "templates/patch.json"},
 	}
 
 	chart := &charts.Chart{
 		Name:      "patch",
-		Dir:       "patch",
 		Namespace: conf.values.Global.Namespace,
 		RawValues: rawValues,
 		Files:     files,
-		Fs:        static.Templates,
+		Fs:        static.WithDefaultChart("patch"),
 	}
 	buf, err := chart.Render()
 	if err != nil {