github-vet / github-vet/rangeloop-pointer-findings

cyverse-de/interapps-runner: run.go; 153 LoC

Open
#14,401 0 comments 0 reactions 0 assignees View on GitHub
fresh large
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
PR metrics pending

Description

Found a possible issue in [cyverse-de/interapps-runner](https://www.github.com/cyverse-de/interapps-runner) at [run.go](https://github.com/cyverse-de/interapps-runner/blob/0014ad4cac3924d864357c67581b56dbb322e71c/run.go#L378-L530)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.

> range-loop variable idx used in defer or goroutine at line 416

[Click here to see the code in its original context.](https://github.com/cyverse-de/interapps-runner/blob/0014ad4cac3924d864357c67581b56dbb322e71c/run.go#L378-L530)

Click here to show the 153 line(s) of Go which triggered the analyzer.

```go
for idx, step := range r.composer.job.Steps {
stepStatus := messaging.Success
var stepErr error

running(r.client, r.composer.job,
fmt.Sprintf(
"Running tool container %s:%s with arguments: %s",
step.Component.Container.Image.Name,
step.Component.Container.Image.Tag,
strings.Join(step.Arguments(), " "),
),
)

stdout, err := os.Create(path.Join(r.logsDir, fmt.Sprintf("docker-compose-step-stdout-%d", idx)))
if err != nil {
log.Error(err)
}
defer stdout.Close()

stderr, err := os.Create(path.Join(r.logsDir, fmt.Sprintf("docker-compose-step-stderr-%d", idx)))
if err != nil {
log.Error(err)
}
defer stderr.Close()

proxystdout, err := os.Create(path.Join(r.logsDir, fmt.Sprintf("docker-compose-step-proxy-stdout-%d", idx)))
if err != nil {
log.Error(err)
}
defer proxystdout.Close()

proxystderr, err := os.Create(path.Join(r.logsDir, fmt.Sprintf("docker-compose-step-proxy-stderr-%d", idx)))
if err != nil {
log.Error(err)
}
defer proxystderr.Close()

go func() {
if err = r.execDockerCompose(ctx, ProxyServiceName(idx), os.Environ(), proxystdout, proxystderr); err != nil {
running(r.client, r.composer.job, fmt.Sprintf("error running proxy %s", err.Error()))
}
}()

ingressID := r.composer.IngressID()

// Used to get the messaging status code and errors from the ConfigureK8s
// function.
configChan := make(chan asyncReturn)

// Call the ConfigureK8s function in a goroutine. It's cancelable, so pass
// in the context.
go func(ctx context.Context, c chan asyncReturn) {
code, k8serr := r.ConfigureK8s(ctx, ingressID)

// Ship the status code and err back to the calling goroutine.
c <- asyncReturn{
statusCode: code,
err: k8serr,
}
}(ctx, configChan)

// Used to get the status code and error from the step execution.
execChan := make(chan asyncReturn)

go func(ctx context.Context, c chan asyncReturn) {
svcname := fmt.Sprintf("step_%d", idx)
if err = r.execDockerCompose(ctx, svcname, os.Environ(), stdout, stderr); err != nil {
running(r.client, r.composer.job,
fmt.Sprintf(
"Error running tool container %s:%s with arguments '%s': %s",
step.Component.Container.Image.Name,
step.Component.Container.Image.Tag,
strings.Join(step.Arguments(), " "),
err.Error(),
),
)

c <- asyncReturn{
statusCode: messaging.StatusStepFailed,
err: err,
}
return
}

running(r.client, r.composer.job,
fmt.Sprintf("Tool container %s:%s with arguments '%s' finished successfully",
step.Component.Container.Image.Name,
step.Component.Container.Image.Tag,
strings.Join(step.Arguments(), " "),
),
)
c <- asyncReturn{
statusCode: messaging.Success,
err: nil,
}
}(ctx, execChan)

shouldExit := false
for !shouldExit {
select {
case execReturn := <-execChan: // step exection is done
stepStatus = execReturn.statusCode
stepErr = execReturn.err
if err != nil {
log.Println("error from step execution, canceling contexts")
cancel()
}
shouldExit = true
break
case configReturn := <-configChan: // k8s configuration is done
stepStatus = configReturn.statusCode
stepErr = configReturn.err
if err != nil {
log.Println("error from k8s configuration, canceling contexts")
cancel()
shouldExit = true
} else {
shouldExit = false // this will probably exit before the step
}
break
case <-ctx.Done(): // The context got canceled
stepStatus = messaging.StatusStepFailed
stepErr = ctx.Err()
shouldExit = true
break
}
}

// I'm not sure if ignoring the errors here (aside from logging them) is the
// right thing to do, but it's easy to fix if it becomes a problem. Just
// return messaging.StatusStepFailed and the error.
log.Printf("deleting K8s endpoint %s\n", ingressID)
if err = DeleteK8SEndpoint(ctx, r.appExposerBaseURL, r.appExposerHeader, ingressID); err != nil {
running(r.client, r.composer.job, fmt.Sprintf("Error deleting K8s endpoint: %s", err.Error()))
}
log.Printf("done deleting K8s endpoint %s\n", ingressID)

log.Printf("deleting K8s service %s\n", ingressID)
if err = DeleteK8SService(ctx, r.appExposerBaseURL, r.appExposerHeader, ingressID); err != nil {
running(r.client, r.composer.job, fmt.Sprintf("Error deleting K8s service: %s", err.Error()))
}
log.Printf("done deleting K8s service %s\n", ingressID)

log.Printf("deleting K8s ingress %s\n", ingressID)
if err = DeleteK8SIngress(ctx, r.appExposerBaseURL, r.appExposerHeader, ingressID); err != nil {
running(r.client, r.composer.job, fmt.Sprintf("Error deleting K8s ingress: %s", err.Error()))
}
log.Printf("done deleting K8s ingress %s\n", ingressID)

if stepErr != nil {
return stepStatus, stepErr
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 0014ad4cac3924d864357c67581b56dbb322e71c

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with run.go lines 378-530, especially the range loop and its defer and goroutine bodies. Trace whether idx, step, and related variables are shared across iterations, then classify the analyzer finding based on the observed behavior; done means the issue has a clear Bug, Mitigated, or Desirable Behavior classification.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, go, kubernetes
Domain
devops, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.