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

Initial functionality #9

Merged
merged 48 commits into from
Mar 24, 2022
Merged

Initial functionality #9

merged 48 commits into from
Mar 24, 2022

Conversation

dav3r
Copy link
Member

@dav3r dav3r commented Mar 10, 2022 •
edited
Loading

🗣 Description

The PR provides the initial functionality of this project, a script that simplifies the process of opening sessions to AWS instances using SSM, either with or without SSH, and includes helpful tab autocompletion of various parameters on the command line.

💭 Motivation and context

There are two main benefits to using this tool:

  • The command to open a session is much simpler than without it.
  • There is no need to separately look up the ID of the AWS instance you want to connect to, since the tab completion in this tool does it for you.

If you need to connect to many different AWS instances across multiple AWS accounts via the CLI, this tool will save you a bunch of time.

Note that since this repository/module supports non-SSH connections, @felddy and I discussed the pros and cons of renaming it (and appropriately adjusting the argument names and related code), but we ultimately agreed that "awssh" could refer to opening any shell via AWS. Also, it seemed like extra work for minimal benefits.

🧪 Testing

I verified that the basic functionality all works as intended and all automated tests pass.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

✅ Post-merge checklist

  • Add a tag or create a release.

felddy and others added 30 commits July 13, 2021 14:01
@felddy
Initial commit of working code
6542b34 
This is the first commit of a working utility.
It still has a bunch of TODOs.
 - Functions need docs
 - Better README
 - Clean up logging.
 - Ton of linter errors
 - Many more
@felddy
Rename completer tool
a76c670 
Renamed the competer tool so that bash would not offer it
as a completion when typing awssh<tab>
@felddy
Allow completion matches on instance name
2807eea 
This was requested by @dav3r

It is a bit of a weird case, where the current word doesn't match
any candidate.
@felddy
Add better suggestions for name completions
b741068
@felddy
Simplify logic for instance name selection
4e7815a
@felddy
Add support for AWSSH_USER env variable
b566467
@felddy
Add profile filter environment variable
413e271
@felddy
Document supported environment variables
8fd5c5f
@felddy
Handle instances that do no have tags
bc3e92f
@felddy
Add CodeQL shield and remove broken ones
f9f7f96
@felddy
Rename project to awssh
16d87a7
@felddy
Merge branch 'develop' of github.com:cisagov/ssm-ssh into first-commits
58e4f89
@felddy
Rename files and references to match project
3a57e1e
@felddy
Appease mypy
d8f41b9
@felddy
Make subprocess helper function private
12f6015
@felddy
Remove unsupported Python versions
bfe7386
@felddy
Add missing docstrings and refactor types
85b6637
@felddy
Normalize and correct quoting of numerics
6e99d90
@felddy
Document subprocess security for bandit
cc40fdc
@dav3r
Merge branch 'develop' into first-commits
ecf2a9b
@dav3r
Cleanup from isort hook
c841c6d
@dav3r
Add --no-ssh flag to open an SSM shell session without using ssh
52e8516
@dav3r
Remove f-strings from logging messages
f3b9c99
@dav3r
Clean up usage text
163ae4f
@dav3r
Add typing libraries to satisfy mypy checks
19b7627
@dav3r
Ignore type checking for schema module and add an explanatory comment
5bd4c85
@dav3r
Add more details to README documentation
c05662f
@dav3r
Update old references to renamed module
1048916
@dav3r
Set correct python version requirement
3d60ded 
Also, update keywords from skeleton template and delete unused package_data line
@dav3r
Explicitly call sys.exit after SchemaError
e2bd4ba
mcdonnnj
mcdonnnj requested changes Mar 11, 2022
View reviewed changes
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Strong work from all involved. I mostly have some housekeeping change requests.

src/awssh/__init__.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
dav3r
dav3r commented Mar 11, 2022
View reviewed changes
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
dav3r
dav3r commented Mar 11, 2022
View reviewed changes
src/awssh/autocompleter.py Outdated Show resolved Hide resolved
dav3r and others added 4 commits March 11, 2022 16:12
@dav3r @mcdonnnj
Make main() return None
d0654de 
This is in line with our standard of only specifying a return code if there is an error.

Co-authored-by: Nick <[email protected]>
@dav3r @mcdonnnj
Prefer console to bash formatting for examples
bd8fc15 
Co-authored-by: Nick <[email protected]>
@dav3r @mcdonnnj
Close log file if it exists
c525d1f 
Co-Authored-By: Nick <[email protected]>
@dav3r @mcdonnnj
Make ParsedState a data class
971b187 
This enables cleaner class initialization code.

Co-Authored-By: Nick <[email protected]>
@dav3r dav3r requested review from mcdonnnj and jsf9k March 14, 2022 19:07
jsf9k
jsf9k approved these changes Mar 14, 2022
View reviewed changes
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

Approval intensifies

mcdonnnj
mcdonnnj requested changes Mar 14, 2022
View reviewed changes
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Thanks for your continued work on getting this out the door! I had some mypy related items that I missed on my initial review I would like resolved.

setup.py Outdated Show resolved Hide resolved
src/awssh/awssh.py Show resolved Hide resolved
@dav3r dav3r mentioned this pull request Mar 16, 2022
Open
1 task
dav3r and others added 2 commits March 16, 2022 15:10
@dav3r @mcdonnnj
Override default mypy flags and add type-checking dependencies
2467d06 
We override the default flags in order to exclude the --ignore-missing-imports flag.  As a result of that, we add some additional dependencies needed to successfully do type-checking.

Co-Authored-By: Nick <[email protected]>
@dav3r @mcdonnnj
Move packages related to type-checking to extras_require["test"] section
3db8629 
Since these packages are not required for the tool to function, they do not belong in the install_requires section.

Co-Authored-By: Nick <[email protected]>
@dav3r dav3r requested review from mcdonnnj and jsf9k March 16, 2022 21:16
mcdonnnj
mcdonnnj approved these changes Mar 24, 2022
View reviewed changes
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Just one more mypy dependency but otherwise LGTM ✔ Thanks for your work getting this over the finish line 💪💪💪

setup.py Show resolved Hide resolved
@dav3r dav3r merged commit 0947c7c into develop Mar 24, 2022
@dav3r dav3r deleted the first-commits branch March 24, 2022 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcdonnnj mcdonnnj mcdonnnj approved these changes

@jsf9k jsf9k jsf9k approved these changes

@felddy felddy felddy approved these changes

Assignees

@dav3r dav3r

Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dav3r @felddy @jsf9k @mcdonnnj