Validate localized SqlClient resources in OneBranch builds

[cheenamalhotra]

Summary

• Validate localized resource keys and untranslated English values during the SqlClient build.
• Allow LCL-approved identical values through a culture/key allowlist.
• Report actionable failures per resource file.

Next

• Setup automation to tranfer loc changes to applicable release branches.

Non-Official Build

Validation

• Tests added or updated
• Public API changes documented (not applicable)
• Verified against customer repro (not applicable)
• Ensure no breaking changes introduced

[Copilot]

🟡 Changes recommended

The validator is incompatible with documented PowerShell 5.1 support and does not reject localized-only keys or stale allowlist entries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds OneBranch validation for localized SqlClient resources.

Changes:

• Validates localized resource keys and untranslated values.
• Adds culture-specific allowlist support and Pester tests.
• Integrates validation into SqlClient builds and documentation.

File summaries

File	Description
eng/pipelines/onebranch/steps/validate-localization-step.yml	Defines the localization validation pipeline step.
eng/pipelines/onebranch/scripts/validate-localization.ps1	Implements localized resource validation.
eng/pipelines/onebranch/scripts/tests/validate-localization.Tests.ps1	Tests localization validation behavior.
eng/pipelines/onebranch/scripts/tests/README.md	Documents validation test coverage.
eng/pipelines/onebranch/jobs/build-buildproj-job.yml	Runs validation during SqlClient builds.
.github/instructions/onebranch-pipeline-design.instructions.md	Documents the validation workflow.
.config/LocalizationValidationAllowlist.json	Lists approved identical localized values.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.69%. Comparing base (7054399) to head (11d85ba).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4635      +/-   ##
==========================================
- Coverage   65.92%   63.69%   -2.23%     
==========================================
  Files         290      284       -6     
  Lines       44987    67966   +22979     
==========================================
+ Hits        29656    43291   +13635     
- Misses      15331    24675    +9344     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	63.69% <ø> (?)

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

☔ View full report in Codecov by Harness.
📢 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.

[Copilot]

🟡 Changes recommended

Warning-only builds can still fail on stale allowlist entries, and this behavior needs test coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[benrr101]

This seems somewhat beneficial. It's probably good to attempt to catch missing localizations before they are potentially released. Though, I think we should run the validation all the time - and have it fail the build if the validation fails, just like we do with all other validations.

I'd also like to see some explanation of why values are exempted and how the powershell script is doing the validation.

My only quibbles about the entire concept are:

1. While it's good to break the build before we might release a build that's missing localization, it'd be better if we can have this validation somewhere sooner in the release process. Getting all the way to the end only to find out localization wasn't completed would be a major disruption to a release.
2. I worry that this might be a bit too brittle. We've already carved out a lot of exceptions, and I wonder about localizations that differ by only a character being ignored. (eg, "Invalid file." in en-us and "Invalid file" in pt-br). I suspect rather than trying to address all these fuzzy concerns, it will be better to just ensure all localizable strings exist in all languages (regardless of what those strings actually contain), and automate localization better.
3. Outside the scope of this PR, but since this comes on the heels of my request for automation around localization backports, I want to make sure we aren't crossing signals. This PR still doesn't address automating localization backports - it just blocks releases from going out if they're missing localization. A good idea, but let's also automate localization backports, and maybe even improve our localization pipeline story.

[Copilot]

🟡 Changes recommended

The shared build job currently makes localization findings fail Official builds instead of remaining warning-only.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

.github/instructions/onebranch-pipeline-design.instructions.md:31

• This states that every empty resource fails validation, but the script deliberately permits an empty localized value when the English value is empty (and the test at validate-localization.Tests.ps1:82-87 requires that behavior). Qualify the statement so future pipeline changes do not treat these valid empty pairs as failures.

The SqlClient build job runs `steps/validate-localization-step.yml` before building the driver. Validation always fails the build for missing, obsolete, empty, or untranslated resources. Approved identical translations are listed by culture and resource key in `.config/LocalizationValidationAllowlist.json`.

.github/instructions/onebranch-pipeline-design.instructions.md:31

• This statement is no longer accurate for the required warning-only Official-build mode. Document that validation findings fail enforcing builds but are emitted as warnings in the scheduled Official build, while malformed configuration still fails immediately.

The SqlClient build job runs `steps/validate-localization-step.yml` before building the driver. Validation always fails the build for missing, obsolete, empty, or untranslated resources. Approved identical translations are listed by culture and resource key in `.config/LocalizationValidationAllowlist.json`.

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[cheenamalhotra]

Regarding your points @benrr101

While it's good to break the build before we might release a build that's missing localization, it'd be better if we can have this validation somewhere sooner in the release process. Getting all the way to the end only to find out localization wasn't completed would be a major disruption to a release.

This is purposely kept inside the MDS build job, because localization files will come to all other packages too (which we are missing today), and we will need to validate those as well. So keeping this validation close to the resp. package build is what I went for here.

I worry that this might be a bit too brittle. We've already carved out a lot of exceptions, and I wonder about localizations that differ by only a character being ignored. (eg, "Invalid file." in en-us and "Invalid file" in pt-br). I suspect rather than trying to address all these fuzzy concerns, it will be better to just ensure all localizable strings exist in all languages (regardless of what those strings actually contain), and automate localization better.

Actually its not, because all the strings are technically different in all languages, that's all we care for. We're not looking for correctness or anything, we just want to make sure same strings don't exist in all files - which happens when new strings are added, but not yet localized.

Outside the scope of this PR, but since this comes on the heels of my request for automation around localization backports, I want to make sure we aren't crossing signals. This PR still doesn't address automating localization backports - it just blocks releases from going out if they're missing localization. A good idea, but let's also automate localization backports, and maybe even improve our localization pipeline story.

Yes, this was not the intent of this PR, it is something to be done later to automating backporting localized file changes.