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

fix ci after migrating our pip mirror to git-lfs #699

Merged
merged 5 commits into from Jan 16, 2020
Merged

fix ci after migrating our pip mirror to git-lfs #699

merged 5 commits into from Jan 16, 2020

Conversation

redshiftzero
Copy link
Contributor

Description

@ntoll hit a CI failure in #698, that's due to the removal of the fetch-wheels target we made in freedomofpress/securedrop-builder#124

I'm putting this up for awareness, still working on another outstanding git-lfs issue inline tho..

Test Plan

  • CI should be fixed

@redshiftzero
Copy link
Contributor Author

redshiftzero commented Jan 14, 2020

This is me running git lfs fetch with GIT_TRACE=1 in a CI machine (which prints out what git-lfs is doing in more detail). The problem is on the download of the wheels step, we're POSTing to a git-lfs batch API endpoint to get the list of objects to download and getting an authentication error:

[...snip...]
18:40:50.453924 trace git-lfs: run_command: ssh -- git@github.com git-lfs-authenticate freedomofpress/securedrop-debian-packaging.git download
18:40:50.624636 trace git-lfs: api: batch 59 files
18:40:50.624868 trace git-lfs: HTTP: POST https://lfs.github.com/freedomofpress/securedrop-debian-packaging/objects/batch
18:40:50.698513 trace git-lfs: HTTP: 403
18:40:50.698616 trace git-lfs: HTTP: {"message":"Invalid deploy key for freedomofpress/securedrop-debian-packaging","documentation_url":"https://github.com/contact"}
18:40:50.698687 trace git-lfs: api error: Invalid deploy key for freedomofpress/securedrop-debian-packaging
batch response: Invalid deploy key for freedomofpress/securedrop-debian-packaging
error: failed to fetch some objects from 'https://github.com/freedomofpress/securedrop-debian-packaging.git/info/lfs'

Possibly related upstream issue git-lfs/git-lfs#1553

I'm not following why a key would be required to download objects, we are specifying download in the call to git-lfs-authenticate (the git lfs batch API documentation here seems to say the same), so I need to dig further... maybe I'm missing something obvious

@emkll emkll changed the title [wip] fix ci after migrating our pip mirror to git-lfs fix ci after migrating our pip mirror to git-lfs Jan 15, 2020
@emkll
Copy link
Contributor

emkll commented Jan 15, 2020

Thanks @redshiftzero for the debugging steps, I think this is now fixed, i pushed two commits (and rebased on lastest master to resolve conflicts) :

  • the first removes gitlfs install steps, as it is handled by the install-deps scripts [1]
  • The second fixes the git lfs clone operation, as the .gitconfig option by default in CircleCI automatically rewrites GitHub URLs with their SSH equivalents (which in turn uses CircleCI's automatically provisioned SSH deploy key for the repo). However, the ticket you referred to states that git-lfs does not work with GitHub deploy keys [2].

[1] https://github.com/freedomofpress/securedrop-debian-packaging/blob/1284cd536b8cb570e868362f769addf6241150c6/scripts/install-deps
[2]

Thanks for all your comments. As it turned out, also in our case the cause was the use of a wrong SSH key. Esp. if Git is configured to clone via HTTPS but Git LFS is configured to authenticate via SSH, many users are not aware that they in fact need an SSH key pair. So far Git LFS was quite unclear about the root cause in this case, so I've create PR #1599 to improve on this.

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Re-fetching git-lfs assets...
Updated git hooks.
Git LFS initialized.
fetch: Fetching reference refs/heads/master

Works as expected. CI is also green. Approving this and then all of us have to rebase our PRs.

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.

None yet

3 participants