Paths

Table of Contentst

-
keyserver/
-
Dockerfile
-
docker-compose.yml

yarn cleaninstall from Docker image
ClosedPublic
Actions

Authored by ashoat on Apr 12 2022, 8:27 AM.

Details

Reviewers

tomek
• jim

Commits

rCOMMca61a68e4d26: yarn cleaninstall from Docker image

Summary

For proper build caching, we want to first handle the node_modules, and then later copy in the actual files.

Depends on D3712

Test Plan

Make sure I can build the Node.js keyserver from Docker

Diff Detail

Repository

rCOMM Comm

Branch

ashoat/dockerize_node

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Apr 12 2022, 8:27 AM

Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 12 2022, 8:27 AM

Making @jimpo a blocking reviewer to make sure I get his perspective on the Dockerfile. In particular, curious if using multiple subsequent COPY commands is bad

Harbormaster completed remote builds in B8126: Diff 11354.Apr 12 2022, 8:37 AM

ashoat requested review of this revision.Apr 12 2022, 8:37 AM

tomek accepted this revision.Apr 15 2022, 8:43 AM

• jim requested changes to this revision.Apr 18 2022, 5:15 AM

• jim added inline comments.

keyserver/Dockerfile
9 ↗	(On Diff #11354)	I'd do the `update-alternatives` in a separate RUN statement -- no need for them to be together
keyserver/docker-compose.yml
1 ↗	(On Diff #11354)	Version 1 is deprecated... should be "3" at least
8 ↗	(On Diff #11354)	Any reason the default container name isn't OK? It's probably "keyserver_node_1".

This revision now requires changes to proceed.Apr 18 2022, 5:15 AM

ashoat added inline comments.Apr 18 2022, 9:56 PM

keyserver/Dockerfile
9 ↗	(On Diff #11354)	Okay, will separate. Just wondering – why is it that the other commands (`apt-get`-related) are better to keep together, but it's fine to have `update-alternatives` be separate?
keyserver/docker-compose.yml
1 ↗	(On Diff #11354)	This is really bad, not sure why I didn't just look at the `version` string in `services/docker-compose.yml`. That said, the docs are pretty confusing: The Compose spec merges the legacy 2.x and 3.x versions, aggregating properties across these formats and is implemented by Compose 1.27.0+. `services/docker-compose.yml` declares version 3.9, which I can't find anywhere in the docs. I did find this question, and one of the comments there references the Docker release notes to indicate 3.9 has been released. So I guess I'll use that...
8 ↗	(On Diff #11354)	Eh why not have something better? To be honest I included this based on a template somewhere, but I'm not a huge fan of the `_1` suffix in the default that you quoted

Review

Harbormaster completed remote builds in B8284: Diff 11580.Apr 18 2022, 10:09 PM

• jim requested changes to this revision.Apr 18 2022, 11:51 PM

• jim added inline comments.

keyserver/Dockerfile
9 ↗	(On Diff #11354)	Because the `apt-get update` adds a bunch of data to the apt cache and the `rm` removes it, so if these were separated you'd get a layer with the apt cache which you don't want. Basically, a `RUN` should do stuff and clean up after itself. `update-alternatives` isn't part of the cleanup so it can be separate.
keyserver/docker-compose.yml
8 ↗	(On Diff #11354)	Not a big deal, you can keep if you want but it's non-standard. And the default naming scheme lets you know which services are in the same docker-compose because they are grouped with the directory name. "keyserver_node" vs "node-keyserver". And in this case keyserver is a singleton so it makes sense that you'd drop the "_1" but it's part of the default name so you can scale up the number of containers per service. That's prob incompatible with port publishing though.

This revision now requires changes to proceed.Apr 18 2022, 11:51 PM

• jim accepted this revision.Apr 18 2022, 11:51 PM

This revision is now accepted and ready to land.Apr 18 2022, 11:51 PM

Closed by commit rCOMMca61a68e4d26: yarn cleaninstall from Docker image (authored by ashoat). · Explain WhyApr 19 2022, 9:53 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMca61a68e4d26: yarn cleaninstall from Docker image.

ashoat mentioned this in D3847: [keyserver] Get rid of container_name for Node in docker-compose.yml.Apr 25 2022, 7:46 PM

ashoat mentioned this in rCOMMf7c00984513b: [keyserver] Get rid of container_name for Node in docker-compose.yml.Apr 27 2022, 11:40 AM

Revision Contents
Changeset List

Path

Size

keyserver/

Dockerfile

25 lines

docker-compose.yml

10 lines

Diff 11580

View Options

keyserver/Dockerfile

Show First 20 Lines • Show All 64 Lines • ▼ Show 20 Lines	async function apnPush({
invariant(apnProvider, `keyserver/secrets/${pushProfile}.json should exist`);		invariant(apnProvider, `keyserver/secrets/${pushProfile}.json should exist`);

const results = await Promise.all(		const results = await Promise.all(
targetedNotifications.map(({ notification, deviceToken }) => {		targetedNotifications.map(({ notification, deviceToken }) => {
return apnProvider.send(notification, deviceToken);		return apnProvider.send(notification, deviceToken);
}),		}),
);		);

const mergedResults = { sent: [], failed: [] };		const errors: Array<ResponseFailure> = [];
for (const result of results) {		for (const result of results) {
mergedResults.sent.push(...result.sent);		errors.push(...result.failed);
mergedResults.failed.push(...result.failed);
}		}

const errors = [];		const invalidTokens: Array<string> = [];
const invalidTokens = [];		for (const error of errors) {
for (const error of mergedResults.failed) {
errors.push(error);
/* eslint-disable eqeqeq */		/* eslint-disable eqeqeq */
if (		if (
error.status == apnTokenInvalidationErrorCode \|\|		error.status == apnTokenInvalidationErrorCode \|\|
(error.status == apnBadRequestErrorCode &&		(error.status == apnBadRequestErrorCode &&
error.response.reason === apnBadTokenErrorString)		error.response.reason === apnBadTokenErrorString)
) {		) {
invalidTokens.push(error.device);		invalidTokens.push(error.device);
}		}
/* eslint-enable eqeqeq */		/* eslint-enable eqeqeq */
}		}
if (invalidTokens.length > 0) {		if (invalidTokens.length > 0) {
return { errors, invalidTokens };		return { errors, invalidTokens };
} else if (errors.length > 0) {		} else if (errors.length > 0) {
return { errors };		return { errors };
} else {		} else {
return { success: true };		return { success: true };
}		}
}		}

type FCMPushResult = {		type WritableFCMPushResult = {
+success?: true,		success?: true,
+fcmIDs?: $ReadOnlyArray<string>,		fcmIDs?: $ReadOnlyArray<string>,
+errors?: $ReadOnlyArray<FirebaseError>,		errors?: $ReadOnlyArray<FirebaseError \| mixed>,
+invalidTokens?: $ReadOnlyArray<string>,		invalidTokens?: $ReadOnlyArray<string>,
};		};
		export type FCMPushResult = $ReadOnly<WritableFCMPushResult>;
async function fcmPush({		async function fcmPush({
targetedNotifications,		targetedNotifications,
collapseKey,		collapseKey,
codeVersion,		codeVersion,
}: {		}: {
+targetedNotifications: $ReadOnlyArray<TargetedAndroidNotification>,		+targetedNotifications: $ReadOnlyArray<TargetedAndroidNotification>,
+codeVersion: ?number,		+codeVersion: ?number,
+collapseKey?: ?string,		+collapseKey?: ?string,
Show All 11 Lines	async function fcmPush({
if (collapseKey) {		if (collapseKey) {
options.collapseKey = collapseKey;		options.collapseKey = collapseKey;
}		}
// firebase-admin is extremely barebones and has a lot of missing or poorly		// firebase-admin is extremely barebones and has a lot of missing or poorly
// thought-out functionality. One of the issues is that if you send a		// thought-out functionality. One of the issues is that if you send a
// multicast messages and one of the device tokens is invalid, the resultant		// multicast messages and one of the device tokens is invalid, the resultant
// won't explain which of the device tokens is invalid. So we're forced to		// won't explain which of the device tokens is invalid. So we're forced to
// avoid the multicast functionality and call it once per deviceToken.		// avoid the multicast functionality and call it once per deviceToken.
const promises = [];		const results = await Promise.all(
for (const { notification, deviceToken } of targetedNotifications) {		targetedNotifications.map(({ notification, deviceToken }) => {
promises.push(		return fcmSinglePush(fcmProvider, notification, deviceToken, options);
fcmSinglePush(fcmProvider, notification, deviceToken, options),		}),
);		);
}
const pushResults = await Promise.all(promises);

const errors = [];		const errors = [];
const ids = [];		const ids = [];
const invalidTokens = [];		const invalidTokens = [];
for (let i = 0; i < pushResults.length; i++) {		for (let i = 0; i < results.length; i++) {
const pushResult = pushResults[i];		const pushResult = results[i];
for (const error of pushResult.errors) {		for (const error of pushResult.errors) {
errors.push(error);		errors.push(error.error);
if (fcmTokenInvalidationErrors.has(error.errorInfo.code)) {		const errorCode =
		error.type === 'firebase_error'
		? error.error.errorInfo.code
		: undefined;
		if (errorCode && fcmTokenInvalidationErrors.has(errorCode)) {
invalidTokens.push(targetedNotifications[i].deviceToken);		invalidTokens.push(targetedNotifications[i].deviceToken);
}		}
}		}
for (const id of pushResult.fcmIDs) {		for (const id of pushResult.fcmIDs) {
ids.push(id);		ids.push(id);
}		}
}		}

const result = {};		const result: WritableFCMPushResult = {};
if (ids.length > 0) {		if (ids.length > 0) {
result.fcmIDs = ids;		result.fcmIDs = ids;
}		}
if (errors.length > 0) {		if (errors.length > 0) {
result.errors = errors;		result.errors = errors;
} else {		} else {
result.success = true;		result.success = true;
}		}
if (invalidTokens.length > 0) {		if (invalidTokens.length > 0) {
result.invalidTokens = invalidTokens;		result.invalidTokens = invalidTokens;
}		}
return { ...result };		return result;
}		}

		type FCMSinglePushError =
		\| { +type: 'firebase_error', +error: FirebaseError }
		\| { +type: 'exception', +error: mixed };
		type FCMSinglePushResult = {
		+fcmIDs: $ReadOnlyArray<string>,
		+errors: $ReadOnlyArray<FCMSinglePushError>,
		};
async function fcmSinglePush(		async function fcmSinglePush(
provider: FirebaseApp,		provider: FirebaseApp,
notification: Object,		notification: Object,
deviceToken: string,		deviceToken: string,
options: Object,		options: Object,
) {		): Promise<FCMSinglePushResult> {
try {		try {
const deliveryResult = await provider		const deliveryResult = await provider
.messaging()		.messaging()
.sendToDevice(deviceToken, notification, options);		.sendToDevice(deviceToken, notification, options);
const errors = [];		const errors = [];
const ids = [];		const ids = [];
for (const fcmResult of deliveryResult.results) {		for (const fcmResult of deliveryResult.results) {
if (fcmResult.error) {		if (fcmResult.error) {
errors.push(fcmResult.error);		errors.push({ type: 'firebase_error', error: fcmResult.error });
} else if (fcmResult.messageId) {		} else if (fcmResult.messageId) {
ids.push(fcmResult.messageId);		ids.push(fcmResult.messageId);
}		}
}		}
return { fcmIDs: ids, errors };		return { fcmIDs: ids, errors };
} catch (e) {		} catch (e) {
return { fcmIDs: [], errors: [e] };		return { fcmIDs: [], errors: [{ type: 'exception', error: e }] };
}		}
}		}

async function getUnreadCounts(		async function getUnreadCounts(
userIDs: string[],		userIDs: string[],
): Promise<{ [userID: string]: number }> {		): Promise<{ [userID: string]: number }> {
const visPermissionExtractString = `$.${threadPermissions.VISIBLE}.value`;		const visPermissionExtractString = `$.${threadPermissions.VISIBLE}.value`;
const notificationExtractString = `$.${threadSubscriptions.home}`;		const notificationExtractString = `$.${threadSubscriptions.home}`;
const query = SQL`		const query = SQL`
SELECT user, COUNT(thread) AS unread_count		SELECT user, COUNT(thread) AS unread_count
FROM memberships		FROM memberships
WHERE user IN (${userIDs}) AND last_message > last_read_message		WHERE user IN (${userIDs}) AND last_message > last_read_message
AND role > 0		AND role > 0
AND JSON_EXTRACT(permissions, ${visPermissionExtractString})		AND JSON_EXTRACT(permissions, ${visPermissionExtractString})
AND JSON_EXTRACT(subscription, ${notificationExtractString})		AND JSON_EXTRACT(subscription, ${notificationExtractString})
GROUP BY user		GROUP BY user
`;		`;
const [result] = await dbQuery(query);		const [result] = await dbQuery(query);
const usersToUnreadCounts = {};		const usersToUnreadCounts: { [string]: number } = {};
for (const row of result) {		for (const row of result) {
usersToUnreadCounts[row.user.toString()] = row.unread_count;		usersToUnreadCounts[row.user.toString()] = row.unread_count;
}		}
for (const userID of userIDs) {		for (const userID of userIDs) {
if (usersToUnreadCounts[userID] === undefined) {		if (usersToUnreadCounts[userID] === undefined) {
usersToUnreadCounts[userID] = 0;		usersToUnreadCounts[userID] = 0;
}		}
}		}
return usersToUnreadCounts;		return usersToUnreadCounts;
}		}

export type WebPushError = {		export type WebPushError = {
+statusCode: number,		+statusCode: number,
+headers: { +[string]: string },		+headers: { +[string]: string },
+body: string,		+body: string,
};		};
type WebPushResult = {		type WritableWebPushResult = {
+success?: true,		success?: true,
+errors?: $ReadOnlyArray<WebPushError>,		errors?: $ReadOnlyArray<WebPushError>,
+invalidTokens?: $ReadOnlyArray<string>,		invalidTokens?: $ReadOnlyArray<string>,
		};
		type WebPushResult = $ReadOnly<WritableWebPushResult>;
		type WebPushAttempt = {
		+error?: WebPushError,
};		};
async function webPush(		async function webPush(
targetedNotifications: $ReadOnlyArray<TargetedWebNotification>,		targetedNotifications: $ReadOnlyArray<TargetedWebNotification>,
): Promise<WebPushResult> {		): Promise<WebPushResult> {
await ensureWebPushInitialized();		await ensureWebPushInitialized();

const pushResults = await Promise.all(		const pushResults: $ReadOnlyArray<WebPushAttempt> = await Promise.all(
targetedNotifications.map(		targetedNotifications.map(
async ({ notification, deviceToken: deviceTokenString }) => {		async ({ notification, deviceToken: deviceTokenString }) => {
const deviceToken: PushSubscriptionJSON = JSON.parse(deviceTokenString);		const deviceToken: PushSubscriptionJSON = JSON.parse(deviceTokenString);
const notificationString = JSON.stringify(notification);		const notificationString = JSON.stringify(notification);
try {		try {
await webpush.sendNotification(deviceToken, notificationString);		await webpush.sendNotification(deviceToken, notificationString);
} catch (error) {		} catch (error) {
return { error };		return ({ error }: WebPushAttempt);
}		}
return {};		return {};
},		},
),		),
);		);

const errors = [];		const errors = [];
const invalidTokens = [];		const invalidTokens = [];
const deviceTokens = targetedNotifications.map(		const deviceTokens = targetedNotifications.map(
({ deviceToken }) => deviceToken,		({ deviceToken }) => deviceToken,
);		);
for (let i = 0; i < pushResults.length; i++) {		for (let i = 0; i < pushResults.length; i++) {
const pushResult = pushResults[i];		const pushResult = pushResults[i];
if (pushResult.error) {		const { error } = pushResult;
errors.push(pushResult.error);		if (error) {
if (webInvalidTokenErrorCodes.includes(pushResult.error.statusCode)) {		errors.push(error);
		if (webInvalidTokenErrorCodes.includes(error.statusCode)) {
invalidTokens.push(deviceTokens[i]);		invalidTokens.push(deviceTokens[i]);
}		}
}		}
}		}

const result = {};		const result: WritableWebPushResult = {};
if (errors.length > 0) {		if (errors.length > 0) {
result.errors = errors;		result.errors = errors;
} else {		} else {
result.success = true;		result.success = true;
}		}
if (invalidTokens.length > 0) {		if (invalidTokens.length > 0) {
result.invalidTokens = invalidTokens;		result.invalidTokens = invalidTokens;
}		}
return { ...result };		return result;
}		}

export type WNSPushError = any \| string \| Response;		export type WNSPushError = any \| string \| Response;
type WNSPushResult = {		type WritableWNSPushResult = {
+success?: true,		success?: true,
+wnsIDs?: $ReadOnlyArray<string>,		wnsIDs?: $ReadOnlyArray<string>,
+errors?: $ReadOnlyArray<WNSPushError>,		errors?: $ReadOnlyArray<WNSPushError>,
+invalidTokens?: $ReadOnlyArray<string>,		invalidTokens?: $ReadOnlyArray<string>,
};		};
		type WNSPushResult = $ReadOnly<WritableWNSPushResult>;
async function wnsPush(		async function wnsPush(
targetedNotifications: $ReadOnlyArray<TargetedWNSNotification>,		targetedNotifications: $ReadOnlyArray<TargetedWNSNotification>,
): Promise<WNSPushResult> {		): Promise<WNSPushResult> {
const token = await getWNSToken();		const token = await getWNSToken();
if (!token && process.env.NODE_ENV === 'development') {		if (!token && process.env.NODE_ENV === 'development') {
console.log(`no keyserver/secrets/wns_config.json so ignoring notifs`);		console.log(`no keyserver/secrets/wns_config.json so ignoring notifs`);
return { success: true };		return { success: true };
}		}
Show All 31 Lines	if (pushResult.error) {
) {		) {
invalidTokens.push(deviceTokens[i]);		invalidTokens.push(deviceTokens[i]);
}		}
} else {		} else {
notifIDs.push(pushResult.wnsID);		notifIDs.push(pushResult.wnsID);
}		}
}		}

const result = {};		const result: WritableWNSPushResult = {};
if (notifIDs.length > 0) {		if (notifIDs.length > 0) {
result.wnsIDs = notifIDs;		result.wnsIDs = notifIDs;
}		}
if (errors.length > 0) {		if (errors.length > 0) {
result.errors = errors;		result.errors = errors;
} else {		} else {
result.success = true;		result.success = true;
}		}
if (invalidTokens.length > 0) {		if (invalidTokens.length > 0) {
result.invalidTokens = invalidTokens;		result.invalidTokens = invalidTokens;
}		}
return { ...result };		return result;
}		}

async function wnsSinglePush(token: string, notification: string, url: string) {		async function wnsSinglePush(token: string, notification: string, url: string) {
const parsedURL = new URL(url);		const parsedURL = new URL(url);
const domain = parsedURL.hostname.split('.').slice(-3);		const domain = parsedURL.hostname.split('.').slice(-3);
if (		if (
domain[0] !== 'notify' \|\|		domain[0] !== 'notify' \|\|
domain[1] !== 'windows' \|\|		domain[1] !== 'windows' \|\|
▲ Show 20 Lines • Show All 63 Lines • Show Last 20 Lines