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

Interpolate values from SSM Parameter Store when initializing Dotenv #233

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@dbackeus
Copy link
Contributor

commented Apr 15, 2019

This is a πŸ™‹β€β™‚οΈ feature or enhancement.

  • I've added tests (if it's a bug, feature or enhancement)
  • I've adjusted the documentation (if it's a feature or enhancement)
  • The test suite passes (run bundle exec rspec to verify this)

Summary

Mimics the same behaviour and interpolation syntax found in serverless, ie: ${smm:<parameter-name>} (https://serverless.com/framework/docs/providers/aws/guide/variables/#reference-variables-using-the-ssm-parameter-store). However it will default to decrypting secure parameters and does not attempt to add any weird syntax for toggling the functionality. It seems to me that 99.9% of cases will want decrypted values. If anyone has any real world use cases for wanting encrypted values instead we could expand on the syntax with a disabling feature later.

We also introduce conventions for relative vs absolute paths. For example:
${ssm:relative-variable} will be expanded to ${ssm:/<app-name>/<jets-environment>/relative-variable} while an absolute path (prefixed with /) will not be expanded.

If Jets fails to find a given parameter at AWS it will immediately abort the process with a helpful error message.

Note that it is mandatory to wrap .env file variables in single quotes to avoid losing the dollar sign since dollar signs are also the conventional way of referencing other variables in bash (see https://stackoverflow.com/a/37876900 for explanation). For example:

# Good:
MY_SSM_VARIABLE='${ssm:parameter-name}'

# Bad:
MY_SSM_VARIABLE=${ssm:parameter-name}
MY_SSM_VARIABLE="${ssm:parameter-name}"

I will be happy add documentation for this feature after we have 100% settled on the general behaviour and implementation details.

Context

This work was initiated after discussing SSM parameters in Jets at https://community.rubyonjets.com/t/thoughts-on-ssm-parameter-store-for-configuration

Version Changes

I would go for a minor version bump. Note that we're potentially breaking backwards compatibility if someone is already using this syntax in their own .env files but this seems highly unlikely.

Interpolate values from SSM Parameter Store when initializing Dotenv
Uses the same interpolation syntax as serverless, ie: ${smm:<parameter-name>}
https://serverless.com/framework/docs/providers/aws/guide/variables/#reference-variables-using-the-ssm-parameter-store

We also introduce conventions for relative vs absolute paths. For example:
MY_RELATIVE_VARIABLE='${ssm:relative-variable}' will be expanded to ${ssm:/<app-name>/<jets-environment>/relative-variable} while an absolute path (prefixed with /) will not be expanded.

If Jets fails to find a given parameter at AWS it will immediately abort the process with a helpful error message.

Note that it is mandatory to wrap .env file variables in single quotes to avoid losing the dollar sign  since dollar signs are also the conventional way of referencing other variables in bash (see https://stackoverflow.com/a/37876900 for explanation).

This work was initiated after discussing SSM parameters in Jets at https://community.rubyonjets.com/t/thoughts-on-ssm-parameter-store-for-configuration
)
expect_any_instance_of(Aws::SSM::Client).to receive(:get_parameter)
.with(name: "/demo/test/username", with_decryption: true)
.and_return(username_response_double)

This comment has been minimized.

Copy link
@dbackeus

dbackeus Apr 15, 2019

Author Contributor

I'm not a big fan of all the detailed mocking required in the specs. But gets the job done I guess.

I was originally surprised by these calls already being stubbed via Aws.config.update(stub_responses: true). Maybe there are more conventional ways of mocking the Aws libraries than what I'm doing here?

This comment has been minimized.

Copy link
@tongueroo

tongueroo May 16, 2019

Owner

Yeah, mocking is a lot of work. Think this is fine, it works πŸ‘ Note, for specs, usually I ask myself, is it saving me overall development time.

env = Jets::Dotenv.new.load!

expect(env.fetch("AUTENTICATED_URL")).to eq "https://my-user:my-password@host.com"
expect(ENV.fetch("AUTENTICATED_URL")).to eq "https://my-user:my-password@host.com"

This comment has been minimized.

Copy link
@dbackeus

dbackeus Apr 15, 2019

Author Contributor

It's important to confirm that both the returned env hash as well as global ENV have been mutated since both are used in the Jets codebase.

That said it's not good practice to mutate global ENV in specs since it can end up being a source of heisen-specs. Let me know if you have any preferences for reverting ENV to its original state after running the spec.

This comment has been minimized.

Copy link
@tongueroo

tongueroo May 16, 2019

Owner

Yes, can see how folks think it is not a good practice to change the ENV. Think this is fine for test also though.

@tongueroo

This comment has been minimized.

Copy link
Owner

commented May 16, 2019

Been thinking about the syntax. Think that this is a little cleaner:

MY_SSM_VARIABLE=ssm:parameter-name
@dbackeus

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

That's indeed cleaner but I guess that implies sacrificing the ability to compose multiple ssm variables together (ie. "https://${ssm:username}:${ssm:password}@host.com")?

I'm inclined to thinking it might be ok to make that sacrifice though... serverless allows composing but I don't see myself wanting to use that feature personally.

@tongueroo

This comment has been minimized.

Copy link
Owner

commented May 16, 2019

RE: sacrificing the ability to compose multiple ssm variables
RE: I'm inclined to thinking it might be ok to make that sacrifice though

Ah I see. Thanks for pointing out. Yup, think it's worth the trade-off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.