Persist Authentication header for authority-scope URL when repo has submodules#5511
Persist Authentication header for authority-scope URL when repo has submodules#5511sphanley wants to merge 3 commits intomicrosoft:masterfrom
Conversation
|
Hi @sphanley , Thank you for your time and for providing this solution. We encountered the same issue and were able to implement a similar workaround, which has been working well for fetching and pulling. However, we noticed there may be a potential issue when pushing, where Git could raise a “Header duplication” error. I’m not certain this will occur after applying the fix, but I thought it was worth mentioning. |
|
Azure Pipelines: 1 pipeline(s) require an authorized user to comment /azp run to run. |
|
Thanks for flagging that, @Howrang! I could imagine how there could end up being a duplicate header issue if values were stored for both |
Context
This PR should fix #4114 (which was closed for inactivity but is still an unresolved issue), which describes a longstanding issue with the Checkout task. When the
submodulestask property is set totrue, the Checkout task will strip the path and query string from the repository URL, providing an authorization header scoped to the host URL so that submodule repositories can be fetched successfully. However, when thepersistCredentialsproperty is set, the task only persists an authorization header for the full repository URL, and does not persist the additional header needed for submodules. This means that even when ostensibly persisting credentials, users are unable to perform any subsequent git operations which interact with submodules hosted in the same Azure DevOps project.Description
This PR adds logic to the section of code responsible for persisting credentials during checkout, such that if submodules are also enabled, then the same authorization header used during the initial submodule sync will also be persisted. This header is also added to the
configModificationsdictionary, which ensures that it will be cleaned up in the post-checkout task alongside the existing persisted header.Risk Assessment (Low / Medium / High)
I believe that this change is low risk, since it is persisting the same header which is already utilized during the initial checkout for projects with submodules, and the persisted value should be cleaned up by the existing code at the end of the pipeline job .
Unit Tests Added or Updated (Yes / No)
No unit tests were added or updated, since there does not appear to be any existing test coverage for the persisting of credentials.
Additional Testing Performed
As an outside contributor, I was not able to test these changes. I believe they are straightforward and should function as intended, especially since they are mostly based on existing code present in other parts of the checkout task. However, I would appreciate any help in validating the change.
Change Behind Feature Flag (No)
Since this is essentially a bugfix to close a breaking gap in current functionality, I don't think it would make sense to be put behind a feature flag.
Tech Design / Approach
This change is designed to conform as closely as possible to the existing design of the relevant code.
Documentation Changes Required (No)
Logging Added/Updated (No)
Telemetry Added/Updated (No)
Rollback Scenario and Process (Yes)
Dependency Impact Assessed and Regression Tested (No)
These changes are highly localized and should have no impact to other dependencies.