Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce spec.code.sha256 field to help with s3 code changes #88

Merged
merged 5 commits into from
May 7, 2024
Merged

Introduce spec.code.sha256 field to help with s3 code changes #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5fd9a01
Update to ACK runtime `v0.25.0`, code-generator `v0.25.0` (#87)
ack-bot Mar 22, 2023
3ba2ca7
revise
Vandita2020 Mar 31, 2024
9981248
E2E tests
Vandita2020 Apr 9, 2024
b955873
New revision with Architecture update
Vandita2020 May 2, 2024
6024afe
Merge branch 'main' into s3_codeSHA
Vandita2020 May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apis/v1alpha1/ack-generate-metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ api_directory_checksum: c67645b15db39980ba51ff6303c34c5aafc55a9e
api_version: v1alpha1
aws_sdk_go_version: v1.44.181
generator_config_info:
file_checksum: f0fa26f0d3c577f5800eb183e0ec1cea865a75ee
file_checksum: 4038e6b669d1d6966f8e729a3ed323bf03593680
original_file_name: generator.yaml
last_modification:
reason: API generation
9 changes: 8 additions & 1 deletion apis/v1alpha1/generator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ resources:
- path: Status.State
in: [ "Active" ]
fields:
Code.SHA256:
type: string
compare:
is_ignored: true
set:
- ignore: "to"
method: Create
Code.S3Bucket:
references:
resource: Bucket
Expand Down Expand Up @@ -72,7 +79,7 @@ resources:
path: ReservedConcurrentExecutions
Code:
compare:
is_ignored: true
is_ignored: false
set:
- ignore: true
operation: ReadOne
Expand Down
1 change: 1 addition & 0 deletions apis/v1alpha1/types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions config/crd/bases/lambda.services.k8s.aws_functions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ spec:
type: string
s3ObjectVersion:
type: string
sha256:
type: string
zipFile:
format: byte
type: string
Expand Down
9 changes: 8 additions & 1 deletion generator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ resources:
- path: Status.State
in: [ "Active" ]
fields:
Code.SHA256:
type: string
compare:
is_ignored: true
set:
- ignore: "to"
method: Create
Code.S3Bucket:
references:
resource: Bucket
Expand Down Expand Up @@ -72,7 +79,7 @@ resources:
path: ReservedConcurrentExecutions
Code:
compare:
is_ignored: true
is_ignored: false
set:
- ignore: true
operation: ReadOne
Expand Down
2 changes: 2 additions & 0 deletions helm/crds/lambda.services.k8s.aws_functions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ spec:
type: string
s3ObjectVersion:
type: string
sha256:
type: string
zipFile:
format: byte
type: string
Expand Down
35 changes: 35 additions & 0 deletions pkg/resource/function/delta.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

143 changes: 48 additions & 95 deletions pkg/resource/function/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,25 +102,15 @@ func (rm *resourceManager) customUpdateFunction(
}
}
}
if delta.DifferentAt("Spec.Architectures") {
err = rm.updateFunctionArchitectures(ctx, desired, latest)
if err != nil {
return nil, err
}
}

// Only try to update Spec.Code or Spec.Configuration at once. It is
// not correct to sequentially call UpdateFunctionConfiguration and
// UpdateFunctionCode because both of them can put the function in a
// Pending state.
a-hilaly marked this conversation as resolved.
Show resolved Hide resolved
switch {
case delta.DifferentAt("Spec.Code"):
err = rm.updateFunctionCode(ctx, desired, delta)
case delta.DifferentAt("Spec.Code.ImageURI") || delta.DifferentAt("Spec.Code.SHA256") || delta.DifferentAt("Spec.Architectures"):
err = rm.updateFunctionCode(ctx, desired, delta, latest)
if err != nil {
// If the source image is not available, we get an error like this:
// "InvalidParameterValueException: Source image 1234567890.dkr.ecr.us-east-2.amazonaws.com/my-lambda:my-tag does not exist. Provide a valid source image."
// Because this may be recoverable (i.e. the image may be pushed once a build completes),
// we requeue the function for reconciliation after one minute.
if strings.Contains(err.Error(), "Provide a valid source image.") {
return nil, requeueWaitWhileSourceImageDoesNotExist
} else {
Expand All @@ -131,6 +121,7 @@ func (rm *resourceManager) customUpdateFunction(
"Spec.Code",
"Spec.Tags",
"Spec.ReservedConcurrentExecutions",
"Spec.FunctionEventInvokeConfig",
"Spec.CodeSigningConfigARN"):
err = rm.updateFunctionConfiguration(ctx, desired, delta)
if err != nil {
Expand Down Expand Up @@ -377,16 +368,17 @@ func (rm *resourceManager) updateFunctionTags(
return nil
}

// updateFunctionArchitectures calls UpdateFunctionCode to update architecture for lambda
// updateFunctionsCode calls UpdateFunctionCode to update a specific lambda
// function code.
func (rm *resourceManager) updateFunctionArchitectures(
func (rm *resourceManager) updateFunctionCode(
ctx context.Context,
desired *resource,
delta *ackcompare.Delta,
latest *resource,
) error {
var err error
rlog := ackrtlog.FromContext(ctx)
exit := rlog.Trace("rm.updateFunctionArchitectures")
exit := rlog.Trace("rm.updateFunctionCode")
defer exit(err)

dspec := desired.ko.Spec
Expand All @@ -400,60 +392,30 @@ func (rm *resourceManager) updateFunctionArchitectures(
input.Architectures = nil
}

if latest.ko.Spec.Code != nil {
if latest.ko.Spec.PackageType != nil && *latest.ko.Spec.PackageType == "Image" {
input.ImageUri = latest.ko.Spec.Code.ImageURI
} else if latest.ko.Spec.PackageType != nil && *latest.ko.Spec.PackageType == "Zip" {
input.S3Bucket = latest.ko.Spec.Code.S3Bucket
input.S3Key = latest.ko.Spec.Code.S3Key
}
}

_, err = rm.sdkapi.UpdateFunctionCodeWithContext(ctx, input)
rm.metrics.RecordAPICall("UPDATE", "UpdateFunctionArchitectures", err)
if err != nil {
return err
}

return nil
}

// updateFunctionsCode calls UpdateFunctionCode to update a specific lambda
// function code.
func (rm *resourceManager) updateFunctionCode(
ctx context.Context,
desired *resource,
delta *ackcompare.Delta,
) error {
var err error
rlog := ackrtlog.FromContext(ctx)
exit := rlog.Trace("rm.updateFunctionCode")
defer exit(err)

if delta.DifferentAt("Spec.Code.S3Key") &&
!delta.DifferentAt("Spec.Code.S3Bucket") &&
!delta.DifferentAt("Spec.Code.S3ObjectVersion") &&
!delta.DifferentAt("Spec.Code.ImageURI") {
log := ackrtlog.FromContext(ctx)
log.Info("updating code.s3Key field is not currently supported.")
return nil
}

dspec := desired.ko.Spec
input := &svcsdk.UpdateFunctionCodeInput{
FunctionName: aws.String(*dspec.Name),
}

if dspec.Code != nil {
switch {
case dspec.Code.ImageURI != nil:
input.ImageUri = dspec.Code.ImageURI
case dspec.Code.S3Bucket != nil,
dspec.Code.S3Key != nil,
dspec.Code.S3ObjectVersion != nil:
input.S3Bucket = dspec.Code.S3Bucket
input.S3Key = dspec.Code.S3Key
input.S3ObjectVersion = dspec.Code.S3ObjectVersion
if delta.DifferentAt("Spec.Code.SHA256") && dspec.Code.SHA256 != nil {
if dspec.Code.S3Key != nil {
input.S3Key = aws.String(*dspec.Code.S3Key)
}
if dspec.Code.S3Bucket != nil {
input.S3Bucket = aws.String(*dspec.Code.S3Bucket)
}
if dspec.Code.S3ObjectVersion != nil {
input.S3ObjectVersion = aws.String(*dspec.Code.S3ObjectVersion)
}
} else if delta.DifferentAt("Spec.Code.ImageURI") && dspec.Code.ImageURI != nil {
if dspec.Code.ImageURI != nil {
input.ImageUri = aws.String(*dspec.Code.ImageURI)
}

} else { // We need to pass the latest code to Update API call,
//if there is change in architecture and no change in Code
if latest.ko.Spec.PackageType != nil && *latest.ko.Spec.PackageType == "Image" {
input.ImageUri = latest.ko.Spec.Code.ImageURI
Comment on lines +413 to +414
Copy link
Member

Choose a reason for hiding this comment

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

I think we only need to update the image if the ImageURI changed

Copy link
Member Author

Choose a reason for hiding this comment

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

This code is for passing code values if there is change in Architecture field. To update Architecture field we need to pass the code again too, this handles case that even if there is no change in code, it will just pass the current values of it along with updated architecture field.

} else if latest.ko.Spec.PackageType != nil && *latest.ko.Spec.PackageType == "Zip" {
input.S3Bucket = latest.ko.Spec.Code.S3Bucket
input.S3Key = latest.ko.Spec.Code.S3Key
}
}
}

Expand Down Expand Up @@ -501,36 +463,27 @@ func customPreCompare(
a *resource,
b *resource,
) {
// No need to compare difference in S3 Key/Bucket/ObjectVersion. As in sdkFind() there is a copy 'ko := r.ko.DeepCopy()'
// of S3 Key/Bucket/ObjectVersion passed. This 'ko' then stores the values of latest S3 fields which API returns
// and compares it with desired field values. Since the API doesn't return values of S3 fields, it doesn't
// notice any changes between desired and latest, hence fails to recognize the update in the values.

// To solve this we created a new field 'Code.SHA256' to store the hash value of deployment package. Any change
// in hash value refers to change in S3 Key/Bucket/ObjectVersion and controller can recognize the change in
// desired and latest value of 'Code.SHA256' and hence calls the update function.

if ackcompare.HasNilDifference(a.ko.Spec.Code, b.ko.Spec.Code) {
delta.Add("Spec.Code", a.ko.Spec.Code, b.ko.Spec.Code)
} else if a.ko.Spec.Code != nil && b.ko.Spec.Code != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Should we consider removing this customPreCompare, generate all this code, and then rely on the custom update function to lookup *a.ko.Spec.PackageType == "Image"|"Zip"

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be better to generate this code for Code.S3Bucket, Code.S3Key and Code.ImageURI, but we will still need this custom compare code for Code.SHA256, as that is not auto-generated and we need to check if it has been changes in order to let ACK know that deployment package has been changed.

if ackcompare.HasNilDifference(a.ko.Spec.Code.ImageURI, b.ko.Spec.Code.ImageURI) {
delta.Add("Spec.Code.ImageURI", a.ko.Spec.Code.ImageURI, b.ko.Spec.Code.ImageURI)
} else if a.ko.Spec.Code.ImageURI != nil && b.ko.Spec.Code.ImageURI != nil {
if *a.ko.Spec.Code.ImageURI != *b.ko.Spec.Code.ImageURI {
delta.Add("Spec.Code.ImageURI", a.ko.Spec.Code.ImageURI, b.ko.Spec.Code.ImageURI)
}
}
//TODO(hialylmh) handle Spec.Code.S3bucket changes
// if ackcompare.HasNilDifference(a.ko.Spec.Code.S3Bucket, b.ko.Spec.Code.S3Bucket) {
// delta.Add("Spec.Code.S3Bucket", a.ko.Spec.Code.S3Bucket, b.ko.Spec.Code.S3Bucket)
// } else if a.ko.Spec.Code.S3Bucket != nil && b.ko.Spec.Code.S3Bucket != nil {
// if *a.ko.Spec.Code.S3Bucket != *b.ko.Spec.Code.S3Bucket {
// delta.Add("Spec.Code.S3Bucket", a.ko.Spec.Code.S3Bucket, b.ko.Spec.Code.S3Bucket)
// }
// }
if ackcompare.HasNilDifference(a.ko.Spec.Code.S3Key, b.ko.Spec.Code.S3Key) {
delta.Add("Spec.Code.S3Key", a.ko.Spec.Code.S3Key, b.ko.Spec.Code.S3Key)
} else if a.ko.Spec.Code.S3Key != nil && b.ko.Spec.Code.S3Key != nil {
if *a.ko.Spec.Code.S3Key != *b.ko.Spec.Code.S3Key {
delta.Add("Spec.Code.S3Key", a.ko.Spec.Code.S3Key, b.ko.Spec.Code.S3Key)
}
}
if ackcompare.HasNilDifference(a.ko.Spec.Code.S3ObjectVersion, b.ko.Spec.Code.S3ObjectVersion) {
delta.Add("Spec.Code.S3ObjectVersion", a.ko.Spec.Code.S3ObjectVersion, b.ko.Spec.Code.S3ObjectVersion)
} else if a.ko.Spec.Code.S3ObjectVersion != nil && b.ko.Spec.Code.S3ObjectVersion != nil {
if *a.ko.Spec.Code.S3ObjectVersion != *b.ko.Spec.Code.S3ObjectVersion {
delta.Add("Spec.Code.S3ObjectVersion", a.ko.Spec.Code.S3ObjectVersion, b.ko.Spec.Code.S3ObjectVersion)
if a.ko.Spec.PackageType != nil && *a.ko.Spec.PackageType == "Zip" {
if a.ko.Spec.Code.SHA256 != nil {
if ackcompare.HasNilDifference(a.ko.Spec.Code.SHA256, b.ko.Status.CodeSHA256) {
delta.Add("Spec.Code.SHA256", a.ko.Spec.Code.SHA256, b.ko.Status.CodeSHA256)
} else if a.ko.Spec.Code.SHA256 != nil && b.ko.Status.CodeSHA256 != nil {
if *a.ko.Spec.Code.SHA256 != *b.ko.Status.CodeSHA256 {
delta.Add("Spec.Code.SHA256", a.ko.Spec.Code.SHA256, b.ko.Status.CodeSHA256)
}
}
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions test/e2e/resources/function_code_s3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: lambda.services.k8s.aws/v1alpha1
kind: Function
metadata:
name: $FUNCTION_NAME
annotations:
services.k8s.aws/region: $AWS_REGION
spec:
name: $FUNCTION_NAME
architectures: [$ARCHITECTURES]
code:
s3Bucket: $BUCKET_NAME
s3Key: $LAMBDA_FILE_NAME
sha256: $HASH
role: $LAMBDA_ROLE
runtime: python3.9
handler: main
description: function created by ACK lambda-controller e2e tests
reservedConcurrentExecutions: $RESERVED_CONCURRENT_EXECUTIONS
codeSigningConfigARN: "$CODE_SIGNING_CONFIG_ARN"
2 changes: 2 additions & 0 deletions test/e2e/resources/lambda_function/updated_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
if __name__ == "__main__":
print("Updated Hello ACK!")
11 changes: 11 additions & 0 deletions test/e2e/service_bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
LAMBDA_FUNCTION_FILE_PATH = f"./resources/lambda_function/{LAMBDA_FUNCTION_FILE}"
LAMBDA_FUNCTION_FILE_PATH_ZIP = f"./resources/lambda_function/{LAMBDA_FUNCTION_FILE_ZIP}"

LAMBDA_FUNCTION_UPDATED_FILE = "updated_main.py"
LAMBDA_FUNCTION_UPDATED_FILE_ZIP = "updated_main.zip"
LAMBDA_FUNCTION_UPDATED_FILE_PATH = f"./resources/lambda_function/{LAMBDA_FUNCTION_UPDATED_FILE}"
LAMBDA_FUNCTION_UPDATED_FILE_PATH_ZIP = f"./resources/lambda_function/{LAMBDA_FUNCTION_UPDATED_FILE_ZIP}"

AWS_SIGNING_PLATFORM_ID = "AWSLambda-SHA384-ECDSA"

def zip_function_file(src: str, dst: str):
Expand Down Expand Up @@ -139,6 +144,12 @@ def service_bootstrap() -> Resources:
LAMBDA_FUNCTION_FILE_PATH_ZIP,
resources.FunctionsBucket.name,
)

zip_function_file(LAMBDA_FUNCTION_UPDATED_FILE_PATH, LAMBDA_FUNCTION_UPDATED_FILE_PATH_ZIP)
upload_function_to_bucket(
LAMBDA_FUNCTION_UPDATED_FILE_PATH_ZIP,
resources.FunctionsBucket.name,
)
except BootstrapFailureException as ex:
exit(254)
return resources
Expand Down
Loading