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

Dedup pipeline compile code and unwind dependency issues #1401

Closed
wants to merge 2 commits into from
Closed

Dedup pipeline compile code and unwind dependency issues #1401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24b4771
start
6543 Nov 6, 2022
6cddee4
document
6543 Nov 6, 2022
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: 2 additions & 0 deletions cli/exec/exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func runExec(c *cli.Context, file, repoPath string) error {
return err
}

// use pipeline.StepBuilder
axes, err := matrix.ParseString(string(dat))
if err != nil {
return fmt.Errorf("Parse matrix fail")
Expand Down Expand Up @@ -166,6 +167,7 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error
}

// lint the yaml file
// TODO: trusted = flag
if lerr := linter.New(linter.WithTrusted(true)).Lint(conf); lerr != nil {
return lerr
}
Expand Down
15 changes: 8 additions & 7 deletions docs/docs/92-development/05-architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@

### main package hierarchy

| package | meaning | imports
|------------|--------------------------------------------------------------|----------
| `cmd/**` | parse command-line args & environment to stat server/cli/agent | all other
| `agent/**` | code only agent (remote worker) will need | `pipeline`, `shared`
| `cli/**` | code only cli tool does need | `pipeline`, `shared`, `woodpecker-go`
| `server/**`| code only server will need | `pipeline`, `shared`
| `shared/**`| code shared for all three main tools (go help utils) | only std and external libs
| package | meaning | imports
|---------------|--------------------------------------------------------------|----------
| `cmd/**` | parse command-line args & environment to stat server/cli/agent | all other
| `agent/**` | code only agent (remote worker) will need | `pipeline`, `shared`
| `cli/**` | code only cli tool does need | `pipeline`, `shared`, `woodpecker-go`
| `pipeline/**` | code to parse, compile and run pipelines | pipeline backend libs
| `server/**` | code only server will need | `pipeline`, `shared`
| `shared/**` | code shared for all three main tools (go help utils) | only std and external libs
| `woodpecker-go/**` | go client for server rest api | std

### Server
Expand Down
45 changes: 40 additions & 5 deletions pipeline/stepBuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,44 @@ import (
"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/compiler"
"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/linter"
"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/matrix"

// TODO: remove server dependency
"github.com/woodpecker-ci/woodpecker/server"
forge_types "github.com/woodpecker-ci/woodpecker/server/forge/types"
Copy link
Member Author

Choose a reason for hiding this comment

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

we might need a pipeline/types package ...

"github.com/woodpecker-ci/woodpecker/server/model"
)

// StepBuilder Takes the hook data and the yaml and returns in internal data model
type StepBuilder struct {
type StepBuilder interface {
Build() ([]*Item, error)
CurrentPipeline() *model.Pipeline
}

func NewStepBuilder(
repo *model.Repo,
curr *model.Pipeline,
last *model.Pipeline,
netrc *model.Netrc,
secs []*model.Secret,
regs []*model.Registry,
link string,
yamls []*forge_types.FileMeta,
envs map[string]string,
) StepBuilder {
return &stepBuilder{
Repo: repo,
Curr: curr,
Last: last,
Netrc: netrc,
Secs: secs,
Regs: regs,
Link: link,
Yamls: yamls,
Envs: envs,
}
}

type stepBuilder struct {
Repo *model.Repo
Curr *model.Pipeline
Last *model.Pipeline
Expand All @@ -58,7 +89,11 @@ type Item struct {
Config *backend.Config
}

func (b *StepBuilder) Build() ([]*Item, error) {
func (b *stepBuilder) CurrentPipeline() *model.Pipeline {
return b.Curr
}

func (b *stepBuilder) Build() ([]*Item, error) {
var items []*Item

b.Yamls = forge_types.SortByName(b.Yamls)
Expand Down Expand Up @@ -216,7 +251,7 @@ func containsItemWithName(name string, items []*Item) bool {
return false
}

func (b *StepBuilder) envsubst(y string, environ map[string]string) (string, error) {
func (b *stepBuilder) envsubst(y string, environ map[string]string) (string, error) {
return envsubst.Eval(y, func(name string) string {
env := environ[name]
if strings.Contains(env, "\n") {
Expand All @@ -226,15 +261,15 @@ func (b *StepBuilder) envsubst(y string, environ map[string]string) (string, err
})
}

func (b *StepBuilder) environmentVariables(metadata frontend.Metadata, axis matrix.Axis) map[string]string {
func (b *stepBuilder) environmentVariables(metadata frontend.Metadata, axis matrix.Axis) map[string]string {
environ := metadata.Environ()
for k, v := range axis {
environ[k] = v
}
return environ
}

func (b *StepBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[string]string, metadata frontend.Metadata, stepID int64) (*backend.Config, error) {
func (b *stepBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[string]string, metadata frontend.Metadata, stepID int64) (*backend.Config, error) {
var secrets []compiler.Secret
for _, sec := range b.Secs {
if !sec.Match(b.Curr.Event) {
Expand Down
26 changes: 13 additions & 13 deletions pipeline/stepBuilder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
func TestGlobalEnvsubst(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Envs: map[string]string{
"KEY_K": "VALUE_V",
"IMAGE": "scratch",
Expand Down Expand Up @@ -60,7 +60,7 @@ pipeline:
func TestMissingGlobalEnvsubst(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Envs: map[string]string{
"KEY_K": "VALUE_V",
"NO_IMAGE": "scratch",
Expand Down Expand Up @@ -94,7 +94,7 @@ pipeline:
func TestMultilineEnvsubst(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{
Message: `aaa
Expand Down Expand Up @@ -131,7 +131,7 @@ pipeline:
func TestMultiPipeline(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -165,7 +165,7 @@ pipeline:
func TestDependsOn(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -211,7 +211,7 @@ depends_on:
func TestRunsOn(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -247,7 +247,7 @@ runs_on:
func TestPipelineName(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{Config: ".woodpecker"},
Curr: &model.Pipeline{},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -282,7 +282,7 @@ pipeline:
func TestBranchFilter(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{Branch: "dev"},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -328,7 +328,7 @@ pipeline:
func TestRootWhenFilter(t *testing.T) {
t.Parallel()

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: &model.Pipeline{Event: "tester"},
Last: &model.Pipeline{},
Expand Down Expand Up @@ -376,7 +376,7 @@ func TestZeroSteps(t *testing.T) {

pipeline := &model.Pipeline{Branch: "dev"}

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: pipeline,
Last: &model.Pipeline{},
Expand Down Expand Up @@ -410,7 +410,7 @@ func TestZeroStepsAsMultiPipelineDeps(t *testing.T) {

pipeline := &model.Pipeline{Branch: "dev"}

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: pipeline,
Last: &model.Pipeline{},
Expand Down Expand Up @@ -458,7 +458,7 @@ func TestZeroStepsAsMultiPipelineTransitiveDeps(t *testing.T) {

pipeline := &model.Pipeline{Branch: "dev"}

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: pipeline,
Last: &model.Pipeline{},
Expand Down Expand Up @@ -514,7 +514,7 @@ func TestTree(t *testing.T) {
Event: model.EventPush,
}

b := StepBuilder{
b := stepBuilder{
Repo: &model.Repo{},
Curr: pipeline,
Last: &model.Pipeline{},
Expand Down
16 changes: 6 additions & 10 deletions server/pipeline/filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,12 @@ import (
)

func zeroSteps(currentPipeline *model.Pipeline, forgeYamlConfigs []*forge_types.FileMeta) bool {
b := pipeline.StepBuilder{
Repo: &model.Repo{},
Curr: currentPipeline,
Last: &model.Pipeline{},
Netrc: &model.Netrc{},
Secs: []*model.Secret{},
Regs: []*model.Registry{},
Link: "",
Yamls: forgeYamlConfigs,
}
b := pipeline.NewStepBuilder(&model.Repo{}, currentPipeline, &model.Pipeline{},
&model.Netrc{}, []*model.Secret{}, []*model.Registry{},
"",
forgeYamlConfigs,
make(map[string]string),
)

pipelineItems, err := b.Build()
if err != nil {
Expand Down
19 changes: 7 additions & 12 deletions server/pipeline/items.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,12 @@ func createPipelineItems(ctx context.Context, store store.Store,
envs[k] = v
}

b := pipeline.StepBuilder{
Repo: repo,
Curr: currentPipeline,
Last: last,
Netrc: netrc,
Secs: secs,
Regs: regs,
Envs: envs,
Link: server.Config.Server.Host,
Yamls: yamls,
}
b := pipeline.NewStepBuilder(repo, currentPipeline, last,
netrc, secs, regs,
server.Config.Server.Host,
yamls,
envs)

pipelineItems, err := b.Build()
if err != nil {
currentPipeline, uerr := UpdateToStatusError(store, *currentPipeline, err)
Expand All @@ -87,7 +82,7 @@ func createPipelineItems(ctx context.Context, store store.Store,
return currentPipeline, nil, err
}

currentPipeline = pipeline.SetPipelineStepsOnPipeline(b.Curr, pipelineItems)
currentPipeline = pipeline.SetPipelineStepsOnPipeline(b.CurrentPipeline(), pipelineItems)

return currentPipeline, pipelineItems, nil
}