Skip to content

Fix temp directory leak in getDelve #1665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SebTardif wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix temp directory leak in getDelve #1665

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions pkg/build/gobuild.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,33 +297,42 @@ func doesPlatformSupportDebugging(platform v1.Platform) bool {
return platform.OS == "linux" && (platform.Architecture == "amd64" || platform.Architecture == "arm64")
}

func getDelve(ctx context.Context, platform v1.Platform) (string, error) {
func getDelve(ctx context.Context, platform v1.Platform) (string, string, error) {
const delveCloneURL = "https://github.com/go-delve/delve.git"

if platform.OS == "" || platform.Architecture == "" {
return "", fmt.Errorf("platform os (%q) or arch (%q) is empty",
return "", "", fmt.Errorf("platform os (%q) or arch (%q) is empty",
platform.OS,
platform.Architecture,
)
}

env, err := buildEnv(platform, os.Environ(), nil)
if err != nil {
return "", fmt.Errorf("could not create env for Delve build: %w", err)
return "", "", fmt.Errorf("could not create env for Delve build: %w", err)
}

tmpInstallDir, err := os.MkdirTemp("", "delve")
if err != nil {
return "", fmt.Errorf("could not create tmp dir for Delve installation: %w", err)
return "", "", fmt.Errorf("could not create tmp dir for Delve installation: %w", err)
}

// Clean up the entire temp directory on any error path.
success := false
defer func() {
if !success {
os.RemoveAll(tmpInstallDir)
}
}()

cloneDir := filepath.Join(tmpInstallDir, "delve")
err = os.MkdirAll(cloneDir, 0755)
if err != nil {
return "", fmt.Errorf("making dir for delve clone: %w", err)
return "", "", fmt.Errorf("making dir for delve clone: %w", err)
}
err = git.Clone(ctx, cloneDir, delveCloneURL)
if err != nil {
return "", fmt.Errorf("cloning delve repo: %w", err)
return "", "", fmt.Errorf("cloning delve repo: %w", err)
}
osArchDir := fmt.Sprintf("%s_%s", platform.OS, platform.Architecture)
delveBinaryPath := filepath.Join(tmpInstallDir, "bin", osArchDir, "dlv")
Expand All @@ -349,15 +358,15 @@ func getDelve(ctx context.Context, platform v1.Platform) (string, error) {

log.Printf("Building Delve for %s", platform)
if err := cmd.Run(); err != nil {
os.RemoveAll(tmpInstallDir)
return "", fmt.Errorf("go build Delve: %w: %s", err, output.String())
return "", "", fmt.Errorf("go build Delve: %w: %s", err, output.String())
}

if _, err := os.Stat(delveBinaryPath); err != nil {
return "", fmt.Errorf("could not find Delve binary at %q: %w", delveBinaryPath, err)
return "", "", fmt.Errorf("could not find Delve binary at %q: %w", delveBinaryPath, err)
}

return delveBinaryPath, nil
success = true
return delveBinaryPath, tmpInstallDir, nil
}

func build(ctx context.Context, buildCtx buildContext) (string, error) {
Expand Down Expand Up @@ -1171,11 +1180,11 @@ func (g *gobuild) buildOne(ctx context.Context, refStr string, base v1.Image, pl
delvePath := "" // path for delve in image
if g.useDebugging(*platform) {
// get delve locally
delveBinary, err := getDelve(ctx, *platform)
delveBinary, delveDir, err := getDelve(ctx, *platform)
if err != nil {
return nil, fmt.Errorf("building Delve: %w", err)
}
defer os.RemoveAll(filepath.Dir(delveBinary))
defer os.RemoveAll(delveDir)

delvePath = path.Join("/ko-app", filepath.Base(delveBinary))

Expand Down
Loading