Skip to content

Pre-requisite Changes Enable smithy query + rest-xml clients #3321

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 18, 2025
Merged

Pre-requisite Changes Enable smithy query + rest-xml clients #3321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
511f0b7
Enable Smithy Query, rest xml clients
sbera87 Feb 28, 2025
92ed8ad
backwards compatible changes
sbera87 Mar 11, 2025
510c06a
reuse code
sbera87 Mar 13, 2025
89be26a
move all legacy presigners
sbera87 Mar 13, 2025
0122fd4
disable smithy client generation
sbera87 Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
304 changes: 304 additions & 0 deletions src/aws-cpp-sdk-core/include/smithy/client/AwsLegacyClient.h
View file
Open in desktop

Large diffs are not rendered by default.

179 changes: 82 additions & 97 deletions src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ namespace client
auto httpResponseOutcome = MakeRequestSync(request, requestName, method, std::move(endpointCallback));
return m_serializer->Deserialize(std::move(httpResponseOutcome), GetServiceClientName(), requestName);
}

Aws::String GeneratePresignedUrl(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so fan of having on presigner now, only question i really have is why is this in the the Smithy client and the others legcy. I think something that we could talk about is "what the public interface of a pre signer should be". but since this is already here, im open to moving forward without having a answer for that for now

EndpointUpdateCallback&& endpointCallback,
Aws::Http::HttpMethod method,
Expand All @@ -238,110 +238,66 @@ namespace client
const Aws::Http::HeaderValueCollection& customizedHeaders,
const std::shared_ptr<Aws::Http::ServiceSpecificParameters> serviceSpecificParameters) const
{
AwsSmithyClientAsyncRequestContext ctx;
auto authSchemeOptionOutcome = SelectAuthSchemeOption( ctx);
auto authSchemeOption = std::move(authSchemeOptionOutcome.GetResultWithOwnership());

Aws::Endpoint::EndpointParameters epParams = Aws::Endpoint::EndpointParameters();
const auto authSchemeEpParams = authSchemeOption.endpointParameters();
epParams.insert(epParams.end(), authSchemeEpParams.begin(), authSchemeEpParams.end());
if(serviceSpecificParameters)
{
auto bucketIt = serviceSpecificParameters->parameterMap.find("bucketName");
if(bucketIt != serviceSpecificParameters->parameterMap.end())
ExtractUriCallback getUriCallback = [&](Aws::Http::URI& uri, Aws::String& signerRegionOverride,
Aws::String& signerServiceNameOverride, const AuthSchemeOption& authSchemeOption) -> bool{

Aws::Endpoint::EndpointParameters epParams = Aws::Endpoint::EndpointParameters();
const auto authSchemeEpParams = authSchemeOption.endpointParameters();
epParams.insert(epParams.end(), authSchemeEpParams.begin(), authSchemeEpParams.end());
if(serviceSpecificParameters)
{
auto bucket = bucketIt->second;
epParams.emplace_back(Aws::String("Bucket"), bucket);
auto bucketIt = serviceSpecificParameters->parameterMap.find("bucketName");
if(bucketIt != serviceSpecificParameters->parameterMap.end())
{
auto bucket = bucketIt->second;
epParams.emplace_back(Aws::String("Bucket"), bucket);
}
}
}

auto epResolutionOutcome = this->ResolveEndpoint(std::move(epParams), std::move(endpointCallback));
if (!epResolutionOutcome.IsSuccess())
{
AWS_LOGSTREAM_ERROR(ServiceNameT, "Presigned URL generating failed. Encountered error: " << epResolutionOutcome.GetError().GetMessage());
return {};
}
auto endpoint = std::move(epResolutionOutcome.GetResultWithOwnership());
const Aws::Http::URI& uri = endpoint.GetURI();
auto signerRegionOverride = region;
auto signerServiceNameOverride = serviceName;
//signer name is needed for some identity resolvers
if (endpoint.GetAttributes()) {
if (endpoint.GetAttributes()->authScheme.GetSigningRegion()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegion()->c_str();
}
if (endpoint.GetAttributes()->authScheme.GetSigningRegionSet()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegionSet()->c_str();
auto epResolutionOutcome = this->ResolveEndpoint(std::move(epParams), std::move(endpointCallback));
if (!epResolutionOutcome.IsSuccess())
{
AWS_LOGSTREAM_ERROR(ServiceNameT, "Presigned URL generating failed. Encountered error: " << epResolutionOutcome.GetError().GetMessage());
return false;
}
if (endpoint.GetAttributes()->authScheme.GetSigningName()) {
signerServiceNameOverride = endpoint.GetAttributes()->authScheme.GetSigningName()->c_str();
auto endpoint = std::move(epResolutionOutcome.GetResultWithOwnership());
uri = endpoint.GetURI();
signerRegionOverride = region;
signerServiceNameOverride = serviceName;
//signer name is needed for some identity resolvers
if (endpoint.GetAttributes()) {
if (endpoint.GetAttributes()->authScheme.GetSigningRegion()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegion()->c_str();
}
if (endpoint.GetAttributes()->authScheme.GetSigningRegionSet()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegionSet()->c_str();
}
if (endpoint.GetAttributes()->authScheme.GetSigningName()) {
signerServiceNameOverride = endpoint.GetAttributes()->authScheme.GetSigningName()->c_str();
}
}
}
std::shared_ptr<HttpRequest> request = CreateHttpRequest(uri, method, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod);
request->SetServiceSpecificParameters(serviceSpecificParameters);
for (const auto& it: customizedHeaders)
{
request->SetHeaderValue(it.first.c_str(), it.second);
}
if (AwsClientRequestSigning<AuthSchemesVariantT>::PreSignRequest(request, authSchemeOption, m_authSchemes, signerRegionOverride, signerServiceNameOverride, expirationInSeconds).IsSuccess())
{
return request->GetURIString();
}
return {};
}
return true;
};

//legacy
Aws::String GeneratePresignedUrl(const Aws::Http::URI& uri,
Aws::Http::HttpMethod method,
const Aws::String& region,
const Aws::String& serviceName,
long long expirationInSeconds,
const Aws::Http::HeaderValueCollection& customizedHeaders,
const std::shared_ptr<Aws::Http::ServiceSpecificParameters> serviceSpecificParameters) const
{
std::shared_ptr<HttpRequest> request = CreateHttpRequest(uri, method, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod);
request->SetServiceSpecificParameters(serviceSpecificParameters);
for (const auto& it: customizedHeaders)
{
request->SetHeaderValue(it.first.c_str(), it.second);
}
AwsSmithyClientAsyncRequestContext ctx;
auto authSchemeOptionOutcome = SelectAuthSchemeOption( ctx);
auto authSchemeOption = std::move(authSchemeOptionOutcome.GetResultWithOwnership());
if (AwsClientRequestSigning<AuthSchemesVariantT>::PreSignRequest(request, authSchemeOption, m_authSchemes, region, serviceName, expirationInSeconds).IsSuccess())
{
return request->GetURIString();
}
return {};
}


Aws::String GeneratePresignedUrl(const Aws::Endpoint::AWSEndpoint& endpoint,
Aws::Http::HttpMethod method,
const Aws::String& region,
const Aws::String& serviceName,
long long expirationInSeconds,
const Aws::Http::HeaderValueCollection& customizedHeaders,
const std::shared_ptr<Aws::Http::ServiceSpecificParameters> serviceSpecificParameters) const
{
const Aws::Http::URI& uri = endpoint.GetURI();
auto signerRegionOverride = region;
auto signerServiceNameOverride = serviceName;
// signer name is needed for some identity resolvers
if (endpoint.GetAttributes()) {
if (endpoint.GetAttributes()->authScheme.GetSigningRegion()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegion()->c_str();
}
if (endpoint.GetAttributes()->authScheme.GetSigningRegionSet()) {
signerRegionOverride = endpoint.GetAttributes()->authScheme.GetSigningRegionSet()->c_str();
}
if (endpoint.GetAttributes()->authScheme.GetSigningName()) {
signerServiceNameOverride = endpoint.GetAttributes()->authScheme.GetSigningName()->c_str();
CreateHttpRequestCallback createHttpRequestCallback = [&customizedHeaders, &serviceSpecificParameters](const Aws::Http::URI& uri, const Aws::Http::HttpMethod& method) -> std::shared_ptr<HttpRequest> {
std::shared_ptr<HttpRequest> request = CreateHttpRequest(uri, method, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod);
request->SetServiceSpecificParameters(serviceSpecificParameters);
for (const auto& it: customizedHeaders)
{
request->SetHeaderValue(it.first.c_str(), it.second);
}
}
return GeneratePresignedUrl(uri, method, signerRegionOverride, signerServiceNameOverride, expirationInSeconds, customizedHeaders, serviceSpecificParameters);
return request;
};

return GeneratePresignedUrl(
std::move(getUriCallback),
std::move(createHttpRequestCallback),
method,
region,
serviceName,
expirationInSeconds);
}

/* Service client specific config, the actual object is stored in AwsSmithyClientBase by pointer
* In order to avoid config object duplication, smithy template client access it by a reference.
* So that base client has it by base config pointer, child smithy client has it by child config reference.
Expand All @@ -352,6 +308,35 @@ namespace client
Aws::UnorderedMap<Aws::String, AuthSchemesVariantT> m_authSchemes{};
std::shared_ptr<SerializerT> m_serializer{};
private:
using ExtractUriCallback = std::function<bool (Aws::Http::URI&, Aws::String& region, Aws::String& serviceName,const AuthSchemeOption&)>;
using CreateHttpRequestCallback = std::function<std::shared_ptr<HttpRequest> (const Aws::Http::URI&, const Aws::Http::HttpMethod&)>;

Aws::String GeneratePresignedUrl(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having four generate presigned URL methods seems excessive and that we are creating bloat in this class. can we do something similar to how we did AwsLegacyClientT and move this into a PresignMixinT then invoke a single smithy client method?

In fact we already do this on the old client but have it as a signer to look up. lets just add it as a mix in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to have all legacy presigners in legacy client and smithy client have only one implementation. Under the hood, they all reuse a private implementation .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a to duplicate this method body?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are overlaps in the message body for the auth scheme selection bit, but differs in processing uri with request. I will push a commit with an attempt to reuse the overlapping block.

ExtractUriCallback&& getUriCallback,
CreateHttpRequestCallback&& createHttpRequestCallback,
Aws::Http::HttpMethod method,
const Aws::String& region,
const Aws::String& serviceName,
long long expirationInSeconds) const
{
AwsSmithyClientAsyncRequestContext ctx;
auto authSchemeOptionOutcome = SelectAuthSchemeOption( ctx);
auto authSchemeOption = std::move(authSchemeOptionOutcome.GetResultWithOwnership());
Aws::Http::URI uri;
Aws::String signerRegionOverride = region;
Aws::String signerServiceNameOverride = serviceName;
if(!getUriCallback(uri , signerRegionOverride, signerServiceNameOverride, authSchemeOption))
{
return {};
}
std::shared_ptr<HttpRequest> request = createHttpRequestCallback(uri, method);
if (AwsClientRequestSigning<AuthSchemesVariantT>::PreSignRequest(request, authSchemeOption, m_authSchemes, signerRegionOverride, signerServiceNameOverride, expirationInSeconds).IsSuccess())
{
return request->GetURIString();
}
return {};
}

friend class AwsLegacyClientT<ServiceNameT, ResponseT, AwsSmithyClientT<ServiceNameT,
ServiceClientConfigurationT,
ServiceAuthSchemeResolverT,
Expand Down
2 changes: 1 addition & 1 deletion src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SigV4Signer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ namespace smithy {
}
return {identity.accessKeyId(), identity.secretAccessKey()};
}();
auto result = legacySigner.PresignRequest(*httpRequest, legacyCreds, region.c_str(), serviceName.c_str(), expirationTimeInSeconds);
auto result = legacySigner.PresignRequest(*httpRequest, legacyCreds, region.empty() ? nullptr : region.c_str(), serviceName.empty() ? nullptr : serviceName.c_str(), expirationTimeInSeconds);

return (result ? SigningFutureOutcome(std::move(httpRequest)) :
SigningError(Aws::Client::CoreErrors::CLIENT_SIGNING_FAILURE, "", "presign failed",
Expand Down
58 changes: 32 additions & 26 deletions ...rc/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/CppClientGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,40 +718,43 @@ protected SdkFileEntry generateClientSmithyHeaderFile(final ServiceModel service
VelocityContext context = createContext(serviceModel);
context.put("CppViewHelper", CppViewHelper.class);
context.put("RequestlessOperations", requestlessOperations);
Optional<String> firstAuthScheme = serviceModel.getAuthSchemes().stream().filter(entry->ResolverMapping.containsKey(entry)).findFirst();
if(firstAuthScheme.isPresent())
selectAuthschemeResolver(serviceModel, context);
String fileName = String.format("include/aws/%s/%sClient.h", serviceModel.getMetadata().getProjectName(),
serviceModel.getMetadata().getClassNamePrefix());

return makeFile(template, context, fileName, true);
}

private void selectAuthschemeResolver(final ServiceModel serviceModel, VelocityContext context)
{
if(serviceModel.getAuthSchemes().size() > 1)
{
context.put("AuthSchemeResolver", ResolverMapping.get(firstAuthScheme.get()));
context.put("AuthSchemeResolver", "SigV4MultiAuthSchemeResolver");
}
else
{
throw new RuntimeException(String.format("authSchemes '%s'",serviceModel.getAuthSchemes().stream().collect(Collectors.toList())
));
Optional<String> firstAuthScheme = serviceModel.getAuthSchemes().stream().filter(entry->ResolverMapping.containsKey(entry)).findFirst();

if(firstAuthScheme.isPresent())
{
context.put("AuthSchemeResolver", ResolverMapping.get(firstAuthScheme.get()));
}
else
{
throw new RuntimeException(String.format("authSchemes '%s'",serviceModel.getAuthSchemes().stream().collect(Collectors.toList())));
}
}
context.put("AuthSchemeVariants", serviceModel.getAuthSchemes().stream().map(this::mapAuthSchemes).collect(Collectors.joining(",")));

String fileName = String.format("include/aws/%s/%sClient.h", serviceModel.getMetadata().getProjectName(),
serviceModel.getMetadata().getClassNamePrefix());

return makeFile(template, context, fileName, true);
}

protected SdkFileEntry GenerateSmithyClientSourceFile(final ServiceModel serviceModel, int i) {
protected SdkFileEntry GenerateSmithyClientSourceFile(final ServiceModel serviceModel, int i, Optional<String> templateFile) {

Template template = velocityEngine.getTemplate("/com/amazonaws/util/awsclientgenerator/velocity/cpp/smithy/SmithyClientSource.vm", StandardCharsets.UTF_8.name());
String templatePath = templateFile.orElse("/com/amazonaws/util/awsclientgenerator/velocity/cpp/smithy/SmithyClientSource.vm");
Template template = velocityEngine.getTemplate(templatePath, StandardCharsets.UTF_8.name());

VelocityContext context = createContext(serviceModel);
context.put("CppViewHelper", CppViewHelper.class);
Optional<String> firstAuthScheme = serviceModel.getAuthSchemes().stream().filter(entry->ResolverMapping.containsKey(entry)).findFirst();
if(firstAuthScheme.isPresent())
{
context.put("AuthSchemeResolver", ResolverMapping.get(firstAuthScheme.get()));
}
else
{
throw new RuntimeException(String.format("authSchemes '%s'",serviceModel.getAuthSchemes().stream().collect(Collectors.toList())
));
}
selectAuthschemeResolver(serviceModel, context);
context.put("AuthSchemeMapEntries", createAuthSchemeMapEntries(serviceModel));

final String fileName;
Expand Down Expand Up @@ -788,7 +791,8 @@ protected SdkFileEntry GenerateLegacyClientSourceFile(final ServiceModel service
"aws.auth#sigv4a", "smithy::SigV4aAuthScheme",
"bearer", "smithy::BearerTokenAuthScheme",
"v4","smithy::SigV4AuthScheme",
"sigv4-s3express","S3::S3ExpressSigV4AuthScheme"
"sigv4-s3express","S3::S3ExpressSigV4AuthScheme",
"v2","smithy::SigV4AuthScheme"
);

protected String mapAuthSchemes(final String authSchemeName) {
Expand All @@ -804,14 +808,16 @@ protected String mapAuthSchemes(final String authSchemeName) {
"aws.auth#sigv4a", "smithy::SigV4aAuthSchemeOption::sigV4aAuthSchemeOption",
"bearer", "smithy::BearerTokenAuthSchemeOption::bearerTokenAuthSchemeOption",
"v4", "smithy::SigV4AuthSchemeOption::sigV4AuthSchemeOption",
"sigv4-s3express", "S3::S3ExpressSigV4AuthSchemeOption::s3ExpressSigV4AuthSchemeOption"
"sigv4-s3express", "S3::S3ExpressSigV4AuthSchemeOption::s3ExpressSigV4AuthSchemeOption",
"v2", "smithy::SigV4AuthSchemeOption::sigV4AuthSchemeOption"
);

private static final Map<String, String> ResolverMapping = ImmutableMap.of(
protected static final Map<String, String> ResolverMapping = ImmutableMap.of(
"aws.auth#sigv4", "SigV4AuthSchemeResolver",
"aws.auth#sigv4a", "SigV4aAuthSchemeResolver",
"bearer", "BearerTokenAuthSchemeResolver",
"v4", "SigV4AuthSchemeResolver"
"v4", "SigV4AuthSchemeResolver",
"v2", "SigV4AuthSchemeResolver"
);


Expand Down
3 changes: 2 additions & 1 deletion ...ain/java/com/amazonaws/util/awsclientgenerator/generators/cpp/JsonCppClientGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

Expand Down Expand Up @@ -176,7 +177,7 @@ protected List<SdkFileEntry> generateClientSourceFile( List<ServiceModel> servic
{
if(serviceModels.get(index).isUseSmithyClient() && !serviceModels.get(index).hasEventStreamingRequestShapes())
{
return GenerateSmithyClientSourceFile(serviceModels.get(index), index);
return GenerateSmithyClientSourceFile(serviceModels.get(index), index, Optional.empty());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reuse base class function

}
else
{
Expand Down
Loading
Loading