-
Notifications
You must be signed in to change notification settings - Fork 55
feat:[PEA-2706] Refresh credentials implementation for data shipper #2407
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ public List<Map<String, String>> uploadVulnerabilityInfo(String dataSource) { | |
| LOGGER.info("Started Vulnerability collection for - {}", dataSource); | ||
| List<Map<String, String>> errorList = new ArrayList<>(); | ||
| AmazonS3 s3Client = AmazonS3ClientBuilder.standard().withCredentials( | ||
| new AWSStaticCredentialsProvider(new CredentialProvider().getCredentials(S3_ACCOUNT, S3_ROLE))).withRegion(S3_REGION).build(); | ||
| (new CredentialProvider().getCredentials(S3_ACCOUNT, S3_ROLE))).withRegion(S3_REGION).build(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate refreshable provider construction and lifecycle handling.
rg -n -C3 \
'STSAssumeRoleSessionCredentialsProvider|new CredentialProvider\(\)\.getCredentials|\.close\(\)|\.shutdown\(\)' \
jobs/pacman-data-shipper/src/main/javaRepository: PaladinCloud/CE Length of output: 8530 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Candidate files =="
fd -a 'VulnerabilityAssociationManager.java|DynamoDBHelper.java|CredentialProvider.java|LambdaInvoker.java' jobs/pacman-data-shipper/src/main/java || true
echo
echo "== Build/config for AWS SDK version =="
fd -a 'pom.xml|build.gradle|build.gradle.kts|settings.gradle|settings.gradle.kts' jobs/pacman-data-sdk-shipper . | sed 's#^\./##' || true
rg -n -C3 'aws-java-sdk|software.amazon.awssdk|dependency: aws-java-sdk|sdk-bom|version\.aws|<aws' -S . || true
echo
echo "== File contents =="
for f in \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/entity/VulnerabilityAssociationManager.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/DynamoDBHelper.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/config/CredentialProvider.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman(datashipper/util/LambdaInvoker.java
do
echo "--- $f ---"
wc -l "$f"
cat -n "$f" || true
doneRepository: PaladinCloud/CE Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== pom relevant dependencies =="
sed -n '1,120p' jobs/pacman-data-shipper/pom.xml
echo
echo "== target files =="
for f in \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/entity/VulnerabilityAssociationManager.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/DynamoDBHelper.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/config/CredentialProvider.java \
jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/LambdaInvoker.java
do
echo "--- $f ---"
wc -l "$f"
cat -n "$f"
done
echo
echo "== AWS SDK 1.11.490 STSAssumeRoleSessionCredentialsProvider definitions from jars/remote =="
python3 - <<'PY'
import json, urllib.request, tempfile, zipfile, os, re, pathlib
urls = [
"https://repo1.maven.org/maven2/com/amazonaws/aws-java-sdk-sts/1.11.490/aws-java-sdk-sts-1.11.490.jar",
"https://repo1.maven.org/maven2/com/amazonaws/aws-java-sdk-parent/1.11.490/aws-java-sdk-parent-1.11.490.pom",
]
for url in urls:
print(f"FETCH {url}")
with urllib.request.urlopen(url, timeout=20) as r:
data = r.read()
if url.endswith(".jar"):
with tempfile.NamedTemporaryFile(suffix=".jar") as tf:
tf.write(data)
tf.flush()
with zipfile.ZipFile(tf.name) as z:
names = [n for n in z.namelist() if "STSAssumeRoleSessionCredentialsProvider" in n or "CredentialsProviderUtil" in n or "Refreshable" in n]
print("entries:", names)
for name in names[:12]:
text = z.read(name).decode("utf-8", errors="replace")
lines = text.splitlines()
for i,l in enumerate(lines[:120], 1):
if any(kw in l for kw in ["close", "shutdown", "new STSSecurityTokenService", "AWSSecurityTokenService", "STSAssumeRoleSessionCredentialsProvider", "closeStsClient"]):
print(f"{name}:{i}: {l}")
PYRepository: PaladinCloud/CE Length of output: 26406 🌐 Web query:
💡 Result: In the AWS SDK for Java 1.11, calling the close method on an STSAssumeRoleSessionCredentialsProvider does not explicitly close the underlying STS client [1][2]. The close method is primarily responsible for shutting down the background thread used for asynchronous credential refreshing [1][3]. The STS client used by this provider is typically managed separately by the user, especially if it was passed into the provider's builder [2]. If you created an STS client specifically to use with the credentials provider, you must manage its lifecycle independently and call its close method when it is no longer required [1][2]. The SDK does not automatically close resources such as custom ExecutorServices or the STS client itself upon closing the credentials provider [1][2]. In summary: 1. The close method on STSAssumeRoleSessionCredentialsProvider terminates the background refresh thread [1][4]. 2. You must manually close the STS client instance used by the provider if it is no longer needed [2]. 3. Similarly, if you provided a custom ExecutorService to the provider, you are responsible for shutting it down [1][2]. Citations:
Close STS credential sessions after each request-scoped AWS client.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
|
|
||
| for (Map.Entry<String, String> entry : sourceFileToIndexMapping.entrySet()) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: PaladinCloud/CE
Length of output: 50371
🏁 Script executed:
Repository: PaladinCloud/CE
Length of output: 50371
🌐 Web query:
AWS Java SDK 1 x AWSSecurityTokenServiceClientBuilder defaultClient region environment variable AWS_DEFAULT_REGION💡 Result:
In the AWS SDK for Java 1.x, the AWSSecurityTokenServiceClientBuilder.defaultClient method uses the DefaultAwsRegionProviderChain to determine the AWS region [1][2][3]. This provider chain does not check the AWS_DEFAULT_REGION environment variable by default; it specifically prioritizes the AWS_REGION environment variable [4][5][6]. The standard region lookup process used by the SDK for Java 1.x builder is: 1. Explicit region configuration set via the builder (e.g.,.withRegion) [4][5]. 2. The AWS_REGION environment variable [4][5]. 3. The AWS shared configuration file (typically ~/.aws/config) [4][5]. 4. The Amazon EC2 instance metadata service [4][5]. Because AWS_DEFAULT_REGION is not part of this specific provider chain for the Java 1.x SDK, it will be ignored if AWS_REGION is not set. If you require the SDK to recognize AWS_DEFAULT_REGION, you would typically need to manually configure the client or implement a custom region provider, as the built-in default chain is explicitly defined to use AWS_REGION [5][7][6]. Note that the AWS SDK for Java 1.x reached end-of-support on December 31, 2025, and users are encouraged to migrate to the AWS SDK for Java 2.x [8].
Citations:
Set the STS client region from
base.region.AWSSecurityTokenServiceClientBuilder.defaultClient()does not readbase.regionand usesAWS_REGIONinstead. Since this job only appliesbase.region, the base-account credential refresh path can pick any other AWS region for STS and fail on AssumeRole.Proposed fix
🤖 Prompt for AI Agents