Skip to content

Commit

Permalink
fix updater confused by blank commit (#176)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakecoffman authored Sep 7, 2023
1 parent 2113018 commit b3bb53e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 9 deletions.
2 changes: 1 addition & 1 deletion cmd/dependabot/internal/cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func readArguments(cmd *cobra.Command) (*model.Input, error) {
Provider: provider,
Repo: repo,
Directory: directory,
Commit: &commit,
Commit: commit,
Branch: nil,
Hostname: nil,
APIEndpoint: nil,
Expand Down
4 changes: 2 additions & 2 deletions internal/infra/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (p *RunParams) Validate() error {
if p.Job == nil {
return fmt.Errorf("job is required")
}
if p.Job.Source.Commit != nil && *p.Job.Source.Commit != "" && !gitShaRegex.MatchString(*p.Job.Source.Commit) {
if p.Job.Source.Commit != "" && !gitShaRegex.MatchString(p.Job.Source.Commit) {
return fmt.Errorf("commit must be a SHA, or not provided")
}
return nil
Expand Down Expand Up @@ -137,7 +137,7 @@ func Run(params RunParams) error {
}

func generateOutput(params RunParams, api *server.API, outFile *os.File) ([]byte, error) {
if params.Job.Source.Commit == nil {
if params.Job.Source.Commit == "" {
// store the SHA we worked with for reproducible tests
params.Job.Source.Commit = api.Actual.Input.Job.Source.Commit
}
Expand Down
13 changes: 9 additions & 4 deletions internal/infra/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ func putUpdaterInputs(ctx context.Context, cli *client.Client, cert, id string,
return fmt.Errorf("failed to copy cert to container: %w", err)
}

data, err := json.Marshal(FileFetcherJobFile{Job: job})
data, err := JobFile{Job: job}.ToJSON()
if err != nil {
return fmt.Errorf("failed to marshal job file: %w", err)
}
if t, err := tarball(guestInputDir, string(data)); err != nil {
if t, err := tarball(guestInputDir, data); err != nil {
return fmt.Errorf("failed create input tarball: %w", err)
} else if err = cli.CopyToContainer(ctx, id, "/", t, opt); err != nil {
return fmt.Errorf("failed to copy input to container: %w", err)
Expand Down Expand Up @@ -278,11 +278,16 @@ func (u *Updater) Close() error {
})
}

// FileFetcherJobFile is the payload passed to file updater containers.
type FileFetcherJobFile struct {
// JobFile is the payload passed to file updater containers.
type JobFile struct {
Job *model.Job `json:"job"`
}

func (j JobFile) ToJSON() (string, error) {
data, err := json.Marshal(j)
return string(data), err
}

func tarball(name, contents string) (*bytes.Buffer, error) {
var buf bytes.Buffer
t := tar.NewWriter(&buf)
Expand Down
21 changes: 21 additions & 0 deletions internal/infra/updater_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package infra

import (
"github.com/dependabot/cli/internal/model"
"os"
"path/filepath"
"strings"
"testing"
)

Expand Down Expand Up @@ -48,3 +50,22 @@ func Test_mountOptions(t *testing.T) {
}
}
}

func TestJobFile_ToJSON(t *testing.T) {
t.Run("empty commit doesn't pass in empty string", func(t *testing.T) {
job := JobFile{
Job: &model.Job{
Source: model.Source{
Commit: "",
},
},
}
json, err := job.ToJSON()
if err != nil {
t.Fatal(err)
}
if strings.Contains(json, `"commit"`) {
t.Errorf("expected JSON to not contain commit: %v", json)
}
})
}
2 changes: 1 addition & 1 deletion internal/model/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Source struct {
Repo string `json:"repo" yaml:"repo,omitempty"`
Directory string `json:"directory" yaml:"directory,omitempty"`
Branch *string `json:"branch" yaml:"branch,omitempty"`
Commit *string `json:"commit" yaml:"commit,omitempty"`
Commit string `json:"commit,omitempty" yaml:"commit,omitempty"`

Hostname *string `json:"hostname" yaml:"hostname,omitempty"` // Must be provided if APIEndpoint is
APIEndpoint *string `json:"api-endpoint" yaml:"api-endpoint,omitempty"` // Must be provided if Hostname is
Expand Down
2 changes: 1 addition & 1 deletion internal/server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (a *API) pushResult(kind string, actual *model.UpdateWrapper) error {

if msg, ok := actual.Data.(model.MarkAsProcessed); ok {
// record the commit SHA so the test is reproducible
a.Actual.Input.Job.Source.Commit = &msg.BaseCommitSha
a.Actual.Input.Job.Source.Commit = msg.BaseCommitSha
}

return nil
Expand Down

0 comments on commit b3bb53e

Please sign in to comment.