From 5de1c0657b1ac6f7bff791ef8964d2eea29a7825 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 28 May 2021 01:39:43 +0300 Subject: [PATCH 1/6] feat(registry-target): Allow batched updates w/ new config Fixes #48 by introducing a new registry target config structure while keeping backward compatibility. Now we expect two dicts under the names `apps` and `sdks` where the keys the canonical names and the values are individual configs for these canonical names. It also does a single commit for all changes instead of one commit per canonical name. This new structure also has the side effect of making it very hard to miss required configuration. Tested with `craft` in dry-run mode. --- .craft.yml | 18 +-- CHANGELOG.md | 1 + src/artifact_providers/base.ts | 16 +- src/commands/publish.ts | 1 + src/targets/awsLambdaLayer.ts | 8 +- src/targets/ghPages.ts | 6 +- src/targets/registry.ts | 271 +++++++++++++++++++-------------- src/targets/upm.ts | 4 +- src/utils/async.ts | 3 +- src/utils/githubApi.ts | 2 +- src/utils/packagePath.ts | 2 +- src/utils/registry.ts | 49 +++--- 12 files changed, 214 insertions(+), 167 deletions(-) diff --git a/.craft.yml b/.craft.yml index b2be79831..1efc4fe6d 100644 --- a/.craft.yml +++ b/.craft.yml @@ -1,7 +1,4 @@ -minVersion: '0.21.0' -github: - owner: getsentry - repo: craft +minVersion: '0.22.2' changelogPolicy: auto requireNames: - /^sentry-craft.*\.tgz$/ @@ -19,13 +16,12 @@ targets: metadata: cacheControl: 'public, max-age=300' - name: registry - type: app - urlTemplate: 'https://downloads.sentry-cdn.com/craft/{{version}}/{{file}}' - checksums: - - algorithm: sha256 - format: hex - config: - canonical: 'app:craft' + apps: + 'app:craft': + urlTemplate: 'https://downloads.sentry-cdn.com/craft/{{version}}/{{file}}' + checksums: + - algorithm: sha256 + format: hex - name: docker source: us.gcr.io/sentryio/craft target: getsentry/craft diff --git a/CHANGELOG.md b/CHANGELOG.md index d1ad7d1f0..93b734152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - feat(publish): Ability to merge to non-default (#245) - fix(logging): Proper scoping and log levels (#247) +- feat(registry-target): Allow batched updates w/ new config (#249) ## 0.22.2 diff --git a/src/artifact_providers/base.ts b/src/artifact_providers/base.ts index fc3c44ad1..249f58ab0 100644 --- a/src/artifact_providers/base.ts +++ b/src/artifact_providers/base.ts @@ -96,7 +96,7 @@ export abstract class BaseArtifactProvider { /** Cache for storing mapping between revisions and a list of their artifacts */ protected readonly fileListCache: { - [key: string]: RemoteArtifact[] | undefined; + [key: string]: Promise | undefined; } = {}; /** Cache for checksums computed for the files stored on disk */ @@ -240,23 +240,23 @@ export abstract class BaseArtifactProvider { ): Promise { this.logger.debug(`Fetching artifact list for revision \`${revision}\`.`); // check the cache first - const cached = this.fileListCache[revision]; - if (cached) { + if (this.fileListCache[revision]) { this.logger.debug(`Found list in cache.`); - return cached; + } else { + // Cache the promise immediately to cause any subsequent calls during the + // fetch to use the pending promise instead of fetching again in parallel + this.fileListCache[revision] = this.doListArtifactsForRevision(revision); } - // the data wasn't in the cache, so now we have to go get it - let artifacts; + let artifacts: RemoteArtifact[]; try { - artifacts = await this.doListArtifactsForRevision(revision); + artifacts = (await this.fileListCache[revision]) || []; } catch (err) { this.logger.error( `Unable to retrieve artifact list for revision ${revision}!` ); throw err; } - this.fileListCache[revision] = artifacts; if (artifacts.length === 0) { this.logger.info(`No artifacts found for revision ${revision}`); diff --git a/src/commands/publish.ts b/src/commands/publish.ts index f5ff256a9..c131568df 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -226,6 +226,7 @@ async function getTargetList( continue; } try { + logger.debug(`Creating target ${targetConfig.name}:`, targetConfig); const target = new targetClass( targetConfig, artifactProvider, diff --git a/src/targets/awsLambdaLayer.ts b/src/targets/awsLambdaLayer.ts index 8c7d9d7ae..954650b1e 100644 --- a/src/targets/awsLambdaLayer.ts +++ b/src/targets/awsLambdaLayer.ts @@ -7,7 +7,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GithubRemote, + GitHubRemote, } from '../utils/githubApi'; import { TargetConfig } from '../schemas/project_config'; @@ -24,9 +24,7 @@ import { createSymlinks } from '../utils/symlink'; import { withTempDir } from '../utils/files'; import { isDryRun } from '../utils/helpers'; import { isPreviewRelease } from '../utils/version'; -import { getRegistryGithubRemote } from '../utils/registry'; - -const DEFAULT_REGISTRY_REMOTE: GithubRemote = getRegistryGithubRemote(); +import { DEFAULT_REGISTRY_REMOTE } from '../utils/registry'; /** Config options for the "aws-lambda-layer" target. */ interface AwsLambdaTargetConfig { @@ -35,7 +33,7 @@ interface AwsLambdaTargetConfig { /** AWS secret access key, set as `AWS_SECRET_ACCESS_KEY`. */ awsSecretAccessKey: string; /** Git remote of the release registry. */ - registryRemote: GithubRemote; + registryRemote: GitHubRemote; /** Should layer versions of prereleases be pushed to the registry? */ linkPrereleases: boolean; } diff --git a/src/targets/ghPages.ts b/src/targets/ghPages.ts index d7958476c..cc3425d94 100644 --- a/src/targets/ghPages.ts +++ b/src/targets/ghPages.ts @@ -11,7 +11,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GithubRemote, + GitHubRemote, } from '../utils/githubApi'; import { isDryRun } from '../utils/helpers'; import { extractZipArchive } from '../utils/system'; @@ -143,7 +143,7 @@ export class GhPagesTarget extends BaseTarget { */ public async commitArchiveToBranch( directory: string, - remote: GithubRemote, + remote: GitHubRemote, branch: string, archivePath: string, version: string @@ -221,7 +221,7 @@ export class GhPagesTarget extends BaseTarget { const username = await getAuthUsername(this.github); - const remote = new GithubRemote( + const remote = new GitHubRemote( githubOwner, githubRepo, username, diff --git a/src/targets/registry.ts b/src/targets/registry.ts index 80b57c3e2..5cc86590f 100644 --- a/src/targets/registry.ts +++ b/src/targets/registry.ts @@ -1,8 +1,6 @@ import { mapLimit } from 'async'; import * as Github from '@octokit/rest'; -import rimraf from 'rimraf'; import simpleGit, { SimpleGit } from 'simple-git'; -import * as path from 'path'; import { GithubGlobalConfig, TargetConfig } from '../schemas/project_config'; import { ConfigurationError, reportError } from '../utils/errors'; @@ -11,7 +9,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GithubRemote, + GitHubRemote, } from '../utils/githubApi'; import { renderTemplateSafe } from '../utils/strings'; import { isPreviewRelease } from '../utils/version'; @@ -27,20 +25,14 @@ import { ChecksumEntry, getArtifactChecksums, } from '../utils/checksum'; -import * as registryUtils from '../utils/registry'; -import { getPackageDirPath } from '../utils/packagePath'; +import { + DEFAULT_REGISTRY_REMOTE, + getPackageManifest, + updateManifestSymlinks, + RegistryPackageType, +} from '../utils/registry'; import { isDryRun } from '../utils/helpers'; -import { withRetry } from '../utils/async'; - -const DEFAULT_REGISTRY_REMOTE: GithubRemote = registryUtils.getRegistryGithubRemote(); - -/** Type of the registry package */ -export enum RegistryPackageType { - /** App is a generic package type that doesn't belong to any specific registry */ - APP = 'app', - /** SDK is a package hosted in one of public registries (PyPI, NPM, etc.) */ - SDK = 'sdk', -} +import { filterAsync, withRetry } from '../utils/async'; /** "registry" target options */ export interface RegistryConfig { @@ -48,8 +40,6 @@ export interface RegistryConfig { type: RegistryPackageType; /** Unique package cannonical name, including type and/or registry name */ canonicalName: string; - /** Git remote of the release registry */ - registryRemote: GithubRemote; /** Should we create registry entries for pre-releases? */ linkPrereleases: boolean; /** URL template for file assets */ @@ -76,12 +66,12 @@ interface ArtifactData { * Target responsible for publishing to Sentry's release registry: https://github.com/getsentry/sentry-release-registry/ */ export class RegistryTarget extends BaseTarget { - /** The information of the canonical local checkout of the registry */ - private static localRepo: undefined | LocalRegistry; /** Target name */ public readonly name = 'registry'; + /** Git remote of the release registry */ + public readonly remote: GitHubRemote; /** Target options */ - public readonly registryConfig: RegistryConfig; + public readonly registryConfig: RegistryConfig[]; /** Github client */ public readonly github: Github; /** Github repo configuration */ @@ -93,6 +83,13 @@ export class RegistryTarget extends BaseTarget { githubRepo: GithubGlobalConfig ) { super(config, artifactProvider, githubRepo); + const remote = this.config.remote; + if (remote) { + const [owner, repo] = remote.split('/', 2); + this.remote = new GitHubRemote(owner, repo); + } else { + this.remote = DEFAULT_REGISTRY_REMOTE; + } this.github = getGithubClient(); this.githubRepo = githubRepo; this.registryConfig = this.getRegistryConfig(); @@ -101,7 +98,34 @@ export class RegistryTarget extends BaseTarget { /** * Extracts Registry target options from the raw configuration. */ - public getRegistryConfig(): RegistryConfig { + public getRegistryConfig(): RegistryConfig[] { + if (!this.config.sdks && !this.config.apps) { + this.logger.warn( + 'You are using a deprecated registry traget config, please update.' + ); + return [this.getLegacyRegistryConfig()]; + } + + const items = []; + for (const canonicalName of Object.keys(this.config.sdks || {})) { + items.push({ + ...this.config.sdks[canonicalName], + type: RegistryPackageType.SDK, + canonicalName, + }); + } + for (const canonicalName of Object.keys(this.config.apps || {})) { + items.push({ + ...this.config.apps[canonicalName], + type: RegistryPackageType.APP, + canonicalName, + }); + } + + return items; + } + + private getLegacyRegistryConfig(): RegistryConfig { const registryType = this.config.type; if ( [RegistryPackageType.APP, RegistryPackageType.SDK].indexOf( @@ -157,7 +181,6 @@ export class RegistryTarget extends BaseTarget { checksums, linkPrereleases, onlyIfPresent, - registryRemote: DEFAULT_REGISTRY_REMOTE, type: registryType, urlTemplate, }; @@ -175,11 +198,12 @@ export class RegistryTarget extends BaseTarget { * @param revision Git commit SHA to be published */ public async addFileLinks( + registryConfig: RegistryConfig, manifest: { [key: string]: any }, version: string, revision: string ): Promise { - if (!this.registryConfig.urlTemplate) { + if (!registryConfig.urlTemplate) { return; } @@ -194,7 +218,7 @@ export class RegistryTarget extends BaseTarget { const fileUrls: { [_: string]: string } = {}; for (const artifact of artifacts) { fileUrls[artifact.filename] = renderTemplateSafe( - this.registryConfig.urlTemplate, + registryConfig.urlTemplate, { file: artifact.filename, revision, @@ -224,23 +248,24 @@ export class RegistryTarget extends BaseTarget { * @param revision Git commit SHA to be published */ public async getArtifactData( + registryConfig: RegistryConfig, artifact: RemoteArtifact, version: string, revision: string ): Promise { const artifactData: ArtifactData = {}; - if (this.registryConfig.urlTemplate) { - artifactData.url = renderTemplateSafe(this.registryConfig.urlTemplate, { + if (registryConfig.urlTemplate) { + artifactData.url = renderTemplateSafe(registryConfig.urlTemplate, { file: artifact.filename, revision, version, }); } - if (this.registryConfig.checksums.length > 0) { + if (registryConfig.checksums.length > 0) { artifactData.checksums = await getArtifactChecksums( - this.registryConfig.checksums, + registryConfig.checksums, artifact, this.artifactProvider ); @@ -264,6 +289,7 @@ export class RegistryTarget extends BaseTarget { * @param revision Git commit SHA to be published */ public async addFilesData( + registryConfig: RegistryConfig, packageManifest: { [key: string]: any }, version: string, revision: string @@ -271,10 +297,7 @@ export class RegistryTarget extends BaseTarget { // Clear existing data delete packageManifest.files; - if ( - !this.registryConfig.urlTemplate && - this.registryConfig.checksums.length === 0 - ) { + if (!registryConfig.urlTemplate && registryConfig.checksums.length === 0) { this.logger.warn( 'No URL template or checksums, not adding any file data' ); @@ -293,7 +316,12 @@ export class RegistryTarget extends BaseTarget { const files: { [key: string]: any } = {}; await mapLimit(artifacts, MAX_DOWNLOAD_CONCURRENCY, async artifact => { - const fileData = await this.getArtifactData(artifact, version, revision); + const fileData = await this.getArtifactData( + registryConfig, + artifact, + version, + revision + ); files[artifact.filename] = fileData; }); @@ -312,6 +340,7 @@ export class RegistryTarget extends BaseTarget { * @param revision Git commit SHA to be published */ public async getUpdatedManifest( + registryConfig: RegistryConfig, packageManifest: { [key: string]: any }, canonical: string, version: string, @@ -328,12 +357,17 @@ export class RegistryTarget extends BaseTarget { const updatedManifest = { ...packageManifest, version }; // Add file links if it's a generic app (legacy) - if (this.registryConfig.type === RegistryPackageType.APP) { - await this.addFileLinks(updatedManifest, version, revision); + if (registryConfig.type === RegistryPackageType.APP) { + await this.addFileLinks( + registryConfig, + updatedManifest, + version, + revision + ); } // Add various file-related data - await this.addFilesData(updatedManifest, version, revision); + await this.addFilesData(registryConfig, updatedManifest, version, revision); return updatedManifest; } @@ -345,25 +379,23 @@ export class RegistryTarget extends BaseTarget { * @param version The new version * @param revision Git commit SHA to be published */ - private async commitVersionToRegistry( + private async updateVersionInRegistry( + registryConfig: RegistryConfig, localRepo: LocalRegistry, version: string, revision: string ): Promise { - const canonicalName = this.registryConfig.canonicalName; - const packageDirPath = getPackageDirPath( - this.registryConfig.type, - canonicalName - ); - const packageManifest = registryUtils.getPackageManifest( + const canonicalName = registryConfig.canonicalName; + const { versionFilePath, packageManifest } = await getPackageManifest( localRepo.dir, - packageDirPath, + registryConfig.type, + canonicalName, version ); - const versionFilePath = path.join(packageDirPath, `${version}.json`); - registryUtils.updateManifestSymlinks( + updateManifestSymlinks( await this.getUpdatedManifest( + registryConfig, packageManifest, canonicalName, version, @@ -373,15 +405,10 @@ export class RegistryTarget extends BaseTarget { versionFilePath, packageManifest.version || undefined ); - - // Commit - await localRepo.git - .add(['.']) - .commit(`craft: release "${canonicalName}", version "${version}"`); } private async cloneRegistry(directory: string): Promise { - const remote = this.registryConfig.registryRemote; + const remote = this.remote; const username = await getAuthUsername(this.github); remote.setAuth(username, getGithubApiToken()); @@ -396,78 +423,88 @@ export class RegistryTarget extends BaseTarget { return git; } + public async getValidItems( + version: string, + revision: string + ): Promise { + return filterAsync(this.registryConfig, async registryConfig => { + if (!registryConfig.linkPrereleases && isPreviewRelease(version)) { + this.logger.info( + `Preview release detected, skipping ${registryConfig.canonicalName}` + ); + return false; + } + + // If we have onlyIfPresent specified, check that we have any of matched files + const onlyIfPresentPattern = registryConfig.onlyIfPresent; + if (onlyIfPresentPattern) { + const artifacts = await this.artifactProvider.filterArtifactsForRevision( + revision, + { + includeNames: onlyIfPresentPattern, + } + ); + if (artifacts.length === 0) { + this.logger.warn( + `No files found that match "${onlyIfPresentPattern.toString()}", skipping the target.` + ); + return false; + } + } + return true; + }); + } + /** * Modifies/adds meta information regarding the package we are publishing */ public async publish(version: string, revision: string): Promise { - if (!this.registryConfig.linkPrereleases && isPreviewRelease(version)) { - this.logger.info('Preview release detected, skipping the target'); - return undefined; - } + const items = await this.getValidItems(version, revision); - // If we have onlyIfPresent specified, check that we have any of matched files - const onlyIfPresentPattern = this.registryConfig.onlyIfPresent; - if (onlyIfPresentPattern) { - const artifacts = await this.artifactProvider.filterArtifactsForRevision( - revision, - { - includeNames: onlyIfPresentPattern, - } - ); - if (artifacts.length === 0) { - this.logger.warn( - `No files found that match "${onlyIfPresentPattern.toString()}", skipping the target.` - ); - return undefined; - } + if (items.length === 0) { + this.logger.warn('No suitable items found, bailing'); + return; } - return this.doPublish(version, revision); - } + await withTempDir( + async dir => { + const localRepo = { + dir, + git: await this.cloneRegistry(dir), + }; + await Promise.all( + items.map(registryConfig => + this.updateVersionInRegistry( + registryConfig, + localRepo, + version, + revision + ) + ) + ); - private async doPublish(version: string, revision: string) { - if (!RegistryTarget.localRepo) { - await withTempDir( - async dir => { - RegistryTarget.localRepo = { - dir, - git: await this.cloneRegistry(dir), - }; - process.on('beforeExit', () => { - RegistryTarget.localRepo = undefined; - rimraf(localRepo.dir, () => { - /* intentionally don't block on deletion */ - }); - }); - }, - // We will clean the directory after pushing - false, - 'craft-release-registry-' - ); - } - let localRepo: LocalRegistry; - if (RegistryTarget.localRepo) { - localRepo = RegistryTarget.localRepo; - } else { - // XXX(BYK): This should NEVER happen - throw new Error( - `Local registry missing, it should have been cloned at this stage!` - ); - } - this.commitVersionToRegistry(localRepo, version, revision); - - // Push! - if (!isDryRun()) { - this.logger.info(`Pushing the changes...`); - // Ensure we are still up to date with upstream - await withRetry(() => - localRepo.git - .pull('origin', 'master', ['--rebase']) - .push('origin', 'master') - ); - } else { - this.logger.info('[dry-run] Not pushing the changes.'); - } + // Commit + await localRepo.git + .add(['.']) + .commit( + `craft: release "${this.githubRepo.repo}", version "${version}"` + ); + // Push! + if (!isDryRun()) { + this.logger.info(`Pushing the changes...`); + // Ensure we are still up to date with upstream + await withRetry(() => + localRepo.git + .pull('origin', 'master', ['--rebase']) + .push('origin', 'master') + ); + } else { + this.logger.info('[dry-run] Not pushing the changes.'); + } + }, + true, + 'craft-release-registry-' + ); this.logger.info('Release registry updated.'); } diff --git a/src/targets/upm.ts b/src/targets/upm.ts index c276b089a..63b49eb38 100644 --- a/src/targets/upm.ts +++ b/src/targets/upm.ts @@ -4,7 +4,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GithubRemote, + GitHubRemote, } from '../utils/githubApi'; import { GithubTarget } from './github'; @@ -105,7 +105,7 @@ export class UpmTarget extends BaseTarget { ); const username = await getAuthUsername(this.github); - const remote = new GithubRemote( + const remote = new GitHubRemote( this.config.releaseRepoOwner, this.config.releaseRepoName, username, diff --git a/src/utils/async.ts b/src/utils/async.ts index 74f8e42d3..e2e15b8ab 100644 --- a/src/utils/async.ts +++ b/src/utils/async.ts @@ -13,8 +13,7 @@ import { reportError } from './errors'; export async function filterAsync( array: T[], predicate: (arg: T) => boolean | Promise, - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - thisArg?: any + thisArg?: unknown ): Promise { const verdicts = await Promise.all(array.map(predicate, thisArg)); return array.filter((_element, index) => verdicts[index]); diff --git a/src/utils/githubApi.ts b/src/utils/githubApi.ts index 19c283d14..045e9d547 100644 --- a/src/utils/githubApi.ts +++ b/src/utils/githubApi.ts @@ -7,7 +7,7 @@ import { ConfigurationError } from './errors'; /** * Abstraction for GitHub remotes */ -export class GithubRemote { +export class GitHubRemote { /** GitHub owner */ public readonly owner: string; /** GitHub repository name */ diff --git a/src/utils/packagePath.ts b/src/utils/packagePath.ts index 785bb9612..b5c996924 100644 --- a/src/utils/packagePath.ts +++ b/src/utils/packagePath.ts @@ -1,5 +1,5 @@ import * as path from 'path'; -import { RegistryPackageType } from '../targets/registry'; +import { RegistryPackageType } from '../utils/registry'; import { ConfigurationError } from './errors'; /** diff --git a/src/utils/registry.ts b/src/utils/registry.ts index e88a26d00..d5d17a3ad 100644 --- a/src/utils/registry.ts +++ b/src/utils/registry.ts @@ -1,10 +1,19 @@ -import * as fs from 'fs'; +import { promises as fsPromises, existsSync } from 'fs'; import * as path from 'path'; import { logger } from '../logger'; import { createSymlinks } from './symlink'; import { reportError } from './errors'; -import { GithubRemote } from './githubApi'; +import { GitHubRemote } from './githubApi'; +import { getPackageDirPath } from '../utils/packagePath'; + +/** Type of the registry package */ +export enum RegistryPackageType { + /** App is a generic package type that doesn't belong to any specific registry */ + APP = 'app', + /** SDK is a package hosted in one of public registries (PyPI, NPM, etc.) */ + SDK = 'sdk', +} /** * Gets the package manifest version in the given directory. @@ -13,18 +22,26 @@ import { GithubRemote } from './githubApi'; * @param packageDirPath The package directory. * @param version The package version. */ -export function getPackageManifest( +export async function getPackageManifest( baseDir: string, - packageDirPath: string, + type: RegistryPackageType, + canonicalName: string, version: string -): any { +): Promise<{ versionFilePath: string; packageManifest: any }> { + const packageDirPath = getPackageDirPath(type, canonicalName); const versionFilePath = path.join(baseDir, packageDirPath, `${version}.json`); - if (fs.existsSync(versionFilePath)) { + if (existsSync(versionFilePath)) { reportError(`Version file for "${version}" already exists. Aborting.`); } const packageManifestPath = path.join(baseDir, packageDirPath, 'latest.json'); - logger.debug('Reading the current configuration from "latest.json"...'); - return JSON.parse(fs.readFileSync(packageManifestPath).toString()) || {}; + logger.debug('Reading the current configuration from', packageManifestPath); + return { + versionFilePath, + packageManifest: + JSON.parse( + await fsPromises.readFile(packageManifestPath, { encoding: 'utf-8' }) + ) || {}, + }; } /** @@ -36,22 +53,20 @@ export function getPackageManifest( * @param versionFilePath The path of the version file. * @param previousVersion The previous version. */ -export function updateManifestSymlinks( +export async function updateManifestSymlinks( updatedManifest: unknown, version: string, versionFilePath: string, previousVersion: string -): void { +): Promise { const manifestString = JSON.stringify(updatedManifest, undefined, 2) + '\n'; logger.debug('Updated manifest', manifestString); logger.debug(`Writing updated manifest to "${versionFilePath}"...`); - fs.writeFileSync(versionFilePath, manifestString); + await fsPromises.writeFile(versionFilePath, manifestString); createSymlinks(versionFilePath, version, previousVersion); } -/** - * Returns a GithubRemote object to the sentry release registry. - */ -export function getRegistryGithubRemote(): GithubRemote { - return new GithubRemote('getsentry', 'sentry-release-registry'); -} +export const DEFAULT_REGISTRY_REMOTE = new GitHubRemote( + 'getsentry', + 'sentry-release-registry' +); From d371a6176186578090fd4ce6cda225ee0ff47ca5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 28 May 2021 10:04:23 +0300 Subject: [PATCH 2/6] revert Github/GitHub change --- src/targets/awsLambdaLayer.ts | 4 ++-- src/targets/ghPages.ts | 6 +++--- src/targets/registry.ts | 6 +++--- src/targets/upm.ts | 4 ++-- src/utils/githubApi.ts | 2 +- src/utils/registry.ts | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/targets/awsLambdaLayer.ts b/src/targets/awsLambdaLayer.ts index 954650b1e..5ff7bb130 100644 --- a/src/targets/awsLambdaLayer.ts +++ b/src/targets/awsLambdaLayer.ts @@ -7,7 +7,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GitHubRemote, + GithubRemote, } from '../utils/githubApi'; import { TargetConfig } from '../schemas/project_config'; @@ -33,7 +33,7 @@ interface AwsLambdaTargetConfig { /** AWS secret access key, set as `AWS_SECRET_ACCESS_KEY`. */ awsSecretAccessKey: string; /** Git remote of the release registry. */ - registryRemote: GitHubRemote; + registryRemote: GithubRemote; /** Should layer versions of prereleases be pushed to the registry? */ linkPrereleases: boolean; } diff --git a/src/targets/ghPages.ts b/src/targets/ghPages.ts index cc3425d94..d7958476c 100644 --- a/src/targets/ghPages.ts +++ b/src/targets/ghPages.ts @@ -11,7 +11,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GitHubRemote, + GithubRemote, } from '../utils/githubApi'; import { isDryRun } from '../utils/helpers'; import { extractZipArchive } from '../utils/system'; @@ -143,7 +143,7 @@ export class GhPagesTarget extends BaseTarget { */ public async commitArchiveToBranch( directory: string, - remote: GitHubRemote, + remote: GithubRemote, branch: string, archivePath: string, version: string @@ -221,7 +221,7 @@ export class GhPagesTarget extends BaseTarget { const username = await getAuthUsername(this.github); - const remote = new GitHubRemote( + const remote = new GithubRemote( githubOwner, githubRepo, username, diff --git a/src/targets/registry.ts b/src/targets/registry.ts index 5cc86590f..69f57d50d 100644 --- a/src/targets/registry.ts +++ b/src/targets/registry.ts @@ -9,7 +9,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GitHubRemote, + GithubRemote, } from '../utils/githubApi'; import { renderTemplateSafe } from '../utils/strings'; import { isPreviewRelease } from '../utils/version'; @@ -69,7 +69,7 @@ export class RegistryTarget extends BaseTarget { /** Target name */ public readonly name = 'registry'; /** Git remote of the release registry */ - public readonly remote: GitHubRemote; + public readonly remote: GithubRemote; /** Target options */ public readonly registryConfig: RegistryConfig[]; /** Github client */ @@ -86,7 +86,7 @@ export class RegistryTarget extends BaseTarget { const remote = this.config.remote; if (remote) { const [owner, repo] = remote.split('/', 2); - this.remote = new GitHubRemote(owner, repo); + this.remote = new GithubRemote(owner, repo); } else { this.remote = DEFAULT_REGISTRY_REMOTE; } diff --git a/src/targets/upm.ts b/src/targets/upm.ts index 63b49eb38..c276b089a 100644 --- a/src/targets/upm.ts +++ b/src/targets/upm.ts @@ -4,7 +4,7 @@ import { getAuthUsername, getGithubApiToken, getGithubClient, - GitHubRemote, + GithubRemote, } from '../utils/githubApi'; import { GithubTarget } from './github'; @@ -105,7 +105,7 @@ export class UpmTarget extends BaseTarget { ); const username = await getAuthUsername(this.github); - const remote = new GitHubRemote( + const remote = new GithubRemote( this.config.releaseRepoOwner, this.config.releaseRepoName, username, diff --git a/src/utils/githubApi.ts b/src/utils/githubApi.ts index 045e9d547..19c283d14 100644 --- a/src/utils/githubApi.ts +++ b/src/utils/githubApi.ts @@ -7,7 +7,7 @@ import { ConfigurationError } from './errors'; /** * Abstraction for GitHub remotes */ -export class GitHubRemote { +export class GithubRemote { /** GitHub owner */ public readonly owner: string; /** GitHub repository name */ diff --git a/src/utils/registry.ts b/src/utils/registry.ts index d5d17a3ad..42e9e2823 100644 --- a/src/utils/registry.ts +++ b/src/utils/registry.ts @@ -4,7 +4,7 @@ import * as path from 'path'; import { logger } from '../logger'; import { createSymlinks } from './symlink'; import { reportError } from './errors'; -import { GitHubRemote } from './githubApi'; +import { GithubRemote } from './githubApi'; import { getPackageDirPath } from '../utils/packagePath'; /** Type of the registry package */ @@ -66,7 +66,7 @@ export async function updateManifestSymlinks( createSymlinks(versionFilePath, version, previousVersion); } -export const DEFAULT_REGISTRY_REMOTE = new GitHubRemote( +export const DEFAULT_REGISTRY_REMOTE = new GithubRemote( 'getsentry', 'sentry-release-registry' ); From ba82dfc3dceca847fda777e7fe7417844f7d6a5d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 28 May 2021 10:09:07 +0300 Subject: [PATCH 3/6] better typing --- src/artifact_providers/base.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/artifact_providers/base.ts b/src/artifact_providers/base.ts index 249f58ab0..8c9366d74 100644 --- a/src/artifact_providers/base.ts +++ b/src/artifact_providers/base.ts @@ -91,12 +91,12 @@ export abstract class BaseArtifactProvider { protected readonly logger: typeof loggerRaw; /** Cache for local paths to downloaded files */ protected readonly downloadCache: { - [key: string]: Promise | undefined; + [key: string]: Promise; } = {}; /** Cache for storing mapping between revisions and a list of their artifacts */ protected readonly fileListCache: { - [key: string]: Promise | undefined; + [key: string]: Promise; } = {}; /** Cache for checksums computed for the files stored on disk */ @@ -250,7 +250,7 @@ export abstract class BaseArtifactProvider { let artifacts: RemoteArtifact[]; try { - artifacts = (await this.fileListCache[revision]) || []; + artifacts = await this.fileListCache[revision]; } catch (err) { this.logger.error( `Unable to retrieve artifact list for revision ${revision}!` From 524fc4ca8b6ad5c61069d58d14b59c4b65704b6c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 2 Jun 2021 00:20:50 +0300 Subject: [PATCH 4/6] review feedback + DRY up --- README.md | 35 ++++++++++++++--------------- src/targets/registry.ts | 49 +++++++++++++++++++++-------------------- tsconfig.build.json | 5 ++++- 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 701aec721..29aaad227 100644 --- a/README.md +++ b/README.md @@ -794,6 +794,9 @@ the corresponding package directory can be found inside "packages" directory of release regsitry. Type "app" indicates that the package's version files are located in "apps" directory of the registry. +It is strongly discouraged to have multiple `registry` targets in a config as it +supports grouping/batching multiple apps and SDKs in a single target. + **Environment** _none_ @@ -802,30 +805,26 @@ _none_ | Option | Description | | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `type` | Type of the package: can be "sdk" or "app". | -| `config.canonical` | Canonical name of the package that includes package registry name (e.g. NPM, PyPI) and the full package name. | -| `urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | -| `linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | -| `checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of "sha256", "sha384", and "sha512) and format ("base64" and "hex"). | -| `onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | +| `apps` | List of `app` configs as a dict, keyed by their canonical names (example: `app:craft`) | +| `sdks` | List of `sdk` configs as a dict, keyed by their canonical names (example: `maven:io.sentry:sentry`) | +| `(sdks|apps).urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | +| `(sdks|apps).linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | +| `(sdks|apps).checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of `sha256`, `sha384`, and `sha512`) and format (`base64` and `hex`). | +| `(sdks|apps).onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | **Example** ```yaml targets: - name: registry - type: sdk - config: - canonical: 'npm:@sentry/browser' - - - name: registry - type: app - urlTemplate: 'https://example.com/{{version}}/{{file}}' - config: - canonical: 'npm:@sentry/browser' - checksums: - - algorithm: sha256 - format: hex + sdks: + 'npm:@sentry/browser': + apps: + 'npm:@sentry/browser': + urlTemplate: 'https://example.com/{{version}}/{{file}}' + checksums: + - algorithm: sha256 + format: hex ``` ### Cocoapods (`cocoapods`) diff --git a/src/targets/registry.ts b/src/targets/registry.ts index 69f57d50d..435fb4880 100644 --- a/src/targets/registry.ts +++ b/src/targets/registry.ts @@ -41,11 +41,11 @@ export interface RegistryConfig { /** Unique package cannonical name, including type and/or registry name */ canonicalName: string; /** Should we create registry entries for pre-releases? */ - linkPrereleases: boolean; + linkPrereleases?: boolean; /** URL template for file assets */ urlTemplate?: string; /** Types of checksums to compute for artifacts */ - checksums: ChecksumEntry[]; + checksums?: ChecksumEntry[]; /** Pattern that allows to skip the target if there's no matching file */ onlyIfPresent?: RegExp; } @@ -62,6 +62,11 @@ interface ArtifactData { }; } +const BATCH_KEYS = { + sdks: RegistryPackageType.SDK, + apps: RegistryPackageType.APP, +}; + /** * Target responsible for publishing to Sentry's release registry: https://github.com/getsentry/sentry-release-registry/ */ @@ -99,30 +104,22 @@ export class RegistryTarget extends BaseTarget { * Extracts Registry target options from the raw configuration. */ public getRegistryConfig(): RegistryConfig[] { - if (!this.config.sdks && !this.config.apps) { + const items = Object.entries(BATCH_KEYS).flatMap(([key, type]) => + Object.entries(this.config[key] || {}).map(([canonicalName, conf]) => ({ + ...(conf as Record), + type, + canonicalName, + })) + ); + + if (items.length === 0 && this.config.type) { this.logger.warn( - 'You are using a deprecated registry traget config, please update.' + 'You are using a deprecated registry target config, please update.' ); return [this.getLegacyRegistryConfig()]; + } else { + return items; } - - const items = []; - for (const canonicalName of Object.keys(this.config.sdks || {})) { - items.push({ - ...this.config.sdks[canonicalName], - type: RegistryPackageType.SDK, - canonicalName, - }); - } - for (const canonicalName of Object.keys(this.config.apps || {})) { - items.push({ - ...this.config.apps[canonicalName], - type: RegistryPackageType.APP, - canonicalName, - }); - } - - return items; } private getLegacyRegistryConfig(): RegistryConfig { @@ -263,7 +260,7 @@ export class RegistryTarget extends BaseTarget { }); } - if (registryConfig.checksums.length > 0) { + if (registryConfig.checksums && registryConfig.checksums.length > 0) { artifactData.checksums = await getArtifactChecksums( registryConfig.checksums, artifact, @@ -297,7 +294,11 @@ export class RegistryTarget extends BaseTarget { // Clear existing data delete packageManifest.files; - if (!registryConfig.urlTemplate && registryConfig.checksums.length === 0) { + if ( + !registryConfig.urlTemplate && + registryConfig.checksums && + registryConfig.checksums.length === 0 + ) { this.logger.warn( 'No URL template or checksums, not adding any file data' ); diff --git a/tsconfig.build.json b/tsconfig.build.json index 1142f69b6..c6a9d3b9f 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -1,7 +1,10 @@ { "extends": "@sentry/typescript/tsconfig.json", "compilerOptions": { - "lib": ["es2018"], + "lib": [ + "es2018", + "ES2019.Array" + ], "target": "es2018", "skipLibCheck": true, "forceConsistentCasingInFileNames": true, From 3bfcf8c8bcc1b7a96826a18559fc0ea5bca682ff Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Tue, 1 Jun 2021 21:21:18 +0000 Subject: [PATCH 5/6] ref: Lint fixes --- README.md | 16 ++++++++-------- tsconfig.build.json | 5 +---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 29aaad227..3178c5f3c 100644 --- a/README.md +++ b/README.md @@ -803,14 +803,14 @@ _none_ **Configuration** -| Option | Description | -| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `apps` | List of `app` configs as a dict, keyed by their canonical names (example: `app:craft`) | -| `sdks` | List of `sdk` configs as a dict, keyed by their canonical names (example: `maven:io.sentry:sentry`) | -| `(sdks|apps).urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | -| `(sdks|apps).linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | -| `(sdks|apps).checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of `sha256`, `sha384`, and `sha512`) and format (`base64` and `hex`). | -| `(sdks|apps).onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | +| Option | Description | +| ------ | --------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `apps` | List of `app` configs as a dict, keyed by their canonical names (example: `app:craft`) | +| `sdks` | List of `sdk` configs as a dict, keyed by their canonical names (example: `maven:io.sentry:sentry`) | +| `(sdks | apps).urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | +| `(sdks | apps).linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | +| `(sdks | apps).checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of `sha256`, `sha384`, and `sha512`) and format (`base64` and `hex`). | +| `(sdks | apps).onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | **Example** diff --git a/tsconfig.build.json b/tsconfig.build.json index c6a9d3b9f..f0f957aeb 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -1,10 +1,7 @@ { "extends": "@sentry/typescript/tsconfig.json", "compilerOptions": { - "lib": [ - "es2018", - "ES2019.Array" - ], + "lib": ["es2018", "ES2019.Array"], "target": "es2018", "skipLibCheck": true, "forceConsistentCasingInFileNames": true, From 9dbf675203d257183d50d1b81557c36a6a365bb1 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 2 Jun 2021 00:51:08 +0300 Subject: [PATCH 6/6] fix wonky readme --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 3178c5f3c..bb816e05e 100644 --- a/README.md +++ b/README.md @@ -803,14 +803,14 @@ _none_ **Configuration** -| Option | Description | -| ------ | --------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `apps` | List of `app` configs as a dict, keyed by their canonical names (example: `app:craft`) | -| `sdks` | List of `sdk` configs as a dict, keyed by their canonical names (example: `maven:io.sentry:sentry`) | -| `(sdks | apps).urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | -| `(sdks | apps).linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | -| `(sdks | apps).checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of `sha256`, `sha384`, and `sha512`) and format (`base64` and `hex`). | -| `(sdks | apps).onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | +| Option | Description | +| ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `apps` | List of `app` configs as a dict, keyed by their canonical names (example: `app:craft`) | +| `sdks` | List of `sdk` configs as a dict, keyed by their canonical names (example: `maven:io.sentry:sentry`) | +| `(sdks\|apps).urlTemplate` | **optional** URL template that will be used to generate download links for "app" package type. | +| `(sdks\|apps).linkPrereleases` | **optional** Update package versions even if the release is a preview release, "false" by default. | +| `(sdks\|apps).checksums` | **optional** A list of checksums that will be computed for matched files (see `includeNames`). Every checksum entry is an object with two attributes: algorithm (one of `sha256`, `sha384`, and `sha512`) and format (`base64` and `hex`). | +| `(sdks\|apps).onlyIfPresent` | **optional** A file pattern. The target will be executed _only_ when the matched file is found. | **Example**