Support Plan Preview for Lambda

[t-kikuc]

What this PR does / why we need it:

Enabled Plan Preview for Lambda, which compares FunctionManifests.

This is almost the same as ECS's PlanPreview(#4881).

Which issue(s) this PR fixes:

Fixes #5045

Does this PR introduce a user-facing change?: no

• How are users affected by this change: no
• Is this breaking change: no
• How to migrate (if breaking change): no

How it works:

When calling PlanPreview as GitHub Actions:

[codecov]

Codecov Report

Attention: Patch coverage is 19.44444% with 116 lines in your changes missing coverage. Please review.

Project coverage is 22.46%. Comparing base (f71f39c) to head (8d021ab).
Report is 27 commits behind head on master.

Files	Patch %	Lines
pkg/app/piped/planpreview/lambdadiff.go	0.00%	78 Missing ⚠️
pkg/app/piped/platformprovider/lambda/cache.go	0.00%	30 Missing ⚠️
pkg/app/piped/platformprovider/lambda/diff.go	82.35%	3 Missing and 3 partials ⚠️
pkg/app/piped/planpreview/builder.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5046      +/-   ##
==========================================
+ Coverage   22.43%   22.46%   +0.02%     
==========================================
  Files         520      526       +6     
  Lines       56852    57251     +399     
==========================================
+ Hits        12756    12862     +106     
- Misses      43069    43357     +288     
- Partials     1027     1032       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[t-kikuc]

/review

[github-actions]

PR Analysis

Main theme

"Introducing Lambda diff functionality to plan-preview"

PR summary

"This PR adds functionality to generate a diff for AWS Lambda applications during the plan-preview execution within PipeCD. It includes the diff logic, caching, and necessary test cases."

Type of PR

Enhancement

PR Feedback:

General suggestions

The PR successfully introduces an enhancement to include the AWS Lambda application kind in the plan-preview mechanism. It follows a consistent design with the existing application kinds and provides necessary testing and caching support. However, there are opportunities for improvement to ensure error handling and code structure best practices are followed.

Code feedback

• relevant file: pkg/app/pipectl/cmd/planpreview/planpreview_test.go
  suggestion: |
  While updating the plan-preview tests, ensure that the new expected output for errors matches the expected formatting. The error messages should provide sufficient context for debugging purposes.
  relevant line: + Error: "wrong application configuration",

• relevant file: pkg/app/piped/planpreview/builder.go
  suggestion: |
  important - Consider adding error logging for the new case model.ApplicationKind_LAMBDA:. Consistent with other cases, if an error occurs during the lambdadiff process, it is valuable to have this error capture in the log for easier troubleshooting.
  relevant line: + dr, err = b.lambdadiff(ctx, app, targetDSP, preCommit, &buf)

• relevant file: pkg/app/piped/planpreview/lambdadiff.go
  suggestion: |
  medium - Consider revising error messages to include the application ID, which can be helpful in identifying the source of the problem. For example, the error message format could be "failed to load lambda manifests for application (app-id: %s) at the head commit: %v".
  relevant line: + fmt.Fprintf(buf, "failed to load lambda manifests at the head commit (%v)\n", err)

• relevant file: pkg/app/piped/platformprovider/lambda/cache.go
  suggestion: |
  medium - Refactor repeated logging messages within FunctionManifestCache.Get and FunctionManifestSimpleCache.Put into a separate function to reduce code redundancy and improve maintainability.
  relevant line: + c.Logger.Error("failed while retrieving function manifest from cache",

• relevant file: pkg/app/piped/platformprovider/lambda/diff.go
  suggestion: |
  important - The renderByCommand function is not handling the case when the diff command is not available or fails to execute properly. Consider adding a fallback to the renderer in case the diff command fails or is not present.
  relevant line: + d, err := diff.RenderByCommand(command, old, new)

Security concerns:

"no"

This code introduces no apparent security issues, such as command injection or sensitive data exposure, within the diff functionality. However, it's important to ensure that any external command execution is properly sanitized and handled securely, as seen in the usage of the diff command to generate differences in manifest files.

[ffjlabo]

[nit] How about remove func renderByCommand and just call diff.RenderByCommand directly?

Suggested change

	d, err := renderByCommand(diffCommand, d.Old, d.New)
	if err != nil {
	d, err := diff.RenderByCommand(diffCommand, d.Old, d.New)
	if err != nil {

[t-kikuc]

@ffjlabo Thank you! I fixed it 8d021ab

[ffjlabo]

LGTM

[khanhtc1202]

🚀