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

Support lists in configuration #100

Open
mcbanderson opened this issue Apr 3, 2020 · 3 comments
Open

Support lists in configuration #100

mcbanderson opened this issue Apr 3, 2020 · 3 comments
Labels
DX-developer-experience UX/UI/DX Developer experience enhancement New feature or request

Comments

@mcbanderson
Copy link
Contributor

We currently only support regular key-value pairs, and map/dict values. We should support list values as well.

How to reproduce:
reflex.yaml

rules:
  aws:
  - my-rule:
      configuration:
        foo:
          - bar
          - baz
      version: latest

reflex build

Expected output:
my-rule.tf

module "my-rule" {
  source            = "git::https://github.com/cloudmitigator/my-rule.git?ref=latest"
  sns_topic_arn     = module.central-sns-topic.arn
  reflex_kms_key_id = module.reflex-kms-key.key_id
  foo = [
    "bar",
    "baz",
  ]
}

Actual output:
my-rule.tf

module "my-rule" {
  source            = "git::https://github.com/cloudmitigator/my-rule.git?ref=latest"
  sns_topic_arn     = module.central-sns-topic.arn
  reflex_kms_key_id = module.reflex-kms-key.key_id
  foo = ['bar', 'baz']
}
@rjulian
Copy link
Contributor

rjulian commented Apr 3, 2020

Should we wait to support this until we see a need for list-type configurations? I can't, with our current infrastructure, think of a time we'd need list-type options.

@mcbanderson
Copy link
Contributor Author

I actually see a pretty immediate need for it. The cloudfront-viewer-tls-protocol rule currently hard codes the list of allowed protocol versions (https://github.com/cloudmitigator/reflex-aws-cloudfront-viewer-tls-protocol/blob/master/source/reflex_aws_cloudfront_viewier_tls_protocol.py#L25), but this really should be configurable. And a list configuration makes the most sense to me.

Adding support to the CLI is going to be trivial, what we'll probably need to put some thought into is how the lists get injected into environment variables. (I suspect we'll need to do a json string that then gets parsed.)

@rjulian
Copy link
Contributor

rjulian commented Apr 3, 2020

So, I saw that there would eventually be the case where we have to inject multiple values into env variables. I think the simplest solution right now is what we'd have to do no matter what using environment variables: comma separated lists. Essentially: ami_id: ami-069494,ami-069999 and then have the lambda split into the list.

Even if we provide the CLI support for putting them as a yaml list under the config item, we'll still have to somehow transfer that to a comma separated string if the ultimate goal is to inject them as env vars to the lambda. Additionally, I don't think it'll be intuitive to end users that the yaml array format is our desired format. I will admit that it's far cleaner looking to do a yaml list.

@rot26 rot26 added DX-developer-experience UX/UI/DX Developer experience enhancement New feature or request labels May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX-developer-experience UX/UI/DX Developer experience enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants