Skip to content

Conversation

a-hilaly
Copy link
Member

@a-hilaly a-hilaly commented Jan 28, 2022

Issue #, if available: aws-controllers-k8s/community#1117

Description of changes:
Functions code signing config should only be called when a function is
created using an s3bucket and a key. Functions created using a container
image cannot get a code signing configuration.

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

Copy link
Contributor

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

I'd like to see an e2e test added that stresses this code path please! :)

@jaypipes
Copy link
Contributor

jaypipes commented Mar 7, 2022

@a-hilaly where do we stand with this PR? :)

@a-hilaly
Copy link
Member Author

a-hilaly commented Mar 7, 2022

@a-hilaly where do we stand with this PR? :)

This PR will need more work than expected. We will need proper support for Images based lambda functions (Initializers, custom hooks and proper e2e for it).
Updated #1062

@a-hilaly
Copy link
Member Author

/hold

@ack-bot ack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 18, 2022
@a-hilaly a-hilaly force-pushed the issue/1117 branch 2 times, most recently from 911b61e to 2f08b31 Compare March 21, 2022 15:07
@a-hilaly a-hilaly force-pushed the issue/1117 branch 2 times, most recently from 0678a82 to 255650e Compare March 25, 2022 16:19
@a-hilaly
Copy link
Member Author

/retest

@gagan-eg
Copy link

Do you know the timeline for this to be merged? We switched to using image based Function because currently S3 Key change updates are not supported.

@a-hilaly
Copy link
Member Author

Do you know the timeline for this to be merged? We switched to using image based Function because currently S3 Key change updates are not supported.

@gagan-eg very soon :)

@a-hilaly
Copy link
Member Author

/retest
/unhold

@ack-bot ack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 26, 2022
@a-hilaly
Copy link
Member Author

/test lambda-kind-e2e

@jaypipes
Copy link
Contributor

jaypipes commented May 2, 2022

@a-hilaly looking at the failure in the e2e test, I think you need to add terminal codes to the generator.yaml so that things like InvalidParameterValueException are understood as terminal exceptions....

@a-hilaly a-hilaly force-pushed the issue/1117 branch 2 times, most recently from b25dcbb to cda221d Compare May 6, 2022 16:35
@a-hilaly
Copy link
Member Author

a-hilaly commented May 6, 2022

/retest

1 similar comment
@a-hilaly
Copy link
Member Author

a-hilaly commented May 6, 2022

/retest

@a-hilaly a-hilaly force-pushed the issue/1117 branch 2 times, most recently from 54941f8 to 4b241ff Compare May 6, 2022 21:42
…en a functions packageType is `Image`

Functions code signing config should only be called when a function is
created using an s3bucket and a key. Functions created using a container
image cannot get a code signing configuration.
Copy link
Contributor

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

Really nice work on this, @a-hilaly 👍

@jaypipes
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label May 23, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented May 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, jaypipes, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [A-Hilaly,jaypipes,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit c7ea860 into aws-controllers-k8s:main May 23, 2022
michaelhtm pushed a commit to rushmash91/lambda-controller that referenced this pull request Feb 5, 2025
### Update ACK runtime to `v0.14.1`

#### stdout for `make build-controller`:

```
building ack-generate ... ok.
==== building mq-controller ====
Copying common custom resource definitions into mq
Building Kubernetes API objects for mq
Generating deepcopy code for mq
Generating custom resource definitions for mq
Building service controller for mq
Generating RBAC manifests for mq
Running gofmt against generated code for mq
Updating additional GitHub repository maintenance files
==== building mq-controller release artifacts ====
Building release artifacts for mq-v0.0.5
Generating common custom resource definitions
Generating custom resource definitions for mq
Generating RBAC manifests for mq
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants