Fix a few issues with functionbeat #8713
Conversation
ph
commented
Oct 23, 2018
ph
commented
- Allow users to define the S3 bucket used for artifact
- Replace the log group name with valid string
- Enforce function name to only contains some chars
- Enforce log group name validation
- Add better code handling when waiting for cloudformation status.
- Allow users to define the S3 bucket used for artifact - Replace the log group name with valid string - Enforce function name to only contains some chars - Enforce log group name validation - Add better code handling when waiting for cloudformation status.
| @@ -11,9 +11,12 @@ | |||
| # Configure functions to run on AWS Lambda, currently we assume that the credentials | |||
| # are present in the environment to correctly create the function when using the CLI. | |||
| # | |||
| # Configure which S3 bucket we should upload the lambda artifact. | |||
| functionbeat.provider.aws.deploy_bucket: "functionbeat-deploy" | |||
|
|
|||
There was a problem hiding this comment.
I have mixed feeling about how to display to display provider options vs functions.
What would be the best way to make it clean in the YML?
There was a problem hiding this comment.
I'd say the format chosen very much fits our conventions. If there is a chance we might introduce even more settings you can write:
functionbeat.provider.aws:
deploy_bucket: ...
<more settings> ?
I'm in favor of having functionbeat.provider.aws.functions: top-level. This reduces the amount of indentation and copy'n paste errors.
Alternatively:
functionbeat.providers.aws:
deploy_bucket: ...
functionbeat.functions:
- provider: aws
type: cloudwatch_logs
...
I wonder if we even need to have an explicit provider on functions. Normally the provider is implicit by function type. By having providers 'implicit', we can't limit the max number of providers in a config file to 1.
| @@ -22,6 +24,11 @@ import ( | |||
| "github.com/elastic/beats/x-pack/functionbeat/provider/aws/transformer" | |||
| ) | |||
|
|
|||
| var ( | |||
| logGroupNamePattern = "^[\\.\\-_/#A-Za-z0-9]+$" | |||
There was a problem hiding this comment.
use backticks for raw strings. Then you don't have to escape the escapes.
| "invalid characters in log group name '%s', name must match regular expression: '%s'", | ||
| s, | ||
| logGroupNamePattern, | ||
| ) |
There was a problem hiding this comment.
not everyone will understand the regex being printed (it even contains escapes). How about: name must consist only of upper or lowercase ascii letters, digits, or special characters "._#/"
- Allow users to define the S3 bucket used for artifact - Replace the log group name with valid string - Enforce function name to only contains some chars - Enforce log group name validation - Add better code handling when waiting for cloudformation status.
- Allow users to define the S3 bucket used for artifact - Replace the log group name with valid string - Enforce function name to only contains some chars - Enforce log group name validation - Add better code handling when waiting for cloudformation status.
- Allow users to define the S3 bucket used for artifact - Replace the log group name with valid string - Enforce function name to only contains some chars - Enforce log group name validation - Add better code handling when waiting for cloudformation status.
