From 0274bd4226758b1341930bd529ffcccea76cd96c Mon Sep 17 00:00:00 2001 From: Maxwell Weru Date: Sat, 24 Aug 2024 16:12:21 +0300 Subject: [PATCH] Improvement to credentials handling - Handle setting `index-url` instead of `url` for `python_index` registries in the server version/component. - Handle setting both `host` and `url` for `composer_repository` registries. - Better logic around building extra credentials. > This is likely the last modification before we try to move to the proxy used by `dependabot-cli`. --- extension/task/IDependabotConfig.ts | 2 +- extension/task/utils/parseConfigFile.ts | 42 ++++++++++---- extension/tests/utils/parseConfigFile.test.ts | 2 +- .../Workflow/UpdateRunnerTests.cs | 16 +++++- .../Workflow/UpdateRunner.cs | 55 +++++++++++-------- 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/extension/task/IDependabotConfig.ts b/extension/task/IDependabotConfig.ts index fe5ce3cc..714fe2c9 100644 --- a/extension/task/IDependabotConfig.ts +++ b/extension/task/IDependabotConfig.ts @@ -115,7 +115,7 @@ export interface IDependabotRegistry { * It should not have the scheme. */ 'registry'?: string | null | undefined; - /** The hostname for 'terraform_registry' types */ + /** The hostname for `terraform_registry` and `composer_repository` types */ 'host'?: string | null | undefined; /** The username to access the registry */ diff --git a/extension/task/utils/parseConfigFile.ts b/extension/task/utils/parseConfigFile.ts index 7bbda8f0..e158f679 100644 --- a/extension/task/utils/parseConfigFile.ts +++ b/extension/task/utils/parseConfigFile.ts @@ -4,6 +4,7 @@ import { getVariable } from 'azure-pipelines-task-lib/task'; import * as fs from 'fs'; import { load } from 'js-yaml'; import * as path from 'path'; +import { URL } from 'url'; import { IDependabotConfig, IDependabotRegistry, IDependabotUpdate } from '../IDependabotConfig'; import { convertPlaceholder } from './convertPlaceholder'; import { ISharedVariables } from './getSharedVariables'; @@ -266,19 +267,36 @@ function parseRegistries(config: any): Record { throw new Error(`The value 'url' in dependency registry config '${registryConfigKey}' is missing`); } if (url) { - // Some credentials do not use the 'url' property in the Ruby updater. - // npm_registry and docker_registry use 'registry' which should be stripped off the scheme. - // terraform_registry uses 'host' which is the hostname from the given URL. - - if (type === 'docker_registry' || type === 'npm_registry') { - parsed.registry = url.replace('https://', '').replace('http://', ''); - } else if (type === 'terraform_registry') { - parsed.host = new URL(url).hostname; - } else if (type === 'python_index') { - parsed['index-url'] = url; - } else { - parsed.url = url; + /* + * Some credentials do not use the 'url' property in the Ruby updater. + * The 'host' and 'registry' properties are derived from the given URL. + * The 'registry' property is derived from the 'url' by stripping off the scheme. + * The 'host' property is derived from the hostname of the 'url'. + * + * 'npm_registry' and 'docker_registry' use 'registry' only. + * 'terraform_registry' uses 'host' only. + * 'composer_repository' uses both 'url' and 'host'. + * 'python_index' uses 'index-url' instead of 'url'. + */ + + if (URL.canParse(url)) { + const parsedUrl = new URL(url); + + const addRegistry = type === 'docker_registry' || type === 'npm_registry'; + if (addRegistry) parsed.registry = url.replace('https://', '').replace('http://', ''); + + const addHost = type === 'terraform_registry' || type === 'composer_repository'; + if (addHost) parsed.host = parsedUrl.hostname; } + + if (type === 'python_index') parsed['index-url'] = url; + + const skipUrl = + type === 'docker_registry' || + type === 'npm_registry' || + type === 'terraform_registry' || + type === 'python_index'; + if (!skipUrl) parsed.url = url; } }); return registries; diff --git a/extension/tests/utils/parseConfigFile.test.ts b/extension/tests/utils/parseConfigFile.test.ts index c2d2930c..864218a2 100644 --- a/extension/tests/utils/parseConfigFile.test.ts +++ b/extension/tests/utils/parseConfigFile.test.ts @@ -46,7 +46,7 @@ describe('Parse registries', () => { expect(registry.url).toBe('https://repo.packagist.com/example-company/'); expect(registry['index-url']).toBe(undefined); expect(registry.registry).toBe(undefined); - expect(registry.host).toBe(undefined); + expect(registry.host).toBe('repo.packagist.com'); expect(registry.key).toBe(undefined); expect(registry.token).toBe(undefined); expect(registry.organization).toBe(undefined); diff --git a/server/Tingle.Dependabot.Tests/Workflow/UpdateRunnerTests.cs b/server/Tingle.Dependabot.Tests/Workflow/UpdateRunnerTests.cs index 506f9441..10dc0a55 100644 --- a/server/Tingle.Dependabot.Tests/Workflow/UpdateRunnerTests.cs +++ b/server/Tingle.Dependabot.Tests/Workflow/UpdateRunnerTests.cs @@ -101,7 +101,8 @@ public void MakeExtraCredentials_Works() Assert.Equal("composer_repository", Assert.Contains("type", credential)); Assert.Equal("https://repo.packagist.com/example-company/", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); - Assert.DoesNotContain("host", credential); + Assert.DoesNotContain("index-url", credential); + Assert.Equal("repo.packagist.com", Assert.Contains("host", credential)); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); Assert.DoesNotContain("organization", credential); @@ -117,6 +118,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("docker_registry", Assert.Contains("type", credential)); Assert.DoesNotContain("url", credential); Assert.Equal("registry.hub.docker.com", Assert.Contains("registry", credential)); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -133,6 +135,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("git", Assert.Contains("type", credential)); Assert.Equal("https://github.com", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -149,6 +152,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("hex_organization", Assert.Contains("type", credential)); Assert.DoesNotContain("url", credential); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.Equal("key_1234567890", Assert.Contains("key", credential)); Assert.DoesNotContain("token", credential); @@ -165,6 +169,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("hex_repository", Assert.Contains("type", credential)); Assert.Equal("https://private-repo.example.com", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -181,6 +186,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("maven_repository", Assert.Contains("type", credential)); Assert.Equal("https://artifactory.example.com", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -197,6 +203,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("npm_registry", Assert.Contains("type", credential)); Assert.DoesNotContain("url", credential); Assert.Equal("npm.pkg.github.com", Assert.Contains("registry", credential)); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.Equal("tkn_1234567890", Assert.Contains("token", credential)); @@ -213,6 +220,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("nuget_feed", Assert.Contains("type", credential)); Assert.Equal("https://pkgs.dev.azure.com/contoso/_packaging/My_Feed/nuget/v3/index.json", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -227,8 +235,9 @@ public void MakeExtraCredentials_Works() // python-index credential = credentials[8]; Assert.Equal("python_index", Assert.Contains("type", credential)); - Assert.Equal("https://pkgs.dev.azure.com/octocat/_packaging/my-feed/pypi/example", Assert.Contains("url", credential)); + Assert.DoesNotContain("url", credential); Assert.DoesNotContain("registry", credential); + Assert.Equal("https://pkgs.dev.azure.com/octocat/_packaging/my-feed/pypi/example", Assert.Contains("index-url", credential)); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.DoesNotContain("token", credential); @@ -236,6 +245,7 @@ public void MakeExtraCredentials_Works() Assert.DoesNotContain("repo", credential); Assert.DoesNotContain("auth-key", credential); Assert.DoesNotContain("public-key-fingerprint", credential); + Assert.Equal("https://pkgs.dev.azure.com/octocat/_packaging/my-feed/pypi/example", Assert.Contains("index-url", credential)); Assert.Equal("octocat@example.com", Assert.Contains("username", credential)); Assert.Equal("pwd_1234567890", Assert.Contains("password", credential)); Assert.Equal("true", Assert.Contains("replaces-base", credential)); @@ -245,6 +255,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("rubygems_server", Assert.Contains("type", credential)); Assert.Equal("https://rubygems.pkg.github.com/octocat/github_api", Assert.Contains("url", credential)); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.DoesNotContain("host", credential); Assert.DoesNotContain("key", credential); Assert.Equal("tkn_1234567890", Assert.Contains("token", credential)); @@ -261,6 +272,7 @@ public void MakeExtraCredentials_Works() Assert.Equal("terraform_registry", Assert.Contains("type", credential)); Assert.DoesNotContain("url", credential); Assert.DoesNotContain("registry", credential); + Assert.DoesNotContain("index-url", credential); Assert.Equal("terraform.example.com", Assert.Contains("host", credential)); Assert.DoesNotContain("key", credential); Assert.Equal("tkn_1234567890", Assert.Contains("token", credential)); diff --git a/server/Tingle.Dependabot/Workflow/UpdateRunner.cs b/server/Tingle.Dependabot/Workflow/UpdateRunner.cs index bd5a1b7c..236f676b 100644 --- a/server/Tingle.Dependabot/Workflow/UpdateRunner.cs +++ b/server/Tingle.Dependabot/Workflow/UpdateRunner.cs @@ -384,18 +384,17 @@ internal static IList> MakeCredentialsMetadata(IList< return credentials.Select(cred => { var values = new Dictionary { ["type"] = cred["type"], }; - cred.TryGetValue("host", out var host); - // pull host from registry if available - if (string.IsNullOrWhiteSpace(host)) + // if no host, pull host from url, index-url, or registry if available + if (!cred.TryGetValue("host", out var host) || string.IsNullOrWhiteSpace(host)) { - host = cred.TryGetValue("registry", out var registry) && Uri.TryCreate($"https://{registry}", UriKind.Absolute, out var u) ? u.Host : host; - } + if (cred.TryGetValue("url", out var url) || cred.TryGetValue("index-url", out url)) { } + else if (cred.TryGetValue("registry", out var registry)) url = $"https://{registry}"; - // pull host from registry if url - if (string.IsNullOrWhiteSpace(host)) - { - host = cred.TryGetValue("url", out var url) && Uri.TryCreate(url, UriKind.Absolute, out var u) ? u.Host : host; + if (url is not null && Uri.TryCreate(url, UriKind.Absolute, out var u)) + { + host = u.Host; + } } values.AddIfNotDefault("host", host); @@ -427,23 +426,31 @@ internal static IList> MakeExtraCredentials(ICollecti values.AddIfNotDefault("token", ConvertPlaceholder(v.Token, secrets)); values.AddIfNotDefault("replaces-base", v.ReplacesBase is true ? "true" : null); - // Some credentials do not use the 'url' property in the Ruby updater. - // npm_registry and docker_registry use 'registry' which should be stripped off the scheme. - // terraform_registry uses 'host' which is the hostname from the given URL. - - if (type == "docker_registry" || type == "npm_registry") - { - values.Add("registry", v.Url!.Replace("https://", "").Replace("http://", "")); - } - else if (type == "terraform_registry") + /* + * Some credentials do not use the 'url' property in the Ruby updater. + * The 'host' and 'registry' properties are derived from the given URL. + * The 'registry' property is derived from the 'url' by stripping off the scheme. + * The 'host' property is derived from the hostname of the 'url'. + * + * 'npm_registry' and 'docker_registry' use 'registry' only. + * 'terraform_registry' uses 'host' only. + * 'composer_repository' uses both 'url' and 'host'. + * 'python_index' uses 'index-url' instead of 'url'. + */ + + if (Uri.TryCreate(v.Url, UriKind.Absolute, out var url)) { - values.Add("host", new Uri(v.Url!).Host); - } - else - { - values.AddIfNotDefault("url", v.Url!); + var addRegistry = type is "docker_registry" or "npm_registry"; + if (addRegistry) values.Add("registry", $"{url.Host}{url.PathAndQuery}".TrimEnd('/')); + + var addHost = type is "terraform_registry" or "composer_repository"; + if (addHost) values.Add("host", url.Host); } - var useRegistryProperty = type.Contains("npm") || type.Contains("docker"); + + if (type is "python_index") values.AddIfNotDefault("index-url", v.Url); + + var skipUrl = type is "docker_registry" or "npm_registry" or "terraform_registry" or "python_index"; + if (!skipUrl) values.AddIfNotDefault("url", v.Url); return values; }).ToList();