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.

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

Support host wildcards in Shell app #601

Merged
merged 16 commits into from
Aug 4, 2020
Merged

Conversation

msquee
Copy link
Contributor

@msquee msquee commented Jul 20, 2020

This PR adds support for using wildcards such as *.ten.osc.edu in the SSH allowlist. Closes #599

Example:
Set ENV variable OOD_SSHHOST_ALLOWLIST to owens.osc.edu:*.ten.osc.edu then you can SSH into any host that matches *.ten.osc.edu from the Shell app without adding every FQDN manually.

  • Adds two helper functions: hostInAllowList() and generateWildcardGlob()
  • Includes tests for both helper functions
  • Tests have 100% code coverage

You can run tests with code coverage tests with command yarn jest --verbose --coverage or run test suites with yarn test

@msquee msquee requested a review from ericfranz July 20, 2020 23:21
@msquee msquee changed the title Support host wildcards in shell Support host wildcards in Shell app Jul 20, 2020
@msquee msquee added the ready label Jul 21, 2020
@msquee msquee self-assigned this Jul 21, 2020
@msquee
Copy link
Contributor Author

msquee commented Jul 21, 2020

We really should start moving functions into a utilities module so we can write tests. The current architecture of the Shell app doesn't make tests easy. I'll talk with @samirmansour and @matthu017 offline so everyone is on the same page about it.

Copy link
Contributor

@ericfranz ericfranz left a comment

Choose a reason for hiding this comment

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

Discussed on Slack simpler solution that can be backed by tests.

@ericfranz ericfranz removed the ready label Jul 22, 2020
apps/shell/app.js Outdated Show resolved Hide resolved
@msquee msquee requested a review from ericfranz July 23, 2020 19:54
@msquee msquee added the ready label Jul 23, 2020
apps/shell/app.js Outdated Show resolved Hide resolved
apps/shell/app.js Outdated Show resolved Hide resolved
@msquee msquee requested a review from ericfranz August 3, 2020 05:25
@johrstrom johrstrom self-requested a review August 3, 2020 15:09
johrstrom
johrstrom previously approved these changes Aug 3, 2020
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

This looks ready to me. As a follow up, because this adds tests, we need enable those tests in our rake tasks.

@ericfranz ericfranz merged commit e45aba5 into master Aug 4, 2020
@ericfranz ericfranz deleted the feature/shell-wildcards branch August 4, 2020 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allowlist to support wildcards
3 participants