Skip to content

Commit acfc595

Browse files
kai-ionkai-ion
committed
Add Profile computed getters using Aws::Crt::Optional for proper null handling:
 - GetServicesName() returns services definition name or empty Optional - GetEndpointUrl() returns global endpoint URL or empty Optional - Add static Profile::GetServiceEndpointUrl() helper for endpoint resolution - Takes profile, services map, and serviceId as parameters - Maintains Profile as stateless data container - Add AWSConfigFileProfileConfigLoader::GetServices() const accessor Uses Aws::Crt::Optional instead of empty strings to properly represent "no value" state. Static helper pattern keeps Profile ABI-stable while enabling service endpoint resolution without coupling to loader internals.
1 parent 7403d97 commit acfc595

File tree

5 files changed

+118
-99
lines changed

5 files changed

+118
-99
lines changed

‎src/aws-cpp-sdk-core/include/aws/core/config/AWSConfigFileProfileConfigLoader.h‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,9 @@ namespace Aws
4141
void SetFileName(const Aws::String& fileName) { m_fileName = fileName; }
4242

4343
/**
44-
* Get service-specific endpoint URL for a given profile and service.
44+
* Get services map for endpoint resolution.
4545
*/
46-
const Aws::String* GetServiceEndpointUrl(const Aws::String& profileName, const Aws::String& serviceId) const;
47-
48-
/**
49-
* Get global endpoint URL for a given profile.
50-
*/
51-
const Aws::String* GetGlobalEndpointUrl(const Aws::String& profileName) const;
46+
const Aws::Map<Aws::String, Aws::Map<Aws::String, Aws::String>>& GetServices() const;
5247

5348
protected:
5449
virtual bool LoadInternal() override;

‎src/aws-cpp-sdk-core/include/aws/core/config/AWSProfileConfig.h‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <aws/core/utils/memory/stl/AWSString.h>
99
#include <aws/core/utils/memory/stl/AWSMap.h>
1010
#include <aws/core/auth/AWSCredentials.h>
11+
#include <aws/crt/Optional.h>
1112

1213
namespace Aws
1314
{
@@ -93,18 +94,24 @@ namespace Aws
9394
return iter->second;
9495
}
9596

96-
inline const Aws::String& GetServicesName() const {
97-
auto iter = m_allKeyValPairs.find("services");
98-
static const Aws::String empty;
99-
return (iter != m_allKeyValPairs.end()) ? iter->second : empty;
97+
inline Aws::Crt::Optional<Aws::String> GetServicesName() const {
98+
const Aws::String& service = GetValue("services");
99+
return service.empty() ? Aws::Crt::Optional<Aws::String>() : Aws::Crt::Optional<Aws::String>(service);
100100
}
101101

102-
inline const Aws::String& GetEndpointUrl() const {
103-
auto iter = m_allKeyValPairs.find("endpoint_url");
104-
static const Aws::String empty;
105-
return (iter != m_allKeyValPairs.end()) ? iter->second : empty;
102+
inline Aws::Crt::Optional<Aws::String> GetEndpointUrl() const {
103+
const Aws::String& endpoint = GetValue("endpoint_url");
104+
return endpoint.empty() ? Aws::Crt::Optional<Aws::String>() : Aws::Crt::Optional<Aws::String>(endpoint);
106105
}
107106

107+
/**
108+
* Static helper that get service-specific endpoint URL for a given service.
109+
*/
110+
static Aws::Crt::Optional<Aws::String> GetServiceEndpointUrl(
111+
const Profile& profile,
112+
const Aws::Map<Aws::String, Aws::Map<Aws::String, Aws::String>>& services,
113+
const Aws::String& serviceId);
114+
108115
inline bool IsSsoSessionSet() const { return m_ssoSessionSet; }
109116
inline const SsoSession& GetSsoSession() const { return m_ssoSession; }
110117
inline void SetSsoSession(const SsoSession& value) { m_ssoSessionSet = true; m_ssoSession = value; }

‎src/aws-cpp-sdk-core/source/config/AWSConfigFileProfileConfigLoader.cpp‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ namespace Aws
176176

177177
// New service block: "s3 =" (right hand side empty)
178178
if (!left.empty() && right.empty()) {
179-
activeServiceId = StringUtils::ToLower(left.c_str());
179+
activeServiceId = StringUtils::ToUpper(left.c_str());
180180
StringUtils::Replace(activeServiceId, " ", "_");
181181
continue;
182182
}
@@ -738,39 +738,9 @@ namespace Aws
738738
return false;
739739
}
740740

741-
const Aws::String* AWSConfigFileProfileConfigLoader::GetServiceEndpointUrl(const Aws::String& profileName, const Aws::String& serviceId) const
741+
const Aws::Map<Aws::String, Aws::Map<Aws::String, Aws::String>>& AWSConfigFileProfileConfigLoader::GetServices() const
742742
{
743-
auto profileIter = m_profiles.find(profileName);
744-
if (profileIter == m_profiles.end())
745-
{
746-
return nullptr;
747-
}
748-
749-
const auto& servicesName = profileIter->second.GetServicesName();
750-
if (servicesName.empty()) {
751-
return nullptr;
752-
}
753-
754-
auto servicesIter = m_services.find(servicesName);
755-
if (servicesIter == m_services.end()) {
756-
return nullptr;
757-
}
758-
759-
Aws::String key = StringUtils::ToLower(serviceId.c_str());
760-
StringUtils::Replace(key, " ", "_");
761-
auto serviceIter = servicesIter->second.find(key);
762-
return (serviceIter == servicesIter->second.end()) ? nullptr : &serviceIter->second;
763-
}
764-
765-
const Aws::String* AWSConfigFileProfileConfigLoader::GetGlobalEndpointUrl(const Aws::String& profileName) const
766-
{
767-
auto profileIter = m_profiles.find(profileName);
768-
if (profileIter == m_profiles.end())
769-
{
770-
return nullptr;
771-
}
772-
const Aws::String& endpointUrl = profileIter->second.GetEndpointUrl();
773-
return endpointUrl.empty() ? nullptr : &endpointUrl;
743+
return m_services;
774744
}
775745
} // Config namespace
776746
} // Aws namespace

‎src/aws-cpp-sdk-core/source/config/AWSProfileConfig.cpp‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0.
4+
*/
5+
6+
#include <aws/core/config/AWSProfileConfig.h>
7+
#include <aws/core/utils/StringUtils.h>
8+
9+
namespace Aws
10+
{
11+
namespace Config
12+
{
13+
using namespace Aws::Utils;
14+
15+
Aws::Crt::Optional<Aws::String> Profile::GetServiceEndpointUrl(
16+
const Profile& profile,
17+
const Aws::Map<Aws::String, Aws::Map<Aws::String, Aws::String>>& services,
18+
const Aws::String& serviceId)
19+
{
20+
auto servicesName = profile.GetServicesName();
21+
if (!servicesName.has_value()) {
22+
return Aws::Crt::Optional<Aws::String>();
23+
}
24+
25+
auto servicesIter = services.find(*servicesName);
26+
if (servicesIter == services.end()) {
27+
return Aws::Crt::Optional<Aws::String>();
28+
}
29+
30+
Aws::String key = StringUtils::ToUpper(serviceId.c_str());
31+
StringUtils::Replace(key, " ", "_");
32+
auto serviceIter = servicesIter->second.find(key);
33+
34+
return (serviceIter == servicesIter->second.end() || serviceIter->second.empty()) ?
35+
Aws::Crt::Optional<Aws::String>() :
36+
Aws::Crt::Optional<Aws::String>(serviceIter->second);
37+
}
38+
}
39+
}

‎tests/aws-cpp-sdk-core-tests/aws/config/ServiceEndpointsConfigFileLoaderTest.cpp‎

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -39,34 +39,29 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificEndpoints)
3939
ASSERT_NE(profiles.end(), profiles.find("default"));
4040

4141
// Test global endpoint
42-
auto globalEndpoint = loader.GetGlobalEndpointUrl("default");
43-
ASSERT_NE(nullptr, globalEndpoint);
42+
const auto& profile = profiles["default"];
43+
auto globalEndpoint = profile.GetEndpointUrl();
44+
ASSERT_TRUE(globalEndpoint.has_value());
4445
ASSERT_STREQ("https://global.example.com", globalEndpoint->c_str());
4546

4647
// Test service-specific endpoints
47-
auto s3Endpoint = loader.GetServiceEndpointUrl("default", "s3");
48-
ASSERT_NE(nullptr, s3Endpoint);
48+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
49+
ASSERT_TRUE(s3Endpoint.has_value());
4950
ASSERT_STREQ("http://localhost:9000", s3Endpoint->c_str());
5051

51-
auto dynamoEndpoint = loader.GetServiceEndpointUrl("default", "dynamodb");
52-
ASSERT_NE(nullptr, dynamoEndpoint);
52+
auto dynamoEndpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "dynamodb");
53+
ASSERT_TRUE(dynamoEndpoint.has_value());
5354
ASSERT_STREQ("http://localhost:8000", dynamoEndpoint->c_str());
5455

5556
// Test case insensitive service lookup
56-
auto s3EndpointUpper = loader.GetServiceEndpointUrl("default", "S3");
57-
ASSERT_NE(nullptr, s3EndpointUpper);
57+
auto s3EndpointUpper = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "S3");
58+
ASSERT_TRUE(s3EndpointUpper.has_value());
5859
ASSERT_STREQ("http://localhost:9000", s3EndpointUpper->c_str());
5960

6061
// Test non-existent service
61-
auto nonExistent = loader.GetServiceEndpointUrl("default", "nonexistent");
62-
ASSERT_EQ(nullptr, nonExistent);
63-
64-
// Test non-existent profile
65-
auto nonExistentProfile = loader.GetServiceEndpointUrl("nonexistent", "s3");
66-
ASSERT_EQ(nullptr, nonExistentProfile);
67-
68-
auto nonExistentProfileGlobal = loader.GetGlobalEndpointUrl("nonexistent");
69-
ASSERT_EQ(nullptr, nonExistentProfileGlobal);
62+
auto nonExistent = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "nonexistent");
63+
ASSERT_FALSE(nonExistent.has_value());
64+
7065
}
7166

7267
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificEndpointsOnly)
@@ -83,19 +78,21 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificEndpointsOnly)
8378

8479
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
8580
ASSERT_TRUE(loader.Load());
81+
auto profiles = loader.GetProfiles();
82+
const auto& profile = profiles["dev-minio"];
8683

8784
// Test that global endpoint is null when not set
88-
auto globalEndpoint = loader.GetGlobalEndpointUrl("dev-minio");
89-
ASSERT_EQ(nullptr, globalEndpoint);
85+
auto globalEndpoint = profile.GetEndpointUrl();
86+
ASSERT_FALSE(globalEndpoint.has_value());
9087

9188
// Test service-specific endpoint
92-
auto s3Endpoint = loader.GetServiceEndpointUrl("dev-minio", "s3");
93-
ASSERT_NE(nullptr, s3Endpoint);
89+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
90+
ASSERT_TRUE(s3Endpoint.has_value());
9491
ASSERT_STREQ("https://play.min.io:9000", s3Endpoint->c_str());
9592

9693
// Test case insensitive lookup
97-
auto s3EndpointUpper = loader.GetServiceEndpointUrl("dev-minio", "S3");
98-
ASSERT_NE(nullptr, s3EndpointUpper);
94+
auto s3EndpointUpper = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "S3");
95+
ASSERT_TRUE(s3EndpointUpper.has_value());
9996
ASSERT_STREQ("https://play.min.io:9000", s3EndpointUpper->c_str());
10097
}
10198

@@ -110,15 +107,17 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestGlobalEndpointOnly)
110107

111108
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
112109
ASSERT_TRUE(loader.Load());
110+
auto profiles = loader.GetProfiles();
111+
const auto& profile = profiles["dev-global"];
113112

114113
// Test global endpoint
115-
auto globalEndpoint = loader.GetGlobalEndpointUrl("dev-global");
116-
ASSERT_NE(nullptr, globalEndpoint);
114+
auto globalEndpoint = profile.GetEndpointUrl();
115+
ASSERT_TRUE(globalEndpoint.has_value());
117116
ASSERT_STREQ("https://play.min.io:9000", globalEndpoint->c_str());
118117

119118
// Test that service-specific endpoint is null when not set
120-
auto s3Endpoint = loader.GetServiceEndpointUrl("dev-global", "s3");
121-
ASSERT_EQ(nullptr, s3Endpoint);
119+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
120+
ASSERT_FALSE(s3Endpoint.has_value());
122121
}
123122

124123
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificAndGlobalEndpoints)
@@ -136,20 +135,22 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificAndGlobalEndpoin
136135

137136
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
138137
ASSERT_TRUE(loader.Load());
138+
auto profiles = loader.GetProfiles();
139+
const auto& profile = profiles["dev-s3-specific-and-global"];
139140

140141
// Test service-specific S3 endpoint
141-
auto s3Endpoint = loader.GetServiceEndpointUrl("dev-s3-specific-and-global", "s3");
142-
ASSERT_NE(nullptr, s3Endpoint);
142+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
143+
ASSERT_TRUE(s3Endpoint.has_value());
143144
ASSERT_STREQ("https://play.min.io:9000", s3Endpoint->c_str());
144145

145146
// Test global endpoint for other services
146-
auto globalEndpoint = loader.GetGlobalEndpointUrl("dev-s3-specific-and-global");
147-
ASSERT_NE(nullptr, globalEndpoint);
147+
auto globalEndpoint = profile.GetEndpointUrl();
148+
ASSERT_TRUE(globalEndpoint.has_value());
148149
ASSERT_STREQ("http://localhost:1234", globalEndpoint->c_str());
149150

150151
// Test that non-configured service returns null (would fall back to global)
151-
auto dynamoEndpoint = loader.GetServiceEndpointUrl("dev-s3-specific-and-global", "dynamodb");
152-
ASSERT_EQ(nullptr, dynamoEndpoint);
152+
auto dynamoEndpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "dynamodb");
153+
ASSERT_FALSE(dynamoEndpoint.has_value());
153154
}
154155

155156
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestMultipleServicesInDefinition)
@@ -168,20 +169,22 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestMultipleServicesInDefinition)
168169

169170
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
170171
ASSERT_TRUE(loader.Load());
172+
auto profiles = loader.GetProfiles();
173+
const auto& profile = profiles["dev"];
171174

172175
// Test S3 endpoint
173-
auto s3Endpoint = loader.GetServiceEndpointUrl("dev", "s3");
174-
ASSERT_NE(nullptr, s3Endpoint);
176+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
177+
ASSERT_TRUE(s3Endpoint.has_value());
175178
ASSERT_STREQ("http://localhost:4567", s3Endpoint->c_str());
176179

177180
// Test Elastic Beanstalk endpoint with normalized service ID
178-
auto ebEndpoint = loader.GetServiceEndpointUrl("dev", "Elastic Beanstalk");
179-
ASSERT_NE(nullptr, ebEndpoint);
181+
auto ebEndpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "Elastic Beanstalk");
182+
ASSERT_TRUE(ebEndpoint.has_value());
180183
ASSERT_STREQ("http://localhost:8000", ebEndpoint->c_str());
181184

182185
// Test direct normalized lookup
183-
auto ebEndpointDirect = loader.GetServiceEndpointUrl("dev", "elastic_beanstalk");
184-
ASSERT_NE(nullptr, ebEndpointDirect);
186+
auto ebEndpointDirect = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "elastic_beanstalk");
187+
ASSERT_TRUE(ebEndpointDirect.has_value());
185188
ASSERT_STREQ("http://localhost:8000", ebEndpointDirect->c_str());
186189
}
187190

@@ -198,14 +201,16 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestIgnoreGlobalEndpointInServicesS
198201

199202
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
200203
ASSERT_TRUE(loader.Load());
204+
auto profiles = loader.GetProfiles();
205+
const auto& profile = profiles["testing"];
201206

202207
// Test that global endpoint in services section is ignored
203-
auto globalEndpoint = loader.GetGlobalEndpointUrl("testing");
204-
ASSERT_EQ(nullptr, globalEndpoint);
208+
auto globalEndpoint = profile.GetEndpointUrl();
209+
ASSERT_FALSE(globalEndpoint.has_value());
205210

206211
// Test that service-specific endpoints return null (no services configured)
207-
auto s3Endpoint = loader.GetServiceEndpointUrl("testing", "s3");
208-
ASSERT_EQ(nullptr, s3Endpoint);
212+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profile, loader.GetServices(), "s3");
213+
ASSERT_FALSE(s3Endpoint.has_value());
209214
}
210215

211216
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestSourceProfileEndpointIsolation)
@@ -227,24 +232,27 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestSourceProfileEndpointIsolation)
227232

228233
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
229234
ASSERT_TRUE(loader.Load());
235+
auto profiles = loader.GetProfiles();
236+
const auto& profileB = profiles["B"];
237+
const auto& profileA = profiles["A"];
230238

231239
// Test that profile B gets EC2 endpoint from its own services definition
232-
auto ec2Endpoint = loader.GetServiceEndpointUrl("B", "ec2");
233-
ASSERT_NE(nullptr, ec2Endpoint);
240+
auto ec2Endpoint = Profile::GetServiceEndpointUrl(profileB, loader.GetServices(), "ec2");
241+
ASSERT_TRUE(ec2Endpoint.has_value());
234242
ASSERT_STREQ("https://profile-b-ec2-endpoint.aws", ec2Endpoint->c_str());
235243

236244
// Test that profile B has no global endpoint (doesn't inherit from profile A)
237-
auto globalEndpointB = loader.GetGlobalEndpointUrl("B");
238-
ASSERT_EQ(nullptr, globalEndpointB);
245+
auto globalEndpointB = profileB.GetEndpointUrl();
246+
ASSERT_FALSE(globalEndpointB.has_value());
239247

240248
// Test that profile A still has its own global endpoint
241-
auto globalEndpointA = loader.GetGlobalEndpointUrl("A");
242-
ASSERT_NE(nullptr, globalEndpointA);
249+
auto globalEndpointA = profileA.GetEndpointUrl();
250+
ASSERT_TRUE(globalEndpointA.has_value());
243251
ASSERT_STREQ("https://profile-a-endpoint.aws/", globalEndpointA->c_str());
244252

245253
// Test that other services in profile B return null (no chaining to profile A)
246-
auto s3Endpoint = loader.GetServiceEndpointUrl("B", "s3");
247-
ASSERT_EQ(nullptr, s3Endpoint);
254+
auto s3Endpoint = Profile::GetServiceEndpointUrl(profileB, loader.GetServices(), "s3");
255+
ASSERT_FALSE(s3Endpoint.has_value());
248256
}
249257
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestIgnoreConfiguredEndpointUrls)
250258
{

0 commit comments

Comments
 (0)