Skip to content

Commit

Permalink
Merge branch 'issue-402' (fix #402)
Browse files Browse the repository at this point in the history
rhysd committed Feb 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents 3e2f8ea + 94bf6f3 commit 2eeaa12
Showing 10 changed files with 81 additions and 16 deletions.
17 changes: 15 additions & 2 deletions ast.go
Original file line number Diff line number Diff line change
@@ -762,6 +762,19 @@ type Service struct {
Container *Container
}

// Services is a mapping from service ID to its configuration.
// https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idservices
type Services struct {
// Value is a mapping from service ID to its Service instances. Keys are in lower case since
// they are case-insensitive.
Value map[string]*Service
// Expression is an expression assigned to the services mapping by ${{ }} placeholder. Otherwise
// this field is nil.
Expression *String
// Pos is a position in source.
Pos *Pos
}

// Output is output entry of the job.
// https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_idoutputs
type Output struct {
@@ -863,9 +876,9 @@ type Job struct {
ContinueOnError *Bool
// Container is container configuration to run the job.
Container *Container
// Services is map from service names to service configurations. Keys are in lower case since they are case-insensitive.
// Services is a services configurations.
// https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_idservices
Services map[string]*Service
Services *Services
// WorkflowCall is a workflow call by 'uses:'.
// https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_iduses
WorkflowCall *WorkflowCall
27 changes: 19 additions & 8 deletions parse.go
Original file line number Diff line number Diff line change
@@ -970,6 +970,24 @@ func (p *parser) parseContainer(sec string, pos *Pos, n *yaml.Node) *Container {
return ret
}

func (p *parser) parseServices(n *yaml.Node) *Services {
ret := &Services{Pos: posAt(n)}
if e := p.mayParseExpression(n); e != nil {
ret.Expression = e
} else {
services := p.parseSectionMapping("services", n, false, false) // XXX: Is the key case-insensitive?
ss := make(map[string]*Service, len(services))
for _, s := range services {
ss[s.id] = &Service{
Name: s.key,
Container: p.parseContainer("services", s.key.Pos, s.val),
}
}
ret.Value = ss
}
return ret
}

// https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes
func (p *parser) parseTimeoutMinutes(n *yaml.Node) *Float {
f := p.parseFloat(n)
@@ -1205,14 +1223,7 @@ func (p *parser) parseJob(id *String, n *yaml.Node) *Job {
ret.Container = p.parseContainer("container", k.Pos, v)
stepsOnlyKey = k
case "services":
services := p.parseSectionMapping("services", v, false, false) // XXX: Is the key case-insensitive?
ret.Services = make(map[string]*Service, len(services))
for _, s := range services {
ret.Services[s.id] = &Service{
Name: s.key,
Container: p.parseContainer("services", s.key.Pos, s.val),
}
}
ret.Services = p.parseServices(v)
case "uses":
call.Uses = p.parseString(v, false)
callOnlyKey = k
6 changes: 4 additions & 2 deletions rule_credentials.go
Original file line number Diff line number Diff line change
@@ -24,8 +24,10 @@ func (rule *RuleCredentials) VisitJobPre(n *Job) error {
if n.Container != nil {
rule.checkContainer("\"container\" section", n.Container)
}
for _, s := range n.Services {
rule.checkContainer(fmt.Sprintf("%q service", s.Name.Value), s.Container)
if n.Services != nil {
for _, s := range n.Services.Value {
rule.checkContainer(fmt.Sprintf("%q service", s.Name.Value), s.Container)
}
}
return nil
}
6 changes: 4 additions & 2 deletions rule_env_var.go
Original file line number Diff line number Diff line change
@@ -29,8 +29,10 @@ func (rule *RuleEnvVar) VisitJobPre(n *Job) error {
if n.Container != nil {
rule.checkEnv(n.Container.Env)
}
for _, s := range n.Services {
rule.checkEnv(s.Container.Env)
if n.Services != nil {
for _, s := range n.Services.Value {
rule.checkEnv(s.Container.Env)
}
}
return nil
}
7 changes: 5 additions & 2 deletions rule_expression.go
Original file line number Diff line number Diff line change
@@ -238,8 +238,11 @@ func (rule *RuleExpression) VisitJobPre(n *Job) error {
rule.checkFloat(n.TimeoutMinutes, "jobs.<job_id>.timeout-minutes")
rule.checkContainer(n.Container, "jobs.<job_id>.container", "")

for _, s := range n.Services {
rule.checkContainer(s.Container, "jobs.<job_id>.services", "<service_id>")
if n.Services != nil {
rule.checkObjectExpression(n.Services.Expression, "services", "jobs.<job_id>.services")
for _, s := range n.Services.Value {
rule.checkContainer(s.Container, "jobs.<job_id>.services", "<service_id>")
}
}

rule.checkWorkflowCall(n.WorkflowCall)
1 change: 1 addition & 0 deletions testdata/err/context_availability.out
Original file line number Diff line number Diff line change
@@ -30,3 +30,4 @@
/test\.yaml:200:22: context "runner" is not allowed here\. .+ \[expression\]/
/test\.yaml:208:19: context "runner" is not allowed here\. .+ \[expression\]/
/test\.yaml:210:18: context "runner" is not allowed here\. .+ \[expression\]/
/test\.yaml:217:34: context "env" is not allowed here\. .+ \[expression\]/
7 changes: 7 additions & 0 deletions testdata/err/context_availability.yaml
Original file line number Diff line number Diff line change
@@ -210,3 +210,10 @@ jobs:
group: ${{ runner.OS }}
steps:
- run: echo hello
test-services:
runs-on: ubuntu-latest
# jobs.<job_id>.services
# ERROR at env
services: ${{ inputs.bool || env.FOO }}
steps:
- run: echo
2 changes: 2 additions & 0 deletions testdata/err/expr_check_in_services.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test.yaml:9:15: "services" section is scalar node but mapping node is expected [syntax-check]
test.yaml:14:15: type of expression at "services" must be object but found type string [expression]
17 changes: 17 additions & 0 deletions testdata/err/expr_check_in_services.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
on: push
jobs:
ok1:
services: ${{ fromJSON('...') }}
runs-on: ubuntu-latest
steps:
- run: echo
err1:
services: redis
runs-on: ubuntu-latest
steps:
- run: echo
err2:
services: ${{ 'redis' }}
runs-on: ubuntu-latest
steps:
- run: echo
7 changes: 7 additions & 0 deletions testdata/ok/issue-402-expression-in-services.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
on: push
jobs:
tests:
runs-on: ubuntu-latest
services: ${{ fromJSON('...') }}
steps:
- run: echo

0 comments on commit 2eeaa12

Please sign in to comment.