Support vpc configuration to lambda #4551

sivchari · 2023-08-18T09:44:21Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #4550

Does this PR introduce a user-facing change?:

How are users affected by this change:

Users can configure a vpc configuration for lambda function.

Is this breaking change:

No

How to migrate (if breaking change):

Signed-off-by: sivchari <[email protected]>

…ate a vpc config Signed-off-by: sivchari <[email protected]>

Signed-off-by: sivchari <[email protected]>

codecov · 2023-08-18T09:51:39Z

Codecov Report

Patch coverage has no change and project coverage change: -0.06% ⚠️

Comparison is base (b4a4f6c) 29.99% compared to head (42d4545) 29.93%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4551      +/-   ##
==========================================
- Coverage   29.99%   29.93%   -0.06%     
==========================================
  Files         220      220              
  Lines       25834    25851      +17     
==========================================
- Hits         7749     7739      -10     
- Misses      17438    17465      +27     
  Partials      647      647

Files Changed	Coverage Δ
pkg/app/piped/platformprovider/lambda/client.go	`4.72% <0.00%> (-0.23%)`	⬇️
pkg/app/piped/platformprovider/lambda/function.go	`63.75% <ø> (ø)`

... and 3 files with indirect coverage changes

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

khanhtc1202 · 2023-08-18T10:04:14Z

@sivchari How about adding that vpc configuration here instead of the application configuration? Since it quite specific for Lambda (or AWS service only)
https://github.com/pipe-cd/pipecd/blob/master/pkg/app/piped/platformprovider/lambda/function.go#L58-L73

sivchari · 2023-08-18T13:10:18Z

@khanhtc1202 Thanks, oh, I've already added it, doesn't it ?

https://github.com/pipe-cd/pipecd/pull/4551/files#diff-7b8805cb45eb45990964cb95f8e8afdd1619eb58eeed7b190462d91374d015f6R73

khanhtc1202 · 2023-08-18T13:13:27Z

docs/content/en/docs-dev/user-guide/configuration-reference.md

@@ -121,6 +121,7 @@ spec:
 | timeout | duration | The maximum length of time to execute deployment before giving up. Default is 6h. | No |
 | notification | [DeploymentNotification](#deploymentnotification) | Additional configuration used while sending notification to external services. | No |
 | postSync | [PostSync](#postsync) | Additional configuration used as extra actions once the deployment is triggered. | No |
+| vpcConfig | [VPCConfig](#vpcconfig) | Additional configuration used to link to specific vpc. | No |


@sivchari I mean instead of adding it here, only make the vpc configuration available via Function manifest file. I think we should keep this configuration file contains generic configurations only.

@khanhtc1202
Oops, sorry. I fixed it. Like this ?

Signed-off-by: sivchari <[email protected]>

khanhtc1202 · 2023-08-18T13:41:32Z

docs/content/en/docs-dev/user-guide/configuration-reference.md

@@ -9,7 +9,7 @@ description: >
 ## Kubernetes Application

 ``` yaml
-apiVersion: pipecd.dev/v1beta1
+apiVersion: pipecd.devI mean instead of adding it here, only make the vpc configuration available via Function manifest file. I think we should keep this configuration file contains generic configurations only./v1beta1


what was this? 😆

🐱 stepped on the keyboard ?? 😄

khanhtc1202 · 2023-08-18T13:42:31Z

pkg/app/piped/platformprovider/lambda/function_test.go

+      "vpcConfig": {
+          securityGroupIds: ["sg-1234567890", "sg-0987654321"],
+          subnetIds: ["subnet-1234567890", "subnet-0987654321"]
+      }


Signed-off-by: sivchari <[email protected]>

khanhtc1202

Almost LGTM. Left a simple nits.

khanhtc1202 · 2023-08-18T14:05:47Z

pkg/app/piped/platformprovider/lambda/client.go

+	if fm.Spec.VPCConfig != nil {
+		input.VpcConfig = &types.VpcConfig{
+			SecurityGroupIds: fm.Spec.VPCConfig.SecurityGroupIDs,
+			SubnetIds:        fm.Spec.VPCConfig.SubnetIDs,
+		}
+	}


Could you move this block to L128 or after L152, I want to keep "packing using container image" and "Zip on S3" block near by since they mean to same thing (code of the function).

You are right. I fix.

Signed-off-by: sivchari <[email protected]>

khanhtc1202

Great, thank you 🙌

khanhtc1202 · 2023-08-18T14:15:58Z

@sivchari one testcase is failed, could you check?

Signed-off-by: sivchari <[email protected]>

sivchari · 2023-08-18T14:43:04Z

@khanhtc1202
All test is passed.

khanhtc1202

LGreatTM 👍

Signed-off-by: sZma5a <[email protected]>

Signed-off-by: moko-poi <[email protected]>

sivchari added 3 commits August 18, 2023 18:23

piped/platformprovider/lambda: add VpcConfig field

93a6a49

Signed-off-by: sivchari <[email protected]>

piped/platformprovider/lambda: support lambda client to create or upd…

673c946

…ate a vpc config Signed-off-by: sivchari <[email protected]>

docs: update configuration reference

1ebdf23

Signed-off-by: sivchari <[email protected]>

sivchari requested review from nghialv, khanhtc1202 and kentakozuka as code owners August 18, 2023 09:44

github-actions bot added the area/go label Aug 18, 2023

khanhtc1202 reviewed Aug 18, 2023

View reviewed changes

sivchari added 2 commits August 18, 2023 22:34

docs: delete

a0e7b3b

Signed-off-by: sivchari <[email protected]>

docs: add vpc config description

ac2b825

Signed-off-by: sivchari <[email protected]>

khanhtc1202 reviewed Aug 18, 2023

View reviewed changes

sivchari force-pushed the support-vpc-configuration-to-lambda branch 3 times, most recently from ae05b63 to 2e2eb4e Compare August 18, 2023 13:59

fix: review point

710800c

Signed-off-by: sivchari <[email protected]>

sivchari force-pushed the support-vpc-configuration-to-lambda branch from 2e2eb4e to 710800c Compare August 18, 2023 14:01

khanhtc1202 reviewed Aug 18, 2023

View reviewed changes

platformprovider/lambda: move code to wrap up meaning

edd7d06

Signed-off-by: sivchari <[email protected]>

khanhtc1202 previously approved these changes Aug 18, 2023

View reviewed changes

fix: typo

42d4545

Signed-off-by: sivchari <[email protected]>

sivchari dismissed khanhtc1202’s stale review via 42d4545 August 18, 2023 14:42

khanhtc1202 approved these changes Aug 18, 2023

View reviewed changes

khanhtc1202 merged commit 145d59b into pipe-cd:master Aug 18, 2023
11 of 13 checks passed

github-actions bot mentioned this pull request Aug 23, 2023

Release v0.45.0 #4556

Merged

sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Aug 27, 2023

Support vpc configuration to lambda (pipe-cd#4551)

55209b6

Signed-off-by: sZma5a <[email protected]>

moko-poi pushed a commit to moko-poi/pipecd that referenced this pull request Nov 3, 2023

Support vpc configuration to lambda (pipe-cd#4551)

2d6daa0

Signed-off-by: moko-poi <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support vpc configuration to lambda #4551

Support vpc configuration to lambda #4551

sivchari commented Aug 18, 2023 •

edited

Loading

codecov bot commented Aug 18, 2023 •

edited

Loading

khanhtc1202 commented Aug 18, 2023

sivchari commented Aug 18, 2023

khanhtc1202 Aug 18, 2023

sivchari Aug 18, 2023 •

edited

Loading

khanhtc1202 Aug 18, 2023

sivchari Aug 18, 2023 •

edited

Loading

khanhtc1202 Aug 18, 2023

khanhtc1202 Aug 18, 2023 •

edited

Loading

sivchari Aug 18, 2023

khanhtc1202 left a comment

khanhtc1202 Aug 18, 2023

sivchari Aug 18, 2023

khanhtc1202 left a comment

khanhtc1202 commented Aug 18, 2023

sivchari commented Aug 18, 2023

khanhtc1202 left a comment

Support vpc configuration to lambda #4551

Support vpc configuration to lambda #4551

Conversation

sivchari commented Aug 18, 2023 • edited Loading

codecov bot commented Aug 18, 2023 • edited Loading

Codecov Report

khanhtc1202 commented Aug 18, 2023

sivchari commented Aug 18, 2023

khanhtc1202 Aug 18, 2023

Choose a reason for hiding this comment

sivchari Aug 18, 2023 • edited Loading

Choose a reason for hiding this comment

khanhtc1202 Aug 18, 2023

Choose a reason for hiding this comment

sivchari Aug 18, 2023 • edited Loading

Choose a reason for hiding this comment

khanhtc1202 Aug 18, 2023

Choose a reason for hiding this comment

khanhtc1202 Aug 18, 2023 • edited Loading

Choose a reason for hiding this comment

sivchari Aug 18, 2023

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

khanhtc1202 Aug 18, 2023

Choose a reason for hiding this comment

sivchari Aug 18, 2023

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

khanhtc1202 commented Aug 18, 2023

sivchari commented Aug 18, 2023

khanhtc1202 left a comment

Choose a reason for hiding this comment

sivchari commented Aug 18, 2023 •

edited

Loading

codecov bot commented Aug 18, 2023 •

edited

Loading

sivchari Aug 18, 2023 •

edited

Loading

sivchari Aug 18, 2023 •

edited

Loading

khanhtc1202 Aug 18, 2023 •

edited

Loading