Skip to content

feat:[PEA-646] Response structure modification in data shipper for ta… #2398

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.

Sign up for GitHub

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

Merged
arunpaladin merged 2 commits into from
Jun 18, 2026
Merged

feat:[PEA-646] Response structure modification in data shipper for ta… #2398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33f8068
feat:[PEA-646] Response structure modification in data shipper for ta…
Jun 10, 2026
7fb6d92
Upgrade CodeQL actions to version 3
arunpaladin Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/codeQl.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v2
uses: github/codeql-action/init@v3

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all GitHub Actions uses are SHA-pinned (40-hex) rather than mutable tags.
rg -nP '^\s*uses:\s*[^@]+@([a-f0-9]{40})\s*$|^\s*uses:\s*' .github/workflows/*.y*ml

Repository: PaladinCloud/CE

Length of output: 14775

Pin CodeQL actions to immutable commit SHAs.

Lines 50, 65, and 84 use mutable version tags (@v3) instead of immutable commit SHAs. This creates supply-chain risk and should be pinned to specific commits.

🔒 Suggested fix 
-      uses: github/codeql-action/init@v3
+      uses: github/codeql-action/init@<full_commit_sha>

-      uses: github/codeql-action/autobuild@v3
+      uses: github/codeql-action/autobuild@<full_commit_sha>

-      uses: github/codeql-action/analyze@v3
+      uses: github/codeql-action/analyze@<full_commit_sha>
🧰 Tools
🪛 zizmor (1.25.2)

[error] 50-50: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/codeQl.yaml at line 50, Replace the mutable version tags
(`@v3`) in the three GitHub CodeQL action references with immutable commit SHAs.
For each occurrence of github/codeql-action/init@v3,
github/codeql-action/analyze@v3, and the other CodeQL action on line 84, replace
the `@v3` tag with the full commit SHA (e.g., `@abc123def456`...) to ensure these
actions are pinned to specific immutable commits rather than floating version
tags that could change unexpectedly.

Source: Linters/SAST tools

with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
Expand All @@ -62,7 +62,7 @@ jobs:
# If this step fails, then you should remove it and run the build manually (see below)
- if: matrix.language == 'python' || matrix.language == 'javascript'
name: Autobuild
uses: github/codeql-action/autobuild@v2
uses: github/codeql-action/autobuild@v3

- if: matrix.language == 'java'
name: Build Java
Expand All @@ -81,4 +81,4 @@ jobs:
# ./location_of_script_within_repo/buildscript.sh

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
uses: github/codeql-action/analyze@v3
77 changes: 20 additions & 57 deletions ...an-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/AssetGroupUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
******************************************************************************/
package com.tmobile.cso.pacman.datashipper.util;

import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.gson.*;
import com.tmobile.cso.pacman.datashipper.dao.RDSDBManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -66,7 +65,7 @@ public static List<Map<String, Object>> fetchTypeCounts(String ag) throws Except
String url = ASSET_SVC_BASE_URL + "/count?ag=" + ag;
String typeCountJson = HttpUtil.get(url, AuthManager.getToken());
Map<String, Object> typeCountMap = Util.parseJson(typeCountJson);
return (List<Map<String, Object>>) ((Map<String, Object>) typeCountMap.get("data")).get("assetcount");
return (List<Map<String, Object>>) ((Map<String, Object>) typeCountMap.get("data")).get("assetcount");
}

/**
Expand Down Expand Up @@ -317,65 +316,29 @@ public static String fetchAssetCount(String platform, String accountId) throws E
}

public static List<Map<String, Object>> fetchTaggingSummaryForAssetGroup(String ag) throws Exception {
List<Map<String, Object>> issueInfoList = new ArrayList<>();
List<Map<String, Object>> resultList = new ArrayList<>();
try {

String distributionResponse = LambdaInvoker.invokeTaggingSummaryLambda(ag);
if (distributionResponse == null || distributionResponse.isEmpty()) {
String response = LambdaInvoker.invokeTaggingSummaryLambda(ag);
if (response == null || response.isEmpty()) {
LOGGER.warn("Empty response from tagging summary API for ag: {}", ag);
return issueInfoList;
}
JsonObject distributionJson = JsonParser.parseString(distributionResponse).getAsJsonObject();
JsonObject dataObj = distributionJson.getAsJsonObject("data");
if (dataObj == null || dataObj.isJsonNull()) {
LOGGER.warn("No data found in tagging summary response for ag: {}", ag);
return issueInfoList;
return resultList;
}
Map<String, Object> summaryInfo = new HashMap<>();
summaryInfo.put("totalAssets", dataObj.get("totalAssets").getAsLong());
summaryInfo.put("overallCompliancePercentage", dataObj.get("overallCompliancePercentage").getAsDouble());
summaryInfo.put("overallTaggedCount", dataObj.get("overallTaggedCount").getAsLong());
summaryInfo.put("overallAssetCount", dataObj.get("overallAssetCount").getAsLong());
summaryInfo.put("description", dataObj.get("description").getAsString());

JsonArray assetTypesArray = dataObj.getAsJsonArray("assetTypes");
List<Map<String, Object>> assetTypesList = new ArrayList<>();

for (int i = 0; i < assetTypesArray.size(); i++) {
JsonObject assetTypeObj = assetTypesArray.get(i).getAsJsonObject();

Map<String, Object> assetTypeInfo = new HashMap<>();
assetTypeInfo.put("targetType", assetTypeObj.get("targetType").getAsString());
assetTypeInfo.put("displayName", assetTypeObj.get("displayName").getAsString());
assetTypeInfo.put("assetCount", assetTypeObj.get("assetCount").getAsLong());
assetTypeInfo.put("taggedCount", assetTypeObj.get("taggedCount").getAsLong());
assetTypeInfo.put("untaggedCount", assetTypeObj.get("untaggedCount").getAsLong());
assetTypeInfo.put("compliancePercentage", assetTypeObj.get("compliancePercentage").getAsDouble());

JsonArray tagDetailsArray = assetTypeObj.getAsJsonArray("tagDetails");
List<Map<String, Object>> tagDetailsList = new ArrayList<>();

for (int j = 0; j < tagDetailsArray.size(); j++) {
JsonObject tagDetailObj = tagDetailsArray.get(j).getAsJsonObject();

Map<String, Object> tagInfo = new HashMap<>();
tagInfo.put("tagName", tagDetailObj.get("tagName").getAsString());
tagInfo.put("count", tagDetailObj.get("count").getAsLong());
tagInfo.put("tagCompliancePercentage", tagDetailObj.get("tagCompliancePercentage").getAsDouble());

tagDetailsList.add(tagInfo);
}
TaggingSummaryResponse summaryResponse = new Gson().fromJson(response,
TaggingSummaryResponse.class);

assetTypeInfo.put("tagDetails", tagDetailsList);
assetTypesList.add(assetTypeInfo);
if (summaryResponse.data == null) {
LOGGER.warn("No data found in tagging summary response for ag: {}", ag);
return resultList;
}

summaryInfo.put("assetTypes", assetTypesList);
issueInfoList.add(summaryInfo);
ObjectMapper mapper = new ObjectMapper();
Map<String, Object> summaryInfo = mapper.convertValue(
summaryResponse.data, new TypeReference<Map<String, Object>>() {
});
resultList.add(summaryInfo);
} catch (Exception e) {
LOGGER.error("Error retrieving tagging summary data", e);
throw e;
LOGGER.error("Error retrieving tagging summary data for ag: {}", ag, e);
}
Comment on lines 339 to 341

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Silent error swallowing changes the method's failure contract.

The previous implementation logged and re-threw exceptions, allowing callers to handle failures. The new implementation catches all exceptions and returns an empty list, making it impossible for callers to distinguish between "no tagging data exists" and "the Lambda invocation failed."

This could mask production issues—if the Lambda consistently fails, downstream systems will process empty data without any indication of failure.

Consider either:

  1. Rethrowing after logging (preserves original behavior)
  2. Returning an Optional or a result wrapper that indicates success/failure
  3. At minimum, adding a metric or alert for failures
🛠️ Suggested fix to preserve failure signaling 
         } catch (Exception e) {
             LOGGER.error("Error retrieving tagging summary data for ag: {}", ag, e);
+            throw e;
         }
-        return resultList;
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@jobs/pacman-data-shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/AssetGroupUtil.java`
around lines 339 - 341, The catch block for Exception handling in this method
logs the error but then silently continues without signaling the failure to the
caller, which masks Lambda invocation failures and prevents downstream systems
from distinguishing between missing data and actual failures. Modify the
exception handling to either rethrow the exception after logging (to preserve
the original behavior and allow callers to handle the failure), return an
Optional or result wrapper that explicitly indicates success or failure, or add
monitoring metrics to alert when Lambda failures occur. Ensure the method's
contract clearly communicates whether the operation succeeded or failed to
callers.

return issueInfoList;
return resultList;
}
}
48 changes: 48 additions & 0 deletions ...shipper/src/main/java/com/tmobile/cso/pacman/datashipper/util/TaggingSummaryResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.tmobile.cso.pacman.datashipper.util;

import java.util.List;

public class TaggingSummaryResponse {

public Data data;

public static class Data {
public String ag;
public int totalAssets;
public double overallCompliancePercentage;
public int overallTaggedCount;
public int overallAssetCount;
public String description;
public List<AssetGroupEntry> assetgroups;
}

public static class AssetGroupEntry {
public String ag;
public Stats stats;
}

public static class Stats {
public int totalAssets;
public double overallCompliancePercentage;
public int overallTaggedCount;
public int overallAssetCount;
public String description;
public List<AssetType> assetTypes;
}

public static class AssetType {
public String targetType;
public String displayName;
public int assetCount;
public int taggedCount;
public int untaggedCount;
public double compliancePercentage;
public List<TagDetail> tagDetails;
}

public static class TagDetail {
public String tagName;
public int count;
public double tagCompliancePercentage;
}
}
Loading