Skip to content

Conversation

Alex-Waring
Copy link
Contributor

@Alex-Waring Alex-Waring commented Oct 24, 2024

User description

…block

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

Description

For reasons that are beyond me, enabled_cluster_log_types is a list of strings normally, but when there's a dynamic block added to the resource it becomes a list of list of strings. This PR adds tests to confirm that this is indeed an issue (if you run the test file against the check in master then it fails on aws_eks_cluster.fully_enabled_with_dynamic_block), and fixes the issue by checking the type before itterating.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes

Generated description

Below is a concise technical summary of the changes proposed in this PR:

TopicDetails
Fix logging check Updates the EKSControlPlaneLogging check to handle dynamic blocks in AWS EKS cluster resources
Modified files (1)
  • checkov/terraform/checks/resource/aws/EKSControlPlaneLogging.py
Latest Contributors(2)
UserCommitDate
manuchandrasekhar@gmai...fix-terraform-and-cdk-...January 28, 2024
peleg.litvak@gmail.comfix-terraform-handle-e...November 16, 2022
Add tests Adds test cases for the EKSControlPlaneLogging check, including Terraform configurations and Python unit tests
Modified files (2)
  • tests/terraform/checks/resource/aws/example_EKSControlPlaneLogging/main.tf
  • tests/terraform/checks/resource/aws/test_EKSControlPlaneLogging.py
Latest Contributors(2)
UserCommitDate
peleg.litvak@gmail.comfix-terraform-handle-e...November 16, 2022
gruebelfix-flake8-issue-W391-...March 25, 2022
This pull request is reviewed by Baz. Join @Alex-Waring and the rest of your team on (Baz).

@Alex-Waring
Copy link
Contributor Author

@bo156 @gruebel can you take a look or find someone to please?

@Alex-Waring
Copy link
Contributor Author

@Saarett I see you were active on my other PR, this one is ready for review if you have time please

@tsmithv11
Copy link
Contributor

@Alex-Waring looks good, but can you fixe the flake8 errors?

@MaryArmaly
Copy link
Contributor

Hey @Alex-Waring,
Could you please fix the failing test and merge the latest changes from the main branch into your branch? Thanks!

@Alex-Waring Alex-Waring force-pushed the AWar/eks_control_plane_logging/bug branch from 6883990 to 1f88271 Compare August 20, 2025 14:10
@Alex-Waring
Copy link
Contributor Author

@MaryArmaly can you please re-run the CI?

@Alex-Waring
Copy link
Contributor Author

Hi @tjwald, I no longer have the repo setup for local development here given the time it took for a review. If this is something you are interested in adding to the project then please feel free to commit to this pr.

fix linting and access pattern
@tjwald tjwald merged commit 2a57d95 into bridgecrewio:main Sep 30, 2025
46 checks passed
Saarett pushed a commit that referenced this pull request Sep 30, 2025
#6792)

* fix(terraform): correctly evaluate CKV_AWS_37 when there's a dynamic block

* Update EKSControlPlaneLogging.py

fix linting and access pattern

---------

Co-authored-by: Tj <52830708+tjwald@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.

7 participants