Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ignore] Testing branch with useSraAuth=true #4478

Closed
wants to merge 45 commits into from

Conversation

zoewangg
Copy link
Contributor

@zoewangg zoewangg commented Sep 25, 2023

PR to kick off integ tests against the feature/master/sra-identity-auth-testing branch which has useSraAuth=true.

@zoewangg zoewangg force-pushed the feature/master/sra-identity-auth-testing branch from ec19254 to b6a9ffe Compare September 29, 2023 23:05
@zoewangg zoewangg force-pushed the feature/master/sra-identity-auth-testing branch from b6a9ffe to ba2eab9 Compare October 2, 2023 17:34
zoewangg and others added 16 commits October 2, 2023 13:35
…' into feature/master/sra-identity-auth-testing
…' into feature/master/sra-identity-auth-testing
* Update NoneAuthTypeRequestTest for SRA clients

* Update ClientBuilderTest for SRA clients
In commit d269d6b where code was
uncommented, missed removing the @disabled on one test.
@gosar gosar changed the title [Ignore] testing [Ignore] Testing branch with useSraAuth=true Oct 13, 2023
gosar and others added 15 commits October 13, 2023 13:25
* Remove derived attribute logic for PRESIGNER_EXPIRATION

Because of the 2 instant() calls in the write/read mappings, a basic
write this attribute and read it back, would give different values.

Since this attribute should only be set/read explicitly by our
presigner code, we don't need this mapping logic at all.

Also, update S3PresignerTest to assert isEqualTo instead of isCloseTo
and remove unnecessary override

* Remove PRESIGNER_EXPIRATION mirroring test
A few limitations exist in plugin's ability to affect the SDK's configuration:
1. When plugins modify the configuration at the request-level, it does not affect configuration that is "derived" from the configuration that was modified (e.g. changing the profile file does not change the region, even if the region was derived from the profile file originally).
2. Plugins do not see the default configuration applied by the client.
3. Plugins do not see and cannot modify configuration applied by customers at the request level.

This PR fixes (1), except for credentials derived from profile files. Future PRs will address the remaining limitations. As a result, this PR includes many disabled and commented-out tests that will be re-enabled as these limitations are corrected.

The particularly interesting/risky changes are in AwsDefaultClientBuilder, SdkDefaultClientBuilder, and AttributeMap. Major new testing of functionality is in AttributeMapTest and SdkPluginTest.

This PR makes the following large changes:
1. Add support for "lazy" SDK client options. These options can be set, but not evaluated until they are read. This functionality was added in AttributeMap, which SdkClientConfiguration uses.

This PR makes the following small changes and bug fixes:
1. Add support for equals and hashCode in the default endpoint provider.
2. Fixed a bug where scheduled executors specified by plugins were not treated as unmanaged.
3. Reduced the number of times that the profile file was read during client creation.
4. Add support for specifying a profile file supplier in the ClientOverrideConfiguration.
5. Changed the way "child" classes configure the AttributeMap used to create the HTTP client to match the way other configuration works.
Copy link

sonarcloud bot commented Nov 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

millems and others added 4 commits November 10, 2023 07:30
Before this change, plugins would not see any configuration specified on the client. This limited its ability to decorate existing configuration, and diminished plugin's ability to have intuitive behavior.

This PR does not fix request-level configuration. Request-level plugins do not see request-level override configuration, and request-level override configuration is still used with a higher priority than plugin configuration. This PR does establish a possible pattern for fixing this: allowing override configuration objects to modify the SdkClientConfiguration directly.

Change Summary:
* Default and customer-specified client configuration is now available to SdkPlugins and is also now included in the SdkClient#serviceClientConfiguration.
* Updated ClientOverrideConfiguration to use SdkClientConfiguration internally, and provided internal methods for creating/retrieving these internal configuration objects.
* Updated SDK-provided ServiceClientConfiguration.Builder objects to use SdkClientConfiguration internally, allowing direct modification of these objects and avoiding the need to do translation to these objects from SdkClientConfiguration.
* Removed places where we copied configuration instead of using the SdkClientConfiguration as the source of truth.

Minor Changes:
* Fixed an issue where the non-SRA TOKEN_PROVIDER was not being set, preventing possible compatibility issues with old bearer token clients.
* Simplified TokenProviders initialization logic.
* Fixed a few cache-busting bugs in AttributeMap related to dependency tracking between builder copies.
* Added new methods to SdkClientConfiguration and AttributeMap to allow additional ways to add/modify configuration.
@zoewangg zoewangg closed this Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants