Skip to content

fix: allow yaml anchors - #5893

Closed
ChristopherHX wants to merge 2 commits into
masterfrom
fix-yaml-anchors
Closed

fix: allow yaml anchors#5893
ChristopherHX wants to merge 2 commits into
masterfrom
fix-yaml-anchors

Conversation

@ChristopherHX

Copy link
Copy Markdown
Contributor
  • GitHub Actions finally supports them

See actions/runner#1182 (comment)

* GitHub Actions finally supports them
@github-actions

github-actions Bot commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 2 0 0.01s
✅ REPOSITORY gitleaks yes no 2.77s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY grype yes no 14.4s
✅ REPOSITORY secretlint yes no 1.22s
✅ REPOSITORY trivy-sbom yes no 0.38s
✅ REPOSITORY trufflehog yes no 4.04s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@mergify mergify Bot added the needs-work Extra attention is needed label Aug 4, 2025
@pull-request-size pull-request-size Bot added size/S and removed size/M labels Aug 4, 2025
@codecov

codecov Bot commented Aug 4, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.74%. Comparing base (bd4bc99) to head (5e62222).
⚠️ Report is 25 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5893      +/-   ##
==========================================
+ Coverage   74.65%   74.74%   +0.08%     
==========================================
  Files          73       73              
  Lines       11139    11193      +54     
==========================================
+ Hits         8316     8366      +50     
- Misses       2186     2190       +4     
  Partials      637      637              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify mergify Bot removed the needs-work Extra attention is needed label Aug 4, 2025
Comment thread pkg/schema/schema_test.go
job-with-condition:
runs-on: &label
self-hosted
if: success() || success('joba', 'jobb') || failure() || failure('joba', 'jobb') || always() || cancelled()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We may want to remove this if, since the parameters of success are removed (except from the schema)

Suggested change
if: success() || success('joba', 'jobb') || failure() || failure('joba', 'jobb') || always() || cancelled()

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

At the moment still broken, expressions inside anchors. EvaluateYamlNode needs updates as well to evaluate *os action expressions.

on: push
jobs:
  _:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        os:
        - &os ${{ github.event_name }}
        - *os
        - *os
    steps:
      - name: Checkout
        uses: actions/checkout@v4
      - name: Run Tests
        run: echo 'Running tests... ${{ matrix.os }}'

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

Broken as well

jobs:
  _:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        on: &matrix [push, pull_request]
    steps:
      - name: Checkout
        uses: actions/checkout@v4
      - name: Run Tests
        run: echo 'Running tests...'
on: *matrix

Comment thread pkg/schema/schema.go
if node != nil && node.Kind == yaml.DocumentNode {
return s.UnmarshalYAML(node.Content[0])
}
if node.Kind == yaml.AliasNode {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if node.Kind == yaml.AliasNode {
if node != nil && node.Kind == yaml.AliasNode {

to match the if above so you don't crash on nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

or maybe just return an error if nil is passed in since checkExpression a few lines down would just crash next

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have checked this yaml in a test

func TestNull(t *testing.T) {
	var node yaml.Node
	err := yaml.Unmarshal([]byte(`
on: push
jobs:
  job-with-condition:
    runs-on: self-hosted
    steps:
    -
`), &node)
	if !assert.NoError(t, err) {
		return
	}
	err = (&Node{
		Definition: "workflow-root-strict",
		Schema:     GetWorkflowSchema(),
	}).UnmarshalYAML(&node)
	assert.NoError(t, err)
}

Shows a non zero node passed with !null tag, by nested elements. I assume this can not crash at all.

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

After more consideration, due to yaml anchor problems across the codebase. I am going to discard this PR in favor of https://patch-diff.githubusercontent.com/raw/actions-oss/act-cli/pull/126.patch, it is better to explode yaml anchors right away that fixes a lot of other bugs documented here.

@ChristopherHX
ChristopherHX deleted the fix-yaml-anchors branch September 19, 2025 17:38
@nopeless

nopeless commented Oct 7, 2025

Copy link
Copy Markdown

would love to see a tracking issue for this @ChristopherHX if you don't mind

@ChristopherHX

Copy link
Copy Markdown
Contributor Author

@nopeless Please subscribe to #5947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants