Skip to content

Conversation

@jtsaito
Copy link
Contributor

@jtsaito jtsaito commented Sep 21, 2021

This PR adds support for the secret_environment_variables attribute.

@jansiwy jansiwy self-requested a review September 21, 2021 14:37
@jtsaito
Copy link
Contributor Author

jtsaito commented Sep 21, 2021

@jansiwy Please not the following three points.

  1. I did not add anything to the readme because I was not sure how much we want to highlight the feature. Shall I copy the section on secret environment variables from the inlined cases?
  2. I did not add support for the layers attribute (YAGNI). Please let me know if I should add it.
  3. I added all declarations for secrets to the bottom of the file. Please let me know if you prefer sorting by declaration type, globally.

Copy link
Contributor

@jansiwy jansiwy left a comment

Choose a reason for hiding this comment

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

As this is a public module, I'd recommend to reduce the Babbel-specific parts as much as possible:

  • In a first PR, we might just introduce layers in this module. Building secrets_wrapper_layers from secrets_wrapper_lambda_layer_version_number is not really necessary.
  • And in an updated version of this PR, we might just introduce secret_environment_variables (which sets environment_variables and the permissions) because this is jot strickly relying on the layer. In theory, everyone could implement the same logic in the Lambda function itself, i.e. we coukd drop AWS_LAMBDA_EXEC_WRAPPER here as well.

@jtsaito
Copy link
Contributor Author

jtsaito commented Sep 22, 2021

@jansiwy Okay, just to be sure I understood this correctly:

PR 1. Add support for layers.

PR 2. add secret_environment_variables. consumers of the module will be passing the ARN of our lambda wrapper layer explicitly and set AWS_LAMBDA_EXEC_WRAPPER explicitely. This would look like the following example:

module "lambda-x" {
  source  = "babbel/lambda-with-inline-code/aws"
  version = "1.1.0"

  function_name = "x"

  handler = "index.handler"
  runtime = "nodejs14.x"

  source_dir = "lambda/lambda/src"

  environment_variables = {
    AWS_LAMBDA_EXEC_WRAPPER = "/opt/main"
  }

  secret_environment_variables = {
    PASSWORD = local.password_secret_manager_arn # some example secret manager version ARN
  }
  
  layers = "arn:aws:lambda:xx-easet-1:123456789012:layer:my-layer:1" # our layer version arn
}

Please correct or confirm.

@jtsaito jtsaito requested a review from jansiwy September 22, 2021 08:52
@jtsaito jtsaito mentioned this pull request Sep 22, 2021
@jansiwy
Copy link
Contributor

jansiwy commented Sep 22, 2021

Yes, except that the value of layers is a list.

@jtsaito
Copy link
Contributor Author

jtsaito commented Sep 22, 2021

Replaced by #8

@jtsaito jtsaito closed this Sep 22, 2021
@jtsaito jtsaito deleted the secret-environment-variables branch September 22, 2021 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants