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

Docker improvements #5946

Merged
merged 20 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
4 changes: 4 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.env
icon_cache
logs
dist
5 changes: 2 additions & 3 deletions .env.test
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
TZ="UTC"
ROBOCHIMP_DATABASE_URL="postgresql://postgres:postgres@localhost:5435/robochimp_integration_test?connection_limit=500&pool_timeout=0&schema=public"
DATABASE_URL="postgresql://postgres:postgres@localhost:5435/osb_integration_test?connection_limit=500&pool_timeout=0&schema=public"
CLIENT_ID=111398433321891634
BOT_TOKEN=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
TEST=true
TEST=true
CI=true
36 changes: 7 additions & 29 deletions .github/workflows/integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,15 @@ on:

jobs:
test:
name: Node v${{ matrix.node_version }} - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
runs-on: ubuntu-latest
timeout-minutes: 10
strategy:
matrix:
node_version: [20.15.0]
os: [ubuntu-latest]

steps:
- name: Install System Packages
run: sudo apt-get install -y build-essential libpq-dev

- name: Checkout Project
uses: actions/checkout@v4
- run: corepack enable && corepack install
- name: Use Node.js ${{ matrix.node_version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node_version }}
cache: yarn
- name: Restore CI Cache
uses: actions/cache@v4
with:
path: node_modules
key: ${{ runner.os }}-${{ matrix.node_version }}-${{ hashFiles('**/yarn.lock') }}
- name: Install Dependencies
run: yarn --immutable
- name: Copy Configuration
run: |
pushd src &&
cp config.example.ts config.ts &&
popd && cp .env.test .env
- name: Test
run: yarn test:integration

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

- name: Run Integration Tests
run: docker-compose up --build --abort-on-container-exit
Copy link
Contributor

Choose a reason for hiding this comment

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

The command docker-compose up --build --abort-on-container-exit might not be appropriate for integration tests as it builds and runs all services, which could include non-test related services. Consider specifying only the necessary services or adjusting the docker-compose configuration to ensure only relevant services are included during the test phase.

23 changes: 20 additions & 3 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,29 @@
services:
osb:
db:
image: postgres:16-alpine
command: -c 'max_connections=1000'
restart: always
container_name: osb_database
ports:
- "5435:3001"
- "5435:5435"
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
PGPORT: 3001
PGPORT: 5435
volumes:
- postgres_data:/var/lib/postgresql/data

app:
build:
context: .
args:
GIT_BRANCH: ${GIT_BRANCH}
depends_on:
- db
environment:
ROBOCHIMP_DATABASE_URL: postgresql://postgres:postgres@db:5435/robochimp_integration_test?connection_limit=500&pool_timeout=0&schema=public
DATABASE_URL: postgresql://postgres:postgres@db:5435/osb_integration_test?connection_limit=500&pool_timeout=0&schema=public
WAIT_HOSTS: db:5435

volumes:
postgres_data:
30 changes: 30 additions & 0 deletions dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
FROM node:20.15.0-alpine AS base
WORKDIR /usr/src/app
ENV CI=true
RUN apk add --no-cache dumb-init python3 g++ make git
RUN corepack enable

COPY yarn.lock package.json .yarnrc.yml ./
Copy link
Contributor

Choose a reason for hiding this comment

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

The COPY command is used without specifying ownership, which might lead to files being owned by the root user. This can pose a security risk if the application is expected to run as a non-root user but has access to root-owned files. Consider using the --chown=node:node option with the COPY command to explicitly set file ownership.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the .yarn/ directory copy step could potentially break the yarn install command if the project relies on specific configurations or patches stored in this directory. Consider restoring this step if it's crucial for the build process.


ENTRYPOINT ["dumb-init", "--"]

FROM base AS dependencies
WORKDIR /usr/src/app
RUN yarn install --immutable

FROM base AS build-run
Copy link
Contributor

Choose a reason for hiding this comment

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

The NODE_ENV is set to "development" in the Dockerfile, which is typically not recommended for production builds as it can expose more verbose error messages and potentially sensitive stack traces. Consider using a production setting or managing this through environment variables passed at runtime.

WORKDIR /usr/src/app
ENV NODE_ENV="development"
ENV NODE_OPTIONS="--enable-source-maps --max_old_space_size=4096"

COPY --from=dependencies /usr/src/app/node_modules /usr/src/app/node_modules
COPY . .

ENV CI=true
Copy link
Contributor

Choose a reason for hiding this comment

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

The .env.test file is being copied and renamed to .env in the Dockerfile. This is generally not recommended for production environments as test configurations might include sensitive defaults or settings not suitable for production. Consider using environment-specific configuration files or managing these settings through environment variables.

RUN cp .env.test .env
RUN cp src/config.example.ts src/config.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Copying src/config.example.ts to src/config.ts might not be suitable for all environments. Consider using environment-specific configurations or managing this through environment variables to ensure the Docker container is configured correctly across different environments.


ADD https://github.com/ufoscout/docker-compose-wait/releases/download/2.9.0/wait /wait
RUN chmod +x /wait

CMD /wait && yarn prisma db push --schema='./prisma/robochimp.prisma' && yarn prisma db push --schema='./prisma/schema.prisma' && yarn run build:tsc && yarn vitest run --config vitest.integration.config.mts
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider encapsulating the commands in the CMD instruction into a script to handle potential failures more gracefully and improve maintainability. Additionally, move the TypeScript build step (yarn run build:tsc) to the Docker image build phase to reduce container startup time and catch build issues earlier.

11 changes: 9 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
"test:unit": "vitest run --coverage --config vitest.unit.config.mts",
"test:watch": "vitest --config vitest.unit.config.mts --coverage",
"test:integration": "tsx ./src/scripts/integration-tests.ts",
"watch": "tsc -w -p src"
"watch": "tsc -w -p src",
"build:esbuild": "yarn build:main && yarn build:workers",
"build:main": "esbuild src/index.ts --minify --legal-comments=none --bundle --outdir=./dist --platform=node --loader:.node=file --external:@napi-rs/canvas --external:@prisma/robochimp --external:@prisma/client --external:zlib-sync --external:bufferutil --external:oldschooljs --external:discord.js --external:node-fetch --external:piscina",
"build:workers": "esbuild src/lib/workers/kill.worker.ts src/lib/workers/finish.worker.ts src/lib/workers/casket.worker.ts --minify --legal-comments=none --outdir=./dist/workers --platform=node --loader:.node=file --external:piscina"
},
"dependencies": {
"@napi-rs/canvas": "^0.1.53",
Expand Down Expand Up @@ -51,6 +54,7 @@
"@types/node-fetch": "^2.6.1",
"@vitest/coverage-v8": "^1.6.0",
"concurrently": "^8.2.2",
"esbuild": "0.21.5",
"fast-glob": "^3.3.2",
"prettier": "^3.3.2",
"prisma": "^5.16.1",
Expand All @@ -61,5 +65,8 @@
"engines": {
"node": "20.15.0"
},
"packageManager": "[email protected]"
"packageManager": "[email protected]",
"resolutions": {
"esbuild": "0.21.5"
}
}
5 changes: 1 addition & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import './lib/safeglobals';
import './lib/globals';
import './lib/data/itemAliases';
import './lib/data/trophies';
import './lib/crons';
import './lib/MUser';
import './lib/util/transactItemsFromBank';
import './lib/itemMods';
import './lib/geImage';
import './lib/util/logger';

import { MahojiClient } from '@oldschoolgg/toolkit';
import { init } from '@sentry/node';
Expand Down
2 changes: 1 addition & 1 deletion src/lib/bankImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import fetch from 'node-fetch';
import { Bank } from 'oldschooljs';
import type { Item } from 'oldschooljs/dist/meta/types';
import { toKMB } from 'oldschooljs/dist/util/util';

import { resolveItems } from 'oldschooljs/dist/util/util';

import { BOT_TYPE, BitField, ItemIconPacks, PerkTier, toaPurpleItems } from '../lib/constants';
import { allCLItems } from '../lib/data/Collections';
import { filterableTypes } from '../lib/data/filterables';
Expand Down
10 changes: 5 additions & 5 deletions src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import * as dotenv from 'dotenv';
import { getItemOrThrow, resolveItems } from 'oldschooljs/dist/util/util';
import { z } from 'zod';

import './data/itemAliases';

import { DISCORD_SETTINGS, production } from '../config';
import type { AbstractCommand } from '../mahoji/lib/inhibitors';
import { SkillsEnum } from './skilling/types';
Expand Down Expand Up @@ -534,11 +532,12 @@ const globalConfigSchema = z.object({
clientID: z.string().min(10).max(25),
geAdminChannelID: z.string().default(''),
redisPort: z.coerce.number().int().optional(),
botToken: z.string().min(1)
botToken: z.string().min(1),
isCI: z.coerce.boolean().default(false)
});
dotenv.config({ path: path.resolve(process.cwd(), process.env.TEST ? '.env.test' : '.env') });

if (!process.env.BOT_TOKEN) {
if (!process.env.BOT_TOKEN && !process.env.CI) {
throw new Error(
`You need to specify the BOT_TOKEN environment variable, copy your bot token from your config.ts and put it in the ".env" file like so:\n\nBOT_TOKEN=your_token_here`
);
Expand All @@ -548,7 +547,8 @@ export const globalConfig = globalConfigSchema.parse({
clientID: process.env.CLIENT_ID,
geAdminChannelID: process.env.GE_ADMIN_CHANNEL_ID,
redisPort: process.env.REDIS_PORT,
botToken: process.env.BOT_TOKEN
botToken: process.env.BOT_TOKEN,
isCI: process.env.CI
});

export const ONE_TRILLION = 1_000_000_000_000;
Expand Down
92 changes: 92 additions & 0 deletions src/lib/data/itemAliases.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { modifyItem } from '@oldschoolgg/toolkit';
import { Items } from 'oldschooljs';
import { allTeamCapes } from 'oldschooljs/dist/data/itemConstants';
import { itemNameMap } from 'oldschooljs/dist/structures/Items';
import { cleanString } from 'oldschooljs/dist/util/cleanString';
import { resolveItems } from 'oldschooljs/dist/util/util';

import { getOSItem } from '../util/getOSItem';

export function setItemAlias(id: number, name: string | string[], rename = true) {
const existingItem = Items.get(id);
Expand Down Expand Up @@ -298,3 +303,90 @@ setItemAlias(25_922, 'Antique lamp (hard ca)');
setItemAlias(25_923, 'Antique lamp (elite ca)');
setItemAlias(25_924, 'Antique lamp (master ca)');
setItemAlias(25_925, 'Antique lamp (grandmaster ca)');

/**
* Trophies
*/

// BSO (Twisted) trophies
setItemAlias(24_372, 'BSO dragon trophy');
setItemAlias(24_374, 'BSO rune trophy');
setItemAlias(24_376, 'BSO adamant trophy');
setItemAlias(24_378, 'BSO mithril trophy');
setItemAlias(24_380, 'BSO steel trophy');
setItemAlias(24_382, 'BSO iron trophy');
setItemAlias(24_384, 'BSO bronze trophy');

// Comp. trophies
setItemAlias(25_042, 'Comp. dragon trophy');
setItemAlias(25_044, 'Comp. rune trophy');
setItemAlias(25_046, 'Comp. adamant trophy');
setItemAlias(25_048, 'Comp. mithril trophy');
setItemAlias(25_050, 'Comp. steel trophy');
setItemAlias(25_052, 'Comp. iron trophy');
setItemAlias(25_054, 'Comp. bronze trophy');

// Placeholder trophies
setItemAlias(26_515, 'Placeholder dragon trophy');
setItemAlias(26_513, 'Placeholder rune trophy');
setItemAlias(26_511, 'Placeholder adamant trophy');
setItemAlias(26_509, 'Placeholder mithril trophy');
setItemAlias(26_507, 'Placeholder steel trophy');
setItemAlias(26_505, 'Placeholder iron trophy');
setItemAlias(26_503, 'Placeholder bronze trophy');

export const allTrophyItems = resolveItems([
'BSO dragon trophy',
'BSO rune trophy',
'BSO adamant trophy',
'BSO mithril trophy',
'BSO steel trophy',
'BSO iron trophy',
'BSO bronze trophy',
'Comp. dragon trophy',
'Comp. rune trophy',
'Comp. adamant trophy',
'Comp. mithril trophy',
'Comp. steel trophy',
'Comp. iron trophy',
'Comp. bronze trophy',
'Placeholder dragon trophy',
'Placeholder rune trophy',
'Placeholder adamant trophy',
'Placeholder mithril trophy',
'Placeholder steel trophy',
'Placeholder iron trophy',
'Placeholder bronze trophy'
]);

for (const item of allTrophyItems) {
modifyItem(item, {
tradeable: false,
tradeable_on_ge: false,
customItemData: {
cantBeSacrificed: true
}
});
}

/**
* Item modifications
*/

export interface CustomItemData {
cantBeSacrificed?: true;
}
declare module 'oldschooljs/dist/meta/types' {
interface Item {
customItemData?: CustomItemData;
}
}

for (const item of allTeamCapes) {
modifyItem(item.id, {
price: 100
});
if (getOSItem(item.id).price !== 100) {
throw new Error(`Failed to modify price of item ${item.id}`);
}
}
65 changes: 0 additions & 65 deletions src/lib/data/trophies.ts

This file was deleted.

Loading
Loading