Drop troubleshoot.sh/v1beta3 docs from release files (#6009)

Kots has no support for troubleshoot.sh/v1beta3 specs, so an EC v3
release packaging both a v1beta2 and v1beta3 doc (dual preflight)
would silently have the v1beta3 doc misinterpreted as v1beta2 and
executed. Detect v1beta3 docs by matching the apiVersion header line
directly rather than requiring the whole doc to parse as valid yaml,
since a doc can embed unrendered template syntax (e.g. Helm's
"{{ .Values.x }}") that breaks yaml parsing further down while the
header stays literal text. Packaged Helm chart archives (.tgz/.tar.gz)
are left untouched, since they're binary content, not yaml.

Signed-off-by: Evans Mungai <evans@replicated.com>

Author: banjoh

pkg/kotsutil/kots.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -912,6 +913,39 @@ func IsApiVersionKind(content []byte, apiVersion, kind string) bool {
 	return false
 }
 
+var v1Beta3APIVersionRe = regexp.MustCompile(`(?m)^apiVersion:[ \t]*['"]?troubleshoot\.sh/v1beta3['"]?[ \t]*(#.*)?$`)
+
+// IsV1Beta3Doc reports whether doc is an unsupported troubleshoot.sh/v1beta3
+// doc. Matches the apiVersion header line directly, so it still works on docs
+// with unrendered template syntax (e.g. Helm's "{{ .Values.x }}") further down.
+func IsV1Beta3Doc(doc []byte) bool {
+	return v1Beta3APIVersionRe.Match(doc)
+}
+
+// FilterOutV1Beta3Docs removes v1beta3 documents from content (a possibly
+// multi-doc yaml file). Returns nil if nothing remains.
+func FilterOutV1Beta3Docs(content []byte) []byte {
+	docs := util.ConvertToSingleDocs(content)
+
+	kept := make([][]byte, 0, len(docs))
+	changed := false
+	for _, doc := range docs {
+		if IsV1Beta3Doc(doc) {
+			changed = true
+			continue
+		}
+		kept = append(kept, doc)
+	}
+
+	if !changed {
+		return content
+	}
+	if len(kept) == 0 {
+		return nil
+	}
+	return bytes.Join(kept, []byte("\n---\n"))
+}
+
 func LoadInstallationFromPath(installationFilePath string) (*kotsv1beta1.Installation, error) {
 	installationData, err := os.ReadFile(installationFilePath)
 	if err != nil {

pkg/kotsutil/kots_test.go

@@ -730,6 +730,71 @@ var _ = Describe("Kots", func() {
 	})
 })
 
+func TestIsV1Beta3Doc(t *testing.T) {
+	tests := []struct {
+		name string
+		doc  []byte
+		want bool
+	}{
+		{
+			name: "v1beta2 preflight",
+			doc:  []byte("apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\n"),
+			want: false,
+		},
+		{
+			name: "v1beta3 preflight",
+			doc:  []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n"),
+			want: true,
+		},
+		{
+			name: "v1beta3 host preflight",
+			doc:  []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: HostPreflight\n"),
+			want: true,
+		},
+		{
+			name: "v1beta3 doc, any other kind",
+			doc:  []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: SupportBundle\n"),
+			want: true,
+		},
+		{
+			name: "v1beta3 with quoted apiVersion",
+			doc:  []byte("apiVersion: \"troubleshoot.sh/v1beta3\"\nkind: 'Preflight'\n"),
+			want: true,
+		},
+		{
+			name: "v1beta3 doc whose spec breaks yaml parsing via unrendered helm template syntax",
+			doc: []byte(
+				"apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\nmetadata:\n  name: vendorflow\nspec:\n" +
+					"  collectors:\n    {{- if eq .Chart.Name \"vendorflow\" }}\n    - clusterInfo: {}\n    {{- end }}\n",
+			),
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, kotsutil.IsV1Beta3Doc(tt.doc))
+		})
+	}
+}
+
+func TestFilterOutV1Beta3Docs(t *testing.T) {
+	content := []byte(
+		"apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\nmetadata:\n  name: v2\n" +
+			"\n---\n" +
+			"apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\nmetadata:\n  name: v3\n" +
+			"\n---\n" +
+			"apiVersion: troubleshoot.sh/v1beta3\nkind: HostPreflight\nmetadata:\n  name: hp3\n",
+	)
+
+	filtered := kotsutil.FilterOutV1Beta3Docs(content)
+	assert.Equal(t, "apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\nmetadata:\n  name: v2\n", string(filtered))
+
+	assert.Nil(t, kotsutil.FilterOutV1Beta3Docs([]byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n")))
+
+	unchanged := []byte("apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\n")
+	assert.Equal(t, unchanged, kotsutil.FilterOutV1Beta3Docs(unchanged))
+}
+
 func TestIsKotsKind(t *testing.T) {
 	type args struct {
 		apiVersion string

pkg/upstream/replicated.go

@@ -350,7 +350,11 @@ func readReplicatedAppFromLocalPath(localPath string, localCursor replicatedapp.
 			appPath := strings.TrimPrefix(path, localPath)
 			appPath = strings.TrimLeft(appPath, string(os.PathSeparator))
 
-			release.Manifests[appPath] = contents
+			filtered, ok := filterV1Beta3(appPath, contents)
+			if !ok {
+				return nil
+			}
+			release.Manifests[appPath] = filtered
 
 			return nil
 		})
@@ -465,7 +469,11 @@ func downloadReplicatedApp(replicatedUpstream *replicatedapp.ReplicatedUpstream,
 				return nil, errors.Wrap(err, "failed to read file from tar")
 			}
 
-			release.Manifests[name] = content
+			filtered, ok := filterV1Beta3(name, content)
+			if !ok {
+				continue
+			}
+			release.Manifests[name] = filtered
 		}
 
 		i++
@@ -856,6 +864,25 @@ func findAppInRelease(release *Release) *kotsv1beta1.Application {
 	return app
 }
 
+// filterV1Beta3 drops unsupported troubleshoot.sh/v1beta3 docs from content.
+// ok is false when nothing is left to keep. Helm chart archives (.tgz/.tar.gz)
+// are skipped, since they're binary, not yaml.
+func filterV1Beta3(filename string, content []byte) (filtered []byte, ok bool) {
+	if strings.HasSuffix(filename, ".tgz") || strings.HasSuffix(filename, ".tar.gz") {
+		return content, true
+	}
+
+	filtered = kotsutil.FilterOutV1Beta3Docs(content)
+	if filtered == nil {
+		logger.Infof("skipping v1beta3 file from release: %s", filename)
+		return nil, false
+	}
+	if !bytes.Equal(filtered, content) {
+		logger.Infof("removed v1beta3 doc(s) from release file: %s", filename)
+	}
+	return filtered, true
+}
+
 func releaseToFiles(release *Release) ([]types.UpstreamFile, error) {
 	upstreamFiles := []types.UpstreamFile{}
 

pkg/upstream/replicated_test.go

@@ -105,6 +105,66 @@ func Test_releaseToFiles(t *testing.T) {
 	}
 }
 
+func Test_filterV1Beta3(t *testing.T) {
+	tests := []struct {
+		name        string
+		filename    string
+		content     []byte
+		wantOk      bool
+		wantContent []byte
+	}{
+		{
+			name:        "not a v1beta3 doc",
+			filename:    "manifests/deployment.yaml",
+			content:     []byte("a: b"),
+			wantOk:      true,
+			wantContent: []byte("a: b"),
+		},
+		{
+			name:        "v1beta3 doc, any kind, is dropped entirely",
+			filename:    "manifests/preflight.yaml",
+			content:     []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n"),
+			wantOk:      false,
+			wantContent: nil,
+		},
+		{
+			name:     "only the v1beta3 doc is stripped from a multi-doc file mixing v1beta2 and v1beta3",
+			filename: "manifests/preflight.yaml",
+			content: []byte(
+				"apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\nmetadata:\n  name: v2\n" +
+					"\n---\n" +
+					"apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\nmetadata:\n  name: v3\n",
+			),
+			wantOk:      true,
+			wantContent: []byte("apiVersion: troubleshoot.sh/v1beta2\nkind: Preflight\nmetadata:\n  name: v2\n"),
+		},
+		{
+			name:     "v1beta3 doc is dropped even when a template breaks yaml parsing of the rest of the doc",
+			filename: "manifests/preflight.yaml",
+			content: []byte(
+				"apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\nmetadata:\n  name: vendorflow\nspec:\n" +
+					"  collectors:\n    {{- if eq .Chart.Name \"vendorflow\" }}\n    - clusterInfo: {}\n    {{- end }}\n",
+			),
+			wantOk:      false,
+			wantContent: nil,
+		},
+		{
+			name:        "packaged helm chart archives are left untouched",
+			filename:    "manifests/vendorflow-0.1.0.tgz",
+			content:     []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n"), // not real tgz content, but proves content is never inspected
+			wantOk:      true,
+			wantContent: []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			content, ok := filterV1Beta3(tt.filename, tt.content)
+			require.Equal(t, tt.wantOk, ok)
+			require.Equal(t, tt.wantContent, content)
+		})
+	}
+}
+
 func Test_createConfigValues(t *testing.T) {
 	applicationName := "Test App"
 	appInfo := &template.ApplicationInfo{Slug: "app-slug"}
@@ -735,6 +795,42 @@ spec:
 		}
 	})
 
+	t.Run("drops v1beta3 docs from the release's manifests", func(t *testing.T) {
+		v1beta3Files := map[string][]byte{
+			"manifests/app.yaml":       testFiles["manifests/app.yaml"],
+			"manifests/preflight.yaml": []byte("apiVersion: troubleshoot.sh/v1beta3\nkind: Preflight\n"),
+		}
+
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			var buf bytes.Buffer
+			gzipWriter := gzip.NewWriter(&buf)
+			tarWriter := tar.NewWriter(gzipWriter)
+
+			for name, content := range v1beta3Files {
+				header := &tar.Header{Name: name, Mode: 0600, Size: int64(len(content))}
+				require.NoError(t, tarWriter.WriteHeader(header))
+				_, err := tarWriter.Write(content)
+				require.NoError(t, err)
+			}
+
+			require.NoError(t, tarWriter.Close())
+			require.NoError(t, gzipWriter.Close())
+
+			w.WriteHeader(http.StatusOK)
+			w.Write(buf.Bytes())
+		}))
+		defer server.Close()
+
+		license.V1.Spec.Endpoint = server.URL
+
+		release, err := downloadReplicatedApp(upstr, license, cursor, nil, "")
+		require.NoError(t, err)
+
+		_, ok := release.Manifests["manifests/preflight.yaml"]
+		require.False(t, ok, "expected the v1beta3 preflight to be dropped from the release's manifests")
+		require.Equal(t, testFiles["manifests/app.yaml"], release.Manifests["manifests/app.yaml"])
+	})
+
 	// Test error cases
 	t.Run("HTTP error", func(t *testing.T) {
 		errorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

pkg/util/util.go

@@ -10,12 +10,18 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"regexp"
 
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/kots/pkg/crypto"
 	"github.com/replicatedhq/kotskinds/pkg/licensewrapper"
 )
 
+// yamlDocSeparatorLine matches a "---" yaml document separator that has
+// trailing whitespace or a trailing comment on the same line (both valid per
+// the yaml spec), so it can be normalized to a bare "---" before splitting.
+var yamlDocSeparatorLine = regexp.MustCompile(`(?m)^---[ \t]*(#.*)?$`)
+
 func IsURL(str string) bool {
 	_, err := url.ParseRequestURI(str)
 	if err != nil {
@@ -127,6 +133,9 @@ func ConvertToSingleDocs(doc []byte) [][]byte {
 	singleDocs := [][]byte{}
 	// replace all windows line endings with unix line endings
 	doc = bytes.ReplaceAll(doc, []byte("\r\n"), []byte("\n"))
+	// normalize separators with trailing whitespace or a comment (e.g. "---  " or
+	// "--- # comment") to a bare "---" so they're recognized as document boundaries
+	doc = yamlDocSeparatorLine.ReplaceAll(doc, []byte("---"))
 	docs := bytes.Split(doc, []byte("\n---\n"))
 	for _, doc := range docs {
 		if len(bytes.TrimSpace(doc)) == 0 {

pkg/util/util_test.go

@@ -318,6 +318,16 @@ func TestConvertToSingleDocs(t *testing.T) {
 			doc:  []byte("abc\r\n---\r\n\r\n---\r\ndef"),
 			want: [][]byte{[]byte("abc"), []byte("def")},
 		},
+		{
+			name: "multiple docs with trailing whitespace after separator",
+			doc:  []byte("abc\n---   \ndef"),
+			want: [][]byte{[]byte("abc"), []byte("def")},
+		},
+		{
+			name: "multiple docs with comment after separator",
+			doc:  []byte("abc\n--- # comment\ndef"),
+			want: [][]byte{[]byte("abc"), []byte("def")},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {