Aws-sdk-net: PersistenceManager static constructor and credentials retrieval itself could be more robust

Created on 3 Sep 2017  路  7Comments  路  Source: aws/aws-sdk-net

When user profile is not loaded for the process (common case for IIS with processModel.loadUserProfile set to false, which is the default), it defaults to a specific subdirectory in the %WINDIR%\System32 directory. Currently, PersistenceManager static ctor is assuming that this directory is valid and can be written into, which is, unfortunately, not the case. This not only leads to failure to retrieve default AWS credentials on service startup, but also to a cached (permanently) TypeInitialization exception when accessing PersistenceManager.

I believe that static constructor should defer any file access logic to GetSettingsStoreFolder with some kind of lazy setup (this is good for app startup times also).
Also, the UnauthorizedAccessException probably should be ignored when progressing on staged AWS credential discovery and attempting to load credential data from user profile.

bug modulsdk-core response-requested

Most helpful comment

馃憢 @normj. I'd be happy to tackle this bug.

I'm in a situation where I need to use the SDK running as Network Service and the current PersistenceManager is throwing an error. I've stubbed myself a decent substitute for now, but I would like to fix this bug and get the code merged in proper.

I could use some guidance on how you would like to have the default PeristenceManager provided to the application.

All 7 comments

Agreed we do need to make this more robust.

馃憢 @normj. I'd be happy to tackle this bug.

I'm in a situation where I need to use the SDK running as Network Service and the current PersistenceManager is throwing an error. I've stubbed myself a decent substitute for now, but I would like to fix this bug and get the code merged in proper.

I could use some guidance on how you would like to have the default PeristenceManager provided to the application.

The InMemoryPersistenceManager is absolutely great ! We are eager to see that PR merged.

If you need some help for the tests, let me know.

Thanks @PascalUmaknow It's just a rough patch for now.
If you are setup to run the tests, please give it a shot, I never got that far.

I definitely want to get this merged, otherwise I have to setup a build of this library on my company's CI server. Which is work I rather not do.

@StanleyGoldman PR that addresses this issue has been released as part of version 3.3.106.20 of AWSSDK.Core

Hi @onyxmaster @StanleyGoldman,

I was going through the backlog and came across this issue. From @normj's comments, looks like the issue has been addressed in the version 3.3.106.20 of AWSSDK.Core. Please confirm if we could close this issue.

Thanks,
Ashish

Hi. While I cannot currently verify the issue since I don't work on the project I encountered this anymore, I reviewed the changes introduced with #1561, and they look good to me.

Was this page helpful?
0 / 5 - 0 ratings