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

river/stdlib: add nonsensitive function #3817

Merged
merged 6 commits into from
May 9, 2023

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented May 9, 2023

NOTE TO REVIEWER: I had to do some refactoring to make this PR possible without import cycles. I recommend reviewing commit-by-commit.

The nonsensitive function allows users to explicitly convert a secret value back to a string.

nonsensitive is primarily useful when combined with the remote.vault component, since remote.vault always returns secrets, even if some data stored in Vault is non-sensitive (such as a TLS public key).

nonsensitive is the equivalent to the Terraform function of the same name.

With the intent to add a `nonsensitive` function into the standard
library, rivertypes needs to be moved under river/ to allow it to
eventually remove import cycles.

Plus, given a `nonsensitive` function, the river/ package would be a
better home for the rivertypes package, since the River stdlib should
not import anything outside of River.
This commit modifies rivertypes to import types through the internal
value package instead of the top-level river package. This removes a
potential import cycle allows the stdlib to import rivertypes.
The `nonsensitive` function allows users to explicitly convert a secret
value back to a string.

`nonsensitive` is primarily useful when combined with the remote.vault
component, since remote.vault always returns secrets, even if some data
stored in Vault is non-sensitive (such as a TLS public key).
@ptodev
Copy link
Contributor

ptodev commented May 9, 2023

Do we want to mention this in the remote.vault documentation? I think we probably should, since people interested in such functionality might be using remote.vault and would try find the information in the Export section of the remote.vault docs.

@ptodev
Copy link
Contributor

ptodev commented May 9, 2023

I personally like the approach with nonsensitive as opposed to trying to add extra non-secret config to remote.vault for two reasons:

  • It's quite flexible and could work with other components which only export secrets in the future.
  • It also gives the user full responsibility regarding what gets converted from a secret. The less under-the-hood-magic is happening with secrets, the better.

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Small nit but looks good.

@rfratto rfratto merged commit 454306f into grafana:main May 9, 2023
@rfratto rfratto deleted the river-nonsensitive branch May 9, 2023 16:58
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
* move flow/rivertypes package to river/rivertypes

With the intent to add a `nonsensitive` function into the standard
library, rivertypes needs to be moved under river/ to allow it to
eventually remove import cycles.

Plus, given a `nonsensitive` function, the river/ package would be a
better home for the rivertypes package, since the River stdlib should
not import anything outside of River.

* rivertypes: remove import on river

This commit modifies rivertypes to import types through the internal
value package instead of the top-level river package. This removes a
potential import cycle allows the stdlib to import rivertypes.

* river/stdlib: add `nonsensitive` function

The `nonsensitive` function allows users to explicitly convert a secret
value back to a string.

`nonsensitive` is primarily useful when combined with the remote.vault
component, since remote.vault always returns secrets, even if some data
stored in Vault is non-sensitive (such as a TLS public key).

* remote.vault: document usage of nonsensitive in exports

* address review feedback
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
* move flow/rivertypes package to river/rivertypes

With the intent to add a `nonsensitive` function into the standard
library, rivertypes needs to be moved under river/ to allow it to
eventually remove import cycles.

Plus, given a `nonsensitive` function, the river/ package would be a
better home for the rivertypes package, since the River stdlib should
not import anything outside of River.

* rivertypes: remove import on river

This commit modifies rivertypes to import types through the internal
value package instead of the top-level river package. This removes a
potential import cycle allows the stdlib to import rivertypes.

* river/stdlib: add `nonsensitive` function

The `nonsensitive` function allows users to explicitly convert a secret
value back to a string.

`nonsensitive` is primarily useful when combined with the remote.vault
component, since remote.vault always returns secrets, even if some data
stored in Vault is non-sensitive (such as a TLS public key).

* remote.vault: document usage of nonsensitive in exports

* address review feedback
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 26, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants