feat(lambda): introduce a new kind of Code, CfnParametersCode.

[skinny85]

This Code type is helpful when there is no local file/directory to use Assets with,
and the Lambda is supposed to be deployed in a CodePipeline.

---

Pull Request Checklist

• Testing
  • Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
  • CLI change?: coordinate update of integration tests with team
  • cdk-init template change?: coordinated update of integration tests with team
• Docs
  • jsdocs: All public APIs documented
  • README: README and/or documentation topic updated
• Title and Description
  • Change type: title prefixed with fix, feat will appear in changelog
  • Title: use lower-case and doesn't end with a period
  • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
• Sensitive Modules (requires 2 PR approvers)
  • IAM Policy Document (in @aws-cdk/aws-iam)
  • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  • Grant APIs (only if not based on official documentation with a reference)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

[eladb]

Not sure I understand the use case here. Where is the Lambda code actually coming from? The example seems to imply that there's some lambdaBuildAction that has the artifacts of your lambda function, but I am not sure how this interplays with your infrastructure code...

What happens if this stack is deployed through cdk deploy? What does it mean for the Lambda to use a CodePipelineCode in that case?

[skinny85]

Not sure I understand the use case here. Where is the Lambda code actually coming from? The example seems to imply that there's some lambdaBuildAction that has the artifacts of your lambda function, but I am not sure how this interplays with your infrastructure code...

The use case is deploying your Lambda through a CodePipeline. The lambdaBuildAction packages your Lambda code (compiles it if needed, does npm install if you need it, etc.), and this is where your Lambda code is coming from.

This was the experience we discussed in the doc review for Lambda in CodePipeline.

What happens if this stack is deployed through cdk deploy? What does it mean for the Lambda to use a CodePipelineCode in that case?

That wouldn't work really - you would need to provide the values for the 2 CFN Parameters, and we don't support that from the toolkit I believe. This Stack is meant to be deployed only through a CloudFormation Action.

[rix0rrr]

Is this PR superseded by your other one about exposing the parameter names?

[skinny85]

Is this PR superseded by your other one about exposing the parameter names?

In my opinion, no.

[rix0rrr]

In my opinion, no.

But isn't the other PR also about overriding an asset's source when deployed through a pipeline?

[skinny85]

There is still a usecase that is not handled well, even if that PR is merged, and that is a situation where you don't have any Assets to point to when creating your Lambda construct (example when this comes up: when the CDK code and the Lambda code are mastered in separate codebases).

In that situation, you currently have to do something like this:

        const bucketNameParam = new cdk.CfnParameter(lambdaStack, 'BucketNameLambdaSourceParam', {
            type: 'String',
        });
        const objectKeyParam = new cdk.CfnParameter(lambdaStack, 'ObjectKeyLambdaSourceParam', {
            type: 'String',
        });
        const bucket = s3.Bucket.import(lambdaStack, 'LambdaBucket', {
            bucketName: bucketNameParam.stringValue,
        });
        const lambdaCode = lambda.Code.bucket(bucket, objectKeyParam.stringValue);

// later, in the Pipeline Stack:

        const parameterOverrides: { [name: string]: any } = {};
        parameterOverrides[bucketNameParam.logicalId] = lambdaBuildAction.outputArtifact.bucketName;
        parameterOverrides[objectKeyParam.logicalId] = lambdaBuildAction.outputArtifact.objectKey;
        const cfnDeployAction = new cloudformation.PipelineCreateUpdateStackAction({
            // bunch of stuff here...
            parameterOverrides,
            additionalInputArtifacts: [lambdaBuildAction.outputArtifact],
        });

This is far from ideal. What I want to have is:

        const lambdaCode = lambda.Code.codePipeline(); // name TBD

// later, in the Pipeline Stack:

        const cfnDeployAction = new cloudformation.PipelineCreateUpdateStackAction({
            // bunch of stuff here...
            parameterOverrides: lambdaCode.overrideWith(lambdaBuildAction.outputArtifact),
            additionalInputArtifacts: [lambdaBuildAction.outputArtifact],
        });

[rix0rrr]

I see, thanks for explaining. I would like to see the use case for this type of asset documented very clearly on the class then. Think of our poor users who won't understand! (i.e., me in 3 months ;))

[rix0rrr]

Lambda deployed exclusively through CodePipeline, right? Might be worth a note that if you're using assets, you can override them anyway so no need to change your code then. I'm saying this because I want this to be the 99% case:

• Use an asset, use "cdk deploy" for "personal dev stack"
• That same asset is reused for CI/CD deployments.

I don't want the docs to send people down paths where they're going to be writing complicated conditionals because they think this is what they have to do to deploy via a Pipeline. Or worse, get confused and come ask us.

Maybe it's worth combining those 2 PRs so we can get the messaging and the examples right in one place?

[skinny85]

Since the other PR with overriding Assets has been closed without merging, I kept this title as-is. Let me know if that's fine, or if you still want to change it.

[eladb]

looks good. Artifact.fillParameters is a bit off. It’s very specific to lambda code which doesn’t feel right. I am wondering if there’s room for a more abstract concept for an “s3 path” (basically the tuple bucket+key[+version]) and then lambda.CfnParametersCode can implement this interface or have a property that returns this tuple.

[skinny85]

looks good. Artifact.fillParameters is a bit off. It’s very specific to lambda code which doesn’t feel right. I am wondering if there’s room for a more abstract concept for an “s3 path” (basically the tuple bucket+key[+version]) and then lambda.CfnParametersCode can implement this interface or have a property that returns this tuple.

I thought about this for some time, and I didn't really get anywhere. The only thing that came to me was something like this:

// in the S3 module
export interface S3Coordinates {
  readonly bucketName: string;
  readonly objectKey: string;
  readonly version?: string; // not really needed at this point, but why not
}

, then have lambda.CfnParametersCode implement S3Coordinates, and change Artifact#fillParameters to accept an S3Coordinates instead of a lambda.CfnParametersCode.

I'm not in love with this solution.

How about we invert the control a little bit?

// in the S3 module
export interface S3CoordinatesProvider {
  readonly bucketName: string;
  readonly objectKey: string;
}

// in the CodePipeline module
export class Artifact implements s3.S3CoordinatesProvider {
  // ...
}

// in the Lambda module
export class CfnParametersCode extends /* ... */ {
  // ...

  public fillParameters(provider: s3.S3CoordinatesProvider): { [name: string]: any } {
    const ret: { [name: string]: any } = {};
    ret[this.bucketNameParam.logicalId] = provider.bucketName;
    ret[this.objectKeyParam.logicalId] = provider.objectKey;
    return ret;
  }
}

And its usage looks like this in the CloudFormation Action:

new codepipeline_actions.CloudFormationCreateUpdateStackAction({
  parameterOverrides: {
    ...lambdaCode.fillParameters(lambdaBuildAction.outputArtifact),
  },
  additionalInputArtifacts: [
    lambdaBuildAction.outputArtifact,
  ],
  // ...
});

Thoughts on this?

[eladb]

You are right that it doesn't make sense for CfnParametersCode to expose the coordinates, because those are not the coordinates but rather the parameters... But CoordinateProvider feels like over design. The coordinates come from the artifacts, not from CfnParametersCode.

How about:

parameterOverrides: cfnParametersCode.assign({ 
    bucketName: outputArtifact.bucketName,
    objectKey: outputArtifact.objectKey
}),

Or if we really want to model coordinates, then:

parameterOverrides: cfnParametersCode.assign(outputArtifact.s3Coordinates)

[skinny85]

Or if we really want to model coordinates, then:

parameterOverrides: cfnParametersCode.assign(outputArtifact.s3Coordinates)

I went with this option, let me know if that works.

[skinny85]

Renamed ObjectBucketCoordinates to Coordinates after talking with Elad offline.