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

Support using YAML's anchors and aliases in runbooks #722

Merged
merged 2 commits into from Dec 31, 2023

Conversation

h6ah4i
Copy link
Contributor

@h6ah4i h6ah4i commented Dec 30, 2023

This pull request introduces YAML's anchors & aliases support in runbooks.

example

The aliases are very useful if you want to re-use same fields multiple times.

my_aliases:
  common_request_headers: &common_request_headers
    authorization: 'Bearer **********'
    accept: 'application/json'

steps:
  - req:
      /api-foo:
        get:
          headers:
            <<: *common_request_headers
            additionnal-header: foo
          body:
            application/json: null

  - req:
      /api-bar:
        get:
          headers:
            <<: *common_request_headers
            additionnal-header: bar
          body:
            application/json: null

FYI: This PR is the 4th one that I implemented yesterday.

  1. https://github.com/h6ah4i/runn/tree/feature/add-built-in-function-pick (Thx, already merged 🙏)
  2. https://github.com/h6ah4i/runn/tree/feature/add-built-in-function-omit (Thx, already merged 🙏)
  3. https://github.com/h6ah4i/runn/tree/feature/add-built-in-function-merge (Thx, already merged 🙏)
  4. https://github.com/h6ah4i/runn/tree/feature/support-yaml-anchor-alias-handling 👈 This pull request

@h6ah4i h6ah4i force-pushed the feature/support-yaml-anchor-alias-handling branch from 19949dc to acd345e Compare December 30, 2023 06:26
@k1LoW k1LoW added enhancement New feature or request minor labels Dec 30, 2023
@k1LoW
Copy link
Owner

k1LoW commented Dec 30, 2023

@h6ah4i Thank you for your GREAT WORK!!

That is a very powerful feature. And since it is a YAML feature, it is good that it does not affect the runn features.

By the way, can you think of any disadvantages of implementing this powerful feature?
It seems too powerful to me and I would like to have some input as to whether or not it is a good idea to introduce it.

@k2tzumi I would like to hear your opinion too!

@k1LoW k1LoW requested a review from k2tzumi December 30, 2023 06:44
@h6ah4i
Copy link
Contributor Author

h6ah4i commented Dec 30, 2023

By the way, can you think of any disadvantages of implementing this powerful feature?
It seems too powerful to me and I would like to have some input as to whether or not it is a good idea to introduce it.

I think anchors and aliases are basic YAML feature and it's natural that we can use them. However, in terms of the implementation it depends on goccy/go-yaml heavily and it may introduce some incompatibilities that the current test code does not cover.

@k2tzumi
Copy link
Collaborator

k2tzumi commented Dec 30, 2023

@h6ah4i
Thanks for all the suggestions for improvements!

There was a time when I too was wondering why docker-compose has an anchor function and GitHub Actions does not.

I have not checked the behavior with the improved content, but if an error occurs in a step expanded by this anchor function, how will the error content be displayed?

If it is a configuration file, the Anchor function works very well, but when describing a flow like Github Actions, I think it is also important to be able to easily understand the step when an error is displayed.

@h6ah4i h6ah4i force-pushed the feature/support-yaml-anchor-alias-handling branch from acd345e to ee1c804 Compare December 30, 2023 07:05
@k1LoW
Copy link
Owner

k1LoW commented Dec 30, 2023

I have not checked the behavior with the improved content, but if an error occurs in a step expanded by this anchor function, how will the error content be displayed?

Adding {"testdata/book/yaml_anchor_alias.yml", 7}, to https://github.com/h6ah4i/runn/blob/ee1c8043dcb1fc8d74ae763b1759a03795a3ad6f/runbook_test.go#L210 might help us see that.

@h6ah4i h6ah4i force-pushed the feature/support-yaml-anchor-alias-handling branch from ee1c804 to 6edf31d Compare December 30, 2023 09:36
@h6ah4i h6ah4i force-pushed the feature/support-yaml-anchor-alias-handling branch from 6edf31d to 71b1d0a Compare December 30, 2023 09:42
@h6ah4i h6ah4i force-pushed the feature/support-yaml-anchor-alias-handling branch from 71b1d0a to d0ca454 Compare December 30, 2023 09:53
@h6ah4i
Copy link
Contributor Author

h6ah4i commented Dec 30, 2023

@k1LoW @k2tzumi

Thank you for your advice. I have added more test patterns to the TestPickStepYAML test.


For your reference, here is the example runn command output for error steps using aliases.
👉 yaml_anchor_alias_always_failure.yml

❯ go run cmd/runn/main.go run ./testdata/book/yaml_anchor_alias_always_failure.yml
F

1) ./testdata/book/yaml_anchor_alias_always_failure.yml b1497eb73af72ed261ca8858c6566a4646b8ec74
  Failure/Error: test failed on "YAML's anchor & alias check".steps.check_1 "with anchor & alias": condition is not true

  Condition:
    compare(vars.my_hash_anchor_merged, {a: 1, b: 2, c: 3, d: 100})
    │
    ├── compare(vars["my_hash_anchor_merged"], {a: 1, b: 2, c: 3, d: 100}) => false
    ├── vars["my_hash_anchor_merged"] => {"a":1,"b":2,"c":3,"d":4}
    └── {a: 1, b: 2, c: 3, d: 100} => {"a":1,"b":2,"c":3,"d":100}

  Failure step (./testdata/book/yaml_anchor_alias_always_failure.yml):
  24   check_1:
  25     desc: 'with anchor & alias'
  26     test: |
  27       compare(vars.my_hash_anchor_merged, {a: 1, b: 2, c: 3, d: 100})
  28

2) ./testdata/book/yaml_anchor_alias_always_failure.yml b1497eb73af72ed261ca8858c6566a4646b8ec74
  Failure/Error: test failed on "YAML's anchor & alias check".steps.check_2 "merging in HTTP header part": condition is not true

  Condition:
    current.res.status != 418
    │
    ├── current.res.status => 418
    └── 418 => 418

  Failure step (./testdata/book/yaml_anchor_alias_always_failure.yml):
  29   check_2:
  30     desc: 'merging in HTTP header part'
  31     httpbin:
  32       /status/418:
  33         get:
  34           headers:
  35             <<: *common_req_headers
  36     test:
  37       current.res.status != 418


1 scenario, 0 skipped, 1 failure
exit status 1

Copy link
Owner

@k1LoW k1LoW left a comment

Choose a reason for hiding this comment

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

Looks GREAT To Me!!!

@k1LoW
Copy link
Owner

k1LoW commented Dec 30, 2023

@h6ah4i
This is a big/great feature, so let me wait for @k2tzumi 's take on this.

@k2tzumi
Copy link
Collaborator

k2tzumi commented Dec 31, 2023

We have checked the output content in the event of an error.
For each of the following yaml, errors could be output without panic.

  • testdata/book/yaml_anchor_alias_always_failure_2.yml

    vars:
      param1: "pram1"
      param2: "pram2"
    alias_steps: &alias_steps_anchor
      check_1:
        desc: 'check1'
        dump: vars.param1
        test:
          true
      check_2:
        desc: 'check2'
        dump: vars.param2
        test:
          false
    steps: *alias_steps_anchor

    result.

    $ go run cmd/runn/main.go run testdata/book/yaml_anchor_alias_always_failure_2.yml
    pram1
    pram2
    F
    
    1) testdata/book/yaml_anchor_alias_always_failure_2.yml aa4500bd97be345f0f08986421b946fa09deae98
      Failure/Error: test failed on "[No Description]".steps.check_2 "check2": condition is not true
      
      Condition:
        false
    
        └── false => false
        
    Error: step not found: 1
    exit status 1
  • testdata/book/yaml_anchor_alias_always_failure_3.yml

    vars:
      param1: "pram1"
      param2: "pram2"
    alias_step1: &alias_step1_anchor
      desc: 'check1'
      dump: vars.param1
      test:
        true
    alias_step2: &alias_step2_anchor
      desc: 'check2'
      dump: vars.param2
      test:
        false
    steps:
      - *alias_step1_anchor
      - *alias_step2_anchor

    result.

    $ go run cmd/runn/main.go run testdata/book/yaml_anchor_alias_always_failure_3.yml
    pram1
    pram2
    F
    
    1) testdata/book/yaml_anchor_alias_always_failure_3.yml b1c45727fe87aa86169f37d23792c73b7c0f15e6
      Failure/Error: test failed on "Generated by `runn new`".steps[1] "check2": condition is not true
      
      Condition:
        false
    
        └── false => false
        
      Failure step (testdata/book/yaml_anchor_alias_always_failure_3.yml):
      16   - *alias_step2_anchor
    
    
    1 scenario, 0 skipped, 1 failure
    exit status 1

Although this is an extreme case, I thought it would be good to be able to identify the error even with this content 👍

Copy link
Collaborator

@k2tzumi k2tzumi left a comment

Choose a reason for hiding this comment

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

LGTM!

@k2tzumi k2tzumi merged commit 99ba378 into k1LoW:main Dec 31, 2023
5 checks passed
@github-actions github-actions bot mentioned this pull request Dec 30, 2023
@h6ah4i
Copy link
Contributor Author

h6ah4i commented Dec 31, 2023

Thank you so much for accepting my proposal🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants