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

[SSM-3] Added AWS SSM Parameter Store Integration #662

Closed
wants to merge 2 commits into from

Conversation

rms1000watt
Copy link

@rms1000watt rms1000watt commented Jun 7, 2019

Reference this PR for all the previous conversations: #573

This new PR incorporates all the changes that happened to the code base over the past few weeks

@rms1000watt
Copy link
Author

@mumoshu Could you give this a once over at your convenience?

Figuring out where to inject the datasource.PrepareAll() was tricky. I wasn't able to just pass in the state.SSM to datasource.PrepareAll() because the helmfile has to be rendered first before state.SSM gets populated. However, I need to pass state.SSM to datasource.PrepareAll() before the rendering.. So, it was a chicken and egg situation.

I ended up keeping the same solution as before, where datasource.PrepareAll() will parse the helmfile based on fileOrDir variable before the rendering. I couldn't crack how to do it another way.


lololol this repo hurts my head with all the callbacks 😅😅😅😅

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 13, 2019

@rms1000watt Thanks for your continuous effort! I'm still reading this throughly and thinking about how we should proceed.

@aegershman
Copy link
Contributor

Apologies, don't want to hijack this-- if there's an issue / better place to ask, let me know -- in the future, could this pattern be extended to support other credential providers similar to SSM, e.g. lastpass or credhub or vault, etc.?

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 18, 2019

@aegershman I think it worth a dedicated github issue for it. Would you mind creating one?

Anyway, yeah - I'd love to add support for various datasources like those. Ideally/in longer-term, I'd want to maintain the layer that abstracts various datasources as a separate project, like we use go-getter for sources files from various sources.

@costimuraru
Copy link

@rms1000watt is this PR supposed to add support for defining some thing what is present in #573 ?

ssm:
  - name: east1
    prefix: /dev/us-east-1/redis
    region: us-east-1
  - name: west2
    prefix: /dev/us-west-2/redis
    region: us-west-2

releases:
  - name: redis-test
    chart: stable/redis
    values:
    - fake_key:
        redis_pass_east: {{ ssm "east1:redis_password" }}
        redis_pass_west: {{ ssm "west2:redis_password" }}
        redis_pass_seast: {{ ssm "/dev/ap-southeast-1/redis/redis_password" }}
    - cluster:
        enabled: false
    - persistence:
        enabled: false
    - metrics:
        enabled: false

If so, that would be pretty neat! We're looking to use SSM in our helmfiles.

@rms1000watt
Copy link
Author

rms1000watt commented Jul 9, 2019

@costimuraru Yeah, exactly!! It has the same intent just different code hooking to the main logic. The codebase changed since that initial PR so I took the lazy approach and branched from the new master.

Full disclosure, I've been running off this branch for personal projects so I can use ssm with helmfile. Been loving it.

@costimuraru
Copy link

Awesome, @rms1000watt.
QQ: How is this actually fetching the secrets from SSM? Is it using the default aws credentials provider? Would it be possible to somehow specify the AWS_PROFILE?

@rms1000watt
Copy link
Author

rms1000watt commented Jul 9, 2019

@costimuraru Hmm, I don't want anything to get lost in translation/definitions, so I'll just say it this way:

It behaves the same way as the aws cli behaves

(So yeah, you can export the AWS_PROFILE in your environment and it should obey)

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 10, 2019

@costimuraru @rms1000watt Hey! I'd appreciate it if you could also take a look into #745.

I'd like to cover this feature in a way it harmonizes well with other use-cases and issues we're trying to address in a longer term.

@rms1000watt
Copy link
Author

@mumoshu I know you're suuuuuuper busy with all these projects you maintain. I'm just curious what your gut tells you for the next step on this topic?

I see a few things you've cited:

Not trying to push you in any one direction, just curious about an update is all! ❤️ 👍

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 27, 2019

@rms1000watt Hey! Thanks for the reminder.

I think the last thing that's unclear at the moment is whether we should use requiredEnv to access the SSM params or not.

Other than that I think the design work is complete and we're ready to implement it :shipit:

@barryib
Copy link

barryib commented Oct 9, 2019

It sounds like you guys did a great work on this. Any updates ?

@mumoshu
Copy link
Collaborator

mumoshu commented Nov 3, 2019

I'm closing this as resolved via #906.
Thanks for all your support!

@mumoshu mumoshu closed this Nov 3, 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.

5 participants