Skip to content

fix: detect lockfile changes in GitHub PRs#73

Merged
maxrake merged 1 commit into
mainfrom
detect_lockfile_changes
Jul 1, 2022
Merged

fix: detect lockfile changes in GitHub PRs#73
maxrake merged 1 commit into
mainfrom
detect_lockfile_changes

Conversation

@maxrake
Copy link
Copy Markdown
Contributor

@maxrake maxrake commented Jul 1, 2022

Adding the $GITHUB_WORKSPACE as a safe directory in git is necessary to ensure the GitHub Container action will function properly with the git repository checked out on the runner. However, this recommended workaround was applied too late in the class initialization. The parent class sets the instance predicate attribute for is_lockfile_changed in the __init__ method. Therefore, the workaround needs to be applied in the child class prior to this.

Closes #72

Checklist

  • Does this PR have an associated issue (i.e., closes #<issueNum> in description above)?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?

Testing was performed manually, with a Docker image created from the changes in this PR and pushed to maxrake/phylum-ci:GHA on Docker Hub. This test image was used in a throwaway branch for the action, phylum-dev/phylum-analyze-pr-action@fix_access, and ultimately used in the test runs to show the error is no longer present and the action correctly identifies when the lockfile has not changed:

Run phylum-dev/phylum-analyze-pr-action@fix_access
/usr/bin/docker run --name maxrakephylumciGHA_a21cdc --label 4cd98f --workdir /github/workspace --rm -e INPUT_PHYLUM_TOKEN -e INPUT_CMD -e INPUT_GITHUB_TOKEN -e GITHUB_TOKEN -e PHYLUM_API_KEY -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RETENTION_DAYS -e GITHUB_RUN_ATTEMPT -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_REF_NAME -e GITHUB_REF_PROTECTED -e GITHUB_REF_TYPE -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e GITHUB_ACTION_REPOSITORY -e GITHUB_ACTION_REF -e GITHUB_PATH -e GITHUB_ENV -e GITHUB_STEP_SUMMARY -e RUNNER_OS -e RUNNER_ARCH -e RUNNER_NAME -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true --entrypoint "entrypoint.sh" -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/mytestrepo128/mytestrepo128":"/github/workspace" maxrake/phylum-ci:GHA  "phylum-ci -u [6](https://github.com/maxrake/mytestrepo128/runs/7154881200?check_suite_focus=true#step:4:7)0 -m 60 -e 60 -c 60 -o 60"
 [+] CI environment detected: GitHub Actions
 [+] Confirming pre-requisites ...
 [+] Existing `.phylum_project` file found at: /github/workspace/.phylum_project
 [+] `git` binary found on the PATH
 [+] All pre-requisites met
 [+] GITHUB_HEAD_REF: fix_bug
 [+] GITHUB_BASE_REF: main
 [+] PR base SHA: c46fbdafca2c8c5db40[7](https://github.com/maxrake/mytestrepo128/runs/7154881200?check_suite_focus=true#step:4:8)b56b7[8](https://github.com/maxrake/mytestrepo128/runs/7154881200?check_suite_focus=true#step:4:9)2b[9](https://github.com/maxrake/mytestrepo128/runs/7154881200?check_suite_focus=true#step:4:10)92b91f64ac3
 [+] lockfile in use: /github/workspace/requirements.txt
 [+] The lockfile has not changed. Nothing to do.

Adding the $GITHUB_WORKSPACE as a safe directory in `git` is necessary to ensure the GitHub Container action will function properly with the git repository checked out on the runner. However, the recommended workaround was applied too late in the class initialization. The parent class sets the instance predicate attribute for `is_lockfile_changed` in the `__init__` method. Therefore, the workaround needs to be applied in the child class *prior* to this.

Closes #72
@maxrake maxrake requested a review from a team as a code owner July 1, 2022 19:39
@maxrake maxrake requested a review from andreaphylum July 1, 2022 19:39
@maxrake maxrake self-assigned this Jul 1, 2022
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 1, 2022

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and did not identify any issues.

View this project in the Phylum UI

@maxrake
Copy link
Copy Markdown
Contributor Author

maxrake commented Jul 1, 2022

^ This comment is what we don't want. This PR did not change the project lockfile and so Phylum dependency analysis should not have even been performed.

Swapping reviewers in an attempt to wrap this up before I go on leave...

@maxrake maxrake requested review from kylewillmon and removed request for andreaphylum July 1, 2022 19:40
Copy link
Copy Markdown
Contributor

@kylewillmon kylewillmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find

@maxrake maxrake merged commit c119a4a into main Jul 1, 2022
@maxrake maxrake deleted the detect_lockfile_changes branch July 1, 2022 19:45
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

Successfully merging this pull request may close these issues.

GitHub PRs always report lockfile as changed

2 participants