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: add KubectLayer and release as @aws-cdk/lambda-layer-kubectl-v25 #174

Merged
merged 8 commits into from
Mar 7, 2023
Merged

feat: add KubectLayer and release as @aws-cdk/lambda-layer-kubectl-v25 #174

merged 8 commits into from
Mar 7, 2023

Conversation

CarlosDomingues
Copy link
Contributor

@CarlosDomingues CarlosDomingues commented Mar 1, 2023

Adds support for kubectl 1.25

Closes #166.

@CarlosDomingues CarlosDomingues marked this pull request as ready for review March 1, 2023 18:18
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

This all looks good with just one minor comment (that's our fault lol). Can I trust that you've actually deployed the integ tests and not just updated the snapshots?

API.md.md Outdated Show resolved Hide resolved
@CarlosDomingues
Copy link
Contributor Author

CarlosDomingues commented Mar 1, 2023

This all looks good with just one minor comment (that's our fault lol)

Ok, will fix that later today.

EDIT: Just realized you made a PR earlier 😅

Can I trust that you've actually deployed the integ tests and not just updated the snapshots?

Yes. Please check the screenshots attached and let me know whether the tests looks correct.

deploy-01

deploy-02

@kaizencc
Copy link
Contributor

kaizencc commented Mar 1, 2023

Thank you for the screenshots! They look perfect. I'm good with this PR, and once it's released, we can reference it in the CDK repo as an option. Edit: see ensuing comment.

README.md Outdated Show resolved Hide resolved
src/kubectl-layer.ts Outdated Show resolved Hide resolved
test/kubectl-layer.test.ts Outdated Show resolved Hide resolved
@kaizencc
Copy link
Contributor

kaizencc commented Mar 2, 2023

@CarlosDomingues thanks for your work so far :). We are basically done but as you can see, the lambda description was still referencing helm 3.10. I updated the description, but I think we're gonna get unlucky because the description is part of the lambda hash so the snapshot may need updating. Once that's updated, I shall approve!

@CarlosDomingues
Copy link
Contributor Author

CarlosDomingues commented Mar 3, 2023

Just to confirm, do I need to re-deploy the project with the correct description to update the hashes and commit them?

@kaizencc
Copy link
Contributor

kaizencc commented Mar 3, 2023

@CarlosDomingues nah, just snapshot update is ok in this case :)

@robertd
Copy link
Contributor

robertd commented Mar 6, 2023

@CarlosDomingues @kaizencc Are we using helm 3.10.x just for backward support of k8s 1.22? https://helm.sh/docs/topics/version_skew/

@robertd
Copy link
Contributor

robertd commented Mar 6, 2023

Scratch my previous comment... I just saw that helm was brought up to 3.11.x which supports k8s 1.23.x - 1.26.x. Carry on :)

Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

@CarlosDomingues thanks for your contribution!

@mergify mergify bot merged commit 023ea36 into cdklabs:kubectl-v25/main Mar 7, 2023
mergify bot pushed a commit to aws/aws-cdk that referenced this pull request Mar 9, 2023
homakk pushed a commit to homakk/aws-cdk that referenced this pull request Mar 28, 2023
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.

3 participants