betr-io / terraform-provider-mssql

Terraform provider for Microsoft SQL Server
https://registry.terraform.io/providers/betr-io/mssql/latest
MIT License
35 stars 29 forks source link

Can't create user on Synapse Dedicated Pool (Formerly Azure SQL DWH) #15

Open HL-Sibelco opened 2 years ago

HL-Sibelco commented 2 years ago

We are trying to create a user un a Synapse Dedicated Pool, with a similar script that we use for Azue-SQL-Databases.

Here is the script, the script works fine when used on a SQL-Database

resource "mssql_user" "auser_on_syndp" {
  provider = betrio-mssql

  server {
    host     = module.syn_server.full_name
    login {
      username = var.das_fix["sql_sa"]
      password = module.kv_syn_server_admin_password.secret_value
    }
  }
  database   = module.syndp_database.name
  login_name = module.create_syn_login.alogin.login_name
  username   = module.create_syn_login.alogin.login_name

  #roles    = [ "db_owner" ]
}

But here is the error

│ Error: unable to create user [syndpeu2pocedw9].[adf-eu2-poc-60-122-01]: mssql: Parse error at line: 1, column: 108: Incorrect syntax near 'role_cur'.
│ 
│   with module.provision_datafactories[0].module.adf_create["60-122-01"].module.adf_kv_secrets_syndp_cs["CS_1"].module.create_syn_user.mssql_user.auser_on_syndp,
│   on _modules/das/syn/syn_create_user/main.tf line 110, in resource "mssql_user" "auser_on_syndp":
│  110: resource "mssql_user" "auser_on_syndp" {

Any suggestion or in case this is due to the version, what version would solve it ?

HL-Sibelco commented 2 years ago

Did a download (Zip) of the code and think having found the problem. Roles are assigned by a cursor, but cursors do not exist on the Sql-Subset of Synapse.

magne commented 2 years ago

We don't have the need nor the time to address this issue at the moment since it works for our use cases, but we'll be happy to receive a PR for this feature :smiley:

IshanYadav commented 2 years ago

Did a download (Zip) of the code and think having found the problem. Roles are assigned by a cursor, but cursors do not exist on the Sql-Subset of Synapse.

I'm also facing the same issue. Did you find any solution. If so, Please let me know...

ivanl-out commented 1 week ago

Hi @magne,

I am also facing the same issue.

If you will be accepting PRs for this, I wanted to get your advice first on how to support Synapse without potentially breaking other use cases.

Synapse Dedicated and Serverless SQL pools do not support cursors. The following article from Microsoft describes how to overcome this issue.

The user.go file currently has 3 places where cursors are used. Taking a look at one of them here, the following SQL string is constructed:

DECLARE role_cur CURSOR FOR SELECT name FROM [mydatabase].[sys].[database_principals] WHERE type = 'R' AND name != 'public' AND name COLLATE SQL_Latin1_General_CP1_CI_AS IN (SELECT value FROM String_Split(@roles, ','));
DECLARE @role nvarchar(max);
OPEN role_cur;
FETCH NEXT FROM role_cur INTO @role;
WHILE @@FETCH_STATUS = 0
  BEGIN
    DECLARE @sql nvarchar(max);
    SET @sql = 'ALTER ROLE ' + QuoteName(@role) + ' ADD MEMBER ' + QuoteName(@username);
    EXEC (@sql);
    FETCH NEXT FROM role_cur INTO @role;
  END;
CLOSE role_cur;
DEALLOCATE role_cur;

To get this to work on Synapse, it needs to be adjusted to the following:

USE [master]; -- otherwise we get an error "Operation CREATE EXTERNAL TABLE AS SELECT is not allowed for a replicated database." when using a Serverless pool
CREATE TABLE #role_tbl WITH ( DISTRIBUTION = ROUND_ROBIN ) AS
  SELECT ROW_NUMBER() OVER(ORDER BY (SELECT NULL)) AS Sequence, name [role] FROM [mydatabase].[sys].[database_principals] WHERE type = 'R' AND name != 'public' AND name COLLATE SQL_Latin1_General_CP1_CI_AS IN (SELECT value FROM String_Split(@roles, ','));
USE [mydatabase];
DECLARE @count INT = (SELECT COUNT(*) FROM #role_tbl), @i INT = 1;
WHILE @i <= @count
  BEGIN
    DECLARE @sql nvarchar(max);
    SELECT @sql = 'ALTER ROLE ' + QuoteName([role]) + ' ADD MEMBER ' + QuoteName(@username) FROM #role_tbl WHERE Sequence = @i;
    EXEC (@sql);
    SET @i +=1;
  END;

Do you think it is safe to move away from cursors entirely in your terraform provider, or do you rather think that we should somehow try to detect via TQSL whether cursors are supported and then fall back to the temp table approach if they are not supported?

If we are to detect whether cursors are supported, how might we do this? One idea I have is to run the following statement:

DECLARE role_cur CURSOR FOR SELECT 1;

On Synapse Dedicated and Serverless SQL pools the following error would be thrown:

DECLARE CURSOR is not supported.