Service constructors with significantly neutered production behavior should have been removed and replaced

[jamesivie]

Describe the bug

In the transition from SDK 3.x, a (possibly unintentional) malicious change was made.

Previously, the service object constructors that took a profile name fell back to using the IAM role when a null profile was passed. In 4.x, the constructor still exists, still accepts null, and doesn't throw an exception, but subsequently all calls fail due to the lack of credentials. This is the worst possible way this breaking change could have been handled. With normal usage patterns, developers use local credentials (classically API keys) on their local machines that used the named profile, and passed in null for the profile in production. The way this change was made caused everything to work for devs on their local machines and for unit tests, but fail in production, not even at construction time, but later when the code attempted to use the service with various somewhat confusing access denied errors. Even after finding out there were problems, the only way to figure out what was going wrong was extensive logging or running a debugger inside AWS.

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

When I got my code upgraded from using SDK3.x to SDK4.x compiling and the unit tests running on my local machine, I expected that code to work when deployed to AWS. I expected that whenever possible, breaking changes break in the compilation step.

Current Behavior

When upgrading from SDK 3.x, code compiles without errors and works outside of AWS when debugging and running unit tests, but then when you deploy to AWS, EVERYTHING fails with some kind of access error. Debugging and fixing the code is extremely difficult because it MUST be done on a machine that relies on IAM--that's the only case that broke.

Reproduction Steps

Write code using SDK3.x that uses service constructors that take a profile name, passing a profile name in dev mode and null in production mode (like just about everyone was doing). Next, upgrade to SDK4.x and attempt to get your code to work in production. Good luck!

Possible Solution

When the decision was made to change the behavior of a profile specified as null, ideally those constructors would have been removed completely or marked with proper non-nullable attributes so that compiler errors would have indicated to devs that they needed to change something. Second best would have been to have the constructor throw an exception immediately when null is passed for the profile. I understand if you don't want to make a breaking change for this by removing the constructor, but YOU ALREADY BROKE EVERYONE with this change--they just won't know it until they try to deploy into AWS. Breaking at compile time is MUCH PREFERRED to breaking at production runtime. At a minimum you should throw an exception in the constructor in this case--at least the exception will give developers a clue as to the horrific break-only-in-production change you've made.

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

ANY SDK 3.x and ANY SDK 4.x

Targeted .NET Platform

.NET 9.0

Operating System and version

Windows 11, Amazon Linux 2023

[normj]

We didn't intend a runtime breaking change here but I'm a little confused exactly which code path you are talking about because we don't have any constructors on the service client that take in a profile name. The service client constructors are various forms of AWSCredentials or access key/secret key.

Can you give a code example of what you mean by passing null for the profile?

[jamesivie]

Sorry, not the service constructors directly, but the Profile property of the *Config objects they take. It's been about a month since I finally got through this ordeal, so between AWS support response times and bogus responses, it's been a while. Here are three samples of configs that fail silently until production requests are made.
Old code that worked in SDK3.x and in SDK 4.x still compiled fine and worked fine in dev, but failed in production:

            AmazonS3Config readConfig = new()
            {
                Timeout = TimeSpan.FromSeconds(ReadTimeoutSeconds),
                MaxErrorRetry = ReadMaxErrorRetry,
                MaxConnectionsPerServer = Math.Max(50, Environment.ProcessorCount * 15),
                Profile = AwsConfig.Profile,      // if this is null, PROD fails
                RegionEndpoint = AwsConfig.RegionEndpoint,
            };
            AmazonDynamoDBConfig config = new()
            {
                Profile = AwsConfig.Profile,    // if this is null, PROD fails
                RegionEndpoint = AwsConfig.RegionEndpoint,
            };
            AmazonCloudWatchConfig clientConfig = new AmazonCloudWatchConfig
            {
                Timeout = TimeSpan.FromSeconds(10),
                MaxErrorRetry = Settings.GetSetting("CloudwatchMaxErrorRetry", 2),
                Profile = AwsConfig.Profile,    // if this is null, PROD fails
                RegionEndpoint = AwsConfig.RegionEndpoint,
            };

Change required to get production to work with SDK4.x:

            AmazonS3Config readConfig = string.IsNullOrEmpty(AwsConfig.Profile?.Name) ? new()
                {
                    Timeout = TimeSpan.FromSeconds(ReadTimeoutSeconds),
                    MaxErrorRetry = ReadMaxErrorRetry,
                    MaxConnectionsPerServer = Math.Max(50, Environment.ProcessorCount * 15),
                    RegionEndpoint = AwsConfig.RegionEndpoint,
                }
                : new()
                {
                    Timeout = TimeSpan.FromSeconds(ReadTimeoutSeconds),
                    MaxErrorRetry = ReadMaxErrorRetry,
                    MaxConnectionsPerServer = Math.Max(50, Environment.ProcessorCount * 15),
                    Profile = AwsConfig.Profile,
                    RegionEndpoint = AwsConfig.RegionEndpoint,
                };
            AmazonDynamoDBConfig config = string.IsNullOrEmpty(AwsConfig.Profile?.Name) 
                ? new()
                {
                    RegionEndpoint = AwsConfig.RegionEndpoint,
                } : new()
                {
                    Profile = AwsConfig.Profile,
                    RegionEndpoint = AwsConfig.RegionEndpoint,
                };
            AmazonCloudWatchConfig clientConfig = string.IsNullOrEmpty(AwsConfig.Profile?.Name) ? new()
            {
                Timeout = TimeSpan.FromSeconds(10),
                MaxErrorRetry = Settings.GetSetting("CloudwatchMaxErrorRetry", 2),
                RegionEndpoint = AwsConfig.RegionEndpoint,
            } : new()
            {
                Timeout = TimeSpan.FromSeconds(10),
                MaxErrorRetry = Settings.GetSetting("CloudwatchMaxErrorRetry", 2),
                Profile = AwsConfig.Profile,
                RegionEndpoint = AwsConfig.RegionEndpoint,
            };

[normj]

@jamesivie I'm sorry for the pain this caused. This was not an intentional change and we will get this fixed to avoid the next person having your pain.

@dscpinheiro I think the issue is here in DefaultAWSCredentialsIdentityResolver.ResolveIdentity where the check for profile if (profile != null) should really be something like if (!string.IsNullOrEmpty(profile?.Name)).

[dscpinheiro]

The fix was included in today's release of the SDK (https://github.com/aws/aws-sdk-net/releases/tag/4.0.109.0), and the code you had in V3 should work again.

I'm sorry you had a bad experience with support, in the future if you run into any other problems with V4 please open an issue here on GitHub too.