-
Notifications
You must be signed in to change notification settings - Fork 38
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
add alternate azure git repo urls to credentials for submodules #372
Conversation
input.Credentials = append(input.Credentials, model.Credential{ | ||
"type": "git_source", | ||
"host": "dev.azure.com", | ||
"username": "x-access-token", | ||
"password": "$LOCAL_AZURE_ACCESS_TOKEN", | ||
}) | ||
if len(input.Job.CredentialsMetadata) > 0 { | ||
// Add the metadata since the next section will be skipped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is incorrect; the next section is not skipped and the CredentialsMetadata
is correctly updated. The test has been modified to verify this.
for _, cred := range input.Credentials { | ||
if cred["type"] != "git_source" && cred["type"] != "nuget_feed" { | ||
t.Errorf("expected credentials to be either git_source or nuget_feed, got %s", cred["type"]) | ||
} | ||
} | ||
|
||
// Validate NuGet feeds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior is unchanged, but was not explicitly tested.
expectedGitCredentials := []string{ | ||
"dev.azure.com|org|$LOCAL_AZURE_ACCESS_TOKEN", | ||
"dev.azure.com|x-access-token|$LOCAL_AZURE_ACCESS_TOKEN", | ||
"org.visualstudio.com|x-access-token|$LOCAL_AZURE_ACCESS_TOKEN", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new URL that's added to the credentials.
actualCredentialsMetadataHosts = append(actualCredentialsMetadataHosts, host) | ||
} | ||
|
||
expectedGitCredentalsMetadataHosts := []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what the unnecessary deleted code did, just adding explicit verification here.
When a job is started with the
azure
provider,git_source
credentials are provided for thedev.azure.com
domain and that URL is ultimately used to clone the repo.If a repo has a submodule and specifies the URL with the
dev.azure.com
format, everything is OK because the credentials are properly injected by the proxy, but there is an alternate format that's not handled:org.visualstudio.com/DefaultCollection/project/_git/repo
and if a submodule clone is attempted with that URL, it's not detected as being functionally the same as the other URL and a401
is returned, possibly making the job fail due to missing sources.This PR adds the alternate
org.visualstudio.com
credentials.A manual test was performed, first with the old behavior ensuring the
401
and again with the new behavior ensuring a correct200
when cloning the submodule.