feat: replacement support for more autoreplace managers #17476
Conversation
t-kulmburg
changed the title
# Conflicts: # lib/workers/repository/update/branch/auto-replace.ts
|
@t-kulmburg Thanks for taking this over. I am going to close #13211 in favour of your PR. |
|
I just pushed a fix to enable replacing the same dependency with different other dependencies, based on the 'matchCurrentVersion' constraint. Example Mr. I am now investigating and trying to fix the problem that replacement doesnt work for docker images with digest. If you want any documentation/more tests etc. just let me know! |
|
Another update:
There are several manager that would need additional fields, to be fully supported (gitlab-ci-include needs file, helmv3 would need URL etc., all documented in my demo README). I plan to mark my PR as "Ready for Review" as soon as I have that kustomize issue resolved, |
|
Kustomize is actually replacing as expected, but the option to replace subdirectories is missing as they are not part of the 'depName': MR |
|
@t-kulmburg is it possible to replace the docker registry and the image with this change? |
I assume you mean in a Dockerfile, yes it is possible. |
|
@t-kulmburg you might need to request a new review from viceice and paging @JamieMagee for a review. |
t-kulmburg
requested review from
viceice
and removed request for
JamieMagee
| @@ -19,7 +19,8 @@ export function mergeChildConfig< | |||
| if ( | |||
| option.mergeable && | |||
| childConfig[option.name] && | |||
| parentConfig[option.name] | |||
| parentConfig[option.name] && | |||
| parentConfig[option.name] !== childConfig[option.name] | |||
There was a problem hiding this comment.
Without this I had the problem that my newName, newValue and finally depName changed from one string to an array of single characters.
| const packageName = | ||
| config.replacementName ?? config.packageName ?? config.depName; |
There was a problem hiding this comment.
this "packageName" is used to look up digests. After this change it is able to replace images with digests, as now the correct digest for the replacement is used. Without this the depName would be replaced, but the old digest would be used as the old name would be used in lookUp
![catana-corp[bot]](https://avatars.githubusercontent.com/in/198904?s=60&v=4){kind=small}
| manager: 'html', | ||
| packageFile: 'test', | ||
| }); | ||
| // TODO: fix types (#7154) |
There was a problem hiding this comment.
- You can't assign
t-kulmburgto this TODO because he or she is not a member of your GitHub organisation and
neither a collaborator of this repository.
|
Created 3 new PRs:
should I close this PR now? |
Changes
This is based on a PR from @JamieMagee about adding support for replacements for autoreplace managers
Context
After fixing building errors I added the name-replacement into the doAutoReplace function as they share most of their logic.
If you have any good ideas for better structure, I can refactor the code further.
I also crated a gitlab demo-repo containing a dependency for all managers listed in the PR of @JamieMagee.
As can be seen in the repo README and the Renovate created merge requests, more then half of them are already working with my rather minimal changes.
I also added a brief description of the problems the other managers currently have.
To test the feature even further I added unit tests for Managers that worked in the demo-repo.
My question now would be how you would like to proceed with this topic in general.
Getting the feature to work for all supported managers will probably require a lot more work,
but supporting only the currently working ones will require a detailed documentation on the current status.
I am willing to spend some more time on this in the next days/weeks, so reviews would be appreciated.
Documentation (please check one with an [x])
How I've tested my work (please tick one)
I have verified these changes via: