Merge pull request #8807 from dokku/8802-buildpacks-allow-setting-the-entire-ordered-buildpack-list-in-one-command

Allow replacing buildpack list atomically

plugins/buildpacks/buildpacks_test.go

@@ -2,6 +2,7 @@ package buildpacks
 
 import (
 	"os"
+	"os/user"
 	"path/filepath"
 	"testing"
 
@@ -15,9 +16,20 @@ func setupTestEnvironment(t *testing.T) string {
 		t.Fatal(err)
 	}
 
-	os.Setenv("DOKKU_LIB_ROOT", tmpDir)
-	os.Setenv("PLUGIN_PATH", "/var/lib/dokku/plugins")
-	os.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled")
+	t.Setenv("DOKKU_LIB_ROOT", tmpDir)
+	t.Setenv("PLUGIN_PATH", "/var/lib/dokku/plugins")
+	t.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled")
+
+	current, err := user.Current()
+	if err != nil {
+		t.Fatal(err)
+	}
+	group, err := user.LookupGroupId(current.Gid)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("DOKKU_SYSTEM_USER", current.Username)
+	t.Setenv("DOKKU_SYSTEM_GROUP", group.Name)
 
 	return tmpDir
 }
@@ -139,3 +151,79 @@ func TestGetBuildpacksAppJSONEmptyArray(t *testing.T) {
 	Expect(err).NotTo(HaveOccurred())
 	Expect(buildpacks).To(BeEmpty())
 }
+
+func TestSetBuildpackListReplacesEntireList(t *testing.T) {
+	RegisterTestingT(t)
+	tmpDir := setupTestEnvironment(t)
+	defer teardownTestEnvironment(tmpDir)
+
+	propDir := filepath.Join(tmpDir, "config", "buildpacks", "test-app")
+	Expect(os.MkdirAll(propDir, 0755)).To(Succeed())
+	Expect(os.WriteFile(filepath.Join(propDir, "buildpacks"), []byte("https://github.com/heroku/heroku-buildpack-nodejs.git\nhttps://github.com/heroku/heroku-buildpack-python.git\nhttps://github.com/heroku/heroku-buildpack-ruby.git\n"), 0644)).To(Succeed())
+
+	err := setBuildpackList("test-app", []string{"heroku/ruby", "heroku/nodejs"}, 0)
+	Expect(err).NotTo(HaveOccurred())
+
+	buildpacks, err := getBuildpacks("test-app")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(buildpacks).To(HaveLen(2))
+	Expect(buildpacks[0]).To(Equal("https://github.com/heroku/heroku-buildpack-ruby.git"))
+	Expect(buildpacks[1]).To(Equal("https://github.com/heroku/heroku-buildpack-nodejs.git"))
+}
+
+func TestSetBuildpackListSingleBuildpack(t *testing.T) {
+	RegisterTestingT(t)
+	tmpDir := setupTestEnvironment(t)
+	defer teardownTestEnvironment(tmpDir)
+
+	propDir := filepath.Join(tmpDir, "config", "buildpacks", "test-app")
+	Expect(os.MkdirAll(propDir, 0755)).To(Succeed())
+	Expect(os.WriteFile(filepath.Join(propDir, "buildpacks"), []byte("https://github.com/heroku/heroku-buildpack-nodejs.git\nhttps://github.com/heroku/heroku-buildpack-python.git\n"), 0644)).To(Succeed())
+
+	err := setBuildpackList("test-app", []string{"heroku/ruby"}, 0)
+	Expect(err).NotTo(HaveOccurred())
+
+	buildpacks, err := getBuildpacks("test-app")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(buildpacks).To(HaveLen(1))
+	Expect(buildpacks[0]).To(Equal("https://github.com/heroku/heroku-buildpack-ruby.git"))
+}
+
+func TestSetBuildpackListInvalidLeavesListUnchanged(t *testing.T) {
+	RegisterTestingT(t)
+	tmpDir := setupTestEnvironment(t)
+	defer teardownTestEnvironment(tmpDir)
+
+	propDir := filepath.Join(tmpDir, "config", "buildpacks", "test-app")
+	Expect(os.MkdirAll(propDir, 0755)).To(Succeed())
+	Expect(os.WriteFile(filepath.Join(propDir, "buildpacks"), []byte("https://github.com/heroku/heroku-buildpack-nodejs.git\n"), 0644)).To(Succeed())
+
+	err := setBuildpackList("test-app", []string{"heroku/ruby", "nodejs"}, 0)
+	Expect(err).To(HaveOccurred())
+	Expect(err.Error()).To(ContainSubstring("Invalid buildpack specified"))
+
+	buildpacks, err := getBuildpacks("test-app")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(buildpacks).To(HaveLen(1))
+	Expect(buildpacks[0]).To(Equal("https://github.com/heroku/heroku-buildpack-nodejs.git"))
+}
+
+func TestSetBuildpackListRejectsIndex(t *testing.T) {
+	RegisterTestingT(t)
+	tmpDir := setupTestEnvironment(t)
+	defer teardownTestEnvironment(tmpDir)
+
+	err := setBuildpackList("test-app", []string{"heroku/ruby"}, 1)
+	Expect(err).To(HaveOccurred())
+	Expect(err.Error()).To(ContainSubstring("--index"))
+}
+
+func TestSetBuildpackListRejectsEmpty(t *testing.T) {
+	RegisterTestingT(t)
+	tmpDir := setupTestEnvironment(t)
+	defer teardownTestEnvironment(tmpDir)
+
+	err := setBuildpackList("test-app", []string{}, 0)
+	Expect(err).To(HaveOccurred())
+	Expect(err.Error()).To(ContainSubstring("buildpacks:clear"))
+}

plugins/buildpacks/src/commands/commands.go

@@ -25,6 +25,7 @@ Additional commands:`
     buildpacks:remove <app> <buildpack>, Remove a buildpack set on the app
     buildpacks:report [<app>] [<flag>], Displays a buildpack report for one or more apps
     buildpacks:set [--index 1] <app> <buildpack>, Set new app buildpack at a given position defaulting to the first buildpack if no index is specified
+    buildpacks:set --replace <app> <buildpack> [<buildpack> ...], Replace the entire ordered buildpack list with the specified buildpacks
     buildpacks:set-property [--global|<app>] <key> <value>, Set or clear a buildpacks property for an app`
 )
 

plugins/buildpacks/src/subcommands/subcommands.go

@@ -66,10 +66,11 @@ func main() {
 	case "set":
 		args := flag.NewFlagSet("buildpacks:set", flag.ExitOnError)
 		index := args.Int("index", 0, "--index: the 1-based index of the URL in the list of URLs")
+		replace := args.Bool("replace", false, "--replace: replace the entire ordered buildpack list")
 		args.Parse(os.Args[2:])
 		appName := args.Arg(0)
-		buildpack := args.Arg(1)
-		err = buildpacks.CommandSet(appName, buildpack, *index)
+		_, buildpackList := common.ShiftString(args.Args())
+		err = buildpacks.CommandSet(appName, buildpackList, *index, *replace)
 	case "set-property":
 		args := flag.NewFlagSet("buildpacks:set-property", flag.ExitOnError)
 		global := args.Bool("global", false, "--global: set a global property")

plugins/buildpacks/subcommands.go

@@ -162,12 +162,24 @@ func CommandReport(appName string, format string, infoFlag string) error {
 }
 
 // CommandSet implements buildpacks:set
-func CommandSet(appName string, buildpack string, index int) error {
+func CommandSet(appName string, buildpacks []string, index int, replace bool) error {
 	if err := common.VerifyAppName(appName); err != nil {
 		return err
 	}
 
-	buildpack, err := validBuildpackURL(buildpack)
+	if replace {
+		return setBuildpackList(appName, buildpacks, index)
+	}
+
+	if len(buildpacks) > 1 {
+		return errors.New("Setting multiple buildpacks requires the --replace flag")
+	}
+
+	if len(buildpacks) == 0 {
+		return errors.New("Must specify a buildpack")
+	}
+
+	buildpack, err := validBuildpackURL(buildpacks[0])
 	if err != nil {
 		return err
 	}
@@ -179,6 +191,28 @@ func CommandSet(appName string, buildpack string, index int) error {
 	return common.PropertyListSet("buildpacks", appName, "buildpacks", buildpack, index)
 }
 
+// setBuildpackList atomically replaces the entire ordered buildpack list
+func setBuildpackList(appName string, buildpacks []string, index int) error {
+	if index != 0 {
+		return errors.New("The --index flag cannot be used with --replace")
+	}
+
+	if len(buildpacks) == 0 {
+		return errors.New("Must specify at least one buildpack, use buildpacks:clear to remove all buildpacks")
+	}
+
+	validated := make([]string, 0, len(buildpacks))
+	for _, buildpack := range buildpacks {
+		url, err := validBuildpackURL(buildpack)
+		if err != nil {
+			return err
+		}
+		validated = append(validated, url)
+	}
+
+	return common.PropertyListWrite("buildpacks", appName, "buildpacks", validated)
+}
+
 // CommandSetProperty implements buildpacks:set-property
 func CommandSetProperty(appName string, property string, value string) error {
 	oldStack := ""