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

Rewrite using 1Password JS SDK #80

Open
yafanasiev opened this issue Oct 9, 2024 · 11 comments
Open

Rewrite using 1Password JS SDK #80

yafanasiev opened this issue Oct 9, 2024 · 11 comments

Comments

@yafanasiev
Copy link

Hey! First, thanks for providing the actions of course.

Under the hood, this action installs 1Password CLI and uses it to load secrets. The downside is that installing CLI takes some time and also requires some external dependencies (e.g. unzip). Now that there is a full-fledged JS SDK, are there any plans to rewrite this action using it?

@bxb100
Copy link

bxb100 commented Nov 9, 2024

I wrote this https://github.com/bxb100/load-secrets-action , take a look 😄

@Xuanwo
Copy link

Xuanwo commented Nov 18, 2024

Hi, @edif2008. If you don't have time to maintain this action, how about inviting @bxb100 to help with the rewrite? We can assist with this action and use the pure JS/TS version instead.

@yafanasiev
Copy link
Author

I would be also willing to volunteer to help rewrite the action and maintain it further

@Xuanwo
Copy link

Xuanwo commented Nov 18, 2024

Started a discussion at https://1password-devs.slack.com/archives/C04NP76M709/p1731947485622739

Join in via https://developer.1password.com/joinslack

@edif2008
Copy link
Member

Hey all! 👋🏻
This is super cool and exciting to see. Thank you so much for taking the time to rewrite the GItHub Action using the Connect SDK and the new 1Password SDK. I've taken a quick look at it and it's interesting how it has been rewritten.

Would you be kind to open a Pull Request so that we can take a deeper look at it and review it? That would be much appreciated.

@SimonBarendse
Copy link
Member

One thing we'll want to give a closer look is the integration with the 1Password desktop app. This is supported in CLI, but not (yet) in SDKs.

For users using this action with nektos/act or similar to run actions locally as well, I believe we'd break their workflows if dropping CLI support completely before adding Auth Prompt support in SDKs.

@Xuanwo
Copy link

Xuanwo commented Nov 20, 2024

Would you be kind to open a Pull Request so that we can take a deeper look at it and review it? That would be much appreciated.

@bxb100, what do you think? I'm willing to help if you need it. @yafanasiev also wants to lend a hand, which is greatly appreciated.

For users using this action with nektos/act or similar to run actions locally as well, I believe we'd break their workflows if dropping CLI support completely before adding Auth Prompt support in SDKs.

That's why I'm requesting a v3. This change should definitely be considered a breaking change. I'm okay with having a v3 without integration with the 1Password desktop app support; we can add it once 1Password/terraform-provider-onepassword#140 (comment) is resolved.


Given that this action is not currently active, I suggest developing directly on the main branch. We can then release some RC tags for the community to try out. Finally, we can officially release version 3 of the load-secrets-action.

Additionally, according to the project's governance, I propose adding @bxb100 as a committer (or at least a reviewer initially) so the community can gradually engage in the development after the PR get merged.

I believe 1Password has a strong developer community, and we can collaborate to meet our needs. The 1Password team doesn't need to handle everything on their own; they can act as guardians to ensure this project remains trustworthy and safe. I believe this will make the entire community much healthier.

cc @edif2008 @SimonBarendse

@Xuanwo
Copy link

Xuanwo commented Nov 20, 2024

Also cc @shyim, the maintainer of https://github.com/shyim/1password-load-secrets-action, who will also be interested in this move.

@bxb100
Copy link

bxb100 commented Nov 20, 2024

@edif2008 @SimonBarendse @Xuanwo Thank you all for the discussion on the SDK rewrite. I will push a PR for review ASAP. Honestly, most of the complex logic has already been handled by 1Password’s official SDK, I’m just integrating it into the GitHub Action.

@Xuanwo
Copy link

Xuanwo commented Nov 20, 2024

Also cc @KamaranL, the maintainer of https://github.com/KamaranL/1password-load-secrets-action, who will also be interested in this move.

@Xuanwo
Copy link

Xuanwo commented Nov 20, 2024

@edif2008 @SimonBarendse @Xuanwo Thank you all for the discussion on the SDK rewrite. I will push a PR for review ASAP. Honestly, most of the complex logic has already been handled by 1Password’s official SDK, I’m just integrating it into the GitHub Action.

That's will be great. Looking forward to your PR!

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

No branches or pull requests

5 participants