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(gitlab): Consistent urlencoding and call source specific hostType for gitlab (changelogs, packages, tags, releases) #13577

Closed
wants to merge 3 commits into from

Conversation

rv2673
Copy link
Contributor

@rv2673 rv2673 commented Jan 16, 2022

Changes:

  • Make the repoId url encoding consistent in changelog consistent with the encoding used in datasources.
  • Add hostType ('gitlab-changelog') for the changelog calls done to gitlab so they can have specific hostRules applied to them.
  • Add hostType for gitlab-packages datasource(just like gitlab-tags and gitlab-releases) calls.
  • Add 'gitlab-changelog' to the gitlab fallback types('gitlab-packages' was already there)

Context:

Making it possible to apply hostRules to changelogs and gitlab-packages calls.

Documentation (please check one with an [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:

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

@rv2673 rv2673 changed the title Fix(gitlab): Consistent urlencoding and call source specific hostType for gitlab (changelogs, packages, tags, releases) fix(gitlab): Consistent urlencoding and call source specific hostType for gitlab (changelogs, packages, tags, releases) Jan 16, 2022
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

We should probably do this for GitHub in this PR too.

@rarkins WDYT?

Otherwise LGTM

@rarkins
Copy link
Collaborator

rarkins commented Jan 17, 2022

Is there any blocking reason why we couldn't use gitlab-tags as the datasource where applicable? I'm not too keen to add a new hostType of gitlab-packages

@rv2673
Copy link
Contributor Author

rv2673 commented Jan 17, 2022

@viceice
Wanted to have it first reviewed before expanding scope. I did a quick search in the code where hostType are added or not added.
Currently all datasources except, gitlab-packages and bitbucket-tags add their id as hostType. All presets, platform and changelog calls use the platform id as hostType.
I have locally added tests for the fallbacks and hostTypes for all datasources where it was missing and added a new hostType for changelog calls for both gitlab and github(currently the only ones where it is implemented I believe).
If @rarkins and you are ok with it I can add the changes to this PR and rename it "Adding missing hostType to datasources and adding new hostType for changelog calls"

@rarkins I only added it to the existing datasource type for gitlab-packages, it was already considered an existing hostType in the fallback types, the packages datasource just didn't add it to its calls like all other datasources add their id as hostype(except gitlab-packages and bitbucket-tags)

For changelogs I did add a new hostType, since it would be the least invasive. For gitlab changelog calls could reuse the hostType of gitlab-tags and gitlab-releases, but it also does a call to tree node api, which currently has not hostType.
I am not sure if hostType currently can be overriden per call on the same http object, since the changelog source implementation uses a global GitlabHttp object for all its methods.

@viceice
Copy link
Member

viceice commented Jan 17, 2022

Ok, let's do it in smaller separate PR's.

@@ -19,4 +19,5 @@ export const GITLAB_API_USING_HOST_TYPES = [
'gitlab-releases',
'gitlab-tags',
'gitlab-packages',
'gitlab-changelog',
Copy link
Collaborator

Choose a reason for hiding this comment

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

@viceice are you sure you like this idea?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know a better solution yet. 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have looked for alternative solutions to solve the problem.

The problem is that it is currently impossible to apply hostRules to calls made by the changelog modules. This due to the fact that they use the platform identifier as hostType, which cannot have any rules applied to them since the automatically generated host rule for the platform is always applied last, overriding the authentication set by any previous rule.

Most datasources apply their datasource id to their Http client which adds it as hostType, making it possible to set hostRules for specific datasources. That is also what gitlab-tags and gitlab-releases datasources do. The hostRules also have a fallback mechanism for sources that use Gitlab or Github as source. This makes it possible for gitlab-tags and gitlab-releases to have specific rules configured for private repositories and otherwise fallback to the platform configuration. The changelog http clients,(which are called after datasource lookups) however always use the platform identifier as hostType, which as previously mentioned cannot be overridden.

I can think of only the 3 options to make hostRules configurable for changelogs so that they don't fail when datasources are configured with their own authentication which is not applied to subsequent changelog lookups.

  1. Moving automatically generated platform rule to the beginning, making it possible to override automatically generated rule for call using the platform identifier. Making it possible to override the configured renovate platform token. However there is no ways to differentiate between preset, platform and changelog calls.

    • Which might also break configurations where there are silently ignored hostRules for the platformId hostType.
  2. In the case of the Gitlab changelog retrieval, it does 3 types of calls, tag lookup, checking for existence of changelog md(tree lookup) and looking up releases.

    • tags lookup could use the same hostType that gitlab-tags datsource uses
    • release lookup could use the same hostType that gitlab-releases datsource uses
    • the tree api lookup call is done also done in the platform module with platform identifier as hostType, but that was the problem, so new identifier needs to be introduced, since both tags and releases types calls don't make sense.
  3. Treat the changelog module just like a gitlab-releases or gitlab-tags datasource with its own identifier used as hostType, making it possible to differentiate from other platform calls and apply rules to them, and adding it to the fallback list. (The solution above, which I personally like best, but I am biased.)

@rv2673
Copy link
Contributor Author

rv2673 commented Jan 20, 2022

@rv2673 rv2673 closed this Jan 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Differing behaviour retrieving gitlab-tags and releases between datasource and changelog
3 participants