-
Notifications
You must be signed in to change notification settings - Fork 790
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
Added support for environments on secrets command #95
Added support for environments on secrets command #95
Conversation
🦋 Changeset detectedLatest commit: 511c410 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Having some trouble fixing the tests as they require user input (and a confirmation prompt). Any advice on how to best approach this? Ink doesn't like when I'm feeding inputs through stdin (something in regards to rawData). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Good job on reverse engineering from the dashboard haha. I can't merge this in just yet because we also need to support wrangler v1 style environments. I'll leave this open, marked as changes requested until I figure out details for that too. I'll also figure out something for tests. Thanks very much again!
Thanks for checking it out! Reason I created this PR is because it's not working on V1 or on V2 (previewing or publishing to different environments that rely on different secrets). So not sure if that's something that you're going to be able to support. But happy to update once you figured those details out 🙂 In regards to the tests, I think we would either need to set Ink to accept rawData (of which I don't know it's implications) or we could check-out the testing-library they've build specifically for testing ink apps. But I'll keep an eye on this MR. Thanks again for taking the time to check this draft MR 😃 |
Any new regarding this one? It's only blocker for us to go with wrangler2 😅 |
sorry for the delay on this. I have a comprehensive PR up at #523. Thank you for the initiative! Closing this for now, but I will credit you in the changelog! |
All good, your PR looks to very extensive me like it's including a lot more (including tests 😅). Very kind to still give me some credits, but not sure if I've done all that much 😁 I'll keep an close eye on the repo anyways and will see if there is something I can help out with to make a sizeable contribution 👍 |
First things first, awesome stuff going on here 🚀
Super happy to be able to contribute 🙂
I ran into something while debugging my wrangler-github-action-deployment secrets locally with wrangler V2.
It seemed like the
environment
flag wasn't being passed along, so I looked at the code and saw the todo comments so I thought I'd help out 😅I've reversed engineered the endpoints by checking the dashboard while performing the same actions. I did check wrangler V1 code but couldn't make much sense out of that.
From what I've seen there are two different ways of retrieving secrets. One including variables (so non-secrets), the other one solely secrets. I've gone with the secrets only as it made more sense to me semantically but not sure which one of these endpoints you guys would prefer.
I've tested the GET / PUT / DELETE endpoints defined in this PR through insomnia and confirmed these routes to be listing the actual environment secrets.
I'll be adding tests as well just bear with me while I figure those out. I just wanted to get this out there for when something already required changes
TODO: