fix(cloudsupport): enforce bounded context timeouts for EKS AWS calls by shellyco-code · Pull Request #180 · kubescape/k8s-interface

shellyco-code · 2026-10-06T07:27:14Z

Overview

This PR enforces bounded context timeouts for all AWS SDK calls in cloudsupport/v1/ekssupport.go, aligning EKS support with the precedent established for AKS in PR #149.

Previously, ekssupport.go used unbounded context.TODO() across all operations. When scanning in air-gapped or network-restricted environments where AWS IMDS (169.254.169.254) or STS endpoints are unreachable, calls to config.LoadDefaultConfig and AWS service clients hung indefinitely, stalling the scan runner.

Changes Made

Defined bounded timeout constants in cloudsupport/v1/ekssupport.go:
- eksCallTimeout = 5 * time.Second (for single-call operations: GetClusterDescribe, GetRegion, GetEKSCfgMap)
- eksRBACEnumerationTimeout = 30 * time.Second (for paginated operations: GetDescribeRepositories, GetListEntitiesForPolicies, GetPolicyVersion)
Replaced all 11 unbounded context.TODO() instances with context.WithTimeout(context.Background(), ...) and deferred cancellation.
Updated listPoliciesWithPagination to accept and pass ctx context.Context into paginator.NextPage(ctx) to respect the enumeration deadline.
Added TestEKSTimeoutConstants in cloudsupport/v1/ekssupport_test.go to validate timeout configurations.

How to Test

Run the test suite:

go test -v ./cloudsupport/v1/...
go test ./...
Related issues/PRs:
Fixes #179

Checklist before requesting a review
 My code follows the style guidelines of this project
 I have commented on my code, particularly in hard-to-understand areas
 I have performed a self-review of my code
 I have added thorough tests
 New and existing unit tests pass locally with my changes

Signed-off-by: Komal <komal@example.com>

coderabbitai · 2026-10-06T07:27:23Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 60fd2199-eedf-463d-8e44-a64a78ab29d7

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

matthyx

The timeout work is needed: on main fa34e9d, a controlled EKS endpoint replying after seven seconds returns successfully after seven seconds; this commit correctly cancels at five seconds. However, the shared IAM inventory deadline introduces the confirmed P2 regression below. Verdict: request changes.

History: #179 requests these bounds; merged #149 establishes the AKS precedent, including the maintainer's warning about timing out healthy RBAC enumeration (#149 (comment)). #142/#162 fix region/name discovery and #156 supplies unavailable-error handling; none implements these EKS deadlines. Searches across open/closed/merged PRs and issues used timeout, EKS, ekssupport, context, air-gapped, offline, IMDS, STS and hang, with 100-result caps, plus recent unmerged closures. No superseding fix or explicit rejection of this approach was found; this is a bounded search, not proof of absence.

Validation: credential-free Docker, Go 1.25, go test -p 2 ./cloudsupport/v1/... passed on the reviewed head. Separate controlled-endpoint probes compared main and head and expose the IAM regression. git diff --check and changed-file formatting passed. The new test only asserts constants; please add behavioral coverage. Head CI build/Basic-Test passed (https://github.com/kubescape/k8s-interface/actions/runs/37429761693); License Compliance reports ERROR, with cause undetermined. No live AWS test, full local suite or standalone lint was run.

The AWS call/context plumbing and cancellation cleanup otherwise look sound, with no public API, dependency or format changes. Residual pre-existing limitation: client-go exec authentication can outlive the ConfigMap request deadline; this is not an introduced blocker. Main already supports KS_OFFLINE=true, which skips cloud data altogether and does not replace bounded online discovery. Head/base and existing review threads were rechecked before submission; no existing blocker comments to duplicate.

matthyx · 2026-10-06T09:43:39Z

 	return result, nil
 }

 func (eksSupport *EKSSupport) GetListEntitiesForPolicies(region string) (*ListEntitiesForPolicies, error) {


[P2] Avoid discarding healthy IAM inventories under one fixed 30-second deadline. This context covers config loading, all ListPolicies pages and every serial ListEntitiesForPolicy call; GetPolicyVersion has the same pattern at lines 258–280. A controlled endpoint returning 64 policies and successful entity responses at 500 ms each completes on main, but this head cancels at 30.000 s and returns nil, losing all collected entries. The cloud wrapper then reports ErrCloudDescribeUnavailable. ListPolicies defaults to all policies, including AWS-managed and unattached ones (https://docs.aws.amazon.com/IAM/latest/APIReference/API_ListPolicies.html), so work grows with inventory size. Keep individual requests bounded and separate/configure any overall inventory ceiling, with a regression for successful enumeration beyond 30 seconds. #179 explicitly asks for 30 seconds overall; if dropping healthy inventories at that cutoff is an intentional product decision, please document and test that tradeoff rather than assuming AKS's budget preserves EKS coverage.

I rechecked after “PTAL now”, but the PR still contains only the original commit, ba927a8. No updated implementation or tests are visible yet, so the existing finding remains unresolved. Please push the intended changes to this PR branch so they can be reviewed.

The new commit 4305a74 addresses this original finding. I reran the same controlled-endpoint reproduction: all 64 IAM entries now return successfully in 32.07 seconds, while the EKS describe deadline still works. Thank you for adding the behavioral regression coverage. The latest review has a separate, focused concern about silently ignoring invalid inventory-timeout configuration.

Please check now

…ntories - Bound individual AWS IAM requests by eksCallTimeout (5s) instead of imposing a single 30s deadline across serial loops - Introduce KS_EKS_INVENTORY_TIMEOUT for configurable overall inventory ceilings - Add behavioral test coverage for EKS describe timeouts, IAM policy entities, and policy version parsing - Add regression test verifying enumeration beyond 30s succeeds Signed-off-by: Komal <komal@example.com>

matthyx

Re-reviewed 4305a74 against unchanged main fa34e9d. The previous fixed-30s inventory blocker is addressed: fresh per-request contexts allow healthy enumeration to complete, with an optional overall ceiling. The original controlled-endpoint reproduction now returns all 64 entries in 32.07s; stalled EKS DescribeCluster still cancels at 5.00s. The new behavior tests, including the >30s regression and configured deadline, pass. One new P2 configuration-validation concern remains below; this does not imply that individual AWS calls can hang indefinitely.

Validation: credential-free Docker, Go 1.25; AWS_EC2_METADATA_DISABLED=true go test -p 2 -count=1 -timeout 120s ./cloudsupport/v1/... passed (37.08s). The original TestReview probes also passed (37.09s). git diff --check and changed-file formatting passed. Latest build/Basic-Test CI passed at this exact head: https://github.com/kubescape/k8s-interface/actions/runs/37590009918. License Compliance still reports ERROR, with cause undetermined; current branch protection lists no required status checks. No live AWS validation, full local suite or standalone lint was run.

History and necessity remain as recorded in the earlier review (#179, merged AKS precedent #149, EKS discovery #142/#162 and error handling #156); refreshed repository searches found no superseding timeout fix. Code-review and architecture lanes both rechecked the update; the original architectural blocker is resolved. Remaining nonblocking coverage opportunities are multi-page pagination and isolating both timeout environment variables in tests. The PR description still describes the superseded fixed IAM deadline. Head/base and threads were checked again before submission. Verdict: request changes narrowly for visible validation of supplied inventory-timeout settings.

matthyx · 2026-10-07T10:42:30Z


+func getInventoryContext() (context.Context, context.CancelFunc) {
+	if val, ok := os.LookupEnv(KS_EKS_INVENTORY_TIMEOUT_ENV_VAR); ok && val != "" {
+		if d, err := time.ParseDuration(val); err == nil && d > 0 {


[P2] Make invalid inventory-timeout configuration visible rather than silently removing the ceiling. With KS_EKS_INVENTORY_TIMEOUT=30 (missing the duration unit), time.ParseDuration fails; with no legacy alias set, this function returns context.Background() and collection proceeds without the overall limit the operator attempted to configure. Nonpositive values take the same silent fallback, and an invalid primary value can unexpectedly select the legacy alias. Each individual request remains bounded, but cumulative inventory duration is then unrestricted. Please surface a clear validation diagnostic or return a configuration error, document the fallback/alias precedence, and test an invalid supplied value. This does not require aborting scanning if a documented warning-and-fallback policy is intended.

fix(cloudsupport): enforce bounded context timeouts for EKS AWS calls

ba927a8

Signed-off-by: Komal <komal@example.com>

matthyx added this to KS PRs tracking Oct 6, 2026

matthyx requested changes Oct 6, 2026

View reviewed changes

matthyx requested changes Oct 7, 2026

View reviewed changes

This was referenced Oct 11, 2026

fix(gkesupport): add bounded timeouts for GKE cluster describe, token, and IAM calls #183

Open

fix(cloudsupport): enforce bounded context timeouts for GKE Google Cl… #184

Open

Navigation Menu

Repository navigation

fix(cloudsupport): enforce bounded context timeouts for EKS AWS calls - #180

fix(cloudsupport): enforce bounded context timeouts for EKS AWS calls#180
shellyco-code wants to merge 2 commits into
kubescape:mainfrom
shellyco-code:fix/ekssupport-timeout

shellyco-code commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

matthyx left a comment

Uh oh!

matthyx Oct 6, 2026

Uh oh!

shellyco-code Oct 6, 2026

Uh oh!

matthyx Oct 7, 2026

Uh oh!

matthyx Oct 7, 2026

Uh oh!

shellyco-code Oct 7, 2026

Uh oh!

matthyx left a comment

Uh oh!

matthyx Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shellyco-code commented Oct 6, 2026

Overview

Changes Made

How to Test

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthyx left a comment

Choose a reason for hiding this comment

Uh oh!

matthyx Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

shellyco-code Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

matthyx Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

matthyx Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

shellyco-code Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

matthyx left a comment

Choose a reason for hiding this comment

Uh oh!

matthyx Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading