FIX: append os.Environ to docker build and docker run calls

.github/workflows/build.yml

@@ -9,7 +9,7 @@ on:
   pull_request:
 
 jobs:
-  lint:
+  fmt:
     runs-on: ubuntu-latest
     timeout-minutes: 5
     strategy:
@@ -26,6 +26,19 @@ jobs:
             exit 1
           fi
 
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ">=1.22.0"
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v8
+        with:
+          version: v2.1
+          working-directory: ./v2
+
   test:
     runs-on: ubuntu-latest
     timeout-minutes: 5
@@ -45,7 +58,7 @@ jobs:
     permissions:
       contents: write
     runs-on: ubuntu-latest
-    needs: [lint, test]
+    needs: [fmt, lint, test]
     steps:
       - uses: actions/checkout@v4
       - name: build and release
@@ -58,7 +71,7 @@ jobs:
     permissions:
       contents: write
     runs-on: ubuntu-latest
-    needs: [lint, test, create_github_release]
+    needs: [fmt, lint, test, create_github_release]
     strategy:
       matrix:
         goos: [linux, darwin]

v2/cli_build.go

@@ -26,16 +26,19 @@ type DockerBuildCmd struct {
 	Config string `arg:"" name:"config" help:"configuration" predictor:"config"`
 }
 
-func (r *DockerBuildCmd) Run(cli *Cli, ctx *context.Context) error {
+func (r *DockerBuildCmd) Run(cli *Cli, ctx context.Context) error {
 	config, err := config.LoadConfig(cli.ConfDir, r.Config, true, cli.TemplatesDir)
 	if err != nil {
-		return errors.New("YAML syntax error. Please check your containers/*.yml config files.")
+		return errors.New("YAML syntax error. Please check your containers/*.yml config files")
 	}
 
-	dir := cli.BuildDir + "/" + r.Config
-	if err := os.MkdirAll(dir, 0755); err != nil && !os.IsExist(err) {
-		return err
+	dir := cli.BuildDir
+	if dir == "" {
+		if dir, err = os.MkdirTemp("", "launcher"); err != nil {
+			return errors.New("cannot create temp directory")
+		}
 	}
+	defer os.RemoveAll(dir) //nolint:errcheck
 	if err := config.WriteYamlConfig(dir); err != nil {
 		return err
 	}
@@ -47,18 +50,14 @@ func (r *DockerBuildCmd) Run(cli *Cli, ctx *context.Context) error {
 	pupsArgs := "--skip-tags=precompile,migrate,db"
 	builder := docker.DockerBuilder{
 		Config:    config,
-		Ctx:       ctx,
 		Stdin:     strings.NewReader(config.Dockerfile(pupsArgs, r.BakeEnv)),
 		Dir:       dir,
 		Namespace: namespace,
 		ImageTag:  r.Tag,
 	}
-	if err := builder.Run(); err != nil {
+	if err := builder.Run(ctx); err != nil {
 		return err
 	}
-	cleaner := CleanCmd{Config: r.Config}
-	cleaner.Run(cli)
-
 	return nil
 }
 
@@ -68,11 +67,11 @@ type DockerConfigureCmd struct {
 	Config    string `arg:"" name:"config" help:"config" predictor:"config"`
 }
 
-func (r *DockerConfigureCmd) Run(cli *Cli, ctx *context.Context) error {
+func (r *DockerConfigureCmd) Run(cli *Cli, ctx context.Context) error {
 	config, err := config.LoadConfig(cli.ConfDir, r.Config, true, cli.TemplatesDir)
 
 	if err != nil {
-		return errors.New("YAML syntax error. Please check your containers/*.yml config files.")
+		return errors.New("YAML syntax error. Please check your containers/*.yml config files")
 	}
 
 	var uuidString string
@@ -103,11 +102,10 @@ func (r *DockerConfigureCmd) Run(cli *Cli, ctx *context.Context) error {
 		FromImageName:  namespace + "/" + r.Config + sourceTag,
 		SavedImageName: namespace + "/" + r.Config + targetTag,
 		ExtraEnv:       []string{"SKIP_EMBER_CLI_COMPILE=1"},
-		Ctx:            ctx,
 		ContainerId:    containerId,
 	}
 
-	return pups.Run()
+	return pups.Run(ctx)
 }
 
 type DockerMigrateCmd struct {
@@ -116,10 +114,10 @@ type DockerMigrateCmd struct {
 	SkipPostDeploymentMigrations bool   `env:"SKIP_POST_DEPLOYMENT_MIGRATIONS" help:"Skip post-deployment migrations. Runs safe migrations only. Defers breaking-change migrations. Make sure you run post-deployment migrations after a full deploy is complete if you use this option."`
 }
 
-func (r *DockerMigrateCmd) Run(cli *Cli, ctx *context.Context) error {
+func (r *DockerMigrateCmd) Run(cli *Cli, ctx context.Context) error {
 	config, err := config.LoadConfig(cli.ConfDir, r.Config, true, cli.TemplatesDir)
 	if err != nil {
-		return errors.New("YAML syntax error. Please check your containers/*.yml config files.")
+		return errors.New("YAML syntax error. Please check your containers/*.yml config files")
 	}
 	containerId := "discourse-build-" + uuid.NewString()
 	env := []string{"SKIP_EMBER_CLI_COMPILE=1"}
@@ -140,17 +138,16 @@ func (r *DockerMigrateCmd) Run(cli *Cli, ctx *context.Context) error {
 		PupsArgs:      "--tags=db,migrate",
 		FromImageName: namespace + "/" + r.Config + tag,
 		ExtraEnv:      env,
-		Ctx:           ctx,
 		ContainerId:   containerId,
 	}
-	return pups.Run()
+	return pups.Run(ctx)
 }
 
 type DockerBootstrapCmd struct {
 	Config string `arg:"" name:"config" help:"config" predictor:"config"`
 }
 
-func (r *DockerBootstrapCmd) Run(cli *Cli, ctx *context.Context) error {
+func (r *DockerBootstrapCmd) Run(cli *Cli, ctx context.Context) error {
 	buildStep := DockerBuildCmd{Config: r.Config, BakeEnv: false}
 	migrateStep := DockerMigrateCmd{Config: r.Config}
 	configureStep := DockerConfigureCmd{Config: r.Config}
@@ -165,16 +162,3 @@ func (r *DockerBootstrapCmd) Run(cli *Cli, ctx *context.Context) error {
 	}
 	return nil
 }
-
-type CleanCmd struct {
-	Config string `arg:"" name:"config" help:"config to clean" predictor:"config"`
-}
-
-func (r *CleanCmd) Run(cli *Cli) error {
-	dir := cli.BuildDir + "/" + r.Config
-	os.Remove(dir + "/config.yaml")
-	if err := os.Remove(dir); err != nil {
-		return err
-	}
-	return nil
-}

v2/cli_build_test.go

@@ -39,25 +39,25 @@ var _ = Describe("Build", func() {
 		utils.CmdRunner = CreateNewFakeCmdRunner()
 	})
 
-	AfterEach(func() {
-		os.RemoveAll(testDir)
-	})
-
 	Context("When running build commands", func() {
 		var checkBuildCmd = func(cmd exec.Cmd) {
 			Expect(cmd.String()).To(ContainSubstring("docker build"))
 			Expect(cmd.String()).To(ContainSubstring("--build-arg DISCOURSE_DEVELOPER_EMAILS"))
-			Expect(cmd.Dir).To(Equal(testDir + "/test"))
+			Expect(cmd.Dir).To(Equal(testDir))
 
 			//db password is ignored
 			Expect(cmd.Env).ToNot(ContainElement("DISCOURSE_DB_PASSWORD=SOME_SECRET"))
 			Expect(cmd.Env).ToNot(ContainElement("DISCOURSEDB_SOCKET="))
 			buf := new(strings.Builder)
-			io.Copy(buf, cmd.Stdin)
+			io.Copy(buf, cmd.Stdin) //nolint:errcheck
 			// docker build's stdin is a dockerfile
 			Expect(buf.String()).To(ContainSubstring("COPY config.yaml /temp-config.yaml"))
 			Expect(buf.String()).To(ContainSubstring("--skip-tags=precompile,migrate,db"))
 			Expect(buf.String()).ToNot(ContainSubstring("SKIP_EMBER_CLI_COMPILE=1"))
+
+			// Ensure we clean up the temp dir after building
+			_, err := os.Stat(testDir)
+			Expect(err).To(MatchError(os.IsNotExist, "IsNotExist"))
 		}
 
 		var checkMigrateCmd = func(cmd exec.Cmd) {
@@ -68,7 +68,7 @@ var _ = Describe("Build", func() {
 			Expect(cmd.String()).To(ContainSubstring("--rm"))
 			Expect(cmd.Env).To(ContainElement("DISCOURSE_DB_PASSWORD=SOME_SECRET"))
 			buf := new(strings.Builder)
-			io.Copy(buf, cmd.Stdin)
+			io.Copy(buf, cmd.Stdin) //nolint:errcheck
 			// docker run's stdin is a pups config
 			Expect(buf.String()).To(ContainSubstring("path: /etc/service/nginx/run"))
 		}
@@ -109,7 +109,7 @@ var _ = Describe("Build", func() {
 					"local_discourse/test /bin/bash -c /usr/local/bin/pups --stdin --tags=db,precompile",
 			))
 
-			Expect(cmd.Env).To(Equal([]string{
+			Expect(cmd.Env).To(ContainElements(
 				"DISCOURSE_DB_HOST=data",
 				"DISCOURSE_DB_PASSWORD=SOME_SECRET",
 				"DISCOURSE_DB_PORT=",
@@ -131,10 +131,10 @@ var _ = Describe("Build", func() {
 				"RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR=1.5",
 				"UNICORN_SIDEKIQS=1",
 				"UNICORN_WORKERS=3",
-			}))
+			))
 
 			buf := new(strings.Builder)
-			io.Copy(buf, cmd.Stdin)
+			io.Copy(buf, cmd.Stdin) //nolint:errcheck
 			// docker run's stdin is a pups config
 
 			// web.template.yml is merged with the test config
@@ -161,14 +161,14 @@ var _ = Describe("Build", func() {
 
 		It("Should run docker build with correct arguments", func() {
 			runner := ddocker.DockerBuildCmd{Config: "test"}
-			runner.Run(cli, &ctx)
+			runner.Run(cli, ctx) //nolint:errcheck
 			Expect(len(RanCmds)).To(Equal(1))
 			checkBuildCmd(RanCmds[0])
 		})
 
 		It("Should run docker migrate with correct arguments", func() {
 			runner := ddocker.DockerMigrateCmd{Config: "test"}
-			runner.Run(cli, &ctx)
+			runner.Run(cli, ctx) //nolint:errcheck
 			Expect(len(RanCmds)).To(Equal(1))
 			checkMigrateCmd(RanCmds[0])
 		})
@@ -180,15 +180,15 @@ var _ = Describe("Build", func() {
 
 			It("Should run docker build with correct namespace and custom flags", func() {
 				runner := ddocker.DockerBuildCmd{Config: "test", Tag: "testtag"}
-				runner.Run(cli, &ctx)
+				runner.Run(cli, ctx) //nolint:errcheck
 				Expect(len(RanCmds)).To(Equal(1))
 				checkBuildCmd(RanCmds[0])
 				Expect(RanCmds[0].String()).To(ContainSubstring("testnamespace/test:testtag"))
 			})
 
 			It("Should run docker configure with correct namespace and tags", func() {
 				runner := ddocker.DockerConfigureCmd{Config: "test", SourceTag: "build", TargetTag: "configure"}
-				runner.Run(cli, &ctx)
+				runner.Run(cli, ctx) //nolint:errcheck
 				Expect(len(RanCmds)).To(Equal(3))
 
 				Expect(RanCmds[0].String()).To(MatchRegexp(
@@ -206,15 +206,15 @@ var _ = Describe("Build", func() {
 
 			It("Should run docker migrate with correct namespace", func() {
 				runner := ddocker.DockerMigrateCmd{Config: "test"}
-				runner.Run(cli, &ctx)
+				runner.Run(cli, ctx) //nolint:errcheck
 				Expect(len(RanCmds)).To(Equal(1))
 				Expect(RanCmds[0].String()).To(ContainSubstring("testnamespace/test "))
 			})
 		})
 
 		It("Should allow skip post deployment migrations", func() {
 			runner := ddocker.DockerMigrateCmd{Config: "test", SkipPostDeploymentMigrations: true}
-			runner.Run(cli, &ctx)
+			runner.Run(cli, ctx) //nolint:errcheck
 			Expect(len(RanCmds)).To(Equal(1))
 			cmd := RanCmds[0]
 			Expect(cmd.String()).To(ContainSubstring("docker run"))
@@ -225,14 +225,14 @@ var _ = Describe("Build", func() {
 			Expect(cmd.String()).To(ContainSubstring("--rm"))
 			Expect(cmd.Env).To(ContainElement("DISCOURSE_DB_PASSWORD=SOME_SECRET"))
 			buf := new(strings.Builder)
-			io.Copy(buf, cmd.Stdin)
+			io.Copy(buf, cmd.Stdin) //nolint:errcheck
 			// docker run's stdin is a pups config
 			Expect(buf.String()).To(ContainSubstring("path: /etc/service/nginx/run"))
 		})
 
 		It("Should run docker run followed by docker commit and rm container when configuring", func() {
 			runner := ddocker.DockerConfigureCmd{Config: "test"}
-			runner.Run(cli, &ctx)
+			runner.Run(cli, ctx) //nolint:errcheck
 			Expect(len(RanCmds)).To(Equal(3))
 
 			checkConfigureCmd(RanCmds[0])
@@ -242,7 +242,7 @@ var _ = Describe("Build", func() {
 
 		It("Should run all docker commands for full bootstrap", func() {
 			runner := ddocker.DockerBootstrapCmd{Config: "test"}
-			runner.Run(cli, &ctx)
+			runner.Run(cli, ctx) //nolint:errcheck
 			Expect(len(RanCmds)).To(Equal(5))
 			checkBuildCmd(RanCmds[0])
 			checkMigrateCmd(RanCmds[1])