Skip to content

Stream variable state change event #25966

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

salehjafarli
Copy link
Contributor

@salehjafarli salehjafarli commented Jun 3, 2025

Description

This is a suggestion pr for resolving issue #25899. It is draft and no testing change made yet demonstrate functionality I need. I did not change state of db purposefully to avoid any breaking change, and current state of streamed event is satisfactory for what I want to achieve. Change in variable will be reflected in events api, and the type of change I can infer from checking variables endpoint(update or delete)

Fixes: #25899
Fixes: https://hashicorp.atlassian.net/browse/NMD-773

Testing & Reproduction steps

Links

Contributor Checklist

  • Changelog Entry If this PR changes user-facing behavior, please generate and add a
    changelog entry using the make cl command.
  • Testing Please add tests to cover any new functionality or to demonstrate bug fixes and
    ensure regressions will be caught.
  • Documentation If the change impacts user-facing functionality such as the CLI, API, UI,
    and job configuration, please update the Nomad website documentation to reflect this. Refer to
    the website README for docs guidelines. Please also consider whether the
    change requires notes within the upgrade guide.

Reviewer Checklist

  • Backport Labels Please add the correct backport labels as described by the internal
    backporting document.
  • Commit Type Ensure the correct merge method is selected which should be "squash and merge"
    in the majority of situations. The main exceptions are long-lived feature branches or merges where
    history should be preserved.
  • Enterprise PRs If this is an enterprise only PR, please add any required changelog entry
    within the public repository.

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @salehjafarli! This is looking pretty good. I do wonder if for your particular use case described in #25899 if you wouldn't be better off with a blocking query. After all, that's how we watch for changes to specific variables in template blocks. But if you're watching a very large set of variables, I suppose this option makes more sense and it probably doesn't hurt to include it.

In any case, a few things that'll need here:

  • We'll definitely need tests.
  • We need to make sure we're authenticating access in nomad/stream/event_broker.go, otherwise anyone can read these events.
  • We'll need a changelog entry (make cl)
  • We'll need documentation updates to the table in website/content/api-docs/events.mdx

The other thing that's odd here is that the CLA bot didn't tag you. Have you previously signed HashiCorp's CLA?

Key: before.Path,
FilterKeys: []string{before.Path},
Payload: &structs.VariableEvent{
Variable: before,
Copy link
Member

Choose a reason for hiding this comment

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

The payload will include the encrypted blob, not the decrypted values. We probably don't want to decrypt here but that also means that we should use the variable metadata struct as the event body rather than the contents.

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

Successfully merging this pull request may close these issues.

Events for variable changes
2 participants