Add MSSQLServer module #645

wjin17 · 2023-08-24T04:33:59Z

Add MS Server module

netlify · 2023-08-24T04:34:04Z

✅ Deploy Preview for testcontainers-node ready!

Name	Link
🔨 Latest commit	`56e90a1`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-node/deploys/65a059190ab1c40008773e5e
😎 Deploy Preview	https://deploy-preview-645--testcontainers-node.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

djakielski · 2023-08-27T17:21:39Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+    private readonly password: string
+  ) {
+    super(startedTestContainer);
+    this.port = startedTestContainer.getMappedPort(MSSQL_PORT);


When I was writing the redis module (#642), I also oriented on this style from mysql. But when I start testing it by restaring the container, it will fail, because the container gots new ports after restart. So I moved it to the getter function.

djakielski · 2023-08-27T17:22:52Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+  }
+
+  public getPort(): number {
+    return this.port;


Better use getMappedPort in the getter, to get actual port, also when container was restarted.

return this.getMappedPort(MSSQL_PORT);

Thanks for the review! I updated the PR to use getMappedPort in the getter and I also removed the wait strategy in the module to so users can define their own.

eddumelendez

Thanks for your contribution. I've left some comments. Also, please add more tests where is needed.

packages/modules/mssqlserver/src/mssqlserver-container.ts

eddumelendez · 2023-08-29T22:20:01Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+
+type Edition = "Evaluation" | "Developer" | "Express" | "Web" | "Standard" | "Enterprise" | ProductKey;
+
+const PASSWORD_CATEGORY_VALIDATION_PATTERNS = [/[A-Z]+/, /[a-z]+/, /[0-9]+/, /[@$!%*?&]+/];


Although, this is similar to what testcontainers-java does, the current approach is to leave it to the service itself. Because, it can change and the container implementation in Testcontainers would fail when it shouldn't

eddumelendez · 2023-08-29T22:21:33Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+  public withDatabase(database: string): this {
+    this.database = database;
+    return this;
+  }


AFAIK, you can not set the database name in MSSQL and due to there is no test covering the case I wonder if that works, nowadays.

I left this in as a convenience method for the generating the connection string should the user want to seed the instance during initialization. I can also add a test/example for seeding the database with SQL files and bash scripts but I was worried about adding too many files/adding things out of scope. Let me know how I should proceed.

eddumelendez · 2023-08-29T22:25:18Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+  private database = "master";
+  private username = "SA";
+  private password = "Passw0rd";
+  private edition: Edition = "Developer";


Developer is the default value for MSSQL_PID, so I don't think it is needed. Also, this can added .withEnvironment({ "MSSQL_PID": "Express }) if required.

eddumelendez · 2023-08-29T22:26:45Z

packages/modules/mssqlserver/src/mssqlserver-container.ts

+
+const MSSQL_PORT = 1433;
+
+type ProductKey = `${string}-${string}-${string}-${string}-${string}`;


why is this needed?

Product key comes from the official documentation here:

https://learn.microsoft.com/en-us/sql/linux/sql-server-linux-configure-environment-variables?view=sql-server-ver16#environment-variables

If specifying a product key, it must be in the form of #####-#####-#####-#####-#####, where '#' is a number or a letter.

However, if the edition should be added in by the user using withEnvironment, I can remove all PID/Edition code.

cristianrgreco · 2023-09-12T10:29:46Z

packages/modules/mssqlserver/src/mssqlserver-container.test.ts

+  it("should connect and return a query result", async () => {
+    const container = await new MSSQLServerContainer()
+      .acceptLicense()
+      .withWaitStrategy(Wait.forLogMessage(/.*Recovery is complete.*/, 1))


Could this wait strategy be configured in the module itself?

I left the wait strategy out of the module to allow users to use their own. You can see an example below. I can merge this branch into the PR and update the docs to include this but I wanted to keep the changes to a minimum just in case.

https://github.com/wjin17/testcontainers-node/blob/40385d0af17ef2d9af75165ac99da83c96787470/packages/modules/mssqlserver/src/mssqlserver-container.test.ts#L84

@wjin17 take into account that Testcontainers modules are pre-configured, so, it is totally fair to use the wait strategy in the mssql-server-container.ts.

@eddumelendez Great point!

I've

Merged the change from SA to sa

Removed password validation

Created a default wait strategy* that can be overwritten

Added tests for password validation using the native MSSql instance and a test using the Express edition with environment variables

I had to allow the wait strategy regex to be overwritten because the Express edition requires a different wait regex before the db can be queried.

Let me know if I should add any more changes.

Thanks again for your review!

packages/modules/mssqlserver/src/mssqlserver-container.ts

Co-authored-by: Eddú Meléndez Gonzales <[email protected]>

cristianrgreco · 2023-11-23T13:21:14Z

Hi @wjin17, thanks for the contribution! The test are failing on my M1 mac with the following:

testcontainers:containers [0dbfd11c6c3f] /opt/mssql/bin/sqlservr: Invalid mapping of address 0x400395b000 in reserved address space below 0x400000000000. Possible causes: +65ms
testcontainers:containers [0dbfd11c6c3f] 1) the process (itself, or via a wrapper) starts-up its own running environment sets the stack size limit to unlimited via syscall setrlimit(2); +0ms
testcontainers:containers [0dbfd11c6c3f] 2) the process (itself, or via a wrapper) adjusts its own execution domain and flag the system its legacy personality via syscall personality(2); +0ms
testcontainers:containers [0dbfd11c6c3f] 3) sysadmin deliberately sets the system to run on legacy VA layout mode by adjusting a sysctl knob vm.legacy_va_layout. +0ms

Could it be an issue with ARM/is a fix possible?

Also looks like the Azure dependencies are incompatible with Node 20 which is LTS, perhaps it's supported with a newer version of the dependency?

wjin17 · 2023-11-29T07:34:50Z

Hi @wjin17, thanks for the contribution! The test are failing on my M1 mac with the following:

testcontainers:containers [0dbfd11c6c3f] /opt/mssql/bin/sqlservr: Invalid mapping of address 0x400395b000 in reserved address space below 0x400000000000. Possible causes: +65ms
testcontainers:containers [0dbfd11c6c3f] 1) the process (itself, or via a wrapper) starts-up its own running environment sets the stack size limit to unlimited via syscall setrlimit(2); +0ms
testcontainers:containers [0dbfd11c6c3f] 2) the process (itself, or via a wrapper) adjusts its own execution domain and flag the system its legacy personality via syscall personality(2); +0ms
testcontainers:containers [0dbfd11c6c3f] 3) sysadmin deliberately sets the system to run on legacy VA layout mode by adjusting a sysctl knob vm.legacy_va_layout. +0ms

Could it be an issue with ARM/is a fix possible?

Also looks like the Azure dependencies are incompatible with Node 20 which is LTS, perhaps it's supported with a newer version of the dependency?

Hi @cristianrgreco

Unfortunately, I do not have access to an M1 mac but trying to run mssql on M1 macs seems to be a known issue. Here's a link to a fix from the official MSSQL docker repo mssql docker issue #668.

For the Azure dependency, I tried updating to the latest version of node-mssql using Node 20 but it seems like the official library is still using the deprecated Azure module.

Please let me know how to proceed.

cristianrgreco · 2024-01-10T21:10:29Z

According to tediousjs/node-mssql#1540, node 20 is now supported, will re-run the build now.

wjin17 · 2024-01-11T07:48:10Z

According to tediousjs/node-mssql#1540, node 20 is now supported, will re-run the build now.

Awesome! Thank you for the update.

cristianrgreco · 2024-01-15T18:27:01Z

Thanks for the contribution @wjin17 !

wjin17 added 3 commits August 23, 2023 21:27

add mssql module

b4c2407

add tests

9e516bb

add docs

ee85e37

djakielski reviewed Aug 27, 2023

View reviewed changes

wjin17 added 3 commits August 27, 2023 11:06

use getMappedPort in getter

d14ffe7

remove wait strategy from module

ff41e65

fix weird spacing in docs

791ea7a

wjin17 marked this pull request as ready for review August 29, 2023 07:58

eddumelendez reviewed Aug 29, 2023

View reviewed changes

cristianrgreco reviewed Sep 12, 2023

View reviewed changes

packages/modules/mssqlserver/src/mssqlserver-container.ts Outdated Show resolved Hide resolved

wjin17 and others added 9 commits September 14, 2023 01:37

make sa default param lowercase

80e31f8

Update packages/modules/mssqlserver/src/mssqlserver-container.ts

1426a3f

Co-authored-by: Eddú Meléndez Gonzales <[email protected]>

Merge branch 'mssql' of github.com:wjin17/testcontainers-node into mssql

f04e49f

remove password validation

9cc7854

move wait strategy to module

4fdc326

add password tests using native module

523e389

add additional examples to docs

1a2270f

add express edition test

293dda5

add express edition to docs

96497a4

cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Oct 26, 2023

agosmou mentioned this pull request Oct 26, 2023

Creates Test Plan for Running Integration Tests on Server API Endpoints hackforla/tdm-calculator#1493

Merged

cristianrgreco added 2 commits November 23, 2023 13:20

Merge branch 'main' into mssql

2d40ea3

Add MSSQLServer to list of modules to test

a963c7c

cristianrgreco added 2 commits January 10, 2024 21:24

Merge branch 'main' into mssql

d021e23

Update mssql package version with one which is node 20 compatible

3405f95

Update .nvmrc for Netlify

56e90a1

cristianrgreco merged commit 0129a82 into testcontainers:main Jan 15, 2024
93 of 94 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MSSQLServer module #645

Add MSSQLServer module #645

wjin17 commented Aug 24, 2023

netlify bot commented Aug 24, 2023 •

edited

Loading

djakielski Aug 27, 2023

djakielski Aug 27, 2023

wjin17 Aug 27, 2023

eddumelendez left a comment

eddumelendez Aug 29, 2023

eddumelendez Aug 29, 2023

wjin17 Aug 30, 2023 •

edited

Loading

eddumelendez Aug 29, 2023

eddumelendez Aug 29, 2023

wjin17 Aug 30, 2023

cristianrgreco Sep 12, 2023

wjin17 Sep 15, 2023

eddumelendez Oct 5, 2023

wjin17 Oct 6, 2023

cristianrgreco commented Nov 23, 2023 •

edited

Loading

wjin17 commented Nov 29, 2023

cristianrgreco commented Jan 10, 2024

wjin17 commented Jan 11, 2024

cristianrgreco commented Jan 15, 2024


		type Edition = "Evaluation" \| "Developer" \| "Express" \| "Web" \| "Standard" \| "Enterprise" \| ProductKey;

		const PASSWORD_CATEGORY_VALIDATION_PATTERNS = [/[A-Z]+/, /[a-z]+/, /[0-9]+/, /[@$!%*?&]+/];


		const MSSQL_PORT = 1433;

		type ProductKey = `${string}-${string}-${string}-${string}-${string}`;

Add MSSQLServer module #645

Add MSSQLServer module #645

Conversation

wjin17 commented Aug 24, 2023

netlify bot commented Aug 24, 2023 • edited Loading

✅ Deploy Preview for testcontainers-node ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddumelendez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjin17 Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristianrgreco commented Nov 23, 2023 • edited Loading

wjin17 commented Nov 29, 2023

cristianrgreco commented Jan 10, 2024

wjin17 commented Jan 11, 2024

cristianrgreco commented Jan 15, 2024

netlify bot commented Aug 24, 2023 •

edited

Loading

wjin17 Aug 30, 2023 •

edited

Loading

cristianrgreco commented Nov 23, 2023 •

edited

Loading