Adding support for MSI (Managed Service Identities), and AccessTokens… #222

darrenschwarz · 2019-12-04T02:52:43Z

Hi Guys,

Recently we have had requirement imposed that all sql connections in azure are made use MSI, and hence the need to add token access support.

I'd really appreciate it if anyone has the time to review this implementation.

Scathing criticism welcome <winces> :o)

Thanks

… in sqlConnections.

darrenschwarz · 2019-12-04T03:03:47Z

Just to note that I have added dependencies which may well actually preclude merging, primarily I am just looking for discussion and feedback. If the end result does however prove useful for others that would be be great too.

ckadluba · 2020-04-08T07:02:37Z

Hi @darrenschwarz! Thank you for the PR. It seems to be a very useful enhancement.

I'm currently in the process of doing a lot of refactoring and cleanup work in the sink to make the code better testable and readable and remove some potentially problematic stuff like missing using blocks and writeable static properties. Also I'm adding lots of unit tests.

I would really like to integrate your enhancement but due to the changes in the dev branch the PR will not be able to merge easily. But I promise to add your contribution after I'm done with the refactoring.

ckadluba · 2020-04-20T13:12:19Z

I have now completed integrating Azure Managed Identites as sketched in this PR into the dev branch. Serilog.Sinks.MSSqlServer.5.4.0-dev-00304.nupkg was published to nuget.org and is available for testing.

I had no opportunity to test the feature yet myself. So, if you @darrenschwarz, or anyone else who has Azure Managed Identities available, could help and try it, that would be great.

Version 5.4.0 allows to intialize the sink for using Azure MSI.

    Log.Logger = new LoggerConfiguration()
        .WriteTo.MSSqlServer(
            connectionString: "Server=...",
            sinkOptions: new SinkOptions
            {
                TableName = "LogEventsTable",
                AutoCreateSqlTable = true,
                UseAzureManagedIdentity = true,
                AzureServiceTokenProviderResource = "..."
            })
        .CreateLogger();

nblumhardt · 2020-04-20T21:02:14Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

ckadluba · 2020-04-21T00:12:01Z

I was able to test the Azure Managed Identities authentication with 5.4.0-dev-00304. Here are two screenshots I made during the tests.

This one shows the test program I used.

And here are the resulting log events in the Azure SQL database.

I will still leave this PR open and invite others to report their test results here.

ckadluba · 2020-04-21T00:21:10Z

@ckadluba love seeing all the progress being made on this sink :-)

Thank you very much! 😊

aasif2727 · 2020-04-22T08:21:56Z

Hi ckadluba,
This is exactly what I was looking for the new MSI integration.
How do we provide custom column Options,? Please help.

ckadluba · 2020-04-22T09:25:44Z

@aasif2727 Very good! Please try it out. It would help us a lot.

You can supply column options by passing the columnOptions parameter as documented here.

https://github.com/serilog/serilog-sinks-mssqlserver#code-only-any-net-target

darrenschwarz · 2020-04-24T10:18:55Z

Hi @darrenschwarz! Thank you for the PR. It seems to be a very useful enhancement.

I'm currently in the process of doing a lot of refactoring and cleanup work in the sink to make the code better testable and readable and remove some potentially problematic stuff like missing using blocks and writeable static properties. Also I'm adding lots of unit tests.

I would really like to integrate your enhancement but due to the changes in the dev branch the PR will not be able to merge easily. But I promise to add your contribution after I'm done with the refactoring.

Hi Guys,

Apologies for delayed response. Thanks for taking a look, and really glad it's proved useful. I'll get round to further review asap.

darrenschwarz · 2020-04-24T10:25:50Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

ckadluba · 2020-04-24T11:24:21Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

Cool! Thank you very much.

This could help us get a regular 5.4.0 release out soon. Please let me know if I can help you with anything.

darrenschwarz · 2020-04-25T00:08:02Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

Cool! Thank you very much.

This could help us get a regular 5.4.0 release out soon. Please let me know if I can help you with anything.

Maybe we could catch up on Monday, it would be useful to discuss an optimal test plan. We are keen to get back to inclusion of a regular release in our project.

darrenschwarz · 2020-04-27T23:22:26Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

Cool! Thank you very much.
This could help us get a regular 5.4.0 release out soon. Please let me know if I can help you with anything.

Maybe we could catch up on Monday, it would be useful to discuss an optimal test plan. We are keen to get back to inclusion of a regular release in our project.

FYI, twas an expected hectic start to the week, nevertheless I do intend completing tests in the next couple of days, and will post outcomes here. @ckadluba when done what might be the best channel for a more fluid catch-up?

ckadluba · 2020-04-27T23:41:11Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

Cool! Thank you very much.
This could help us get a regular 5.4.0 release out soon. Please let me know if I can help you with anything.

Maybe we could catch up on Monday, it would be useful to discuss an optimal test plan. We are keen to get back to inclusion of a regular release in our project.

FYI, twas an expected hectic start to the week, nevertheless I do intend completing tests in the next couple of days, and will post outcomes here. @ckadluba when done what might be the best channel for a more fluid catch-up?

Your help is very appreciatet, but no hassle :). Just post results or questions here whenever you are ready, I think it's the best place for now.

darrenschwarz · 2020-05-02T01:52:31Z

@ckadluba love seeing all the progress being made on this sink :-) 👍

Hey Guys, JUst spoken with guys on my current project, and we'll get the updates tested, and integrated. Will keep you updated.

Cool! Thank you very much.
This could help us get a regular 5.4.0 release out soon. Please let me know if I can help you with anything.

Maybe we could catch up on Monday, it would be useful to discuss an optimal test plan. We are keen to get back to inclusion of a regular release in our project.

FYI, twas an expected hectic start to the week, nevertheless I do intend completing tests in the next couple of days, and will post outcomes here. @ckadluba when done what might be the best channel for a more fluid catch-up?

Your help is very appreciatet, but no hassle :). Just post results or questions here whenever you are ready, I think it's the best place for now.

darrenschwarz · 2020-05-03T17:06:10Z

@ckadluba to save me some time can you provide an example appsettings.json excerpt to provide the settings?
"UseAzureManagedIdentity": true,
"AzureServiceTokenProviderResource": "https://database.windows.net/"
Thanks.

ckadluba · 2020-05-03T19:24:54Z

@ckadluba to save me some time can you provide an example appsettings.json excerpt to provide the settings?
"UseAzureManagedIdentity": true,
"AzureServiceTokenProviderResource": "https://database.windows.net/"
Thanks.

Yes, this is similar to the settings that I used. Look at the screenshots above. There you can see my full init code for the sink.

#222 (comment)

The sample app had no appsettings.json. It used only the init code shown in the screenshots. Anyway, a sample appsettings.json containing the Azure Managed Identities properties can be found here (Azure MSI is not activated but still you can see the required structure of the config file):

https://github.com/serilog/serilog-sinks-mssqlserver/blob/dev/sample/WorkerServiceDemo/appsettings.json

Some questions:

Can you post your full init code for the sink and/or appsettings.json you used?
Did your implementation work in the same environment/azure configuration before you swapped in the current sink prerelease nuget?

Regarding screen sharing. Yes, this would be possible. But I would not like to disclose my mail address here. What times and dates would be possible for you to do a session?

darrenschwarz · 2020-05-04T07:54:06Z

@ckadluba to save me some time can you provide an example appsettings.json excerpt to provide the settings?
"UseAzureManagedIdentity": true,
"AzureServiceTokenProviderResource": "https://database.windows.net/"
Thanks.

Yes, this is similar to the settings that I used. Look at the screenshots above. There you can see my full init code for the sink.

#222 (comment)

The sample app had no appsettings.json. It used only the init code shown in the screenshots. Anyway, a sample appsettings.json containing the Azure Managed Identities properties can be found here (Azure MSI is not activated but still you can see the required structure of the config file):

https://github.com/serilog/serilog-sinks-mssqlserver/blob/dev/sample/WorkerServiceDemo/appsettings.json

Some questions:

Can you post your full init code for the sink and/or appsettings.json you used?

Did your implementation work in the same environment/azure configuration before you swapped in the current sink prerelease nuget?

Regarding screen sharing. Yes, this would be possible. But I would not like to disclose my mail address here. What times and dates would be possible for you to do a session?

So the good news is as I suspected the issue was with config. Using the simple config as below everything works as one would hope.

Where there is still as issue is with the current config used in Prod similar to below:

"Serilog": { "Using": [ "Serilog.Settings.Configuration", "Serilog.Sinks.MSSqlServer", "Serilog.Sinks.AzureAnalytics" ], "MinimumLevel": { "Default": "Information", "Override": { "Microsoft": "Warning", "System": "Warning", "IdentityServer4": "Information", "Database": "Information" } }, "WriteTo": { "SubLoggerAudit": { "Name": "Logger", "Args": { "configureLogger": { "Filter": [ { "Name": "ByIncludingOnly", "Args": { "expression": "Contains(@MessageTemplate, '{@event}')" } } ], "AuditTo": [ { "Name": "MSSqlServer", "Args": { "connectionString": "DefaultConnection", "schemaName": "dbo", "tableName": "MyAuthAuditTable", "restrictedToMinimumLevel": "Information", "useAzureManagedIdentity": true, "azureServiceTokenProviderResource": "https://database.windows.net/" } } ] } } }, "SubLoggerAlertLog": { "Name": "Logger", "Args": { "configureLogger": { "Filter": [ { "Name": "ByIncludingOnly", "Args": { "expression": "@Level = 'Error'" } } ], "WriteTo": [ { "Name": "AzureLogAnalytics", "Args": { "workspaceId": "", "authenticationId": "", "logName": "Errors" } } ] } } } } },

@ckadluba I'll get back to you later today with date/times that we could catch up.

ckadluba · 2020-05-04T08:36:59Z

@darrenschwarz Glad to hear that.

The problem seems to be the parameter structure. The sink has a new interface using a SinkOptions object. Within that you have the new properties to control Azure MSI. While your prod configuration seems to use the old interface without SinkOptions where the two parameters for Azure MSI are ignored. I decided to support Azure MSI only with the new interface to avoid adding more and more parameters to the constructors and config extension methods of the sink.

Try to change this

    "AuditTo": [
        {
            "Name": "MSSqlServer",
            "Args": {
                "connectionString": "DefaultConnection",
                "schemaName": "dbo",
                "tableName": "MyAuthAuditTable",
                "restrictedToMinimumLevel": "Information",
                "useAzureManagedIdentity": true,
                "azureServiceTokenProviderResource": "https://database.windows.net/"
            }
        }
    ]

to that:

    "AuditTo": [
        {
            "Name": "MSSqlServer",
            "Args": {
                "connectionString": "DefaultConnection",
                "restrictedToMinimumLevel": "Information",
                "sinkOptionsSection": {
                    "schemaName": "dbo",
                    "tableName": "MyAuthAuditTable",
                    "useAzureManagedIdentity": true,
                    "azureServiceTokenProviderResource": "https://database.windows.net/"
                }
            }
        }
    ]

darrenschwarz · 2020-05-04T11:08:22Z

                "restrictedToMinimumLevel": "Information",

Aha new interface only! Of course makes a lot of sense

@darrenschwarz Glad to hear that.

The problem seems to be the parameter structure. The sink has a new interface using a SinkOptions object. Within that you have the new properties to control Azure MSI. While your prod configuration seems to use the old interface without SinkOptions where the two parameters for Azure MSI are ignored. I decided to support Azure MSI only with the new interface to avoid adding more and more parameters to the constructors and config extension methods of the sink.

Try to change this

    "AuditTo": [
        {
            "Name": "MSSqlServer",
            "Args": {
                "connectionString": "DefaultConnection",
                "schemaName": "dbo",
                "tableName": "MyAuthAuditTable",
                "restrictedToMinimumLevel": "Information",
                "useAzureManagedIdentity": true,
                "azureServiceTokenProviderResource": "https://database.windows.net/"
            }
        }
    ]

to that:

    "AuditTo": [
        {
            "Name": "MSSqlServer",
            "Args": {
                "connectionString": "DefaultConnection",
                "restrictedToMinimumLevel": "Information",
                "sinkOptionsSection": {
                    "schemaName": "dbo",
                    "tableName": "MyAuthAuditTable",
                    "useAzureManagedIdentity": true,
                    "azureServiceTokenProviderResource": "https://database.windows.net/"
                }
            }
        }
    ]

This test is using an instance of Identity Server as a means to raise some events for logging / audit as below....

In Program.cs

and appsettings.json...

Happy days! All working as expected...

...and looks good to go. :)

ckadluba · 2020-05-04T12:04:41Z

Happy days! All working as expected...
...
...and looks good to go. :)

Very cool! Thank you for the contribution and for testing. 👍

I will close this PR now and make a 5.4.0 release soon.

darrenschwarz · 2020-05-06T18:53:31Z

Happy days! All working as expected...
...
...and looks good to go. :)

Very cool! Thank you for the contribution and for testing. 👍

I will close this PR now and make a 5.4.0 release soon.

@ckadluba you can send me contact details to contactforpr222@gmail.com. Thanks

Adding support for MSI (Managed Service Identities), and AccessTokens…

828787c

… in sqlConnections.

ckadluba mentioned this pull request Apr 20, 2020

New interfaces and support for Azure Managed Identities #265

Merged

ckadluba mentioned this pull request Apr 22, 2020

Support automatic table creation on SQL Azure #65

Closed

ckadluba self-assigned this Apr 23, 2020

ckadluba added the enhancement label Apr 23, 2020

ckadluba closed this May 4, 2020

ronaldbarendse mentioned this pull request May 12, 2020

Version 5.4 causes an exception trying to load Microsoft.Azure.Services.AppAuthentication #283

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for MSI (Managed Service Identities), and AccessTokens… #222

Adding support for MSI (Managed Service Identities), and AccessTokens… #222

darrenschwarz commented Dec 4, 2019

darrenschwarz commented Dec 4, 2019

ckadluba commented Apr 8, 2020

ckadluba commented Apr 20, 2020 •

edited

Loading

nblumhardt commented Apr 20, 2020

ckadluba commented Apr 21, 2020

ckadluba commented Apr 21, 2020

aasif2727 commented Apr 22, 2020 •

edited

Loading

ckadluba commented Apr 22, 2020

darrenschwarz commented Apr 24, 2020

darrenschwarz commented Apr 24, 2020

ckadluba commented Apr 24, 2020

darrenschwarz commented Apr 25, 2020 •

edited

Loading

darrenschwarz commented Apr 27, 2020

ckadluba commented Apr 27, 2020

darrenschwarz commented May 2, 2020 •

edited

Loading

darrenschwarz commented May 3, 2020

ckadluba commented May 3, 2020

darrenschwarz commented May 4, 2020

ckadluba commented May 4, 2020

darrenschwarz commented May 4, 2020

ckadluba commented May 4, 2020

darrenschwarz commented May 6, 2020

Adding support for MSI (Managed Service Identities), and AccessTokens… #222

Adding support for MSI (Managed Service Identities), and AccessTokens… #222

Conversation

darrenschwarz commented Dec 4, 2019

darrenschwarz commented Dec 4, 2019

ckadluba commented Apr 8, 2020

ckadluba commented Apr 20, 2020 • edited Loading

nblumhardt commented Apr 20, 2020

ckadluba commented Apr 21, 2020

ckadluba commented Apr 21, 2020

aasif2727 commented Apr 22, 2020 • edited Loading

ckadluba commented Apr 22, 2020

darrenschwarz commented Apr 24, 2020

darrenschwarz commented Apr 24, 2020

ckadluba commented Apr 24, 2020

darrenschwarz commented Apr 25, 2020 • edited Loading

darrenschwarz commented Apr 27, 2020

ckadluba commented Apr 27, 2020

darrenschwarz commented May 2, 2020 • edited Loading

darrenschwarz commented May 3, 2020

ckadluba commented May 3, 2020

darrenschwarz commented May 4, 2020

ckadluba commented May 4, 2020

darrenschwarz commented May 4, 2020

ckadluba commented May 4, 2020

darrenschwarz commented May 6, 2020

ckadluba commented Apr 20, 2020 •

edited

Loading

aasif2727 commented Apr 22, 2020 •

edited

Loading

darrenschwarz commented Apr 25, 2020 •

edited

Loading

darrenschwarz commented May 2, 2020 •

edited

Loading