Update FSM config parser #3586

kai-ion · 2025-10-15T21:07:37Z

Add support for parsing [services] sections with service-specific endpoint_url configuration following AWS SDKs & Tools specification.

Issue #, if available:

Description of changes:

Extend Profile class with endpoint_url and servicesEndpointUrl map
Add FSM states for [services] and [services serviceId] sections
Add GetServiceEndpointUrl() and GetGlobalEndpointUrl() API methods
Support case-insensitive service ID lookup
Add comprehensive test coverage

Supports config format:
[profile default]
endpoint_url = https://global.example.com

[services s3]
endpoint_url = http://localhost:9000

Check all that applies:

Did a review by yourself.
Added proper tests to cover this PR. (If tests are not applicable, explain.)
Checked if this PR is a breaking (APIs have been changed) change.
Checked if this PR will not introduce cross-platform inconsistent behavior.
Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Add support for parsing [services] sections with service-specific endpoint_url configuration following AWS SDKs & Tools specification. Changes: - Extend Profile class with endpoint_url and servicesEndpointUrl map - Add FSM states for [services] and [services serviceId] sections - Add GetServiceEndpointUrl() and GetGlobalEndpointUrl() API methods - Support case-insensitive service ID lookup - Add comprehensive test coverage Supports config format: [profile default] endpoint_url = https://global.example.com [services s3] endpoint_url = http://localhost:9000

sbiscigl · 2025-10-17T15:25:13Z

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

+            /**
+             * Get service-specific endpoint URL for a given profile and service.
+             */
+            const Aws::String* GetServiceEndpointUrl(const Aws::String& profileName, const Aws::String& serviceId) const;


why are we returning a pointer to a string? we should likely be returning by value and rely on RVO or NVRO, or returning bt ref if we actually want to edit the owned string, but we dont want to be returning by pointer.

Updated by using Aws::Crt::Optional<Aws::String> instead of pointer returns.

sbiscigl · 2025-10-17T15:30:00Z

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

                return iter->second;
            }

+            inline const Aws::String& GetServicesName() const {


why return are we returing by ref? we definitely should not be editing the underlying value.

Same as above. Updated to use Aws::Crt::Optional<Aws::String>

sbiscigl · 2025-10-17T15:33:21Z

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

+            inline const Aws::String& GetServicesName() const {
+                auto iter = m_allKeyValPairs.find("services");
+                static const Aws::String empty;
+                return (iter != m_allKeyValPairs.end()) ? iter->second : empty;


This likely not how we should be returning the idea of "lack of service endpoint". likely Crt::Optional is a good candidate for representing "lacking of value" instead of empty string.

Same as above. Updated to use Aws::Crt::Optional<Aws::String>

sbiscigl · 2025-10-17T15:55:32Z

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

+
+                        // New service block: "s3 =" (right hand side empty)
+                        if (!left.empty() && right.empty()) {
+                            activeServiceId = StringUtils::ToLower(left.c_str());


from the spec

This transformation MUST replace any spaces in the service ID with underscores and uppercase all letters

so ToLower here I think should be ToUpper no?

Fixed normalization to use ToUpper

sbiscigl · 2025-10-17T16:03:38Z

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

+            auto profileIter = m_profiles.find(profileName);
+            if (profileIter == m_profiles.end()) 
+            {
+                return nullptr;


yeah dont use "nullptr" to denote absence of a value, or a failed outcome, use a optional, or result type for that

Same as above. Updated to use Aws::Crt::Optional<Aws::String>

sbiscigl · 2025-10-17T17:09:32Z

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

+            /**
+             * Get global endpoint URL for a given profile.
+             */
+            const Aws::String* GetGlobalEndpointUrl(const Aws::String& profileName) const;


Alright i think we have a slightly larger design problem here that we should deal with lets take a look at the two new added APIS

Aws::String GetServiceEndpointUrl(const Aws::String& profileName, const Aws::String& serviceId) const; Aws::String* GetGlobalEndpointUrl(const Aws::String& profileName) const;

both of these functions have something in common, they take a profileName, and even in the comments you mention there is a mapping between profile and endpoint urls. This says to me instead of maintaining a foreign key mapping via this API, this is something that should exist in the profile object, i.e. Profile . so instead of adding this api, lets add this data to the Profile object that we store.

Moved GetEndpointUrl() and GetServicesName() as computed getters in the profile object, also added a static helper that does the lookup for the service.

NVM on the static helper, removed it!
I was only using it as a temporary 'resolver' for the new unit tests anyways

…l 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.

…l handling: - GetServicesName() returns services definition name or empty Optional - GetEndpointUrl() returns global endpoint URL or empty Optional - Add AWSConfigFileProfileConfigLoader::GetServices() const accessor Uses Aws::Crt::Optional instead of empty strings to properly represent "no value" state.

kai-ion added 3 commits October 14, 2025 16:23

update header

0105668

updated implementation to correctly use specification

5f61ca1

kai-ion force-pushed the TheEnd branch from 721d7b2 to 9d3406c Compare October 16, 2025 19:49

sbiscigl reviewed Oct 17, 2025

View reviewed changes

kai-ion force-pushed the TheEnd branch from acfc595 to a1d3e94 Compare October 17, 2025 18:59

kai-ion added 2 commits October 17, 2025 15:29

whitespace

f394a3a

Update FSM config parser #3586

Are you sure you want to change the base?

Update FSM config parser #3586

Conversation

kai-ion commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants