Automated cherry pick of #217

Makefile

@@ -159,7 +159,7 @@ endif
 bundle:
 	rm -rf $(BUNDLE_DIR)/
 	mkdir -p $(BUNDLE_DIR)/$(PLUGIN_NAME)
-	cp $(MANIFEST_FILE) $(BUNDLE_DIR)/$(PLUGIN_NAME)/
+	./build/bin/manifest dist $(BUNDLE_DIR)/$(PLUGIN_NAME)
 	cp -r webapp/pack $(BUNDLE_DIR)/$(PLUGIN_NAME)/
 ifneq ($(wildcard LICENSE.txt),)
 	cp -r LICENSE.txt $(BUNDLE_DIR)/$(PLUGIN_NAME)/

build/manifest/main.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/mattermost/mattermost/server/public/model"
@@ -93,6 +94,14 @@ func main() {
 			panic("failed to apply manifest: " + err.Error())
 		}
 
+	case "dist":
+		if len(os.Args) <= 2 {
+			panic("failed to write manifest to dist directory: expected a destination directory as the first argument")
+		}
+		if err := distManifest(manifest, os.Args[2]); err != nil {
+			panic("failed to write manifest to dist directory: " + err.Error())
+		}
+
 	default:
 		panic("unrecognized command: " + cmd)
 	}
@@ -202,3 +211,20 @@ func applyManifest(manifest *model.Manifest) error {
 
 	return nil
 }
+
+// distManifest writes the resolved manifest (including the build-time version) as plugin.json
+// into the given bundle directory. This ensures the released bundle ships a plugin.json with a
+// version field, rather than the source plugin.json which has none.
+func distManifest(manifest *model.Manifest, destDir string) error {
+	manifestBytes, err := json.MarshalIndent(manifest, "", "  ")
+	if err != nil {
+		return err
+	}
+
+	destPath := filepath.Join(destDir, "plugin.json")
+	if err := os.WriteFile(destPath, manifestBytes, 0600); err != nil {
+		return errors.Wrapf(err, "failed to write %s", destPath)
+	}
+
+	return nil
+}

build/manifest/main_test.go

@@ -0,0 +1,191 @@
+// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package main
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDistManifest(t *testing.T) {
+	t.Run("writes plugin.json with the resolved version into the bundle directory", func(t *testing.T) {
+		destDir := t.TempDir()
+
+		manifest := &model.Manifest{
+			Id:              "focalboard",
+			Name:            "Mattermost Boards",
+			Version:         "9.3.0",
+			ReleaseNotesURL: "https://github.com/mattermost/mattermost-plugin-boards/releases/tag/v9.3.0",
+		}
+
+		require.NoError(t, distManifest(manifest, destDir))
+
+		written, err := os.ReadFile(filepath.Join(destDir, "plugin.json"))
+		require.NoError(t, err)
+
+		var got model.Manifest
+		require.NoError(t, json.Unmarshal(written, &got))
+
+		require.Equal(t, "focalboard", got.Id)
+		require.Equal(t, "9.3.0", got.Version)
+		require.Equal(t, manifest.ReleaseNotesURL, got.ReleaseNotesURL)
+	})
+
+	t.Run("fails when the destination directory does not exist", func(t *testing.T) {
+		manifest := &model.Manifest{Id: "focalboard", Version: "9.3.0"}
+
+		err := distManifest(manifest, filepath.Join(t.TempDir(), "does-not-exist"))
+		require.Error(t, err)
+	})
+}
+
+// writeTempManifest writes a plugin.json into a temp dir, chdirs into it for the duration of the
+// test, and resets the build-time vars afterwards.
+//
+// Tests that call this helper (or otherwise mutate os.Getwd / BuildTagCurrent / BuildTagLatest /
+// BuildHashShort) must not use t.Parallel(); they rely on sequential execution and process-wide
+// cwd/global state.
+func writeTempManifest(t *testing.T, contents string) {
+	t.Helper()
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "plugin.json"), []byte(contents), 0600))
+
+	cwd, err := os.Getwd()
+	require.NoError(t, err)
+	require.NoError(t, os.Chdir(dir))
+
+	t.Cleanup(func() {
+		_ = os.Chdir(cwd)
+		BuildTagCurrent, BuildTagLatest, BuildHashShort = "", "", ""
+	})
+}
+
+// Do not call t.Parallel(); subtests use writeTempManifest and shared build vars.
+func TestFindManifestResolvesVersion(t *testing.T) {
+	const versionlessManifest = `{
+		"id": "focalboard",
+		"homepage_url": "https://github.com/mattermost/mattermost-plugin-boards"
+	}`
+
+	t.Run("uses the tag at the current commit over the latest tag", func(t *testing.T) {
+		writeTempManifest(t, versionlessManifest)
+		BuildTagCurrent = "v9.3.0"
+		BuildTagLatest = "v9.2.0"
+		BuildHashShort = "abc1234"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "9.3.0", manifest.Version)
+	})
+
+	t.Run("falls back to the latest tag when no tag points at the current commit", func(t *testing.T) {
+		writeTempManifest(t, versionlessManifest)
+		BuildTagCurrent = ""
+		BuildTagLatest = "v9.1.0"
+		BuildHashShort = "abc1234"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "9.1.0", manifest.Version)
+	})
+
+	t.Run("falls back to a dev version when no tags exist", func(t *testing.T) {
+		writeTempManifest(t, versionlessManifest)
+		BuildTagCurrent = ""
+		BuildTagLatest = ""
+		BuildHashShort = "abc1234"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "0.0.0+abc1234", manifest.Version)
+	})
+
+	t.Run("preserves a version already present in the manifest", func(t *testing.T) {
+		writeTempManifest(t, `{
+			"id": "focalboard",
+			"version": "1.2.3",
+			"homepage_url": "https://github.com/mattermost/mattermost-plugin-boards"
+		}`)
+		BuildTagCurrent = "v9.3.0"
+		BuildTagLatest = "v9.2.0"
+		BuildHashShort = "abc1234"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "1.2.3", manifest.Version)
+	})
+}
+
+// Do not call t.Parallel(); subtests use writeTempManifest and shared build vars.
+func TestFindManifestReleaseNotesURL(t *testing.T) {
+	t.Run("generates a release notes URL from the latest tag when absent", func(t *testing.T) {
+		writeTempManifest(t, `{
+			"id": "focalboard",
+			"homepage_url": "https://github.com/mattermost/mattermost-plugin-boards"
+		}`)
+		BuildTagLatest = "v9.3.0"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "https://github.com/mattermost/mattermost-plugin-boards/releases/tag/v9.3.0", manifest.ReleaseNotesURL)
+	})
+
+	t.Run("does not overwrite an existing release notes URL", func(t *testing.T) {
+		writeTempManifest(t, `{
+			"id": "focalboard",
+			"homepage_url": "https://github.com/mattermost/mattermost-plugin-boards",
+			"release_notes_url": "https://example.com/custom"
+		}`)
+		BuildTagLatest = "v9.3.0"
+
+		manifest, err := findManifest()
+		require.NoError(t, err)
+		require.Equal(t, "https://example.com/custom", manifest.ReleaseNotesURL)
+	})
+}
+
+// TestBundledManifestFromSourceHasVersion is the regression test most coupled to MM-69594: the
+// source plugin.json ships without a version, but the resolved manifest written into the bundle
+// must carry one.
+//
+// Do not call t.Parallel(); this test mutates process cwd and BuildTagCurrent.
+func TestBundledManifestFromSourceHasVersion(t *testing.T) {
+	cwd, err := os.Getwd()
+	require.NoError(t, err)
+
+	repoRoot := filepath.Join(cwd, "..", "..")
+	source, err := os.ReadFile(filepath.Join(repoRoot, "plugin.json"))
+	require.NoError(t, err)
+
+	var sourceManifest model.Manifest
+	require.NoError(t, json.Unmarshal(source, &sourceManifest))
+	require.Empty(t, sourceManifest.Version, "source plugin.json is expected to ship without a version field")
+
+	require.NoError(t, os.Chdir(repoRoot))
+	BuildTagCurrent = "v9.9.9"
+	t.Cleanup(func() {
+		_ = os.Chdir(cwd)
+		BuildTagCurrent = ""
+	})
+
+	resolved, err := findManifest()
+	require.NoError(t, err)
+
+	destDir := t.TempDir()
+	require.NoError(t, distManifest(resolved, destDir))
+
+	written, err := os.ReadFile(filepath.Join(destDir, "plugin.json"))
+	require.NoError(t, err)
+
+	var bundled model.Manifest
+	require.NoError(t, json.Unmarshal(written, &bundled))
+	require.Equal(t, "9.9.9", bundled.Version)
+	require.Equal(t, sourceManifest.Id, bundled.Id)
+}