Skip to content

Commit 9d9f59f

Browse files
maksim-grebeniuk-sonarsourcesonartech
authored andcommitted
SONARPY-3114 Rule S7621: AWS waiters should be used instead of custom polling loops (#427)
GitOrigin-RevId: 49d2035805ab3ef579efb5de7505b336ee27cf57
1 parent 35e15cc commit 9d9f59f

File tree

10 files changed

+482
-6
lines changed

10 files changed

+482
-6
lines changed
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.Set;
20+
import org.sonar.check.Rule;
21+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
23+
import org.sonar.plugins.python.api.tree.CallExpression;
24+
import org.sonar.plugins.python.api.tree.FunctionDef;
25+
import org.sonar.plugins.python.api.tree.Tree;
26+
import org.sonar.plugins.python.api.tree.WhileStatement;
27+
import org.sonar.python.checks.utils.AwsLambdaChecksUtils;
28+
import org.sonar.python.checks.utils.Expressions;
29+
import org.sonar.python.tree.TreeUtils;
30+
import org.sonar.python.types.v2.TypeCheckMap;
31+
32+
@Rule(key = "S7621")
33+
public class AwsWaitersInsteadOfCustomPollingCheck extends PythonSubscriptionCheck {
34+
35+
private static final Set<String> NON_WAITERS_BOTO3_METHODS = Set.of(
36+
// EC2
37+
"botocore.client.BaseClient.describe_instances",
38+
"botocore.client.BaseClient.describe_instance_status",
39+
"botocore.client.BaseClient.describe_volumes",
40+
"botocore.client.BaseClient.describe_snapshots",
41+
"botocore.client.BaseClient.describe_images",
42+
"botocore.client.BaseClient.describe_vpcs",
43+
"botocore.client.BaseClient.describe_subnets",
44+
"botocore.client.BaseClient.describe_nat_gateways",
45+
"botocore.client.BaseClient.describe_key_pairs",
46+
"botocore.client.BaseClient.get_password_data",
47+
// S3
48+
"botocore.client.BaseClient.head_bucket",
49+
"botocore.client.BaseClient.head_object",
50+
// RDS
51+
"botocore.client.BaseClient.describe_db_instances",
52+
"botocore.client.BaseClient.describe_db_clusters",
53+
"botocore.client.BaseClient.describe_db_snapshots",
54+
// DynamoDB
55+
"botocore.client.BaseClient.describe_table",
56+
// ECS
57+
"botocore.client.BaseClient.describe_services",
58+
"botocore.client.BaseClient.describe_tasks",
59+
// EKS
60+
"botocore.client.BaseClient.describe_cluster",
61+
"botocore.client.BaseClient.describe_nodegroup",
62+
// CloudFormation
63+
"botocore.client.BaseClient.describe_stacks",
64+
"botocore.client.BaseClient.describe_change_set",
65+
// Lambda
66+
"botocore.client.BaseClient.get_function_configuration",
67+
"botocore.client.BaseClient.get_function"
68+
);
69+
70+
private final TypeCheckMap<Boolean> nonWaitersBoto3MethodsTypeCheckMap = new TypeCheckMap<>();
71+
72+
@Override
73+
public void initialize(Context context) {
74+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initializeCheck);
75+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::check);
76+
}
77+
78+
private void initializeCheck(SubscriptionContext ctx) {
79+
NON_WAITERS_BOTO3_METHODS.stream()
80+
.map(fqn -> ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(fqn))
81+
.forEach(check -> nonWaitersBoto3MethodsTypeCheckMap.put(check, true));
82+
}
83+
84+
private void check(SubscriptionContext ctx) {
85+
var callExpression = (CallExpression) ctx.syntaxNode();
86+
if (nonWaitersBoto3MethodsTypeCheckMap.containsForType(TreeUtils.inferSingleAssignedExpressionType(callExpression.callee()))
87+
&& TreeUtils.firstAncestorOfKind(callExpression, Tree.Kind.WHILE_STMT) instanceof WhileStatement whileStatement
88+
&& Expressions.isTruthy(whileStatement.condition())
89+
&& TreeUtils.firstAncestorOfKind(callExpression, Tree.Kind.FUNCDEF) instanceof FunctionDef functionDef
90+
&& AwsLambdaChecksUtils.isLambdaHandler(ctx, functionDef)
91+
) {
92+
ctx.addIssue(callExpression, "Use AWS waiters instead of custom polling loops.");
93+
}
94+
}
95+
96+
97+
}

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ public Stream<Class<?>> getChecks() {
133133
AwsLambdaReservedEnvironmentVariableCheck.class,
134134
AwsLambdaReturnValueAreSerializableCheck.class,
135135
AwsMissingPaginationCheck.class,
136+
AwsWaitersInsteadOfCustomPollingCheck.class,
136137
BackslashInStringCheck.class,
137138
BackticksUsageCheck.class,
138139
BareRaiseInFinallyCheck.class,
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<p>This rule raises an issue when custom polling loops are used instead of AWS waiters for checking resource states.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>When working with AWS resources that require time to reach a desired state, such as EC2 instances starting up, S3 buckets being created, or
4+
DynamoDB tables becoming available, developers often implement custom polling loops to check the resource status repeatedly. However, this approach is
5+
inefficient and error-prone. Custom polling requires manual handling of timing intervals, retry logic, error conditions, and timeout scenarios. It
6+
also tends to be verbose and duplicates logic that AWS already provides in a more robust form. AWS SDK provides waiters, which are purpose-built
7+
abstractions specifically designed to poll AWS resources until they reach a desired state. Waiters handle the complexities of polling automatically,
8+
including appropriate delays, exponential backoff, maximum wait times, and proper error handling.</p>
9+
<h3>What is the potential impact?</h3>
10+
<p>Custom polling implementations can lead to inefficient resource usage due to inappropriate polling intervals, increased complexity and maintenance
11+
overhead, potential race conditions and timing issues, and unreliable error handling that may cause application failures or infinite loops.</p>
12+
<h2>How to fix it</h2>
13+
<p>Replace custom polling loops with AWS waiters. Identify the AWS resource you need to wait for, then use the appropriate waiter method from the
14+
boto3 client. Waiters are available for most AWS services and common state transitions. Use the <code>get_waiter()</code> method on your boto3 client
15+
to obtain the waiter, then call <code>wait()</code> with the appropriate parameters.</p>
16+
<h3>Noncompliant code example</h3>
17+
<pre data-diff-id="1" data-diff-type="noncompliant">
18+
import boto3
19+
import time
20+
21+
ec2_client = boto3.client('ec2', region_name='us-east-1')
22+
23+
while True:
24+
response = ec2_client.describe_instance_status( # Noncompliant
25+
InstanceIds=[instance_id],
26+
IncludeAllInstances=True
27+
)
28+
29+
instance_status = response['Statuses'][0]['InstanceStatus']['Status']
30+
if instance_status == 'ok':
31+
break
32+
33+
time.sleep(10)
34+
</pre>
35+
<h3>Compliant solution</h3>
36+
<pre data-diff-id="1" data-diff-type="compliant">
37+
import boto3
38+
39+
ec2_client = boto3.client('ec2', region_name='us-east-1')
40+
41+
ec2_client.get_waiter('instance_status_ok').wait(
42+
InstanceIds=[instance_id],
43+
IncludeAllInstances=True
44+
)
45+
</pre>
46+
<h2>Resources</h2>
47+
<ul>
48+
<li> boto3 Documentation - <a href="https://boto3.amazonaws.com/v1/documentation/api/latest/guide/clients.html#waiters">Waiters</a> </li>
49+
<li> AWS Documentation - <a href="https://docs.aws.amazon.com/lambda/latest/dg/python-handler.html#python-handler-best-practices">AWS Lambda Python
50+
Handler Best Practices</a> </li>
51+
</ul>
52+
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"title": "AWS waiters should be used instead of custom polling loops",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"aws"
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-S7621",
14+
"sqKey": "S7621",
15+
"scope": "Main",
16+
"quickfix": "infeasible",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "LOW",
20+
"RELIABILITY": "MEDIUM"
21+
},
22+
"attribute": "EFFICIENT"
23+
}
24+
}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,9 @@
290290
"S7516",
291291
"S7517",
292292
"S7519",
293-
"S7617",
294293
"S7613",
294+
"S7617",
295+
"S7621",
295296
"S7622",
296297
"S7632"
297298
]
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class AwsWaitersInsteadOfCustomPollingCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/awsWaitersInsteadOfCustomPolling.py", new AwsWaitersInsteadOfCustomPollingCheck());
27+
}
28+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import boto3
2+
3+
client = boto3.client('s3')
4+
5+
def lambda_handler(event, context):
6+
while True:
7+
response = client.describe_instances() # Noncompliant
8+
9+
while context:
10+
response = client.describe_instances() # OK
11+
12+
response = client.describe_instances() # OK
13+
14+
15+
def not_a_lambda():
16+
while True:
17+
response = client.describe_instances() # OK
18+
19+
response = client.describe_instances() # OK
20+
21+
while a:
22+
response = client.describe_instances() # OK

0 commit comments

Comments
 (0)