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.

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

enhance(build)!: add fork field for OIDC #1221

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions api/oi_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func GetOpenIDConfig(c *gin.Context) {
"build_number",
"build_id",
"repo",
"pull_fork",
"token_type",
"actor",
"actor_scm_id",
Expand Down
30 changes: 30 additions & 0 deletions api/types/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Build struct {
Commit *string `json:"commit,omitempty"`
Sender *string `json:"sender,omitempty"`
SenderSCMID *string `json:"sender_scm_id,omitempty"`
Fork *bool `json:"fork,omitempty"`
Author *string `json:"author,omitempty"`
Email *string `json:"email,omitempty"`
Link *string `json:"link,omitempty"`
Expand Down Expand Up @@ -191,6 +192,7 @@ func (b *Build) Environment(workspace, channel string) map[string]string {
envs["VELA_PULL_REQUEST"] = number
envs["VELA_PULL_REQUEST_SOURCE"] = b.GetHeadRef()
envs["VELA_PULL_REQUEST_TARGET"] = b.GetBaseRef()
envs["VELA_PULL_REQUEST_FORK"] = ToString(b.GetFork())
Copy link
Contributor

Choose a reason for hiding this comment

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

is this info attached to pull requests only? or can we do VELA_REPO_FORK and report it on non-PR events too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the only event that can be triggered on the main repo by content pushed to a fork of that repo is via the PR event. GitHub definitely has metadata about if the repo is a fork, but the only time that really matters is when it's attempting to trigger a build on the parent repo right?

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhh i understand what you mean, yeah i get it. sweet

}

// check if the Build event is tag
Expand Down Expand Up @@ -516,6 +518,19 @@ func (b *Build) GetSenderSCMID() string {
return *b.SenderSCMID
}

// GetFork returns the Fork field.
//
// When the provided Build type is nil, or the field within
// the type is nil, it returns the zero value for the field.
func (b *Build) GetFork() bool {
// return zero value if Build type or Fork field is nil
if b == nil || b.Fork == nil {
return false
}

return *b.Fork
}

// GetAuthor returns the Author field.
//
// When the provided Build type is nil, or the field within
Expand Down Expand Up @@ -971,6 +986,19 @@ func (b *Build) SetSenderSCMID(v string) {
b.SenderSCMID = &v
}

// SetFork sets the Fork field.
//
// When the provided Build type is nil, it
// will set nothing and immediately return.
func (b *Build) SetFork(v bool) {
// return if Build type is nil
if b == nil {
return
}

b.Fork = &v
}

// SetAuthor sets the Author field.
//
// When the provided Build type is nil, it
Expand Down Expand Up @@ -1148,6 +1176,7 @@ func (b *Build) String() string {
Event: %s,
EventAction: %s,
Finished: %d,
Fork: %t,
HeadRef: %s,
Host: %s,
ID: %d,
Expand Down Expand Up @@ -1184,6 +1213,7 @@ func (b *Build) String() string {
b.GetEvent(),
b.GetEventAction(),
b.GetFinished(),
b.GetFork(),
b.GetHeadRef(),
b.GetHost(),
b.GetID(),
Expand Down
18 changes: 18 additions & 0 deletions api/types/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestTypes_Build_Environment(t *testing.T) {
_pull.SetEvent("pull_request")
_pull.SetEventAction("opened")
_pull.SetRef("refs/pulls/1/head")
_pull.SetFork(false)

_tag := testBuild()
_tag.SetEvent("tag")
Expand Down Expand Up @@ -363,6 +364,7 @@ func TestTypes_Build_Environment(t *testing.T) {
"VELA_PULL_REQUEST": "1",
"VELA_PULL_REQUEST_SOURCE": "changes",
"VELA_PULL_REQUEST_TARGET": "",
"VELA_PULL_REQUEST_FORK": "false",
"BUILD_AUTHOR": "OctoKitty",
"BUILD_AUTHOR_EMAIL": "[email protected]",
"BUILD_BASE_REF": "",
Expand Down Expand Up @@ -563,6 +565,14 @@ func TestTypes_Build_Getters(t *testing.T) {
t.Errorf("GetSender is %v, want %v", test.build.GetSender(), test.want.GetSender())
}

if test.build.GetSenderSCMID() != test.want.GetSenderSCMID() {
t.Errorf("GetSenderSCMID is %v, want %v", test.build.GetSenderSCMID(), test.want.GetSenderSCMID())
}

if test.build.GetFork() != test.want.GetFork() {
t.Errorf("GetFork is %v, want %v", test.build.GetFork(), test.want.GetFork())
}

if test.build.GetAuthor() != test.want.GetAuthor() {
t.Errorf("GetAuthor is %v, want %v", test.build.GetAuthor(), test.want.GetAuthor())
}
Expand Down Expand Up @@ -657,6 +667,7 @@ func TestTypes_Build_Setters(t *testing.T) {
test.build.SetCommit(test.want.GetCommit())
test.build.SetSender(test.want.GetSender())
test.build.SetSenderSCMID(test.want.GetSenderSCMID())
test.build.SetFork(test.want.GetFork())
test.build.SetAuthor(test.want.GetAuthor())
test.build.SetEmail(test.want.GetEmail())
test.build.SetLink(test.want.GetLink())
Expand Down Expand Up @@ -762,6 +773,10 @@ func TestTypes_Build_Setters(t *testing.T) {
t.Errorf("SetSenderSCMID is %v, want %v", test.build.GetSenderSCMID(), test.want.GetSenderSCMID())
}

if test.build.GetFork() != test.want.GetFork() {
t.Errorf("SetFork is %v, want %v", test.build.GetFork(), test.want.GetFork())
}

if test.build.GetAuthor() != test.want.GetAuthor() {
t.Errorf("SetAuthor is %v, want %v", test.build.GetAuthor(), test.want.GetAuthor())
}
Expand Down Expand Up @@ -835,6 +850,7 @@ func TestTypes_Build_String(t *testing.T) {
Event: %s,
EventAction: %s,
Finished: %d,
Fork: %t,
HeadRef: %s,
Host: %s,
ID: %d,
Expand Down Expand Up @@ -871,6 +887,7 @@ func TestTypes_Build_String(t *testing.T) {
b.GetEvent(),
b.GetEventAction(),
b.GetFinished(),
b.GetFork(),
b.GetHeadRef(),
b.GetHost(),
b.GetID(),
Expand Down Expand Up @@ -925,6 +942,7 @@ func testBuild() *Build {
b.SetCommit("48afb5bdc41ad69bf22588491333f7cf71135163")
b.SetSender("OctoKitty")
b.SetSenderSCMID("123")
b.SetFork(false)
b.SetAuthor("OctoKitty")
b.SetEmail("[email protected]")
b.SetLink("https://example.company.com/github/octocat/1")
Expand Down
1 change: 1 addition & 0 deletions api/types/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type OpenIDClaims struct {
Image string `json:"image,omitempty"`
ImageName string `json:"image_name,omitempty"`
ImageTag string `json:"image_tag,omitempty"`
PullFork bool `json:"pull_fork,omitempty"`
Ref string `json:"ref,omitempty"`
Repo string `json:"repo,omitempty"`
Request string `json:"request,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ func PostWebhook(c *gin.Context) {
responded := false

// if the webhook was from a Pull event from a forked repository, verify it is allowed to run
if webhook.PullRequest.IsFromFork {
if b.GetFork() {
l.Tracef("inside %s workflow for fork PR build %s/%d", repo.GetApproveBuild(), repo.GetFullName(), b.GetNumber())

switch repo.GetApproveBuild() {
Expand Down
10 changes: 5 additions & 5 deletions compiler/native/environment_test.go

Large diffs are not rendered by default.

15 changes: 5 additions & 10 deletions database/build/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,24 @@ import (

"github.com/sirupsen/logrus"

api "github.com/go-vela/server/api/types"
"github.com/go-vela/server/constants"
"github.com/go-vela/server/database/types"
)

// CleanBuilds updates builds to an error with a provided message with a created timestamp prior to a defined moment.
func (e *engine) CleanBuilds(ctx context.Context, msg string, before int64) (int64, error) {
logrus.Tracef("cleaning pending or running builds created prior to %d", before)

b := new(api.Build)
b.SetStatus(constants.StatusError)
b.SetError(msg)
b.SetFinished(time.Now().UTC().Unix())

build := types.BuildFromAPI(b)

// send query to the database
result := e.client.
WithContext(ctx).
Table(constants.TableBuild).
Where("created < ?", before).
Where("status = 'running' OR status = 'pending'").
Updates(build)
Updates(map[string]interface{}{
"status": constants.StatusError,
"error": msg,
"finished": time.Now().UTC().Unix(),
})

return result.RowsAffected, result.Error
}
4 changes: 2 additions & 2 deletions database/build/clean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func TestBuild_Engine_CleanBuilds(t *testing.T) {
defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()

// ensure the mock expects the name query
_mock.ExpectExec(`UPDATE "builds" SET "status"=$1,"error"=$2,"finished"=$3,"deploy_payload"=$4 WHERE created < $5 AND (status = 'running' OR status = 'pending')`).
WithArgs("error", "msg", NowTimestamp{}, AnyArgument{}, 3).
_mock.ExpectExec(`UPDATE "builds" SET "error"=$1,"finished"=$2,"status"=$3 WHERE created < $4 AND (status = 'running' OR status = 'pending')`).
WithArgs("msg", NowTimestamp{}, "error", 3).
WillReturnResult(sqlmock.NewResult(1, 2))

_sqlite := testSqlite(t)
Expand Down
6 changes: 3 additions & 3 deletions database/build/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ func TestBuild_Engine_CreateBuild(t *testing.T) {

// ensure the mock expects the query
_mock.ExpectQuery(`INSERT INTO "builds"
("repo_id","pipeline_id","number","parent","event","event_action","status","error","enqueued","created","started","finished","deploy","deploy_number","deploy_payload","clone","source","title","message","commit","sender","sender_scm_id","author","email","link","branch","ref","base_ref","head_ref","host","runtime","distribution","approved_at","approved_by","id")
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35) RETURNING "id"`).
WithArgs(1, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, AnyArgument{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1).
("repo_id","pipeline_id","number","parent","event","event_action","status","error","enqueued","created","started","finished","deploy","deploy_number","deploy_payload","clone","source","title","message","commit","sender","sender_scm_id","fork","author","email","link","branch","ref","base_ref","head_ref","host","runtime","distribution","approved_at","approved_by","id")
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36) RETURNING "id"`).
WithArgs(1, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, AnyArgument{}, nil, nil, nil, nil, nil, nil, nil, false, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1).
WillReturnRows(_rows)

_sqlite := testSqlite(t)
Expand Down
2 changes: 2 additions & 0 deletions database/build/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ builds (
commit VARCHAR(500),
sender VARCHAR(250),
sender_scm_id VARCHAR(250),
fork BOOLEAN,
author VARCHAR(250),
email VARCHAR(500),
link VARCHAR(1000),
Expand Down Expand Up @@ -82,6 +83,7 @@ builds (
'commit' TEXT,
sender TEXT,
sender_scm_id TEXT,
fork BOOLEAN,
author TEXT,
email TEXT,
link TEXT,
Expand Down
6 changes: 3 additions & 3 deletions database/build/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ func TestBuild_Engine_UpdateBuild(t *testing.T) {

// ensure the mock expects the query
_mock.ExpectExec(`UPDATE "builds"
SET "repo_id"=$1,"pipeline_id"=$2,"number"=$3,"parent"=$4,"event"=$5,"event_action"=$6,"status"=$7,"error"=$8,"enqueued"=$9,"created"=$10,"started"=$11,"finished"=$12,"deploy"=$13,"deploy_number"=$14,"deploy_payload"=$15,"clone"=$16,"source"=$17,"title"=$18,"message"=$19,"commit"=$20,"sender"=$21,"sender_scm_id"=$22,"author"=$23,"email"=$24,"link"=$25,"branch"=$26,"ref"=$27,"base_ref"=$28,"head_ref"=$29,"host"=$30,"runtime"=$31,"distribution"=$32,"approved_at"=$33,"approved_by"=$34
WHERE "id" = $35`).
WithArgs(1, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, AnyArgument{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1).
SET "repo_id"=$1,"pipeline_id"=$2,"number"=$3,"parent"=$4,"event"=$5,"event_action"=$6,"status"=$7,"error"=$8,"enqueued"=$9,"created"=$10,"started"=$11,"finished"=$12,"deploy"=$13,"deploy_number"=$14,"deploy_payload"=$15,"clone"=$16,"source"=$17,"title"=$18,"message"=$19,"commit"=$20,"sender"=$21,"sender_scm_id"=$22,"fork"=$23,"author"=$24,"email"=$25,"link"=$26,"branch"=$27,"ref"=$28,"base_ref"=$29,"head_ref"=$30,"host"=$31,"runtime"=$32,"distribution"=$33,"approved_at"=$34,"approved_by"=$35
WHERE "id" = $36`).
WithArgs(1, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, AnyArgument{}, nil, nil, nil, nil, nil, nil, nil, false, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1).
WillReturnResult(sqlmock.NewResult(1, 1))

_sqlite := testSqlite(t)
Expand Down
2 changes: 2 additions & 0 deletions database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2539,6 +2539,7 @@ func newResources() *Resources {
buildOne.SetCommit("48afb5bdc41ad69bf22588491333f7cf71135163")
buildOne.SetSender("OctoKitty")
buildOne.SetSenderSCMID("123")
buildOne.SetFork(false)
buildOne.SetAuthor("OctoKitty")
buildOne.SetEmail("[email protected]")
buildOne.SetLink("https://example.company.com/github/octocat/1")
Expand Down Expand Up @@ -2576,6 +2577,7 @@ func newResources() *Resources {
buildTwo.SetCommit("48afb5bdc41ad69bf22588491333f7cf71135164")
buildTwo.SetSender("OctoKitty")
buildTwo.SetSenderSCMID("123")
buildTwo.SetFork(false)
buildTwo.SetAuthor("OctoKitty")
buildTwo.SetEmail("[email protected]")
buildTwo.SetLink("https://example.company.com/github/octocat/2")
Expand Down
1 change: 1 addition & 0 deletions database/testutils/api_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func APIBuild() *api.Build {
Commit: new(string),
Sender: new(string),
SenderSCMID: new(string),
Fork: new(bool),
Author: new(string),
Email: new(string),
Link: new(string),
Expand Down
3 changes: 3 additions & 0 deletions database/types/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type Build struct {
Commit sql.NullString `sql:"commit"`
Sender sql.NullString `sql:"sender"`
SenderSCMID sql.NullString `sql:"sender_scm_id"`
Fork sql.NullBool `sql:"fork"`
Author sql.NullString `sql:"author"`
Email sql.NullString `sql:"email"`
Link sql.NullString `sql:"link"`
Expand Down Expand Up @@ -315,6 +316,7 @@ func (b *Build) ToAPI() *api.Build {
build.SetCommit(b.Commit.String)
build.SetSender(b.Sender.String)
build.SetSenderSCMID(b.SenderSCMID.String)
build.SetFork(b.Fork.Bool)
build.SetAuthor(b.Author.String)
build.SetEmail(b.Email.String)
build.SetLink(b.Link.String)
Expand Down Expand Up @@ -401,6 +403,7 @@ func BuildFromAPI(b *api.Build) *Build {
Commit: sql.NullString{String: b.GetCommit(), Valid: true},
Sender: sql.NullString{String: b.GetSender(), Valid: true},
SenderSCMID: sql.NullString{String: b.GetSenderSCMID(), Valid: true},
Fork: sql.NullBool{Bool: b.GetFork(), Valid: true},
Author: sql.NullString{String: b.GetAuthor(), Valid: true},
Email: sql.NullString{String: b.GetEmail(), Valid: true},
Link: sql.NullString{String: b.GetLink(), Valid: true},
Expand Down
4 changes: 4 additions & 0 deletions database/types/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestTypes_Build_Nullify(t *testing.T) {
Message: sql.NullString{String: "", Valid: false},
Commit: sql.NullString{String: "", Valid: false},
Sender: sql.NullString{String: "", Valid: false},
Fork: sql.NullBool{Bool: false, Valid: false},
Author: sql.NullString{String: "", Valid: false},
Email: sql.NullString{String: "", Valid: false},
Link: sql.NullString{String: "", Valid: false},
Expand Down Expand Up @@ -134,6 +135,7 @@ func TestTypes_Build_ToAPI(t *testing.T) {
want.SetCommit("48afb5bdc41ad69bf22588491333f7cf71135163")
want.SetSender("OctoKitty")
want.SetSenderSCMID("123")
want.SetFork(false)
want.SetAuthor("OctoKitty")
want.SetEmail("[email protected]")
want.SetLink("https://example.company.com/github/octocat/1")
Expand Down Expand Up @@ -230,6 +232,7 @@ func TestTypes_Build_BuildFromAPI(t *testing.T) {
b.SetCommit("48afb5bdc41ad69bf22588491333f7cf71135163")
b.SetSender("OctoKitty")
b.SetSenderSCMID("123")
b.SetFork(false)
b.SetAuthor("OctoKitty")
b.SetEmail("[email protected]")
b.SetLink("https://example.company.com/github/octocat/1")
Expand Down Expand Up @@ -294,6 +297,7 @@ func testBuild() *Build {
Commit: sql.NullString{String: "48afb5bdc41ad69bf22588491333f7cf71135163", Valid: true},
Sender: sql.NullString{String: "OctoKitty", Valid: true},
SenderSCMID: sql.NullString{String: "123", Valid: true},
Fork: sql.NullBool{Bool: false, Valid: true},
Author: sql.NullString{String: "OctoKitty", Valid: true},
Email: sql.NullString{String: "[email protected]", Valid: true},
Link: sql.NullString{String: "https://example.company.com/github/octocat/1", Valid: true},
Expand Down
3 changes: 3 additions & 0 deletions internal/token/mint.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type Claims struct {
IsActive bool `json:"is_active,omitempty"`
IsAdmin bool `json:"is_admin,omitempty"`
Repo string `json:"repo,omitempty"`
PullFork bool `json:"pull_fork,omitempty"`
TokenType string `json:"token_type,omitempty"`
Image string `json:"image,omitempty"`
Request string `json:"request,omitempty"`
Expand Down Expand Up @@ -103,6 +104,7 @@ func (tm *Manager) MintToken(mto *MintTokenOpts) (string, error) {
}

claims.Repo = mto.Repo
claims.PullFork = mto.Build.GetFork()
claims.Subject = fmt.Sprintf("repo:%s:ref:%s:event:%s", mto.Repo, mto.Build.GetRef(), mto.Build.GetEvent())
claims.BuildID = mto.Build.GetID()
claims.BuildNumber = mto.Build.GetNumber()
Expand Down Expand Up @@ -162,6 +164,7 @@ func (tm *Manager) MintIDToken(ctx context.Context, mto *MintTokenOpts, db datab
claims.BuildID = mto.Build.GetID()
claims.Repo = mto.Repo
claims.Event = fmt.Sprintf("%s:%s", mto.Build.GetEvent(), mto.Build.GetEventAction())
claims.PullFork = mto.Build.GetFork()
claims.SHA = mto.Build.GetCommit()
claims.Ref = mto.Build.GetRef()
claims.Subject = fmt.Sprintf("repo:%s:ref:%s:event:%s", mto.Repo, mto.Build.GetRef(), mto.Build.GetEvent())
Expand Down
7 changes: 3 additions & 4 deletions internal/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ var (
// PullRequest defines the data pulled from PRs while
// processing a webhook.
type PullRequest struct {
Comment string
Number int
IsFromFork bool
Labels []string
Comment string
Number int
Labels []string
}

// Webhook defines a struct that is used to return
Expand Down
1 change: 1 addition & 0 deletions mock/server/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const (
"commit": "48afb5bdc41ad69bf22588491333f7cf71135163",
"sender": "OctoKitty",
"sender_scm_id": "0",
"fork": false,
"author": "OctoKitty",
"email": "[email protected]",
"link": "https://vela.example.company.com/github/octocat/1",
Expand Down
1 change: 1 addition & 0 deletions router/middleware/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func TestBuild_Establish(t *testing.T) {
want.SetCommit("")
want.SetSender("")
want.SetSenderSCMID("")
want.SetFork(false)
want.SetAuthor("")
want.SetEmail("")
want.SetLink("")
Expand Down
Loading
Loading