Update mssql.go

[kedarkale27]

query will run on the database passed as the parameter instead of the master database.
This seems the issue of the Go lang
even if we are executing the command use database
the database is not getting selected and the select query runs on the master database
this returns no results if we have a pre-created schema in another database

[hashicorp-cla]

All committers have signed the CLA.

[tyrannosaurus-becks]

Hi @kedarkale27 ! Thanks for working on this.

So I pulled it locally, merged in master, and ran TestMSSQL_Initialize and found that it passes, but I also found that it doesn't hit the updated code because the schema it uses is dbo so it never gets to line 131.

Would you be willing to write an additional test that covers this code, just so that we can be sure it's working and so we can have a test documenting how it's intended to be used?

Thanks!

[kedarkale27]

Hi @kedarkale27 ! Thanks for working on this.

So I pulled it locally, merged in master, and ran TestMSSQL_Initialize and found that it passes, but I also found that it doesn't hit the updated code because the schema it uses is dbo so it never gets to line 131.

Would you be willing to write an additional test that covers this code, just so that we can be sure it's working and so we can have a test documenting how it's intended to be used?

Thanks!

Hi @tyrannosaurus-becks the test case has been added to verify the updated code. Can you please check and approve ?

Thanks!

[tyrannosaurus-becks]

Hi @kedarkale27 , thanks for adding that test. When I pull the PR and try to run it, I get:

tbex@tbex:~/go/src/github.com/hashicorp/vault/physical/mssql$ go test -v
# github.com/hashicorp/vault/physical/mssql [github.com/hashicorp/vault/physical/mssql.test]
./mssql_test.go:85:13: undefined: test

[kedarkale27]

Hi @kedarkale27 , thanks for adding that test. When I pull the PR and try to run it, I get:

tbex@tbex:~/go/src/github.com/hashicorp/vault/physical/mssql$ go test -v
# github.com/hashicorp/vault/physical/mssql [github.com/hashicorp/vault/physical/mssql.test]
./mssql_test.go:85:13: undefined: test

Hi @tyrannosaurus-becks , I have modified the test case to solve this above mentioned error. Can you please check now ?

[tyrannosaurus-becks]

Hi @kedarkale27 ,

I ran the test again, and received this output:

=== RUN   TestMSSQLBackend_schema
--- FAIL: TestMSSQLBackend_schema (0.90s)
    mssql_test.go:92: Failed to create new backend: failed to create mssql table: mssql: The specified schema name "test" either does not exist or you do not have permission to use it.
FAIL

Let me share with you how I'm testing. First, I run MSSQL locally using Docker, like this:

sudo docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=<YourStrong!Passw0rd>' \
   -p 1433:1433 --name sql1 \
   -d mcr.microsoft.com/mssql/server:2017-latest

Then, I run the new test locally using the following environment variables:

export MSSQL_SERVER=localhost
export MSSQL_USERNAME=SA
export MSSQL_PASSWORD='<YourStrong!Passw0rd>'

Once you've gotten the test working, can you please post the output the command line gives you showing the test is successful? Like from running it there, and then viewing the success response? Once you've got that posted, I'd be happy to re-review this PR. Thanks!

[kedarkale27]

Hi @kedarkale27 ,

I ran the test again, and received this output:

=== RUN   TestMSSQLBackend_schema
--- FAIL: TestMSSQLBackend_schema (0.90s)
    mssql_test.go:92: Failed to create new backend: failed to create mssql table: mssql: The specified schema name "test" either does not exist or you do not have permission to use it.
FAIL

Let me share with you how I'm testing. First, I run MSSQL locally using Docker, like this:

sudo docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=<YourStrong!Passw0rd>' \
   -p 1433:1433 --name sql1 \
   -d mcr.microsoft.com/mssql/server:2017-latest

Then, I run the new test locally using the following environment variables:

export MSSQL_SERVER=localhost
export MSSQL_USERNAME=SA
export MSSQL_PASSWORD='<YourStrong!Passw0rd>'

Once you've gotten the test working, can you please post the output the command line gives you showing the test is successful? Like from running it there, and then viewing the success response? Once you've got that posted, I'd be happy to re-review this PR. Thanks!

Hi @tyrannosaurus-becks
I have created a variable schema to fetch the value of MSSQL_SCHEMA from the environment and if it is not present then i am creating the schema as "test".
Please find attached the screen shot of my local setup where the test case is getting passed.

Please let me know if I am missing any step

You can check my latest test case which I have pushed in the PR.

May be you need create a new environment variable MSSQL_SCHEMA which will point to the schema created in the MSSQl server which is other than dbo? just suggesting.
We do not have any docker environment setup in our environment.

Also can you please specify the MSSQL version you are using ?

[tyrannosaurus-becks]

@kedarkale27 the version I'm on is 2017-latest. What version of MSSQL are you using? Also, what would be the value for the MSSQL_SCHEMA variable you're suggesting?

It would be very helpful if these tests can pass using a Docker image because otherwise we may not be able to duplicate your test environment; however, let's figure out what you're using first.

[kedarkale27]

ss using a Docker image because otherwise we may

@tyrannosaurus-becks ,
the version which I am using is SQL Server 2012.
Actually I am a java developer with very limited knowledge of Go lang.

Steps performed by me:

1. fork Vault code
2. make code changes, add basic test case
3. run commands on the cmd (bootstrap, make dev, go test -v)

My current Vault code setup does not point to any SQL sever while running the test. It just runs the test simply like maven test.

Can you please share the steps that I have to perform to setup my environment to use a SQL server that I have. Also can you please share the commands that i have to use to run and test the test case.
If you can share the docker image, then may be we can replicate the steps you are performing.

Sorry for the inconvenience caused to you, please help.

[tyrannosaurus-becks]

Hi @kedarkale27 ,

Ah, gotcha! Well, I think it would be better if we used Docker for testing rather than your test environment. The reason is, we can use Docker and its available images for tests (for free); whereas we won't be able to necessarily duplicate your test environment for our automated tests because we don't have an MSSQL license. If we can't create a test environment in the future, that means we can't make future changes to the code and know whether we're breaking things.

So, I'd recommend installing Docker, and then getting it working using those test steps above here.

I hope that helps!

[kedarkale27]

Hi @kedarkale27 ,

Ah, gotcha! Well, I think it would be better if we used Docker for testing rather than your test environment. The reason is, we can use Docker and its available images for tests (for free); whereas we won't be able to necessarily duplicate your test environment for our automated tests because we don't have an MSSQL license. If we can't create a test environment in the future, that means we can't make future changes to the code and know whether we're breaking things.

So, I'd recommend installing Docker, and then getting it working using those test steps above here.

I hope that helps!

Hi @tyrannosaurus-becks,
I configured my environment to work with docker MSSQL server 2017 as per the steps you mentioned above.
I set the environment variables to the ones you had mentioned above.
I needed to make some changes in the mssql.go file to make the test work. There seems to be some issue with GO lang where it is not using the selected database.
Please pull in the latest code and check. The test case is passing now. :)
Thanks a lot for the help you provided.
I have attached the screen shot below:

[kedarkale27]

Hi @kedarkale27 ,
Ah, gotcha! Well, I think it would be better if we used Docker for testing rather than your test environment. The reason is, we can use Docker and its available images for tests (for free); whereas we won't be able to necessarily duplicate your test environment for our automated tests because we don't have an MSSQL license. If we can't create a test environment in the future, that means we can't make future changes to the code and know whether we're breaking things.
So, I'd recommend installing Docker, and then getting it working using those test steps above here.
I hope that helps!

Hi @tyrannosaurus-becks,
I configured my environment to work with docker MSSQL server 2017 as per the steps you mentioned above.
I set the environment variables to the ones you had mentioned above.
I needed to make some changes in the mssql.go file to make the test work. There seems to be some issue with GO lang where it is not using the selected database.
Please pull in the latest code and check. The test case is passing now. :)
Thanks a lot for the help you provided.
I have attached the screen shot below:

Hi @tyrannosaurus-becks ,
Did you get a chance to check the latest code change?

[tyrannosaurus-becks]

Thanks @kedarkale27 ! It's working for me too. Much appreciated.

[kedarkale27]

Thanks @kedarkale27 ! It's working for me too. Much appreciated.

Thank you @tyrannosaurus-becks ! Can you please let me know when this code will be a part of the main branch ? or when it will be released ?
I do not have the access to merge this pull request to the main branch, are you going to merge this code later?