Skip to content
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

Jump to bottom

feat: add types, fix errors, add conditional status report #1071

Closed
wants to merge 6 commits into from
Closed

feat: add types, fix errors, add conditional status report #1071

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5dd406d
feat: add types, fix errors, add conditional status report
ecrupper Feb 28, 2024
823b44e
add db funcs and source private key from base64 string
ecrupper Feb 28, 2024
5dd7d63
include status report
ecrupper Mar 1, 2024
c279ef8
Merge branch 'main' into ec-updates
ecrupper Mar 5, 2024
aa198ba
Merge branch 'hackathon/github_app' into ec-updates
ecrupper Jun 21, 2024
83d28ac
fix table syntax
ecrupper Jun 24, 2024
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
2 changes: 1 addition & 1 deletion api/build/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func CreateBuild(c *gin.Context) {
input.SetPipelineID(pipeline.GetID())

// create the objects from the pipeline in the database
err = PlanBuild(ctx, database.FromContext(c), p, input, r)
err = PlanBuild(ctx, database.FromContext(c), scm.FromContext(c), p, input, r)
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)

Expand Down
2 changes: 1 addition & 1 deletion api/build/restart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func RestartBuild(c *gin.Context) {
b.SetPipelineID(pipeline.GetID())

// create the objects from the pipeline in the database
err = PlanBuild(ctx, database.FromContext(c), p, b, r)
err = PlanBuild(ctx, database.FromContext(c), scm.FromContext(c), p, b, r)
if err != nil {
util.HandleError(c, http.StatusInternalServerError, err)

Expand Down
17 changes: 9 additions & 8 deletions api/step/plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func PlanSteps(ctx context.Context, database database.Interface, scm scm.Service

// iterate through all pipeline steps
for _, step := range p.Steps {
s, err := planStep(ctx, database, b, step, "")
s, err := planStep(ctx, database, scm, b, r, step, "")
if err != nil {
return steps, err
}
Expand All @@ -61,14 +61,15 @@ func planStep(ctx context.Context, database database.Interface, scm scm.Service,
s.SetStatus(constants.StatusPending)
s.SetCreated(time.Now().UTC().Unix())

id, err := scm.CreateChecks(ctx, r, b.GetCommit(), s.GetName())
if err != nil {
// TODO: make this error more meaningful
return nil, err
}
if c.ReportStatus {
id, err := scm.CreateChecks(ctx, r, b.GetCommit(), s.GetName())
if err != nil {
// TODO: make this error more meaningful
return nil, err
}

// TODO: have to store the check ID somewhere
s.SetCheckID(id)
s.SetCheckID(id)
}

// send API call to create the step
s, err := database.CreateStep(ctx, s)
Expand Down
12 changes: 7 additions & 5 deletions api/step/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,15 @@ func UpdateStep(c *gin.Context) {
return
}

err = scm.FromContext(c).UpdateChecks(ctx, r, s, b.GetCommit())
if err != nil {
retErr := fmt.Errorf("unable to set step check %s: %w", entry, err)
if s.GetCheckID() != 0 {
err = scm.FromContext(c).UpdateChecks(ctx, r, s, b.GetCommit())
if err != nil {
retErr := fmt.Errorf("unable to set step check %s: %w", entry, err)

util.HandleError(c, http.StatusInternalServerError, retErr)
util.HandleError(c, http.StatusInternalServerError, retErr)

return
return
}
}

c.JSON(http.StatusOK, s)
Expand Down
2 changes: 1 addition & 1 deletion api/webhook/post.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func PostWebhook(c *gin.Context) {
// using the same Number and thus create a constraint
// conflict; consider deleting the partially created
// build object in the database
err = build.PlanBuild(ctx, database.FromContext(c), p, b, repo)
err = build.PlanBuild(ctx, database.FromContext(c), scm.FromContext(c), p, b, repo)
if err != nil {
retErr := fmt.Errorf("%s: %w", baseErr, err)

Expand Down
2 changes: 1 addition & 1 deletion cmd/vela-server/schedule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func processSchedule(ctx context.Context, s *library.Schedule, compiler compiler
// using the same Number and thus create a constraint
// conflict; consider deleting the partially created
// build object in the database
err = build.PlanBuild(ctx, database, p, b, r)
err = build.PlanBuild(ctx, database, scm, p, b, r)
if err != nil {
// check if the retry limit has been exceeded
if i < retryLimit-1 {
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

go 1.21

replace github.com/go-vela/types => ../types

Check failure on line 5 in go.mod

View workflow job for this annotation

GitHub Actions / golangci

[golangci] go.mod#L5 

local replacement are not allowed: github.com/go-vela/types (gomoddirectives)
Raw output 
go.mod:5:1: local replacement are not allowed: github.com/go-vela/types (gomoddirectives)
replace github.com/go-vela/types => ../types
^
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
local replacement are not allowed: github.com/go-vela/types (gomoddirectives)


require (
github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb
github.com/DATA-DOG/go-sqlmock v1.5.2
Expand Down
21 changes: 16 additions & 5 deletions scm/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,17 +186,28 @@ func (c *client) newClientToken(token string) *github.Client {
}

// helper function to return the GitHub App token.
func (c *client) newGithubAppToken(r *library.Repo) *github.Client {
func (c *client) newGithubAppToken(r *library.Repo) (*github.Client, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recommendation: looks like it returns a client using a token, not the token itself. maybe remove Token suffix and update comment?

// create a github client based off the existing GitHub App configuration
client, err := github.NewClient(&http.Client{Transport: c.AppsTransport}).WithEnterpriseURLs(c.config.API, c.config.API)
if err != nil {
panic(err)
return nil, err
}

// if repo has an install ID, use it to create an installation token
if r.GetInstallID() != 0 {
// create installation token for the repo
t, _, err := client.Apps.CreateInstallationToken(context.Background(), r.GetInstallID(), &github.InstallationTokenOptions{})
if err != nil {
panic(err)
}

return c.newClientToken(t.GetToken()), nil
}

// list all installations (a.k.a. orgs) where the GitHub App is installed
installations, _, err := client.Apps.ListInstallations(context.Background(), &github.ListOptions{})
if err != nil {
panic(err)
return nil, err
}

var id int64
Expand All @@ -210,7 +221,7 @@ func (c *client) newGithubAppToken(r *library.Repo) *github.Client {

// failsafe in case the repo does not belong to an org where the GitHub App is installed
if id == 0 {
panic(err)
return nil, err
}

// create installation token for the repo
Expand All @@ -219,5 +230,5 @@ func (c *client) newGithubAppToken(r *library.Repo) *github.Client {
panic(err)
}

return c.newClientToken(t.GetToken())
return c.newClientToken(t.GetToken()), nil
}
12 changes: 9 additions & 3 deletions scm/github/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,10 @@ func (c *client) GetBranch(ctx context.Context, u *library.User, r *library.Repo
// CreateChecks defines a function that does stuff...
func (c *client) CreateChecks(ctx context.Context, r *library.Repo, commit, step string) (int64, error) {
// create client from GitHub App
client := c.newGithubAppToken(r)
client, err := c.newGithubAppToken(r)
if err != nil {
return 0, err
}

opts := github.CreateCheckRunOptions{
Name: fmt.Sprintf("vela-%s-%s", commit, step),
Expand All @@ -607,7 +610,10 @@ func (c *client) CreateChecks(ctx context.Context, r *library.Repo, commit, step
// UpdateChecks defines a function that does stuff...
func (c *client) UpdateChecks(ctx context.Context, r *library.Repo, s *library.Step, commit string) error {
// create client from GitHub App
client := c.newGithubAppToken(r)
client, err := c.newGithubAppToken(r)
if err != nil {
return err
}

var (
conclusion string
Expand Down Expand Up @@ -650,7 +656,7 @@ func (c *client) UpdateChecks(ctx context.Context, r *library.Repo, s *library.S
Status: github.String(status),
}

_, _, err := client.Checks.UpdateCheckRun(ctx, r.GetOrg(), r.GetName(), s.GetCheckID(), opts)
_, _, err = client.Checks.UpdateCheckRun(ctx, r.GetOrg(), r.GetName(), s.GetCheckID(), opts)
if err != nil {
return err
}
Expand Down
Loading