feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test #32366

xazhao · 2024-12-02T23:20:02Z

Reason for this change

Setup the alpha module for the EKS L2 rewrite aws-eks-v2-alpha.

This module will be private until all development are done by setting "private": true in package.json,

Also setup unit tests/integration tests so future changes will be visible on test diff.

Description of changes

This is the full copy of existing aws-eks module. I made some minor changes like updating import path to make sure build and all tests pass.

Description of how you validated changes

unit tests and integration tests

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

codecov · 2024-12-02T23:43:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.67%. Comparing base (c5bcfdc) to head (c3f3e11).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32366   +/-   ##
=======================================
  Coverage   78.67%   78.67%           
=======================================
  Files         107      107           
  Lines        7237     7237           
  Branches     1329     1329           
=======================================
  Hits         5694     5694           
  Misses       1357     1357           
  Partials      186      186

Flag	Coverage Δ
suite.unit	`78.67% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`78.67% <ø> (ø)`

pahud · 2024-12-03T00:00:55Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+    skip_crds        = props.get('SkipCrds', False)
+
+    # "log in" to the cluster
+    subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Starting a process with a partial executable path https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html

pahud · 2024-12-03T00:00:57Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/patch/__init__.py

+    role_arn      = props['RoleArn']
+
+    # "log in" to the cluster
+    subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Starting a process with a partial executable path https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html

pahud · 2024-12-03T00:00:59Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+def get_chart_asset_from_url(chart_asset_url):
+    chart_zip = os.path.join(outdir, 'chart.zip')
+    shutil.rmtree(chart_zip, ignore_errors=True)
+    subprocess.check_call(['aws', 's3', 'cp', chart_asset_url, chart_zip])


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Starting a process with a partial executable path https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html

pahud · 2024-12-03T00:01:00Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/get/__init__.py

+    role_arn      = props['RoleArn']
+
+    # "log in" to the cluster
+    subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Starting a process with a partial executable path https://bandit.readthedocs.io/en/latest/plugins/b607_start_process_with_partial_path.html

pahud · 2024-12-03T00:01:02Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+    while retry > 0:
+        try:
+            logger.info(cmnd)
+            output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, shell=True)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

subprocess call with shell=True identified, security issue. https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html

pahud · 2024-12-03T00:01:04Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+                retry = retry - 1
+                logger.info("Broken pipe, retries left: %s" % retry)
+            else:
+                raise Exception(output)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

Similar issue at line number 158.

pahud · 2024-12-03T00:01:06Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+                retry = retry - 1
+                logger.info("Broken pipe, retries left: %s" % retry)
+            else:
+                raise Exception(output)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

Similar issue at line number 203.

pahud · 2024-12-03T00:01:08Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/patch/__init__.py

+    elif request_type == 'Delete':
+        patch_json = restore_patch_json
+    else:
+        raise Exception("invalid request type %s" % request_type)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

pahud · 2024-12-03T00:01:10Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/get/__init__.py

+    elif request_type == 'Delete':
+        pass
+    else:
+        raise Exception("invalid request type %s" % request_type)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

pahud · 2024-12-03T00:01:12Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/patch/__init__.py

+                retry = retry - 1
+                logger.info("kubectl timed out, retries left: %s" % retry)
+            else:
+                raise Exception(output)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

Similar issue at line number 70.

pahud · 2024-12-03T00:01:14Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/apply/__init__.py

+                retry = retry - 1
+                logger.info("kubectl timed out, retries left: %s" % retry)
+            else:
+                raise Exception(output)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

Similar issue at line number 95.

pahud · 2024-12-03T00:01:16Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/index.py

+  if resource_type == 'Custom::AWSCDK-EKS-KubernetesObjectValue':
+    return get_handler(event, context)
+
+  raise Exception("unknown resource type %s" % resource_type)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

pahud · 2024-12-03T00:01:19Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/get/__init__.py

+                logger.info("kubectl timed out, retries left: %s" % retry)
+                retry = retry - 1
+            else:
+                raise Exception(output)


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Using Exception and BaseException can make your code prone to errors and difficult to maintain. Instead, we recommend using one of the Built-in Exceptions or creating a custom exception class that is derived from Exception or one of its subclasses.

pahud · 2024-12-03T00:01:21Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+
+    if request_type == 'Create' or request_type == 'Update':
+        # Ensure chart or chart_asset_url are set
+        if chart == None and chart_asset_url == None:


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The == and != operators use the compared objects' __eq__ method to test if they are equal. To check if an object is a singleton, such as None, we recommend that you use the is identity comparison operator.

Learn more

Similar issue at line numbers 80, 81, 82, and 83.

pahud · 2024-12-03T00:01:23Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+    import subprocess
+
+    cmnd = ['helm', verb, release]
+    if not chart is None:


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Use is not operator rather than not ... is because it is more readable and makes your code easier to understand.

Learn more

Similar issue at line numbers 171, 173, 175, 177, and 183.

pahud · 2024-12-03T00:01:24Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+
+    # Write out the values to a file and include them with the install and upgrade
+    values_file = None
+    if not request_type == "Delete" and not values_text is None:


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Use is not operator rather than not ... is because it is more readable and makes your code easier to understand.

Learn more

pahud · 2024-12-03T00:01:26Z

packages/@aws-cdk/aws-eks-v2-alpha/lib/kubectl-handler/helm/__init__.py

+            chart = get_chart_asset_from_url(chart_asset_url)
+
+        if repository is not None and repository.startswith('oci://'):
+            tmpdir = tempfile.TemporaryDirectory()


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Problem
This line of code might contain a resource leak. Resource leaks can cause your system to slow down or crash.

Fix
Consider closing the following resource: tmpdir. The resource is allocated by call tempfile.TemporaryDirectory. Execution paths that do not contain closure statements were detected. To prevent this resource leak, close tmpdir in a try-finally block or declare it using a with statement.

More info
View details about the with statement in the Python developer's guide (external link).

gracelu0

LGTM

mergify · 2024-12-03T23:43:35Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2024-12-03T23:48:36Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

xazhao · 2024-12-04T00:27:40Z

@Mergifyio refresh

mergify · 2024-12-04T00:27:46Z

refresh

✅ Pull request refreshed

mergify · 2024-12-04T01:11:28Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

GavinZZ

I think this module is too large to provide valuable feedback and reviews. I'll approve this since Xia mentioned that this is pretty much just a copy&paste of the current eks module. I think merging this in now makes sense so that all the future changes are reviewable and those changes are what we care the most about.

mergify · 2024-12-04T19:36:46Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

xazhao · 2024-12-04T19:40:41Z

@mergify Update

mergify · 2024-12-04T19:41:01Z

Update

❌ Sorry but I didn't understand the command. Please consult the commands documentation 📚.

aws-cdk-automation · 2024-12-04T20:12:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: c3f3e11
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-12-04T20:13:12Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-12-04T20:22:49Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

aws-cdk-automation requested a review from a team December 2, 2024 23:20

github-actions bot added the p2 label Dec 2, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Dec 2, 2024

aws-cdk-automation previously requested changes Dec 2, 2024

View reviewed changes

xazhao force-pushed the xazhao/eks-rewrite branch from 53b6ba8 to cff9406 Compare December 2, 2024 23:26

xazhao changed the title ~~feat(eks-v2-alpha): Create the alpha package for the EKS L2 rewrite and setup test.~~ feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test Dec 2, 2024

create eks v2 alpha module

8e24182

xazhao force-pushed the xazhao/eks-rewrite branch from 6936205 to 8e24182 Compare December 2, 2024 23:30

xazhao and others added 2 commits December 2, 2024 15:44

Merge branch 'main' into xazhao/eks-rewrite

a270e61

update yarn lock

e63e4b0

xazhao marked this pull request as ready for review December 2, 2024 23:50

pahud reviewed Dec 3, 2024

View reviewed changes

xazhao added 4 commits December 2, 2024 16:15

fix lint errors

dea25fb

lint

27be657

lint

ef1e7a1

lint

bfbd54b

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 3, 2024

gracelu0 approved these changes Dec 3, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 3, 2024

Merge branch 'main' into xazhao/eks-rewrite

a3b3482

GavinZZ approved these changes Dec 4, 2024

View reviewed changes

Merge branch 'main' into xazhao/eks-rewrite

127c5db

Merge branch 'main' into xazhao/eks-rewrite

c3f3e11

xazhao merged commit b30c823 into main Dec 4, 2024
14 of 16 checks passed

xazhao deleted the xazhao/eks-rewrite branch December 4, 2024 20:22

github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test #32366

feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test #32366

xazhao commented Dec 2, 2024 •

edited

Loading

aws-cdk-automation left a comment

codecov bot commented Dec 2, 2024 •

edited

Loading

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

pahud Dec 3, 2024

gracelu0 left a comment

mergify bot commented Dec 3, 2024

mergify bot commented Dec 3, 2024

xazhao commented Dec 4, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

GavinZZ left a comment

mergify bot commented Dec 4, 2024

xazhao commented Dec 4, 2024

mergify bot commented Dec 4, 2024

aws-cdk-automation commented Dec 4, 2024

mergify bot commented Dec 4, 2024

github-actions bot commented Dec 4, 2024

feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test #32366

feat(eks-v2-alpha): create the alpha package for the EKS L2 rewrite and setup test #32366

Conversation

xazhao commented Dec 2, 2024 • edited Loading

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gracelu0 left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 3, 2024

mergify bot commented Dec 3, 2024

xazhao commented Dec 4, 2024

mergify bot commented Dec 4, 2024

✅ Pull request refreshed

mergify bot commented Dec 4, 2024

GavinZZ left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 4, 2024

xazhao commented Dec 4, 2024

mergify bot commented Dec 4, 2024

❌ Sorry but I didn't understand the command. Please consult the commands documentation 📚.

aws-cdk-automation commented Dec 4, 2024

AWS CodeBuild CI Report

mergify bot commented Dec 4, 2024

github-actions bot commented Dec 4, 2024

xazhao commented Dec 2, 2024 •

edited

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading