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

feat: use google/go-cloud to fetch rules and credentials from object storage #562

Merged
merged 3 commits into from
Nov 2, 2020

Conversation

alekitto
Copy link
Contributor

@alekitto alekitto commented Oct 29, 2020

Related issue

#518

Proposed changes

As proposed in #518, I've used go-cloud library to fetch credentials and rules files from an object storage (S3, Azure Blob Storage or GCS).

Checklist

  • I have read the contributing guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further comments

I'm unsure about how to test this feature: probably I can use mocks to unit test the fetcher, but probably an integration test could be more useful (with a minio service container to test real S3 api calls maybe?)

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Yo, this is awesome! A couple of things that we could do to make this even better:

  1. Add tests for azblob/gs/s3 - I think that gocloud has helpers to help you write tests
  2. Document this feature here and here and here.

Thank you!

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Great work, thank you! :)

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.

2 participants