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.

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

feat: migrate webhook runner configuration to SSM #3728

Merged
merged 28 commits into from
Feb 26, 2024

Conversation

GuptaNavdeep1983
Copy link
Contributor

@GuptaNavdeep1983 GuptaNavdeep1983 commented Jan 18, 2024

This PR migrates the confugration for the webhook from environment variables to SSM to avoid the maximum size of environment variables is reached.

Implementation

The webhook will read the configuration from SSM as json string. As long the lambda is hot the configuration is cached to speed-up the lambda time. In cases of configuration changes Lambda resources will be re-created by Terraform to ensure no cached values are used.

fix: #3594

@spcaipers-arm
Copy link
Contributor

Hey guys, any idea when this is going to be merged?

@GuptaNavdeep1983
Copy link
Contributor Author

Hey guys, any idea when this is going to be merged?

There were few comments on this PR. Addressed and tested them today. Hopefully we should be able to merge this week.

Copy link
Member

@npalm npalm left a comment

Choose a reason for hiding this comment

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

Still working on the review and test.

lambdas/functions/webhook/src/ConfigResolver.ts Outdated Show resolved Hide resolved
modules/webhook/variables.tf Outdated Show resolved Hide resolved
Copy link
Member

@npalm npalm left a comment

Choose a reason for hiding this comment

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

A few remarks

modules/webhook/variables.tf Outdated Show resolved Hide resolved
modules/webhook/policies/lambda-ssm.json Outdated Show resolved Hide resolved
modules/webhook/main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
modules/webhook/webhook.tf Outdated Show resolved Hide resolved
modules/webhook/webhook.tf Outdated Show resolved Hide resolved
Copy link
Member

@npalm npalm left a comment

Choose a reason for hiding this comment

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

PR looks good, made a few minor comments. Can you also check the failing CI?

GuptaNavdeep1983 and others added 3 commits February 20, 2024 21:25
Co-authored-by: Niek Palm <npalm@users.noreply.github.com>
@npalm npalm changed the title fix: move away from lambda environment variable due to size constraints fix: migrate webhook runner configuration to SSM Feb 26, 2024
@npalm npalm changed the title fix: migrate webhook runner configuration to SSM feat: migrate webhook runner configuration to SSM Feb 26, 2024
@GuptaNavdeep1983 GuptaNavdeep1983 merged commit 32492e3 into main Feb 26, 2024
36 checks passed
@GuptaNavdeep1983 GuptaNavdeep1983 deleted the nav/chore/use-ssm branch February 26, 2024 12:28
npalm pushed a commit that referenced this pull request Feb 27, 2024
🤖 I have created a release *beep* *boop*
---


##
[5.8.0](v5.7.1...v5.8.0)
(2024-02-27)


### Features

* Add option to set lambda memory increase webhook memory
([#3778](#3778))
([40bceb6](40bceb6))
* migrate webhook runner configuration to SSM
([#3728](#3728))
([32492e3](32492e3))


### Bug Fixes

* **lambda:** bump the aws group in /lambdas with 6 updates
([#3772](#3772))
([3549bc1](3549bc1))
* **lambda:** bump the aws group in /lambdas with 6 updates
([#3783](#3783))
([b850e85](b850e85))
* **lambda:** bump the octokit group in /lambdas with 2 updates
([#3773](#3773))
([de9985a](de9985a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
rvodden pushed a commit to HumanisingAutonomy/terraform-aws-github-runner that referenced this pull request Mar 3, 2024
🤖 I have created a release *beep* *boop*
---

##
[5.8.0](philips-labs/terraform-aws-github-runner@v5.7.1...v5.8.0)
(2024-02-27)

### Features

* Add option to set lambda memory increase webhook memory
([philips-labs#3778](philips-labs#3778))
([40bceb6](philips-labs@40bceb6))
* migrate webhook runner configuration to SSM
([philips-labs#3728](philips-labs#3728))
([32492e3](philips-labs@32492e3))

### Bug Fixes

* **lambda:** bump the aws group in /lambdas with 6 updates
([philips-labs#3772](philips-labs#3772))
([3549bc1](philips-labs@3549bc1))
* **lambda:** bump the aws group in /lambdas with 6 updates
([philips-labs#3783](philips-labs#3783))
([b850e85](philips-labs@b850e85))
* **lambda:** bump the octokit group in /lambdas with 2 updates
([philips-labs#3773](philips-labs#3773))
([de9985a](philips-labs@de9985a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lambda environment paramaters can exceed 4KB limit
3 participants