Skip to content

Fix panic when Lambda manifest has no tags defined#6215

Merged
t-kikuc merged 1 commit into
pipe-cd:masterfrom
ucpr:fix-nil-lambda-manifest-tags
Sep 10, 2025
Merged

Fix panic when Lambda manifest has no tags defined#6215
t-kikuc merged 1 commit into
pipe-cd:masterfrom
ucpr:fix-nil-lambda-manifest-tags

Conversation

@ucpr
Copy link
Copy Markdown
Contributor

@ucpr ucpr commented Sep 10, 2025

What this PR does:

Fix panic when deploying Lambda functions with manifests that don't have tags defined in the spec.

Why we need it:

According to the PipeCD documentation, the tags field is optional in Lambda function manifests ("Except the tags and the environments field, all others are required fields for the deployment to run").
However, the current implementation assumes that the tags field always exists, causing a panic with "assignment to entry in nil map" error when it's not defined.

Example manifest that causes the panic:

apiVersion: pipecd.dev/v1beta1
kind: LambdaFunction
spec:
  name: example
  role: arn:aws:iam::0000000000:role/example
  image: example
  memory: 256
  timeout: 30
  environments:
    ENV: dev

Piped (v0.52.2) error log:

panic: assignment to entry in nil map [recovered]                                                                                                                         
    panic: assignment to entry in nil map                                                                                                                                 
                                                                                                                                                                          
goroutine 367730 [running]:                                                                                                                                               
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.deferwrap1()                                                                                                      
    /home/runner/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.28.0/trace/span.go:398 +0x25                                                                                  
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc0041a7380, {0x0, 0x0, 0xc00080e180?})                                                                          
    /home/runner/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.28.0/trace/span.go:436 +0xa59                                                                                 
panic({0x24beec0?, 0x417ff90?})                                                                                                                                           
    /opt/hostedtoolcache/go/1.24.1/x64/src/runtime/panic.go:792 +0x132                                                                                                    
github.com/pipe-cd/pipecd/pkg/app/piped/executor/lambda.loadFunctionManifest(_, {_, _}, _)                                                                                
    /home/runner/work/pipecd/pipecd/pkg/app/piped/executor/lambda/lambda.go:85 +0x2c5                                                                                     
github.com/pipe-cd/pipecd/pkg/app/piped/executor/lambda.(*deployExecutor).ensureSync(0xc005ad3600, {0x2ca7548, 0xc006924000})                                             
    /home/runner/work/pipecd/pipecd/pkg/app/piped/executor/lambda/deploy.go:82 +0x7e                                                                                      
github.com/pipe-cd/pipecd/pkg/app/piped/executor/lambda.(*deployExecutor).Execute(0xc005ad3600, {0x2ca8010, 0xc00608da10})                                                
    /home/runner/work/pipecd/pipecd/pkg/app/piped/executor/lambda/deploy.go:68 +0x19e                                                                                     
github.com/pipe-cd/pipecd/pkg/app/piped/controller.(*scheduler).executeStage(0xc0044e0008, {0x2ca8010, 0xc00608da10}, {{{}, {}, {}, 0x0}, 0x0, {0x0, 0x0, ...}, ...}, ...)
    /home/runner/work/pipecd/pipecd/pkg/app/piped/controller/scheduler.go:640 +0x1456                                                                                     
github.com/pipe-cd/pipecd/pkg/app/piped/controller.(*scheduler).Run.func3()                                                                                               
    /home/runner/work/pipecd/pipecd/pkg/app/piped/controller/scheduler.go:350 +0x850                                                                                      
created by github.com/pipe-cd/pipecd/pkg/app/piped/controller.(*scheduler).Run in goroutine 367706                                                                        
    /home/runner/work/pipecd/pipecd/pkg/app/piped/controller/scheduler.go:339 +0x176

This prevents successful deployment of Lambda functions that don't explicitly define tags in their manifests, despite tags being an optional field per the specification.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

  • How are users affected by this change: Users can now deploy Lambda functions without explicitly defining the tags field in their function manifests. The deployment will no
    longer panic and PipeCD management tags will be properly added.
  • Is this breaking change: No
  • How to migrate (if breaking change): N/A

Signed-off-by: ucpr <contact@ucpr.dev>
@ucpr ucpr force-pushed the fix-nil-lambda-manifest-tags branch from 2472b94 to 18ca3c2 Compare September 10, 2025 05:21
@ucpr ucpr marked this pull request as ready for review September 10, 2025 05:23
@ucpr ucpr requested a review from a team as a code owner September 10, 2025 05:23
@t-kikuc t-kikuc added the kind/bug Something isn't working as expected label Sep 10, 2025
Copy link
Copy Markdown
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

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

Thank you so much!

  • the panic point is here (Tags):
    fm.Spec.Tags[provider.LabelManagedBy] = provider.ManagedByPiped
  • ✅ the code location: loadFunctionManifest() seems better than inpovider.LoadFunctionManifest() because the latter one does not need to know the default value of Tags.
  • ✅ This default value does not affect other codes including updateTagsConfig() because currently the case of nil tags is not handled, and handling it as a empty map like this PR is correct.
    func (c *client) updateTagsConfig(ctx context.Context, fm FunctionManifest) error {

@t-kikuc t-kikuc enabled auto-merge (squash) September 10, 2025 05:40
Copy link
Copy Markdown
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Nice catch, thank you 👍

@t-kikuc t-kikuc merged commit b1f1553 into pipe-cd:master Sep 10, 2025
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/go cherry-pick kind/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants