Skip to content

Decouple NpmAuthenticateV0 from packaging-common/npm modules#21911

Open
AlexVTor wants to merge 10 commits intomasterfrom
users/alextorres/NpmAuthenticateV0Refactor
Open

Decouple NpmAuthenticateV0 from packaging-common/npm modules#21911
AlexVTor wants to merge 10 commits intomasterfrom
users/alextorres/NpmAuthenticateV0Refactor

Conversation

@AlexVTor
Copy link
Contributor

@AlexVTor AlexVTor commented Mar 9, 2026

Context

packaging-common/npm/ is a shared package consumed by three tasks: NpmAuthenticateV0, NpmV1, and DownloadGitHubNpmPackageV1. Despite being "shared," each task uses a different subset of the package. The shared abstraction adds dependency coupling and test complexity without meaningful code reuse. This PR will decouple the dependency on a package from the separate repo, while providing the same utility in an improved manner.


Task Name

NpmAuthenticateV0

Description

What changed:

  • Inlined registry parsing, URL normalization, credential formatting, and file I/O (~120 lines replacing 3 external module imports)

  • Replaced the NpmRegistry class with a local NpmrcCredential interface

  • Extracted credential resolution into npmrcCredential.ts and backup management into npmrcBackupManager.ts

  • Consolidated the duplicated WIF auth branches into a single check

  • Improved variable naming throughout (endpointsArray → previouslyAuthenticatedUrls, registry → npmrcEntry, etc.)

  • Rewrote tests to exercise real code paths instead of mocking our own logic

The only remaining packaging-common dependency is locationUtilities (for Azure DevOps service location), which is a general-purpose module used across many tasks.

Risk Assessment (Low / Medium / High)

Medium

Change Behind Feature Flag (Yes / No)

Can this change be behine feature flag, if not why?


Tech Design / Approach

  • Design has been written and reviewed.
  • Any architectural decisions, trade-offs, and alternatives are captured.

Documentation Changes Required (Yes/No)

Indicate whether related documentation needs to be updated.

  • User guides, API specs, system diagrams, or runbooks are updated.
    N/A

Unit Tests Added or Updated (Yes / No)

Indicate whether unit tests were added or modified to reflect these changes.
Y


Additional Testing Performed

List all other tests performed (manual or automated, including integration, regression, scenario tests, etc.).
Local, E2E, Testing Pipelines


Logging Added/Updated (Yes/No)

  • Appropriate log statements are added with meaningful messages.
  • Logging does not expose sensitive data.
  • Log levels are used correctly (e.g., info, warn, error).
    Y

Telemetry Added/Updated (Yes/No)

  • Custom telemetry (e.g., counters, timers, error tracking) is added as needed.
  • Events are tagged with proper metadata for filtering and analysis.
  • Telemetry is validated in staging or test environments.
    N

Rollback Scenario and Process (Yes/No)

  • Rollback plan is documented.
    Rollback via task override can be applied to revert to a previous version

Dependency Impact Assessed and Regression Tested (Yes/No)

  • All impacted internal modules, APIs, services, and third-party libraries are analyzed.
  • Results are reviewed and confirmed to not break existing functionality.
    Y

Checklist

  • Related issue linked (if applicable)
  • Task version was bumped — see versioning guide
  • Verified the task behaves as expected

@AlexVTor AlexVTor requested a review from a team as a code owner March 9, 2026 18:58
@AlexVTor
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@AlexVTor
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant