renovate icon indicating copy to clipboard operation
renovate copied to clipboard

feat(manager/composer): support git-tags hostRules for github.com when updating artifacts

Open etremblay opened this issue 2 years ago • 17 comments

Changes

Take 2 of #16193 considering

When generating the COMPOSER_AUTH json, handle special case for github host rule.

"hostRules": [
    {
      "hostType": "git-tags", 
      "matchHost": "github.com",
      "encrypted": {
        "token": "GitHub Personal Access Token"
      }
    },
]

Context

  • fixes #10691
  • ref #17778

We cannot use the github hostType because it break the update of the dependancy dashboard.

Also, I found that git-tags hostRules where used when doing dependancy lookup so this fix everything related to private github php repositorie

Documentation (please check one with an [x])

  • [x] I have updated the documentation, or
  • [ ] No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • [x] Code inspection only, or
  • [ ] Newly added/modified unit tests, or
  • [ ] No unit tests but ran on a real repository, or
  • [x] Both unit tests + ran on a real repository

etremblay avatar Sep 27 '22 14:09 etremblay

ping :) . artifacts failing. resolving it by manually running docker container with auth.json then pushing lock file

ps. using github app

chrillep avatar Oct 05 '22 08:10 chrillep

Do we know for sure what caused it to fail for others last time, and therefore we can be confident that we're avoiding the problem this time?

rarkins avatar Oct 05 '22 12:10 rarkins

Do we know for sure what caused it to fail for others last time, and therefore we can be confident that we're avoiding the problem this time?

Am afraid it's difficult to know for shure.

The code modified 3 things

  1. Understand hostRules with the git-tags hostType and add it in composer auth.json

Users in #17778 did not mention having such rules. For this to be an issue they would need to hava a broken token in a git-tags host rule and it's not very likely.

  1. We removed the .replace('x-access-token:', '')

This is what I fix here. What I understood form @viceice comment is that x-access-token is possibly added in Github App installations.

  1. We accept only tokens that begins with the ghp_ prefix

We added this condition because github now only accept personnal access token

Support for password authentication was removed on August 13, 2021. Please use a personal access token instead.

Maybe these users have a github installation that still accept non-personnal access token. We can remove this check to improve our chances of not doing a breaking change.

Note that if it's the case, they will probably eventually have to update their tokens for PAT enventually anyway.

etremblay avatar Oct 05 '22 13:10 etremblay

  1. We removed the .replace('x-access-token:', '')

This is what I fix here. What I understood form @viceice comment is that x-access-token is possibly added in Github App installations.

This could be the problem

  1. We accept only tokens that begins with the ghp_ prefix

We added this condition because github now only accept personnal access token

This could also be the problem. The app uses ghs_ tokens (designates an "app" token). Can we skip the check altogether?

rarkins avatar Oct 05 '22 13:10 rarkins

  1. We removed the .replace('x-access-token:', '')

This is what I fix here. What I understood form @viceice comment is that x-access-token is possibly added in Github App installations.

This could be the problem

  1. We accept only tokens that begins with the ghp_ prefix

We added this condition because github now only accept personnal access token

This could also be the problem. The app uses ghs_ tokens (designates an "app" token). Can we skip the check altogether?

Yes we can skip the check and the fix will still work for us. If you set a bad token in your hostRule composer will just fail.

I removed the check but kept the documentation saying you should user a PAT.

etremblay avatar Oct 05 '22 13:10 etremblay

what about add a debug message when it's not. a pat or app token?

viceice avatar Oct 05 '22 14:10 viceice

what about add a debug message when it's not. a pat or app token?

Good idea, I improved the personal access token logic to prefer ghp_ tokens but still use any token available if no ghp_ is set. In that case I log a debug.

https://github.com/renovatebot/renovate/pull/18004/commits/725ee9495d823e6ca21d007f9abc1a586758c38a

etremblay avatar Oct 05 '22 14:10 etremblay

BTW does having in no-api in composer.json for your private repo have any effect on this issue?

https://getcomposer.org/doc/05-repositories.md#git-alternatives

If you set the no-api key to true on a github repository it will clone the repository as it would with any other git repository instead of using the GitHub API. But unlike using the git driver directly, Composer will still attempt to use github's zip files.

chrillep avatar Oct 15 '22 15:10 chrillep

BTW does having in no-api in composer.json for your private repo have any effect on this issue?

https://getcomposer.org/doc/05-repositories.md#git-alternatives

If you set the no-api key to true on a github repository it will clone the repository as it would with any other git repository instead of using the GitHub API. But unlike using the git driver directly, Composer will still attempt to use github's zip files.

Before all this, most of our private repositories had the noapi + type=github and we had the "Support for password authentication was removed" error. Moreover renovate could not list dependencies versions.

Now all our repos use type=vcs + https url ans renovate is able to fetch dependency versions and generate pull request. The only problem is with the composer artifact update (done by composer itself not the renovate codebase). That's what this pull request fix.

etremblay avatar Oct 17 '22 13:10 etremblay

BTW does having in no-api in composer.json for your private repo have any effect on this issue? https://getcomposer.org/doc/05-repositories.md#git-alternatives

If you set the no-api key to true on a github repository it will clone the repository as it would with any other git repository instead of using the GitHub API. But unlike using the git driver directly, Composer will still attempt to use github's zip files.

Before all this, most of our private repositories had the noapi + type=github and we had the "Support for password authentication was removed" error. Moreover renovate could not list dependencies versions.

Now all our repos use type=vcs + https url ans renovate is able to fetch dependency versions and generate pull request. The only problem is with the composer artifact update (done by composer itself not the renovate codebase). That's what this pull request fix.

@etremblay alright we use vcs and no-api.

and also thanks for creating the PR.

chrillep avatar Oct 17 '22 15:10 chrillep

we probably also need to handle the new PAT type soon

https://github.blog/2022-10-18-introducing-fine-grained-personal-access-tokens-for-github/

viceice avatar Oct 19 '22 08:10 viceice

we probably also need to handle the new PAT type soon

https://github.blog/2022-10-18-introducing-fine-grained-personal-access-tokens-for-github/

Indeed. With this version of the code we would only log a debug saying it's not a PAT.

I just generated a token and it's prefixed by github_pat_. I could update this pr to handle it like a ghp

etremblay avatar Oct 19 '22 11:10 etremblay

Added @rarkins suggestion + support for Fine-grainted token + update with main branch

etremblay avatar Oct 27 '22 18:10 etremblay

Updated with master and fixed code coverage

etremblay avatar Dec 19 '22 15:12 etremblay

Any update, by when can we except this released?

haridarshan21 avatar Jan 12 '23 05:01 haridarshan21

Needs conflict resolution

rarkins avatar Jan 12 '23 06:01 rarkins

Needs conflict resolution

Fixed

etremblay avatar Jan 12 '23 13:01 etremblay

This branch should automerge the next time it (a) is up to date with main branch, and (b) has passed all tests

rarkins avatar Jan 13 '23 08:01 rarkins

:tada: This PR is included in version 34.101.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

renovate-release avatar Jan 13 '23 16:01 renovate-release