Update parsing logic to include sso-session #24

waahm7 · 2023-02-28T23:46:29Z

Description of changes:
Changed the profiles map to an array of sections map so that we can handle sso-sessions, which are a different type of section.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2023-02-28T23:47:02Z

Codecov Report

Patch coverage: 91.07% and project coverage change: +0.44 🎉

Comparison is base (c98a341) 67.75% compared to head (4311bee) 68.20%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
+ Coverage   67.75%   68.20%   +0.44%     
==========================================
  Files           9        9              
  Lines        2562     2585      +23     
==========================================
+ Hits         1736     1763      +27     
+ Misses        826      822       -4

Impacted Files	Coverage Δ
source/aws_profile.c	`68.16% <91.07%> (+2.03%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

graebm · 2023-03-06T16:44:06Z

include/aws/sdkutils/aws_profile.h

+AWS_SDKUTILS_API
+const struct aws_profile *aws_profile_collection_get_sso_session(
+    const struct aws_profile_collection *profile_collection,
+    const struct aws_string *profile_name);


~~profile_name~~
name, session_name, identifier, id, session_id ???

Thanks, I have updated it to a generic function.

source/aws_profile.c

DmitriyMusatkin · 2023-03-06T18:34:13Z

include/aws/sdkutils/aws_profile.h

+ * Returns the number of sso-sessions in a collection
+ */
+AWS_SDKUTILS_API
+size_t aws_profile_collection_get_sso_session_count(const struct aws_profile_collection *profile_collection);


should we have a generic api that can return map of all values for a given section type and name?

Thanks, fixed.

source/aws_profile.c

DmitriyMusatkin · 2023-03-06T18:37:32Z

source/aws_profile.c


-    if (element == NULL) {
+    struct aws_hash_element *profile = NULL;
+    aws_hash_table_find(&collection->sections, profile_name, &profile);


nit: hash table find seems inconsistent. first one checks for error, second one ignores it

Thanks, fixed by refactor.

source/aws_profile.c

DmitriyMusatkin · 2023-03-06T18:50:00Z

source/aws_profile.c

@@ -895,6 +991,16 @@ static bool s_parse_profile_declaration(
            return true;
        }

+        s_parse_by_character_predicate(&profile_cursor, s_is_whitespace, NULL, 0);
+    } else if (has_sso_session_prefix) {
+        if (context->profile_collection->profile_source == AWS_PST_CREDENTIALS) {


if we dont allow profiles or other section types in cred files, do we need to have that check inside of specific section type if

Not sure how to improve this, as a profile can be valid without any prefix. This is only invalid if there was a prefix other than profile, and the source is CREDENTIALS.

graebm

fix & ship

graebm · 2023-03-08T16:14:00Z

include/aws/sdkutils/aws_profile.h

+const struct aws_profile *aws_profile_collection_get_section(
+    const struct aws_profile_collection *profile_collection,
+    const struct aws_string *section_name,
+    const enum aws_profile_section_type section_type);


trivial: put type before name. It just makes more sense

Suggested change

const struct aws_profile *aws_profile_collection_get_section(

const struct aws_profile_collection *profile_collection,

const struct aws_string *section_name,

const enum aws_profile_section_type section_type);

const struct aws_profile *aws_profile_collection_get_section(

const struct aws_profile_collection *profile_collection,

const enum aws_profile_section_type section_type,

const struct aws_string *section_name);

graebm · 2023-03-08T16:15:06Z

include/aws/sdkutils/aws_profile.h

@@ -99,12 +109,29 @@ const struct aws_profile *aws_profile_collection_get_profile(
    const struct aws_profile_collection *profile_collection,
    const struct aws_string *profile_name);

+/*
+ * Retrieves a reference to a section with the specified name and type, if it exists, from the profile collection.


mention somewhere that you can get the "default" profile by passing AWS_PROFILE_SECTION_TYPE_PROFILE, "default"

source/aws_profile.c

graebm · 2023-03-08T16:20:31Z

source/aws_profile.c

@@ -620,7 +641,8 @@ static int s_profile_collection_add_profile(
    const struct aws_byte_cursor *profile_name,
    bool has_prefix,
    const struct profile_file_parse_context *context,
-    struct aws_profile **current_profile_out) {
+    struct aws_profile **current_profile_out,
+    const enum aws_profile_section_type section_type) {


similar to earlier comment, move section_type arg so that it comes before profile_name

also, debatable, but does it make to code more clear going forward if we change ~~profile_name~~ -> section_name or name?

I think it would be better to stick to one naming scheme instead of mixing and matching.

Co-authored-by: Michael Graeb <graebm@amazon.com>

waahm7 added 9 commits February 27, 2023 21:27

initial implementation

be95fce

test map of map

4753201

format fix

a6c615c

in progress

4dbdb93

merge

0c791da

new struct

a0d16df

messy but working tests

51b2a7a

refactor

fc84e04

lint fix

07a68f1

waahm7 added 8 commits February 28, 2023 15:49

rename

a327d93

reset s_profile_hash_table_value_destroy

82b8a9b

remove collection variable

7a8ca0e

first test

b074cc5

another test

6d6c65c

add more test

5df7a3a

fix spaces

b96b6f4

update comment

d318127

waahm7 changed the title ~~WIP | Parse sso-session~~ Update parsing logic to include sso-session Mar 1, 2023

waahm7 marked this pull request as ready for review March 1, 2023 17:38

graebm reviewed Mar 6, 2023

View reviewed changes

DmitriyMusatkin reviewed Mar 6, 2023

View reviewed changes

waahm7 added 7 commits March 7, 2023 14:22

Use array instead of map

fba7799

Use generic get & get count function

4ded9eb

Add comment

31f6e26

update comment

17b242b

fix formatting

a9549cc

cleanup

30d0b58

remove blank line

bbee42d

graebm approved these changes Mar 8, 2023

View reviewed changes

DmitriyMusatkin approved these changes Mar 10, 2023

View reviewed changes

waahm7 and others added 3 commits March 14, 2023 12:21

Update source/aws_profile.c

76b987f

Co-authored-by: Michael Graeb <graebm@amazon.com>

refactor

e1f8e09

remove unused variable

4311bee

waahm7 merged commit 9de5923 into main Mar 14, 2023

waahm7 deleted the sso-session-parsing branch March 14, 2023 20:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update parsing logic to include sso-session #24

Update parsing logic to include sso-session #24

waahm7 commented Feb 28, 2023 •

edited

Loading

codecov-commenter commented Feb 28, 2023 •

edited

Loading

graebm Mar 6, 2023 •

edited

Loading

waahm7 Mar 7, 2023

DmitriyMusatkin Mar 6, 2023

waahm7 Mar 7, 2023

DmitriyMusatkin Mar 6, 2023

waahm7 Mar 7, 2023

DmitriyMusatkin Mar 6, 2023

waahm7 Mar 7, 2023 •

edited

Loading

graebm left a comment

graebm Mar 8, 2023

graebm Mar 8, 2023

graebm Mar 8, 2023

graebm Mar 8, 2023

waahm7 Mar 14, 2023 •

edited

Loading

Update parsing logic to include sso-session #24

Update parsing logic to include sso-session #24

Conversation

waahm7 commented Feb 28, 2023 • edited Loading

codecov-commenter commented Feb 28, 2023 • edited Loading

Codecov Report

graebm Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waahm7 Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

graebm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waahm7 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

waahm7 commented Feb 28, 2023 •

edited

Loading

codecov-commenter commented Feb 28, 2023 •

edited

Loading

graebm Mar 6, 2023 •

edited

Loading

waahm7 Mar 7, 2023 •

edited

Loading

waahm7 Mar 14, 2023 •

edited

Loading