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

feat(config): implement custom header field inside HostRules #26225

Merged
merged 27 commits into from
Jan 17, 2024

Conversation

hersentino
Copy link
Contributor

@hersentino hersentino commented Dec 11, 2023

Changes

Adding a 'headers' field to the 'HostRules' configuration, which will be automatically included in the HTTP request header.

Context

I'm making this change because the URL (matchHost) from which I want to retrieve the package version has an authentication system using "X-Auth-Token." Currently, there is no way to pass the token. I came across this discussion that also reports the problem. However, the proposed solution in the discussion cannot work according to the source code. This solution enables genericity

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 select 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 unit tests + ran on a real repository

@CLAassistant
Copy link

CLAassistant commented Dec 11, 2023

CLA assistant check
All committers have signed the CLA.

@hersentino hersentino force-pushed the implement-custom-headers-2 branch 2 times, most recently from ffc40b4 to 9e9d5ff Compare December 11, 2023 10:55
@hersentino hersentino changed the title Implement custom headers field inside HostRules feat(config): implement custom headers field inside HostRules Dec 11, 2023
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Needs discussion about validation and security

lib/util/http/host-rules.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Should be a new globalOnly config option allowedHeaders in lib/config/options

@hersentino hersentino force-pushed the implement-custom-headers-2 branch 3 times, most recently from 3c476e9 to 23e79b9 Compare December 18, 2023 16:32
@hersentino hersentino closed this Dec 19, 2023
@hersentino hersentino reopened this Dec 19, 2023
@hersentino
Copy link
Contributor Author

@rarkins, I've made the changes. Can you confirm if it's okay with you? Sorry for the accidental close/open; it was a misclick

Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Please make use of the anyMatchRegexOrMinimatch() function and do not convert into RegExp internally

lib/types/host-rules.ts Outdated Show resolved Hide resolved
docs/usage/self-hosted-configuration.md Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Please also check that Renovate validates (enforces) that the config (header values must be a string)

docs/usage/self-hosted-configuration.md Outdated Show resolved Hide resolved
docs/usage/self-hosted-configuration.md Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/types/host-rules.ts Outdated Show resolved Hide resolved
lib/util/http/host-rules.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Actually, I think it's essential that we validate repo config for allowed headers and raise a config error if any don't match (instead of waiting until runtime to log a warning)

docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/configuration-options.md Outdated Show resolved Hide resolved
docs/usage/self-hosted-configuration.md Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/validation.ts Outdated Show resolved Hide resolved
lib/config/validation.ts Outdated Show resolved Hide resolved
@hersentino
Copy link
Contributor Author

I added tests to resolve code coverage issues. Could you please rerun the pipeline ?

lib/config/validation.ts Outdated Show resolved Hide resolved
lib/config/validation.ts Outdated Show resolved Hide resolved
lib/util/http/host-rules.ts Outdated Show resolved Hide resolved
lib/util/http/host-rules.ts Outdated Show resolved Hide resolved
lib/util/http/host-rules.ts Outdated Show resolved Hide resolved
@hersentino
Copy link
Contributor Author

@rarkins Could you please rerun the pipeline ?

lib/config/validation.spec.ts Outdated Show resolved Hide resolved
lib/config/validation.spec.ts Outdated Show resolved Hide resolved
rarkins
rarkins previously approved these changes Jan 16, 2024
viceice
viceice previously approved these changes Jan 16, 2024
@viceice viceice added this pull request to the merge queue Jan 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2024
@rarkins rarkins added this pull request to the merge queue Jan 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2024
@rarkins rarkins added this pull request to the merge queue Jan 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2024
@rarkins
Copy link
Collaborator

rarkins commented Jan 17, 2024

@hersentino I think you need to merge from main branch again before this can be successfully merged

@hersentino hersentino dismissed stale reviews from rarkins and viceice via 10dba48 January 17, 2024 08:45
@viceice viceice added this pull request to the merge queue Jan 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 17, 2024
@viceice viceice added this pull request to the merge queue Jan 17, 2024
Merged via the queue into renovatebot:main with commit db9d485 Jan 17, 2024
36 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.139.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 22, 2024
zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 24, 2024
zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 24, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto:no-done-comments Don't say "Done" or "Please review" - request a review instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to set headers using hostRules
7 participants