spinnaker-plugins / aws-lambda-deployment-plugin-spinnaker

Spinnaker plugin to support deployment of AWS Lambda functions via Spinnaker pipelines
Apache License 2.0
23 stars 22 forks source link

Function names depend on Spinnaker Application name #69

Closed eyal-mor closed 3 years ago

eyal-mor commented 3 years ago

This issue is due to how the UI & API interact.

Reproducing

  1. Create a Lambda Create stage.
  2. Click on the Edit Stage As JSON button.
  3. Change the functionUid, functionName to something else (maybe random).
  4. Save Pipeline.
  5. Execute Pipeline

Explanation

When setting up a pipeline manually, the functionName property is generated via a merge with the application name, used as a name space (js excerpt):

functionName = `${ns}-${fn}`

The implementation is found here

Using the application name as a namespace causes automation scripts to become clumsy, weird and will fail when not tailored accurately to the specific usage of the Lambda Plugin.

As an example, the following excerpt is a json that was tailored for function definitions:

{
  "account": "${parameters[\"spinnaker_account\"]}",
  "cloudProvider": "aws",
  "functionName": "${execution.application}-${parameters[\"lambda_function_name\"]}",
  "functionUid": "${parameters[\"lambda_function_name\"]}",
  "type": "Aws.LambdaDeploymentStage"
}

This implementation causes multiple issues:

  1. the application name takes up a piece of the 64 character limit.
  2. Any problem in the Spel expression will cause the stage to break.
  3. Implementing automation is non trivial, there is no obvious relationship between functionUid & functionName.
shaiguitar commented 3 years ago

I want to re-iterate that although the tailored json example works, I am also running into developer experience problems due to this non-obvious implementation and conflation between functionUid and functionName.

shaiguitar commented 3 years ago

It seems like there may be a heuristic the plugin is making to split on the - delimiter. If the application name has a dash in it, this will cause confusion and various unclear errors show up in later stages.

The UI disables the application to have dashes, but the API allows it. This causes some problems when going through the API and using an essentially invalid application name.

Screen Shot 2021-07-13 at 3 03 13 PM
nimakaviani commented 3 years ago

yeah, this makes sense. I dont think we need to tie the function name back to the application name unless chosen by those setting up the pipeline. worst case scenario, colliding function names can be deduped manually by system engineers or based on your stack name etc.

gsapkal commented 3 years ago

Spinnaker clouddriver buckets the functions based on resource naming based on application name as prefix. If we remove the function name -> application relation the the caching and filtering would be difficult .

gsapkal commented 3 years ago

How spinnaker UI will list the functions for an application ?

nimakaviani commented 3 years ago

yes you are right @gsapkal ... looks like namespacing function names is wired down all the way to clouddriver's lambda provider, here.

An unfortunate hard constraint. If we want to modify the conventions, we probably will have to change it all the way down in the lambda provider as well.

nimakaviani commented 3 years ago

application name happens to be the way the clouddriver does the bucketing for the functions of an application. removing application name will break the bucketing and cause lots of issues.