Skip to content
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

feat: add support for fetching a greeting CTA as part of the update flow MONGOSH-1897 #2254

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions config/cta.conf.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

module.exports = {
awsAccessKeyId: process.env.DOWNLOAD_CENTER_AWS_KEY,
awsSecretAccessKey: process.env.DOWNLOAD_CENTER_AWS_SECRET,
ctas: {
// Define the ctas per version here. '*' is the default cta which will be shown if there's no specific cta
// for the current version.
// '*': {
// runs: [
// { text: 'Example', style: 'bold' },
// ]
// },
// '1.2.3': {
// runs: [
// { text: 'Example', style: 'mongosh:uri' },
// ]
// }
},
isDryRun: false,
}
1 change: 1 addition & 0 deletions configs/eslint-config-mongosh/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ module.exports = {
...common.testRules,
...extraJSRules,
...extraTypescriptRules,
'@typescript-eslint/no-non-null-assertion': 'off',
},
},
],
Expand Down
87 changes: 72 additions & 15 deletions packages/build/src/download-center/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
} from '@mongodb-js/dl-center/dist/download-center-config';
import {
ARTIFACTS_BUCKET,
ARTIFACTS_FOLDER,
JSON_FEED_ARTIFACT_KEY,
ARTIFACTS_URL_PUBLIC_BASE,
CONFIGURATION_KEY,
CONFIGURATIONS_BUCKET,
Expand All @@ -32,6 +32,24 @@ import path from 'path';
import semver from 'semver';
import { hashListFiles } from '../run-download-and-list-artifacts';

async function getCurrentJsonFeed(
dlcenterArtifacts: DownloadCenterCls
): Promise<JsonFeed | undefined> {
let existingJsonFeedText;
try {
existingJsonFeedText = await dlcenterArtifacts.downloadAsset(
JSON_FEED_ARTIFACT_KEY
);
} catch (err: any) {
console.warn('Failed to get existing JSON feed text', err);
if (err?.code !== 'NoSuchKey') throw err;
}

return existingJsonFeedText
? JSON.parse(existingJsonFeedText.toString())
: undefined;
}

export async function createAndPublishDownloadCenterConfig(
outputDir: string,
packageInformation: PackageInformationProvider,
Expand Down Expand Up @@ -80,20 +98,8 @@ export async function createAndPublishDownloadCenterConfig(
accessKeyId: awsAccessKeyId,
secretAccessKey: awsSecretAccessKey,
});
const jsonFeedArtifactkey = `${ARTIFACTS_FOLDER}/mongosh.json`;
let existingJsonFeedText;
try {
existingJsonFeedText = await dlcenterArtifacts.downloadAsset(
jsonFeedArtifactkey
);
} catch (err: any) {
console.warn('Failed to get existing JSON feed text', err);
if (err?.code !== 'NoSuchKey') throw err;
}

const existingJsonFeed: JsonFeed | undefined = existingJsonFeedText
? JSON.parse(existingJsonFeedText.toString())
: undefined;
const existingJsonFeed = await getCurrentJsonFeed(dlcenterArtifacts);
const injectedJsonFeed: JsonFeed | undefined = injectedJsonFeedFile
? JSON.parse(await fs.readFile(injectedJsonFeedFile, 'utf8'))
: undefined;
Expand Down Expand Up @@ -122,12 +128,42 @@ export async function createAndPublishDownloadCenterConfig(
await Promise.all([
dlcenter.uploadConfig(CONFIGURATION_KEY, config),
dlcenterArtifacts.uploadAsset(
jsonFeedArtifactkey,
JSON_FEED_ARTIFACT_KEY,
JSON.stringify(newJsonFeed, null, 2)
),
]);
}

export async function updateJsonFeedCTA(
config: UpdateCTAConfig,
DownloadCenter: typeof DownloadCenterCls = DownloadCenterCls
) {
const dlcenterArtifacts = new DownloadCenter({
bucket: ARTIFACTS_BUCKET,
accessKeyId: config.awsAccessKeyId,
secretAccessKey: config.awsSecretAccessKey,
});

const jsonFeed = await getCurrentJsonFeed(dlcenterArtifacts);
if (!jsonFeed) {
throw new Error('No existing JSON feed found');
}

jsonFeed.cta = config.ctas['*'];
for (const version of jsonFeed.versions) {
version.cta = config.ctas[version.version];
}

const patchedJsonFeed = JSON.stringify(jsonFeed, null, 2);
if (config.isDryRun) {
console.warn('Not uploading JSON feed in dry-run mode');
console.warn(`Patched JSON feed: ${patchedJsonFeed}`);
return;
}

await dlcenterArtifacts.uploadAsset(JSON_FEED_ARTIFACT_KEY, patchedJsonFeed);
}

export function getUpdatedDownloadCenterConfig(
downloadedConfig: DownloadCenterConfig,
getVersionConfig: () => ReturnType<typeof createVersionConfig>
Expand Down Expand Up @@ -201,13 +237,32 @@ export function createVersionConfig(
};
}

// TODO: this is duplicated in update-notification-manager.ts
interface GreetingCTADetails {
chunks: {
text: string;
style: string; // TODO: this is actually clr.ts/StyleDefinition
}[];
}

export interface UpdateCTAConfig {
ctas: {
[version: string]: GreetingCTADetails;
};
awsAccessKeyId: string;
awsSecretAccessKey: string;
isDryRun: boolean;
}

interface JsonFeed {
versions: JsonFeedVersionEntry[];
cta?: GreetingCTADetails;
}

interface JsonFeedVersionEntry {
version: string;
downloads: JsonFeedDownloadEntry[];
cta?: GreetingCTADetails;
}

interface JsonFeedDownloadEntry {
Expand Down Expand Up @@ -275,6 +330,8 @@ function mergeFeeds(...args: (JsonFeed | undefined)[]): JsonFeed {
if (index === -1) newFeed.versions.unshift(version);
else newFeed.versions.splice(index, 1, version);
}

newFeed.cta = feed?.cta ?? newFeed.cta;
}
newFeed.versions.sort((a, b) => semver.rcompare(a.version, b.version));
return newFeed;
Expand Down
15 changes: 10 additions & 5 deletions packages/build/src/download-center/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,30 @@ const fallback = require('./fallback.json');
/**
* The S3 bucket for download center configurations.
*/
export const CONFIGURATIONS_BUCKET = 'info-mongodb-com' as const;
export const CONFIGURATIONS_BUCKET = 'info-mongodb-com';

/**
* The S3 object key for the download center configuration.
*/
export const CONFIGURATION_KEY =
'com-download-center/mongosh.multiversion.json' as const;
'com-download-center/mongosh.multiversion.json';

/**
* The S3 bucket for download center artifacts.
*/
export const ARTIFACTS_BUCKET = 'downloads.10gen.com' as const;
export const ARTIFACTS_BUCKET = 'downloads.10gen.com';

/**
* The S3 "folder" for uploaded artifacts.
*/
export const ARTIFACTS_FOLDER = 'compass' as const;
export const ARTIFACTS_FOLDER = 'compass';

/**
* The S3 artifact key for the versions JSON feed.
*/
export const JSON_FEED_ARTIFACT_KEY = `${ARTIFACTS_FOLDER}/mongosh.json`;

export const ARTIFACTS_URL_PUBLIC_BASE =
'https://downloads.mongodb.com/compass/' as const;
'https://downloads.mongodb.com/compass/';

export const ARTIFACTS_FALLBACK = Object.freeze(fallback);
63 changes: 41 additions & 22 deletions packages/build/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import { triggerRelease } from './local';
import type { ReleaseCommand } from './release';
import { release } from './release';
import type { Config, PackageVariant } from './config';
import { updateJsonFeedCTA, UpdateCTAConfig } from './download-center';

export { getArtifactUrl, downloadMongoDb };

const validCommands: (ReleaseCommand | 'trigger-release')[] = [
const validCommands: (ReleaseCommand | 'trigger-release' | 'update-cta')[] = [
'bump',
'compile',
'package',
Expand All @@ -20,11 +21,12 @@ const validCommands: (ReleaseCommand | 'trigger-release')[] = [
'download-crypt-shared-library',
'download-and-list-artifacts',
'trigger-release',
'update-cta',
] as const;

const isValidCommand = (
cmd: string
): cmd is ReleaseCommand | 'trigger-release' =>
): cmd is ReleaseCommand | 'trigger-release' | 'update-cta' =>
(validCommands as string[]).includes(cmd);

if (require.main === module) {
Expand All @@ -38,29 +40,46 @@ if (require.main === module) {
);
}

if (command === 'trigger-release') {
await triggerRelease(process.argv.slice(3));
} else {
const config: Config = require(path.join(
__dirname,
'..',
'..',
'..',
'config',
'build.conf.js'
));
switch (command) {
case 'trigger-release':
await triggerRelease(process.argv.slice(3));
break;
case 'update-cta':
const ctaConfig: UpdateCTAConfig = require(path.join(
__dirname,
'..',
'..',
'..',
'config',
'cta.conf.js'
));

const cliBuildVariant = process.argv
.map((arg) => /^--build-variant=(.+)$/.exec(arg))
.filter(Boolean)[0];
if (cliBuildVariant) {
config.packageVariant = cliBuildVariant[1] as PackageVariant;
validatePackageVariant(config.packageVariant);
}
ctaConfig.isDryRun ||= process.argv.includes('--dry-run');

config.isDryRun ||= process.argv.includes('--dry-run');
await updateJsonFeedCTA(ctaConfig);
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the only step in this approach that I don't fully follow – is the idea that we'd locally run the update-cta command? How would we get the credentials to access the relevant S3 bucket?

We're definitely moving towards a world in which less and less access will be given to individual developers' machines, so I don't think this is something we could easily do, and we may just need to fall back to a variant in which we update the CTA entries along with the version feed during releases only – I think that would be okay though, we're generally free to do mongosh releases whenever necessary.

(In the future, we'll need to make sure that even this kind of flow where we update an existing file in S3 as part of the releases is supported – see e.g. https://jira.mongodb.org/browse/WRITING-16730)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could decouple releases from CTA updates. I had the following in mind - let me know how it sounds:

  1. Updates are published when cta.conf.js is updated. This will have to go through the normal PR flow of accepting changes.
  2. When the change is merged into main, a github action triggers that has access to the S3 credentials through environment secrets and runs the update-cta command.
  3. The secrets are stored in an environment - let's say "production" - that will have protection rules configured, which means the deployment will be paused until the protection rules are satisfied (e.g. approval required by specified people/wait specified number of hours).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a github action triggers that has access to the S3 credentials through environment secrets and runs the update-cta command.

We can do that but I think we'd be going for Evergreen rather than GHA here? That's the only platform I'd really expect to be available to access release infrastructure in the long run. I think we'd encounter some resistance if we wanted to put downloads.mongodb.com secrets into another CI environment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could sort it out - my personal preference is towards GHA and we already have a handful of products being signed and released with secrets stored in GHA, as well as all our code being hosted by github. That being said, if there's a strong desire to only do that on evergreen, we could work with those limitations - the important part here is that the flow of publishing CTA updates happens on CI and in a somewhat automated and reviewable fashion.

default:
const config: Config = require(path.join(
__dirname,
'..',
'..',
'..',
'config',
'build.conf.js'
));

await release(command, config);
const cliBuildVariant = process.argv
.map((arg) => /^--build-variant=(.+)$/.exec(arg))
.filter(Boolean)[0];
if (cliBuildVariant) {
config.packageVariant = cliBuildVariant[1] as PackageVariant;
validatePackageVariant(config.packageVariant);
}

config.isDryRun ||= process.argv.includes('--dry-run');

await release(command, config);
break;
}
})().then(
() => process.exit(0),
Expand Down
14 changes: 12 additions & 2 deletions packages/cli-repl/src/cli-repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,11 @@ export class CliRepl implements MongoshIOProvider {
markTime(TimingCategories.DriverSetup, 'completed SP setup');
const initialized = await this.mongoshRepl.initialize(
initialServiceProvider,
await this.getMoreRecentMongoshVersion()
{
moreRecentMongoshVersion: await this.getMoreRecentMongoshVersion(),
currentVersionCTA:
await this.updateNotificationManager.getGreetingCTAForCurrentVersion(),
}
);
markTime(TimingCategories.REPLInstantiation, 'initialized mongosh repl');
this.injectReplFunctions();
Expand Down Expand Up @@ -1264,21 +1268,27 @@ export class CliRepl implements MongoshIOProvider {
const updateURL = (await this.getConfig('updateURL')).trim();
if (!updateURL) return;

const { version: currentVersion } = require('../package.json');
const localFilePath = this.shellHomeDirectory.localPath(
'update-metadata.json'
);

this.bus.emit('mongosh:fetching-update-metadata', {
updateURL,
localFilePath,
currentVersion,
});
await this.updateNotificationManager.fetchUpdateMetadata(
updateURL,
localFilePath
localFilePath,
currentVersion
);
this.bus.emit('mongosh:fetching-update-metadata-complete', {
latest:
await this.updateNotificationManager.getLatestVersionIfMoreRecent(''),
currentVersion,
hasGreetingCTA:
!!(await this.updateNotificationManager.getGreetingCTAForCurrentVersion()),
});
} catch (err: any) {
this.bus.emit('mongosh:error', err, 'startup');
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-repl/src/clr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export type StyleDefinition =
/** Optionally colorize a string, given a set of style definition(s). */
export default function colorize(
text: string,
style: StyleDefinition,
style: StyleDefinition | undefined,
options: { colors: boolean }
): string {
if (options.colors) {
Expand Down
Loading
Loading