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.

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

Jump to bottom

IOPZ-1847: Support multiple K8s roles for single environment #12

Merged
merged 3 commits into from
Feb 13, 2024
Merged

IOPZ-1847: Support multiple K8s roles for single environment #12

merged 3 commits into from
Feb 13, 2024

Conversation

jordannb
Copy link
Contributor

@jordannb jordannb commented Jan 11, 2024 •
edited
Loading

Testing

  1. Ran ./create-k8s-chained-sessions.sh
  2. Confirmed I can connect with or without specifying a persona (admin, dev-writer, etc.) to connect with (see https://github.com/panorama-ed/school-supplies/pull/167).

@jordannb jordannb force-pushed the IOPZ-1847-role-specific-profiles branch from 86f5f86 to 6f77927 Compare January 11, 2024 19:23
@jordannb jordannb marked this pull request as ready for review January 11, 2024 19:25
@jordannb jordannb changed the title IOPZ-1847: Support multiple K8s roles IOPZ-1847: Support multiple K8s roles for single environment Jan 11, 2024
jordannb
jordannb commented Jan 11, 2024
View reviewed changes
create-k8s-chained-sessions.sh Outdated
createLeappSession "$session" "AWSAdministratorAccess" "eks-admin-1.24"
createLeappSession "$session" "PanoramaK8sEngineeringDefault" "panorama-dev-writer-1.24"
createLeappSession "$session" "PanoramaK8sEngineeringDefault" "panorama-dev-reader-1.24"
createLeappSession "$session" "AWSAdministratorAccess" "eks" "admin" "1.24"
Copy link
Contributor Author

@jordannb jordannb Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Trying to reign in scope creep for now, so I left the eks and panorama keywords, but only to support the IAM roles without needing to change those too right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Those will be changing very soon as part of the in-flight cluster upgrade epic, so I think this makes sense.

@jordannb jordannb force-pushed the IOPZ-1847-role-specific-profiles branch from 6f77927 to c73c83b Compare January 12, 2024 13:21
Pat-Ayres
Pat-Ayres requested changes Jan 12, 2024
View reviewed changes
create-k8s-chained-sessions.sh Outdated
createLeappSession "$session" "AWSAdministratorAccess" "eks-admin-1.24"
createLeappSession "$session" "PanoramaK8sEngineeringDefault" "panorama-dev-writer-1.24"
createLeappSession "$session" "PanoramaK8sEngineeringDefault" "panorama-dev-reader-1.24"
createLeappSession "$session" "AWSAdministratorAccess" "eks" "admin" "1.24"
Copy link
Contributor

Choose a reason for hiding this comment

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

Those will be changing very soon as part of the in-flight cluster upgrade epic, so I think this makes sense.

create-k8s-chained-sessions.sh Outdated
# and can be removed when the keyword is removed from the associated IAM
# roles.
# 4: name of the persona (e.g. admin, dev-writer, etc.) the new session is for
# 5: version of kubernetes, used to determine the associated IAM role
function createLeappSession {
green_echo "creating chained session for $1 with role $3"
Copy link
Contributor

Choose a reason for hiding this comment

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

This echo doesn't make sense anymore with the new arguments. It only outputs panorama or eks instead of a unique role. I think you'll need to do some concatenation to get a role, or just change it to with persona $4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you - I went with the persona here since that's the abstraction people will be abstracting with directly anyways, so I guess it puts the idea out there from the start.

create-k8s-chained-sessions.sh Outdated

green_echo " looking for existing session ${chained_session_name}"
chained_session_id=$(leappSessionId "$chained_session_name" "$chained_role_name")
chained_session_id=$(leappSessionId "$chained_session_name" "${persona_name}")
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is broken. The script should be idempotent, but after running it, the second time it still goes into the session creation logic and creates duplicate sessions.

You'll need to pass in the full role name or change the jq select function to work as a fuzzy filter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been fixed now. I retested by re-running the script; output is below (punycode deprecation errors removed manually for readability):

% ./create-k8s-chained-sessions.sh                                                                                                                                (~/Development/panorama/leapp-setup) panorama-k8s-playground-admin
creating chained session for panorama-k8s-playground with persona admin
    looking for existing session panorama-k8s-playground-admin
    existing session found
creating chained session for panorama-k8s-playground with persona dev-writer
    looking for existing session panorama-k8s-playground-dev-writer
    existing session found
creating chained session for panorama-k8s-playground with persona dev-reader
    looking for existing session panorama-k8s-playground-dev-reader
    existing session found
creating chained session for panorama-k8s-playground with persona data-science-tester
    looking for existing session panorama-k8s-playground-data-science-tester
    existing session found
creating chained session for panorama-k8s-playground-2 with persona admin
    looking for existing session panorama-k8s-playground-2-admin
    existing session found
creating chained session for panorama-k8s-playground-2 with persona dev-writer
    looking for existing session panorama-k8s-playground-2-dev-writer
    existing session found
creating chained session for panorama-k8s-playground-2 with persona dev-reader
    looking for existing session panorama-k8s-playground-2-dev-reader
    existing session found
creating chained session for panorama-k8s-playground-2 with persona data-science-tester
    looking for existing session panorama-k8s-playground-2-data-science-tester
    existing session found
creating chained session for panorama-k8s-staging with persona admin
    looking for existing session panorama-k8s-staging-admin
    existing session found
creating chained session for panorama-k8s-staging with persona dev-writer
    looking for existing session panorama-k8s-staging-dev-writer
    existing session found
creating chained session for panorama-k8s-staging with persona dev-reader
    looking for existing session panorama-k8s-staging-dev-reader
    existing session found
creating chained session for panorama-k8s-staging with persona data-science-tester
    looking for existing session panorama-k8s-staging-data-science-tester
    no existing session found; starting session for panorama-k8s-staging to get role arn
An error occurred (NoSuchEntity) when calling the GetRole operation: The role with name panorama-data-science-tester-1.24 cannot be found.
    creating new profile
    creating new session
creating chained session for panorama-k8s-production with persona admin
    looking for existing session panorama-k8s-production-admin
    existing session found
creating chained session for panorama-k8s-production with persona dev-writer
    looking for existing session panorama-k8s-production-dev-writer
    existing session found
creating chained session for panorama-k8s-production with persona dev-reader
    looking for existing session panorama-k8s-production-dev-reader
    existing session found
creating chained session for panorama-k8s-production with persona data-science-tester
An error occurred (NoSuchEntity) when calling the GetRole operation: The role with name panorama-data-science-tester-1.24 cannot be found.

Copy link
Contributor

Choose a reason for hiding this comment

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

(fyi, you can get rid of the punycode output by changing your host installed node version. I opened a ticket with the leapp team about it with the workaround I used described.

@jordannb jordannb force-pushed the IOPZ-1847-role-specific-profiles branch from c73c83b to 9297d2a Compare January 12, 2024 17:19
@jordannb jordannb merged commit 1fdda4f into main Feb 13, 2024
2 checks passed
@jordannb jordannb deleted the IOPZ-1847-role-specific-profiles branch February 13, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pat-Ayres Pat-Ayres Pat-Ayres approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jordannb @Pat-Ayres