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

Implement handler to wrap fs.writeFile functionality #814

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

aryanjassal
Copy link
Member

@aryanjassal aryanjassal commented Sep 27, 2024

Description

Currently, the VaultsSecretsEdit handler uses vaultOps.updateSecret() to update a secret's contents if it exists. Otherwise, it throws an error. This behaviour convolutes the implementation for secrets edit and secrets write as a lot of needless checks are run at the client side to work around a small oversight.

As such, this PR will aim on laying the groundwork for secrets write by creating a handler which will write secret contents to a secret. If the secret doesn't exist, it will automatically get created.

Issues Fixed

Tasks

  • 1. Remove VaultsSecretsEdit
  • 2. Add VaultsSecretsWriteFile
  • 3. Create vaultOps.writeSecret as a wrapper around fs.writeFile
  • 4. Make tests for VaultsSecretsWriteFile
  • 5. Make tests for vaultOps.writeSecret

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@aryanjassal aryanjassal self-assigned this Sep 27, 2024
Copy link

linear bot commented Sep 27, 2024

@aryanjassal
Copy link
Member Author

Currently, we are using a UnaryHandler to update secrets. For consistency, this behaviour needs to be replicated over to the VaultsSecretsWriteFile handler too.

However, UnaryHandler is inherently not a good choice for streaming file contents. A better choice would be a RawStream, but it needs more work and discussions to set up. As such, it will be done in later iterations. For now, the new handler will keep using UnaryHandler but use the new vaultOps function.

Copy link
Contributor

@tegefaulkes tegefaulkes left a comment

Choose a reason for hiding this comment

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

I skimmed through it and nothing stands out. Looks good.

feat: removed 'VaultsSecretsEdit'

chore: updated tests

chore: added tests for vaultOps
@aryanjassal
Copy link
Member Author

Everything for this PR has been done and approval has also been granted. Merging.

@aryanjassal aryanjassal merged commit 5b78ce1 into staging Sep 30, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants