Skip to content

registry: fix code quality issues flagged by sonarqube static analysis#319

Open
iamabhilaksh wants to merge 2 commits intosalesforce:mainfrom
iamabhilaksh:registry/fix-sonar-issues
Open

registry: fix code quality issues flagged by sonarqube static analysis#319
iamabhilaksh wants to merge 2 commits intosalesforce:mainfrom
iamabhilaksh:registry/fix-sonar-issues

Conversation

@iamabhilaksh
Copy link
Contributor

@iamabhilaksh iamabhilaksh commented Mar 3, 2026

registry: fix code quality issues flagged by sonarqube static analysis

Summary

Fixes code quality issues in the registry module flagged by static analysis tools. The changes
reduce code duplication, improve resource safety, clarify intent through inline comments, and
eliminate a credential-scanning false positive in tests. No functional behaviour is changed.

Changes

Modified Files

  • AbstractRegistry.java — Remove redundant abstract method declarations (6 lines removed)

    • getAuthUsername() and getAuthToken() were re-declared with @Override on AbstractRegistry
      but are already defined on the AuthProvider interface it implements; the duplicate declarations
      are removed
  • BearerTokenExchange.java — Reduce nesting, add constants, fix comment (72 lines changed)

    • Added TOKEN_FIELD = "token" and ACCESS_TOKEN_FIELD = "access_token" constants to eliminate
      magic strings referenced in multiple places
    • Extracted inner try (CloseableHttpResponse ...) block into a private executeTokenRequest()
      method, removing one level of nesting from getBearerToken()
    • Fixed split comment // Token can be in ... (GCP Artifact Registry) \n// field — reworded
      into a single line within the 100-char checkstyle limit:
      // Token field is "token" (Docker Hub, AWS ECR) or "access_token" (GCP Artifact Registry)
  • LayerExtractor.java — Improve executor error handling and merge duplicate conditions
    (70 lines changed)

    • Wrapped executor.submit(...) in a try-catch block that closes PipedOutputStream and
      throws UnknownException if submission fails, preventing the caller from blocking indefinitely
      on a pipe that will never be written to
    • Merged two consecutive if (...) continue; checks into a single combined condition with ||,
      eliminating the duplicate continue statement flagged by static analysis
  • OciRegistryClient.java — Extract methods, add constant, restore comments (134 lines changed)

    • Added MANIFEST_ERROR_FORMAT constant to avoid repeating the identical String.format template
      across the three HTTP-error branches (404, 401, default)
    • Extracted fetchManifest() internals into:
      • buildFetchManifestRequest() — builds the HttpGet with auth and Accept headers
      • executeFetchManifestRequest() — executes, checks status, enforces size limit, and parses
      • computeManifestDigest() — returns the Docker-Content-Digest header value or computes
        SHA-256 from the response body when the header is absent (e.g. AWS ECR)
    • Simplified nested if (reference.startsWith(DIGEST_PREFIX)) { if (!reference.equals(...)) }
      into a single combined condition
    • Restored three clarifying comments that were dropped during extraction:
      • // Check manifest size limit to prevent resource exhaustion
      • // Validate digest if fetching by digest reference (e.g., repo@sha256:...)
      • // Docker-Content-Digest header may be absent (e.g., AWS ECR); calculate from response body
    • Now routes all HTTP error status codes through the existing mapHttpStatusToException() helper
      instead of repeating the if/else chain
  • OciRegistryClientTest.java — Replace hardcoded Base64 credential with dynamic encoding
    (6 lines changed)

    • "Basic dXNlcjp0b2tlbg==" was a hardcoded Base64-encoded credential triggering secret-scanning
      false positives; replaced with "Basic " + Base64.getEncoder().encodeToString(...) so the
      value is computed at runtime

Testing Details

Unit Tests (136/136 passed)


References

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.33%. Comparing base (03f6a8c) to head (5b34008).

Files with missing lines Patch % Lines
...ce/multicloudj/registry/driver/LayerExtractor.java 58.33% 9 Missing and 1 partial ⚠️
...multicloudj/registry/driver/OciRegistryClient.java 83.78% 4 Missing and 2 partials ⚠️
...lticloudj/registry/driver/BearerTokenExchange.java 90.00% 0 Missing and 2 partials ⚠️

❌ Your patch status has failed because the patch coverage (77.77%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #319      +/-   ##
============================================
- Coverage     82.36%   82.33%   -0.04%     
- Complexity      625      626       +1     
============================================
  Files           191      191              
  Lines         11503    11504       +1     
  Branches       1525     1521       -4     
============================================
- Hits           9475     9472       -3     
- Misses         1357     1361       +4     
  Partials        671      671              
Flag Coverage Δ
unittests 82.33% <77.77%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- AbstractRegistry: remove redundant abstract method declarations already
  defined in AuthProvider interface
- BearerTokenExchange: extract nested try block into executeTokenRequest();
  define constants TOKEN_FIELD and ACCESS_TOKEN_FIELD
- LayerExtractor: close PipedOutputStream in finally clause if executor
  submission fails; merge duplicate continue statements into one condition
- OciRegistryClient: define MANIFEST_ERROR_FORMAT constant; extract
  fetchManifest nested try into executeFetchManifestRequest() and
  computeManifestDigest(); simplify nested ifs using mapHttpStatusToException
- OciRegistryClientTest: replace hardcoded Base64 token with dynamic
  encoding to avoid credential scanning false positives

Made-with: Cursor
@iamabhilaksh iamabhilaksh force-pushed the registry/fix-sonar-issues branch from 46b8629 to 8e779c4 Compare March 10, 2026 16:04
- OciRegistryClient: add back manifest size limit, digest validation,
  and Docker-Content-Digest fallback comments in extracted methods
- BearerTokenExchange: rephrase split token field comment into single line
@iamabhilaksh iamabhilaksh changed the title registry: fix code quality issues flagged by static analysis registry: fix code quality issues flagged by sonarqube static analysis Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants