Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Snowflake Keypair App #2537

Merged
merged 11 commits into from
Nov 12, 2021
Merged

Snowflake Keypair App #2537

merged 11 commits into from
Nov 12, 2021

Conversation

evantahler
Copy link
Member

@evantahler evantahler commented Nov 10, 2021

Creates a new snowflake-keypair app to allow connecting to Snowflake databases with keypair authentication rather than password authentication.

  • We still assume that password-based authentication is most common. To that end, the existing password-based app is still called snowflake and this new one is snowflake-keypair
  • Providing the private key in the property format is tricky - notes on how to do so are in the README for this plugin

This PR also makes the selection process better when adding a new App, Source, or Destination. If there is more than one option for the Plugin or App you selected, you'll be taken to a chooser page. If not, we'll make the item for you and take you to the edit page directly

apps

Closes #2532

Checklists

Development

  • Application changes have been tested appropriately

Impact

  • Code follows company security practices and guidelines
  • Security impact of change has been considered
  • Performance impact of change has been considered
  • Possible migration needs considered (model migrations, config migrations, etc.)

Please explain any security, performance, migration, or other impacts if relevant:

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached where applicable.
  • Relevant tags have been added to the PR (bug, enhancement, internal, etc.)

plugins/@grouparoo/snowflake/README.md Show resolved Hide resolved
Comment on lines 33 to 42
export default function Page(props) {
const {
errorHandler,
apps,
plugins,
availablePlugins,
installedPlugins,
}: {
errorHandler: ErrorHandler;
apps: Actions.AppOptions["types"];
plugins: Actions.PluginsAvailableList["plugins"];
installedPlugins: Actions.AppOptions["plugins"];
availablePlugins: Actions.PluginsAvailableList["plugins"];
} = props;
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this change helps clarify what's happening on this page - we actually listing plugins that provide apps (some installed, some that could be installed). We aren't listing apps.

core/src/actions/properties.ts Show resolved Hide resolved
core/bin/dev Show resolved Hide resolved
@evantahler evantahler added the enhancement New feature or request label Nov 11, 2021
Copy link
Member

@pedroslopez pedroslopez left a comment

Choose a reason for hiding this comment

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

Nice! Adding new apps with different options seems pretty smooth!

One thing that I kept thinking about when going through this is the fact that we have two actions that return available plugins, and they are oftentimes combined: plugins:available:list and now app:options. Not exactly sure if there's anything to do about it yet since they do return different things for each plugin, but maybe something to keep an eye out for.

On the Snowflake side: do we need to add some tests for this new app auth method?

core/src/actions/apps.ts Show resolved Hide resolved
plugins/@grouparoo/snowflake/README.md Show resolved Hide resolved
ui/ui-components/pages/app/new/[...plugin].tsx Outdated Show resolved Hide resolved
ui/ui-components/pages/app/new/[...plugin].tsx Outdated Show resolved Hide resolved
@evantahler evantahler mentioned this pull request Nov 12, 2021
7 tasks
@evantahler
Copy link
Member Author

#2543 will be merged here

@evantahler evantahler marked this pull request as ready for review November 12, 2021 16:20
* App and Plugin Action Cleanup

* update notifier test and types

* UI cleanup

* Update ui/ui-components/components/AppSelectorList.tsx

Co-authored-by: Pedro S. Lopez <pedro.lopez@grouparoo.com>

Co-authored-by: Pedro S. Lopez <pedro.lopez@grouparoo.com>
Copy link
Member

@pedroslopez pedroslopez left a comment

Choose a reason for hiding this comment

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

🎉

ui/ui-components/pages/app/new/[...plugin].tsx Outdated Show resolved Hide resolved
@evantahler evantahler merged commit abda13d into main Nov 12, 2021
@evantahler evantahler deleted the 180237226-snowflake-pem-app branch November 12, 2021 21:32
edmundito pushed a commit that referenced this pull request Nov 16, 2021
* Snowflake Keypair App

* fixup bad types

* fix dev GROUPAROO_RUN_MODE ovveride

* AppSlector can make next item directly

* cleanup + new headers

* fix test

* log

* more saftey around preview

* remove unused

* App and Plugin Action Cleanup (#2543)

* App and Plugin Action Cleanup

* update notifier test and types

* UI cleanup

* Update ui/ui-components/components/AppSelectorList.tsx

Co-authored-by: Pedro S. Lopez <pedro.lopez@grouparoo.com>

Co-authored-by: Pedro S. Lopez <pedro.lopez@grouparoo.com>

* remove extra router arg

Co-authored-by: Pedro S. Lopez <pedro.lopez@grouparoo.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snowflake Key Pair Authentication
2 participants