athul / waka-readme

Wakatime Weekly Metrics on your Profile Readme.

https://github.com/athul

MIT License

1.56k stars 301 forks source link

Bug: Failed to build container image #137

Closed AlisaAkiron closed 1 year ago

AlisaAkiron commented 1 year ago

Hi, just received an GitHub Actions workflow failed email and it is this job failed.

Failed Action: https://github.com/LiamSho/LiamSho/actions/runs/5594196665

This workflow use athul/waka-readme@master. Set it to use athul/waka-readme@v0.2.1 works well.

It seems like the docker container was failed to build.

I recommend to add a container build test workflow in this project.

yozachar commented 1 year ago

Tracking upstream https://github.com/pypa/pip/issues/9644

AdebayoEmmanuel commented 1 year ago

From my understanding of the incident, I have put the following together, hope it helps resolve the bug:

Problem statement:

This morning, I woke up to disturbing news: [AdebayoEmmanuel/AdebayoEmmanuel] Run failed: Waka Readme - main (db943f7) - My Wakatime Readme action failed!

run failed email

Troubleshooting done + Findings + Recommendation:

Looking through the GitHub action logs, I observed the exact step of the workflow that was failing:

error log

The error message led me to review line 184 of athul/waka-readme’s requirements.txt (I am using athul/waka-readme@v0.2.2 workflow in my yaml. N/B : issue is persistent with both this version and if you use @master). I found out that this is the line that requires pygithub==1.59.0

line 184 trace

I checked PyGithub’s requirements.txt file and I observed that the requirements were written without pinning their versions using ==.

Recommendation + RCA:

From the error message on the workflow logs, we understand that while declaring the requirements In --require-hashes mode, all requirements must have their versions pinned with == . Whereas, in PyGithub’s requirements.txt file, the requirements were not pinned as required. E.g, pyjwt[crypto]>=2.4.0.

This whole situation resulted to errors that made Docker build to fail with exit code 1 and overall broke the scheduled job.

Let’s try:

Waka-readme can should review how pygithub is included into the requirements.txt (decide if it is necessary to require hash - this looks like a security consideration to me and may be important, so if we have to use require hash mode, then we may have to work together with pygithub to apply the fix in recommendation 2)
PyGithub can update their requirements.txt to hard pin the requirements versions.

AdebayoEmmanuel commented 1 year ago

From my understanding of the incident, I have put the following together, hope it helps resolve the bug:

Problem statement:

This morning, I woke up to disturbing news: [AdebayoEmmanuel/AdebayoEmmanuel] Run failed: Waka Readme - main (db943f7) - My Wakatime Readme action failed!

Troubleshooting done + Findings + Recommendation:

Looking through the GitHub action logs, I observed the exact step of the workflow that was failing:

The error message led me to review line 184 of athul/waka-readme’s requirements.txt (I am using athul/waka-readme@v0.2.2 workflow in my yaml. N/B : issue is persistent with both this version and if you use @master). I found out that this is the line that requires pygithub==1.59.0

I checked PyGithub’s requirements.txt file and I observed that the requirements were written without pinning their versions using ==.

Recommendation + RCA:

From the error message on the workflow logs, we understand that while declaring the requirements In --require-hashes mode, all requirements must have their versions pinned with == . Whereas, in PyGithub’s requirements.txt file, the requirements were not pinned as required. E.g, pyjwt[crypto]>=2.4.0.

This whole situation resulted to errors that made Docker build to fail with exit code 1 and overall broke the scheduled job.

Let’s try:

Waka-readme can should review how pygithub is included into the requirements.txt (decide if it is necessary to require hash - this looks like a security consideration to me and may be important, so if we have to use require hash mode, then we may have to work together with pygithub to apply the fix in recommendation 2)

PyGithub can update their requirements.txt to hard pin the requirements versions.

@joe733

AdebayoEmmanuel commented 1 year ago

Replacing action athul/waka-readme@master to athul/waka-readme@v0.2.1 as recommended here #136 appears to be a working workaround. My workflow ran successfully using @v0.2.1. We can explore the options I recommended above for permanent fix on the master and the latest release @v0.2.2.

yozachar commented 1 year ago

Hi @AdebayoEmmanuel, thanks for the analysis and recommendation. PyGithub as of 42c3b47 does not use pyproject.toml for dependency specification, we do. We then generate requirements.txt just for CI. Speaking of which @LiamSho, I'll add container build test workflow, asap.

In any case PyGithub has nothing to do with the container build failure. It is pip, which is at fault here. Refer: https://github.com/pypa/pip/issues/9644#issue-813456623

Workarounds:

I could generate requirements.txt without hashes, but then compromise on secure install.
Or use --no-deps , but that won't work in this scenario.
pip now is capable of reading from pyproject.toml directly. But pip install . won't accept --require-hashes, which then is equivalent to option 1.

I'll be working on option 3, along container build tests. Feel free to comment your suggestions below.

yozachar commented 1 year ago

This workflow use athul/waka-readme@master. Set it to use athul/waka-readme@v0.2.1 works well.

Switch back to athul/waka-readme@master to receive patch #138.

AdebayoEmmanuel commented 1 year ago

@joe733 Thank you for clarifying the root cause of the incident and for your time and attention to this bug. I can now clearly see that the real origin of the issue is actually from Pip and not Pygithub.

I understand you have been able to resolve the issue now by using pyproject.toml directly without depending on requirements.txt. Good work @joe733 - I was not able to follow up immediately on this thread, but I saw and am impressed by the good work you put in to resolve this.
I am interested in learning how secure install is now being achieved without --require-hashes. You mentioned earlier that pip install won't accept --require-hashes making it equivalent to using requirements.txt without secure installs.
Your commit drew my attention to the contribution guide. I will clone the repo and would be much interested in working together on any future issues as it appears there are no currently open issues to work on at this time. If there is any other way I can contribute, please let me know.

waka-time is by far my favorite accountability partner. ❤️

AdebayoEmmanuel commented 1 year ago

Switching back. Thanks for the update.

On Thu, 20 Jul 2023 at 01:36, Jovial Joe Jayarson @.***> wrote:

This workflow use @. Set it to use @. works well.

Switch back to master to receive the patch.

— Reply to this email directly, view it on GitHub https://github.com/athul/waka-readme/issues/137#issuecomment-1642939194 or unsubscribe https://github.com/notifications/unsubscribe-auth/AWFFLTSE7E4VFYGBLJXJZ33XRB4RHBFKMF2HI4TJMJ2XIZLTS6BKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVIZDCOJSGY2DCNBUGCSG4YLNMWUWQYLTL5WGCYTFNSBKK5TBNR2WLKRSGE4TENRUGE2DKMNENZQW2ZNJNBQXGX3MMFRGK3ECUV3GC3DVMWVDEMZZG4YDKMRYGA32I3TBNVS2S2DBONPWYYLCMVWIFJLWMFWHKZNKGU3TKMBWGQZTCMBSURXGC3LFVFUGC427NRQWEZLMVRZXKYTKMVRXIX3UPFYGLLCJONZXKZKDN5WW2ZLOOSTHI33QNFRXHFUCUR2HS4DFVJZGK4DPONUXI33SPGSXMYLMOVS2SMRXHA2DCNJQGAZIFJDUPFYGLJLJONZXKZNFOZQWY5LFVIYTQMJRGAYTIOBTHGBKI5DZOBS2K3DBMJSWZJLWMFWHKZNKGIYTSMRWGQYTINBQQKSHI6LQMWSWYYLCMVWKK5TBNR2WLKRSGE4TENRUGE2DKMMCUR2HS4DFUVWGCYTFNSSXMYLMOVS2UMRTHE3TANJSHAYDPAVEOR4XAZNFNRQWEZLMUV3GC3DVMWVDKNZVGA3DIMZRGAZKO5DSNFTWOZLSUZRXEZLBORSQ . You are receiving this email because you commented on the thread.

Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

yozachar commented 1 year ago

I am interested in learning how secure install is now being achieved without --require-hashes. You mentioned earlier that pip install won't accept --require-hashes making it equivalent to using requirements.txt without secure installs.

It's pip install . not just pip install that errors on passing --require-hashes:

$ pip install --require-hashes .
Processing /home/us-er/project
ERROR: Can't verify hashes for these file:// requirements because they point to directories:
    file:///home/us-er/project

You can read the documentation here: https://pip.pypa.io/en/stable/topics/secure-installs/#secure-installs.

AdebayoEmmanuel commented 1 year ago

Oh! Right, thanks for pointing that out.

On Thu, 20 Jul 2023, 06:16 Jovial Joe Jayarson, @.***> wrote:

I am interested in learning how secure install is now being achieved without --require-hashes. You mentioned earlier that pip install won't accept --require-hashes making it equivalent to using requirements.txt without secure installs.

It's pip install . not just pip install that errors on passing --require-hashes:

$ pip install --require-hashes .Processing /home/us-er/projectERROR: Can't verify hashes for these file:// requirements because they point to directories: file:///home/us-er/project

You can read the documentation here: https://pip.pypa.io/en/stable/topics/secure-installs/#secure-installs.

— Reply to this email directly, view it on GitHub https://github.com/athul/waka-readme/issues/137#issuecomment-1643217119 or unsubscribe https://github.com/notifications/unsubscribe-auth/AWFFLTVSRDTOM2UM74FKV63XRC5MVBFKMF2HI4TJMJ2XIZLTS6BKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVIZDCOJSGY2DCNBUGCSG4YLNMWUWQYLTL5WGCYTFNSBKK5TBNR2WLKRSGE4TENRUGE2DKMNENZQW2ZNJNBQXGX3MMFRGK3ECUV3GC3DVMWVDEMZZG4YDKMRYGA32I3TBNVS2S2DBONPWYYLCMVWIFJLWMFWHKZNKGU3TKMBWGQZTCMBSURXGC3LFVFUGC427NRQWEZLMVRZXKYTKMVRXIX3UPFYGLLCJONZXKZKDN5WW2ZLOOSTHI33QNFRXHFUCUR2HS4DFVJZGK4DPONUXI33SPGSXMYLMOVS2SMRXHA2DCNJQGAZIFJDUPFYGLJLJONZXKZNFOZQWY5LFVIYTQMJRGAYTIOBTHGBKI5DZOBS2K3DBMJSWZJLWMFWHKZNKGIYTSMRWGQYTINBQQKSHI6LQMWSWYYLCMVWKK5TBNR2WLKRSGE4TENRUGE2DKMMCUR2HS4DFUVWGCYTFNSSXMYLMOVS2UMRTHE3TANJSHAYDPAVEOR4XAZNFNRQWEZLMUV3GC3DVMWVDKNZVGA3DIMZRGAZKO5DSNFTWOZLSUZRXEZLBORSQ . You are receiving this email because you commented on the thread.

Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .