Skip to content

Add more generic support for var_groups#1120

Merged
mrodm merged 7 commits intoelastic:mainfrom
Supplementing:add-var-group-support-everywhere
Mar 31, 2026
Merged

Add more generic support for var_groups#1120
mrodm merged 7 commits intoelastic:mainfrom
Supplementing:add-var-group-support-everywhere

Conversation

@Supplementing
Copy link
Copy Markdown
Contributor

@Supplementing Supplementing commented Mar 23, 2026

Closes https://github.com/elastic/ingest-dev/issues/7186

What does this PR do?

This PR adds more generic support for usage of var_groups. Previously, we only allowed them at the root and streams level, this PR allows them to be used elsewhere.

Why is it important?

In order to support better organization in the UI of items based on grouping, being able to use the var_groups was paramount. See https://github.com/elastic/ingest-dev/issues/6979

Checklist

@Supplementing Supplementing requested a review from a team as a code owner March 23, 2026 21:11
@Supplementing Supplementing added the Team:Fleet Label for the Fleet team label Mar 23, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 762ee316-e6be-4695-a0be-8ce08f3b1a5b

📥 Commits

Reviewing files that changed from the base of the PR and between c7cec12 and d6dd7f3.

📒 Files selected for processing (2)
  • code/go/pkg/validator/validator_test.go
  • spec/changelog.yml
✅ Files skipped from review due to trivial changes (2)
  • code/go/pkg/validator/validator_test.go
  • spec/changelog.yml

📝 Walkthrough

Walkthrough

Schema support for var_groups was added to integration and input package manifests: new var_groups properties at package root, policy template, and input levels, plus migration patches removing them for versions before 3.6.0. A new changelog entry version: 3.6.1-next was added. Tests were extended (validator test) and a new test package good_var_groups_input was added (manifest, templates, fields, docs, changelog, license). The existing good_var_groups test package was updated with additional vars and var_groups. .DS_Store was added to .gitignore.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
spec/integration/manifest.spec.yml (1)

977-984: ⚠️ Potential issue | 🟠 Major

Reorder var_groups removals in the before: 3.6.0 patch.

/definitions/var_groups is removed before properties that still reference it. If patch sequencing enforces schema integrity, this can break backward-compat patching. Remove all .../var_groups properties first, then remove the definition.

Suggested fix
       # Switchable variable groups.
-      - op: remove
-        path: "/definitions/var_groups"
       - op: remove
         path: "/properties/var_groups"
       - op: remove
         path: "/properties/policy_templates/items/properties/var_groups"
       - op: remove
         path: "/properties/policy_templates/items/properties/inputs/items/properties/var_groups"
+      - op: remove
+        path: "/definitions/var_groups"
       - op: remove
         path: "/properties/policy_templates/items/properties/inputs/items/properties/hide_in_var_group_options"

As per coding guidelines: “In version patches, remove property references before removing the definitions they depend on to maintain schema integrity.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@spec/integration/manifest.spec.yml` around lines 977 - 984, Reorder the three
remove ops that target property paths so they run before the remove op for the
"/definitions/var_groups" definition: move the ops for "/properties/var_groups",
"/properties/policy_templates/items/properties/var_groups", and
"/properties/policy_templates/items/properties/inputs/items/properties/var_groups"
ahead of the op for "/definitions/var_groups" so all references to var_groups
are removed before the definition itself (keep the same op entries but change
their sequence).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@spec/changelog.yml`:
- Around line 27-29: The changelog entry in spec/changelog.yml currently uses a
link value of "https://github.com/elastic/package-spec/pull/PLACEHOLDER"; update
the link field to follow changelog conventions by replacing "PLACEHOLDER" with
"TBD" or the real PR URL ("https://github.com/elastic/package-spec/pull/1120")
so the YAML entry under description/type/link reflects the correct link format.

---

Outside diff comments:
In `@spec/integration/manifest.spec.yml`:
- Around line 977-984: Reorder the three remove ops that target property paths
so they run before the remove op for the "/definitions/var_groups" definition:
move the ops for "/properties/var_groups",
"/properties/policy_templates/items/properties/var_groups", and
"/properties/policy_templates/items/properties/inputs/items/properties/var_groups"
ahead of the op for "/definitions/var_groups" so all references to var_groups
are removed before the definition itself (keep the same op entries but change
their sequence).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e2d6dc18-1524-492d-b783-bf68947e9bce

📥 Commits

Reviewing files that changed from the base of the PR and between 3d8508e and 8ee00a9.

⛔ Files ignored due to path filters (8)
  • .DS_Store is excluded by !**/.DS_Store
  • .buildkite/.DS_Store is excluded by !**/.DS_Store
  • .github/.DS_Store is excluded by !**/.DS_Store
  • code/.DS_Store is excluded by !**/.DS_Store
  • compliance/.DS_Store is excluded by !**/.DS_Store
  • spec/.DS_Store is excluded by !**/.DS_Store
  • test/.DS_Store is excluded by !**/.DS_Store
  • test/packages/good_var_groups_input/img/sample-logo.svg is excluded by !**/*.svg
📒 Files selected for processing (11)
  • code/go/pkg/validator/validator_test.go
  • spec/changelog.yml
  • spec/input/manifest.spec.yml
  • spec/integration/manifest.spec.yml
  • test/packages/good_var_groups/manifest.yml
  • test/packages/good_var_groups_input/LICENSE.txt
  • test/packages/good_var_groups_input/agent/input/input.yml.hbs
  • test/packages/good_var_groups_input/changelog.yml
  • test/packages/good_var_groups_input/docs/README.md
  • test/packages/good_var_groups_input/fields/base-fields.yml
  • test/packages/good_var_groups_input/manifest.yml

Comment thread spec/changelog.yml Outdated
Supplementing added a commit to elastic/kibana that referenced this pull request Mar 24, 2026
## Summary

Closes elastic/ingest-dev#7185
Related: elastic/ingest-dev#6549 
Depends on elastic/package-spec#1120

This PR makes `var_groups` usable everywhere, rather than just at the
package root and streams level so that items can be rendered in their
appropriate sections (input, etc) and take full advantage of grouping.
This will unblock elastic/integrations#17495 and
allow us to finalize a draft version of this integration as a
poster-child that utilizes `var_groups` for a clean and minimal UI.

WIP integration utilizing the var_groups at the input level for grouping
based on chosen auth method:
<img width="1709" height="1198" alt="image"
src="https://github.com/user-attachments/assets/433307f4-dcd6-460f-a34f-2a5fa9dc6c3f"
/>


## Testing instructions

If you want to manually build an integration and test, please DM me and
I will send you detailed instructions or walk you through how to set up
a local package-spec and point your elastic-package at it, build it,
etc. Im happy to do so, but in the sake of brevity, I will skip that
here.

The easiest way otherwise is to simply upload this pre-built integration
that I built locally using the changes to package-spec:

[okta-3.14.2.zip](https://github.com/user-attachments/files/26195499/okta-3.14.2.zip)


### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
- [ ] Review the [backport
guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)
and apply applicable `backport:*` labels.

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Comment thread .buildkite/.DS_Store Outdated
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread spec/input/manifest.spec.yml
Comment thread spec/integration/manifest.spec.yml
@Supplementing Supplementing requested a review from mrodm March 25, 2026 18:51
jeramysoucy pushed a commit to jeramysoucy/kibana that referenced this pull request Mar 26, 2026
## Summary

Closes elastic/ingest-dev#7185
Related: elastic/ingest-dev#6549 
Depends on elastic/package-spec#1120

This PR makes `var_groups` usable everywhere, rather than just at the
package root and streams level so that items can be rendered in their
appropriate sections (input, etc) and take full advantage of grouping.
This will unblock elastic/integrations#17495 and
allow us to finalize a draft version of this integration as a
poster-child that utilizes `var_groups` for a clean and minimal UI.

WIP integration utilizing the var_groups at the input level for grouping
based on chosen auth method:
<img width="1709" height="1198" alt="image"
src="https://github.com/user-attachments/assets/433307f4-dcd6-460f-a34f-2a5fa9dc6c3f"
/>


## Testing instructions

If you want to manually build an integration and test, please DM me and
I will send you detailed instructions or walk you through how to set up
a local package-spec and point your elastic-package at it, build it,
etc. Im happy to do so, but in the sake of brevity, I will skip that
here.

The easiest way otherwise is to simply upload this pre-built integration
that I built locally using the changes to package-spec:

[okta-3.14.2.zip](https://github.com/user-attachments/files/26195499/okta-3.14.2.zip)


### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
- [ ] Review the [backport
guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)
and apply applicable `backport:*` labels.

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

LGTM
Since 3.6.0 has been released, the changelog must be updated to include this entry into a new 3.6.1-next section.

Comment thread spec/changelog.yml Outdated
Comment thread spec/input/manifest.spec.yml
Comment thread .buildkite/.DS_Store Outdated
@Supplementing Supplementing requested a review from mrodm March 31, 2026 15:11
@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

@mrodm mrodm merged commit 1e6687f into elastic:main Mar 31, 2026
5 checks passed
@teresaromero teresaromero mentioned this pull request Apr 21, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Fleet Label for the Fleet team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants