rails-lambda / crypteia

🧱🔐 Rust Lambda Extension for any Runtime/Container to preload Secure Environment Variables!
https://lamby.cloud
MIT License
66 stars 7 forks source link

Add should fail if param not found logic #45

Closed jeremiahlukus closed 5 months ago

jeremiahlukus commented 6 months ago

Since I'm am unable to test locally I am unable to say this works. I am assuming throwing an error in get_envs will cause a deploy to fail.

If you enable the test workflow to run I will get the test to pass.

jeremiahlukus commented 6 months ago

https://github.com/rails-lambda/crypteia/issues/44

brcarp commented 6 months ago

@metaskills Do we have the tests disabled again due to configuration issues?

brcarp commented 6 months ago

@jeremiahlukus If you'd like, you should be able to update the test.yml workflow in your branch (or another branch), update the trigger from [push] to [push, workflow_dispatch], and then it can be triggered manually (locally with gh if not in the GUI). I think GitHub will allow you to interact with workflow changes like this from an unmerged feature branch, but I'm not 100% certain. If it does however, you should be able to see if the tests are healthy. If you have the time to dig in further and see what might be needed to get them passing, all the better!

jeremiahlukus commented 6 months ago

I add workflow_dispatch to the test.yaml however i think it needs to be merged in the default branch (main) before it picks up the changes.

I can work on getting all tests to pass once i can run them ha.

All build commands here https://github.com/rails-lambda/crypteia?tab=readme-ov-file#dev-container-cli are failing for me using a mac

EDIT:

I spend a hour or so on it but unable to get the tests to load. If you can get the tests to load on a mac I will spend time on fixing failing ones.

jeremiahlukus commented 6 months ago
Screenshot 2024-06-03 at 9 05 34 PM

Using codespaces

jeremiahlukus commented 6 months ago
Screenshot 2024-06-03 at 9 11 10 PM

Using vscode

metaskills commented 5 months ago

@jeremiahlukus This is a great change! It should definitely be a major 2.0 change/addition.

Thank you so much for attempting to get the tests working. When I first developed this package, I found it hard to get consistent results, even with Dev Containers, and I think you are seeing the same. If we can not build for test, we can not build the distribution packages which even require distinct architectures.

Unfortunately I do not have the time to get this all working. I would be happy to add you as a contributor?

jeremiahlukus commented 5 months ago

Please do! I would love to get the tests working and also get the PR feature in master ha.

It’s also good to know it’s not a local issue and things are just messed up.

If you add me I’ll branch off and get the tests working (eventually). I’ll get it working locally before I start spamming workflows.

metaskills commented 5 months ago

Done. Lemme know if that works. I'll be watching out for any support for releases, etc.

jeremiahlukus commented 5 months ago

Great, I’ll @ you when I get the tests working and able to test this feature again might be a few days but I’ll be working on it in between builds at work ha.

jeremiahlukus commented 5 months ago
Screenshot 2024-06-10 at 7 27 06 PM

fixed codespace

@metaskills mind if i merge so i can run the tests from a branch? its not letting me trigger builds

adding https://github.com/jeremiahlukus/crypteia/blob/main/.github/workflows/test.yml#L2

will let me run the tests on my branch. Im hoping now that i got it to run in codespaces the test will just work but if not ill branch and get them to work.

Dont want to merge without your blessing haha

jeremiahlukus commented 5 months ago

@metaskills all tests pass

https://github.com/rails-lambda/crypteia/actions/runs/9475123575

jeremiahlukus commented 5 months ago
Screenshot 2024-06-11 at 10 36 43 PM
metaskills commented 5 months ago

Amazing! Should we start testing a 2.0.0 preview release?

jeremiahlukus commented 5 months ago

Yes 👍 once you release I’ll test

metaskills commented 5 months ago

Cool, give me a hot minute. I'll report back in a release issue.

avinash-vllbh commented 2 months ago

@jeremiahlukus I see the main change here has been reverted from main. Do you have a sense of when 2.0 might be releasing?

jeremiahlukus commented 2 months ago

@avinash-vllbh 2.0 is released but it doesnt include this. It just has package updates and some logging.

Adding the param not found logic to this app does not have the results you would expect. You should run a script pre deploy to ensure your params are present before deploying or at least that is what i did.

What happened when i added this was a param was not found it exited then got stuck in a loop spinning up lambdas and exiting when loading params.

metaskills commented 2 months ago

I think Avi was speaking to a need where the params were present at deploy time but not on function init during times of service disruptions in AWS. Does that sound right Avi? If so, in this case would the loops @jeremiahlukus described be desirable?