Skip to content

Conversation

@jsoriano
Copy link
Member

Variables in policy templates can have dots in their names. This names are defined as plain strings in yaml files. For example like this:

      - name: jmx.mappings
        type: yaml
        title: JMX Mappings
        multi: false
        required: true
        show_user: true

These variables can be overridden in system tests configuration files, for example like this:

vars:
  hosts:
    - {{Hostname}}:{{Port}}
  jmx.mappings: |
    - mbean: 'java.lang:type=Runtime'
      attributes:
        - attr: Uptime
          field: uptime

These configuration files are parsed with go-ucfg, configured to use . as path separator, so variable names containing dots are expanded. Also, when parsed, they are parsed into an opaque VarVariable structure, so it is not possible to access their content, so overrides cannot be defined for variables with dots in their names.
This happens for example in elastic/integrations#4706.

This issue could be easily solved if ucfg would support preserve as defined in elastic/go-ucfg#124.

This change introduces a hacky workaround to give access to child elements, so they can be also used for overrides.

Alternatives considered:

  • Completely disable variable expansion when parsing this configuration. This could produce breaking changes, and could be misleading, as developers are used to this kind of configurations where handling of objects structure is transparent for them.
  • Implement preserve as defined in Control Structure on unpack go-ucfg#124. Probably a rabbit hole.
  • Parse the configuration as common.MapStr instead of map[string]VarValue, and generate the VarValues later. This could be cleaner, but require some refactors. I may give it a try though.

@jsoriano jsoriano requested a review from a team November 23, 2022 15:58
@jsoriano jsoriano self-assigned this Nov 23, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 23, 2022

❕ Build Aborted

There is a new build on-going so the previous on-going builds have been aborted.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-23T16:22:52.139+0000

  • Duration: 6 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 458
Skipped 0
Total 458

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@jsoriano
Copy link
Member Author

  • Parse the configuration as common.MapStr instead of map[string]VarValue, and generate the VarValues later. This could be cleaner, but require some refactors. I may give it a try though.

Ok, this ended up being much more clear: #1049.

@jsoriano jsoriano closed this Nov 23, 2022
@elasticmachine
Copy link
Collaborator

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-23T16:26:36.975+0000

  • Duration: 28 min 2 sec

Test stats 🧪

Test Results
Failed 0
Passed 866
Skipped 0
Total 866

Steps errors 1

Expand to view the steps failures

Check
  • Took 4 min 7 sec . View more details here
  • Description: make install test-stack-command-8x check-git-clean

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Collaborator

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (35/35) 💚
Files 67.692% (88/130) 👍 0.769
Classes 62.162% (115/185) 👍 0.541
Methods 47.516% (373/785) 👍 0.073
Lines 30.33% (3329/10976) 👎 -0.029
Conditionals 100.0% (0/0) 💚

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants