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

Bug: Failed to build container image #137

Closed
AlisaAkiron opened this issue Jul 19, 2023 · 10 comments · Fixed by #138
Closed

Bug: Failed to build container image #137

AlisaAkiron opened this issue Jul 19, 2023 · 10 comments · Fixed by #138
Assignees
Labels
bug Something isn't working confirmed Bug is confirmed duplicate This issue or pull request already exists upstream Issue is in the upstream code base

Comments

@AlisaAkiron
Copy link

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/[email protected] 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
Copy link
Collaborator

Tracking upstream pypa/pip#9644

@yozachar yozachar added bug Something isn't working to confirm Bug to be confirmed upstream Issue is in the upstream code base labels Jul 19, 2023
@yozachar yozachar added the duplicate This issue or pull request already exists label Jul 19, 2023
@AdebayoEmmanuel
Copy link

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:

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

error log

  1. The error message led me to review line 184 of athul/waka-readme’s requirements.txt (I am using athul/[email protected] 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

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

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
Copy link

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:

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

error log

  1. The error message led me to review line 184 of athul/waka-readme’s requirements.txt (I am using athul/[email protected] 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

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

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
Copy link

Replacing action athul/waka-readme@master to athul/[email protected] 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
Copy link
Collaborator

yozachar commented Jul 19, 2023

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: pypa/pip#9644 (comment)


Workarounds:

  1. I could generate requirements.txt without hashes, but then compromise on secure install.
  2. Or use --no-deps , but that won't work in this scenario.
  3. 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 added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
@yozachar yozachar added confirmed Bug is confirmed and removed to confirm Bug to be confirmed labels Jul 19, 2023
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
yozachar added a commit to yozachar/waka-readme that referenced this issue Jul 19, 2023
- upstream issue: pypa/pip#9644
- runs tests within a container, uses the same `dockerfile`
- ignores `pdm.lock`, removes `requirement.txt`
- update manual contribution steps
- bumps project version

**Related Items**

_Issues_

- Closes athul#137
@yozachar yozachar self-assigned this Jul 19, 2023
@yozachar
Copy link
Collaborator

yozachar commented Jul 20, 2023

This workflow use athul/waka-readme@master. Set it to use athul/[email protected] works well.

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

@AdebayoEmmanuel
Copy link

AdebayoEmmanuel commented Jul 20, 2023

@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
Copy link

AdebayoEmmanuel commented Jul 20, 2023 via email

@yozachar
Copy link
Collaborator

  • 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
Copy link

AdebayoEmmanuel commented Jul 20, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed Bug is confirmed duplicate This issue or pull request already exists upstream Issue is in the upstream code base
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants