Repository navigation
chore: add an Azure template for testing against a real SQL Server - #1915
Merged
Merged
Conversation
The dev container runs SQL Server for Linux and covers the tedious driver, but it cannot exercise msnodesqlv8 against the `SQL Server Native Client 11.0` driver node-mssql defaults to on Windows, and it cannot reproduce behaviour that depends on the engine edition. CI only runs the msnodesqlv8 suite in the Windows job, so a contributor changing that driver has no local way to check their work. The template provisions a Windows VM with SQL Server Developer edition pre-installed. It opens RDP only by default, on the assumption that the suites are run on the VM itself; exposing SQL is opt-in. The Windows administrator and SQL passwords are separate parameters, because the SQL one is written to test/.mssql.json in cleartext and should not also grant administrator access over RDP. The README carries the parts that are easy to get wrong: that the SQL IaaS agent resource is what creates a usable login at all, that the image needs SQL Server Native Client installed before the msnodesqlv8 suite will connect, and that the two drivers need different TLS settings, because msnodesqlv8 builds its connection string without `TrustServerCertificate` and so cannot trust the VM's self-signed certificate at all. The template is not exercised by CI and creates billable resources, which both files state plainly. `contrib/` is outside the `files` allowlist in package.json, so none of it is published to npm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎉 This PR is included in version 12.7.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Bicep template that provisions a Windows VM with SQL Server Developer edition, under
contrib/azure-sql-vm, plus a pointer to it fromCONTRIBUTING.md.Why
CONTRIBUTING.mddocumentsnpm run test-msnodesqlv8, but the dev container cannot run it usefully. Two gaps:node-mssqldefaults to theSQL Server Native Client 11.0ODBC driver rather thanODBC Driver 17 for SQL Server(lib/msnodesqlv8/connection-pool.js). That is the path Windows users actually hit, and CI only exercises it intest-windows— the Linux job's msnodesqlv8 step is commented out. A contributor changing that driver currently has no local way to check their work.EngineEdition9 and has no CLR.This template was used to verify the GHSA-rc93-2r6p-hc2q patch against real SQL Server 2016/2017/2019/2022. It has been deployed successfully, but it is not exercised by CI and both files say so.
Design choices
localhost, which needs no exposed SQL port.exposeSqlPortis opt-in and also flips the SQL VM connectivity type toPUBLIC.sqlAdminPasswordis distinct fromadminPasswordbecause the SQL one is written totest/.mssql.jsonin cleartext and crosses the wire — it should not also unlock RDP as administrator.allowedSourceIpdefaults to empty, which produces no inbound rules at all, so a bare deploy exposes nothing.contrib/is outside thefilesallowlist inpackage.json;npm pack --dry-runlists zero entries from it.The parts that are easy to get wrong
The README documents three things found the hard way:
Microsoft.SqlVirtualMachineresource is not optional — deploy the marketplace image as a plain VM andsais disabled,NT AUTHORITY\SYSTEMis not a sysadmin, and the only sysadmin is that disabledsa, so there is no route to a login short of single-user-mode recovery.sqlncli.msimust be installed before the msnodesqlv8 suite will connect.tediousneedsencrypt: true+trustServerCertificate: true; msnodesqlv8 needsencrypt: false, because its connection string is built from onlyDriver,Server,Database,Uid,Pwd,Trusted_ConnectionandEncrypt—trustServerCertificateis silently dropped, so an encrypted connection cannot trust the VM's self-signed certificate at all. This mirrors what the two CI jobs do.The README also warns that setting
driverintest/.mssql.jsondoes not work as an override:test/msnodesqlv8/msnodesqlv8.jsoverwrites it with'msnodesqlv8', whichconnection-pool.jstreats as "unspecified" and replaces with the platform default. Only a fullconnectionStringsurvives.Verification
az bicep buildcompiles clean.npm run lintpasses (StandardJS doesn't cover.bicep/.md, so nothing here is linted by it).🤖 Generated with Claude Code