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

fix: Implements ListRaw required for export from S3-KMS store #236

Merged
merged 1 commit into from
Oct 18, 2019

Conversation

LinuxBozo
Copy link
Contributor

I noticed that the export implementation uses ListRaw to iterate over the secrets in the store and return them in the requested format, while the env functionality uses List. ListRaw was not included in #195 which implements the S3-KMS backend, so the env command works correctly, but export, which is more flexible, does not. This PR rectifies this so that export will work correctly.

@eculver
Copy link

eculver commented Oct 7, 2019

Ahh, nice find. I'm curious how it breaks? Can you share an example of the export behavior? I wonder if this was just an oversight in the original implementation. It would also be nice if we had a test but given that there aren't any for the s3kms backend at the moment, it's really just a nice-to-have.

Thanks for contributing! Let's let @nickatsegment give a look and we'll see about merging it ASAP.

@LinuxBozo
Copy link
Contributor Author

@eculver The breakage manifests, depending on the format chosen, by returning either nothing, or an empty object. I believe this simply just was an oversight, and may be related to the comment on the implementation in the ssm store: Uses faster AWS APIs with much higher rate-limits. Without further context, this may lead others to believe that ListRaw should always do this, and isn't related to how it actually is used by export as a dependency. It also seems that the duplicative nature of env and export are ripe for some refactor work, though I obviously did not pursue that in my first contribution.

@nickatsegment
Copy link
Contributor

Yeah, the distinction between List and ListRaw is from before I took over maintaining chamber, but I believe it's mostly a speed/API-efficiency hack for SSM. Pretty sure for S3, List is as fast/efficient as we can get, so let's do it!

@nickatsegment nickatsegment merged commit 72a39a7 into segmentio:master Oct 18, 2019
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.

3 participants