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

DockerFile을 제작 및 share.service의 file path 조정 #133

Merged
merged 4 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
*Dockerfile*
node_modules
dist
volumes

Choose a reason for hiding this comment

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

코드 패치에서 몇 가지 점검해야 할 사항이 있습니다:

  1. 의미 확인: volumes가 추가되었는데, 이것이 필요한 이유와 적절한 위치에 있는지 확인해야 합니다. 사용하지 않는다면 제거하는 것이 좋습니다.

  2. 형식 일관성: 각 항목은 정렬되어야 하므로, volumes도 기존 항목들과 같은 형태로 들어가는 것이 좋습니다.

  3. 주석 추가: volumes 항목을 추가한 이유나 관련 정보를 간단히 주석으로 설명하면, 다른 개발자들이 이해하는 데 도움이 될 것입니다.

  4. 기능 테스트: 이 변경사항이 실제로 동작하는지, 의도한 대로 작동하는지 확인할 필요가 있습니다.

위의 제안을 반영하면 코드 품질을 더욱 향상시킬 수 있습니다.

18 changes: 12 additions & 6 deletions docker/Dockerfile.server
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
FROM node:18

ENV NODE_ENV=dev
FROM node:18 AS build

RUN mkdir -p /var/www/otlplus-server
WORKDIR /var/www/otlplus-server

COPY package*.json ./
RUN npm install --force
RUN npm install

COPY . .
RUN npm run client:generate
RUN npm run build

EXPOSE 8000
CMD [ "npm", "run", "start:dev" ]
FROM node:18-slim AS server
RUN apt-get update -y && apt-get install -y openssl

COPY --from=build /var/www/otlplus-server/ /var/www/otlplus-server/

WORKDIR /var/www/otlplus-server

RUN npm prune --production

EXPOSE 8000
2 changes: 1 addition & 1 deletion docker/docker-compose-db-local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ services:
ports:
- '${OTLPLUS_DB_PORT:-43306}:3306'
environment:
- MYSQL_ROOT_HOSTS=%
- MYSQL_ROOT_HOSTS=admin
- MYSQL_ROOT_PASSWORD=${OTLPLUS_DB_PASSWORD:-password}
- MYSQL_DATABASE=otlplus
- TZ=Asia/Seoul

Choose a reason for hiding this comment

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

이 코드 패치를 검토해 본 결과 다음과 같은 몇 가지 주의 사항과 개선 제안을 드립니다:

  1. MYSQL_ROOT_HOSTS 값 변경:

    • %에서 admin으로 변경된 부분은 DB에 대한 root 사용자 접근 권한을 제한하는 것처럼 보입니다. 그러나, 이는 특정 호스트(즉, 'admin'이라는 호스트)에서만 연결을 허용하게 되어, 원격 연결을 필요로 하는 환경에서는 문제가 발생할 수 있습니다.
    • 따라서, 원격 데이터베이스 접근이 필요한지 여부를 확인하고 필요하다면 다시 %로 변경하거나 특정 IP 또는 도메인으로 설정하는 것이 좋습니다.
  2. 환경 변수 보안:

    • MYSQL_ROOT_PASSWORD 값을 기본값으로 설정한 경우, 보안상 문제가 될 수 있습니다. 가능한 한 기본값을 사용하지 않고, 환경 변수를 통해 외부에서 입력받도록 설정하는 것이 좋습니다.
  3. 타임존 설정:

    • TZ=Asia/Seoul 설정은 적절하지만, 애플리케이션이나 컨테이너가 이 환경 변수를 제대로 인식하는지 확인하는 것이 중요합니다.
  4. 주석 및 문서화:

    • 변경 내용을 설명하는 주석을 추가하면 다른 개발자들이 해당 변경의 이유를 이해하는 데 도움이 됩니다.
  5. 버전 관리:

    • 데이터베이스 마이그레이션 시 반드시 관련 서비스나 구조적 변경 사항이 이미 시행되었는지 확인하고, 그에 따라선 데이터 무결성을 체크하는 절차도 필요합니다.

이러한 점들을 고려하여 코드를 수정하거나 테스트할 것을 권장합니다.

Expand Down
10 changes: 7 additions & 3 deletions docker/docker-compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@ version: '3.4'

services:
back:
container_name: otlplus-server-nest
container_name: otlplus-server-nest-dev
platform: linux/amd64
env_file:
- ../env/.env.dev
environment:
- NODE_ENV=dev
- DOCKER_DEPLOY=true
build:
context: ..
dockerfile: ./docker/Dockerfile.server
image: otlplus-server-nest
image: otlplus-server-nest-dev
restart: always
tty: true
ports:
- '8080:8000'
- '58000:8000'
volumes:
- '/etc/timezone:/etc/timezone:ro'
working_dir: /var/www/otlplus-server
command: node dist/src/bootstrap/bootstrap.js

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 코드 리뷰입니다.

문제점 및 버그 위험

  1. 환경 변수 설정:

    • NODE_ENVDOCKER_DEPLOY 환경 변수가 추가되었습니다. 이 변수들이 해당 애플리케이션에서 정확하게 처리되지 않으면, 예상치 못한 동작이 발생할 수 있습니다.
  2. 포트 변경:

    • 포트가 8080:8000에서 58000:8000으로 변경되었습니다. 이것이 기존 클라이언트나 서비스와의 호환성에 영향을 줄 수 있으므로 다른 서비스에 대한 재구성을 고려해야 합니다.
  3. 명령어 실행:

    • command에 지정된 경로가 올바른지 확인해야 합니다. dist/src/bootstrap/bootstrap.js 파일이 실제로 존재하는지, 실행 가능한 상태인지 검토 필요.

개선 제안

  1. 주석 추가:

    • 변경 사항에 대한 주석을 추가하여 향후 유지보수 시 이해를 돕는 것이 좋습니다.
  2. 환경별 구성 파일:

    • env_file과 환경 변수를 별도로 관리하는 방법을 고려해 보세요. 이를 통해 개발, 테스트, 프로덕션 환경을 더욱 명확히 구분할 수 있습니다.
  3. 반복 사용되는 값 상수화:

    • 포트 번호와 같은 하드코딩된 값을 변수나 상수로 관리하면, 추후 유지보수 시 편리합니다.
  4. 실행 속도 최적화:

    • restart 정책을 재검토하여, 원격 서버에서의 안정성을 높일 수 있는지 고려하세요.

전체적으로 디버깅이나 배포 시 발생할 수 있는 잠재적인 문제에 대비하기 위한 추가 검토가 필요합니다.

26 changes: 26 additions & 0 deletions docker/docker-compose.local.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
version: '3.4'

services:
back:
container_name: otlplus-server-nest-local
platform: linux/amd64
env_file:
- ../env/.env.local
environment:
- NODE_ENV=local
- DOCKER_DEPLOY=true
build:
context: ..
dockerfile: ./docker/Dockerfile.server
image: otlplus-server-nest-local
restart: always
tty: true
ports:
- '58000:8000'
expose:
- '8000'
volumes:
- '/etc/timezone:/etc/timezone:ro'
network_mode: 'host'
working_dir: /var/www/otlplus-server
command: node dist/src/bootstrap/bootstrap.js

Choose a reason for hiding this comment

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

코드 패치를 검토한 결과는 다음과 같습니다.

버그 리스크

  1. 네트워크 모드: network_mode: 'host'를 사용하면 컨테이너가 호스트 네트워크를 직접 사용하는데, 이는 포트 충돌 등의 문제를 일으킬 수 있습니다. 다른 서비스와의 충돌 가능성을 고려해야 합니다.

  2. 환경 파일 경로: env_file에서의 상대 경로가 잘못될 경우 환경 변수가 로드되지 않을 수 있습니다. 경로 확인이 필요합니다.

  3. tty 및 restart 설정: tty: true와 함께 restart: always 설정은 일반적인 경우에는 괜찮지만, 특정 상황에서는 불필요하게 컨테이너가 재시작에 빠질 수 있습니다. 필요에 따라 수정할 수 있습니다.

개선 사항

  1. 버전 관리: Docker Compose 파일의 버전을 최신으로 유지하는 것이 좋습니다. 현재는 3.4지만, 최신 안정 버전으로 업데이트할 수 있습니다.

  2. 포트 매핑: exposeports를 동시에 사용하는 것은 다소 중복됩니다. 외부에서 접근 가능한 포트를 명확히 하기 위해 하나만 사용하는 것이 좋습니다.

  3. 볼륨 설정: 볼륨 마운트 경로가 하드코딩되어 있는데, 이를 환경 변수로 관리하면 배포 환경에 따라 유연성을 높일 수 있습니다.

  4. 커맨드 스크립트: command 부분을 별도의 파일로 만들어서 실행할 수 있다면 가독성과 유지보수성이 좋아질 수 있습니다.

  5. 주석 추가: 각 설정에 대한 주석을 추가하면 팀원들이 이해하는 데 도움이 됩니다.

이러한 점들을 고려해 보시기 바랍니다.

23 changes: 23 additions & 0 deletions docker/docker-compose.prod.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
version: '3.4'

services:
back:
container_name: otlplus-server-nest-prod
platform: linux/amd64
env_file:
- ../env/.env.prod
environment:
- NODE_ENV=prod
- DOCKER_DEPLOY=true
build:
context: ..
dockerfile: ./docker/Dockerfile.server
image: otlplus-server-nest-prod
restart: always
tty: true
ports:
- '58000:8000'
volumes:
- '/etc/timezone:/etc/timezone:ro'
working_dir: /var/www/otlplus-server
command: node dist/src/bootstrap/bootstrap.js

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 코드 리뷰입니다.

버그 리스크 및 개선 제안

  1. 환경 변수 관리:

    • .env.prod 파일에서 중요한 비밀 정보(예: API 키)가 안전하게 관리되고 있는지 확인하세요.
  2. 포트 설정:

    • 58000:8000 포트 매핑이 적절한지 확인하십시오. 충돌할 수 있는 포트가 있는지 검증하세요.
  3. 의존성 확인:

    • Dockerfile에서 필요한 Node.js 모듈이 모두 설치되도록 확인합니다. 필요시 docker-compose builddocker-compose up으로 테스트하는 것이 좋습니다.
  4. 파일 위치:

    • dockerfile의 경로가 올바른지, 특히 상대경로(./docker/Dockerfile.server)를 사용할 때 해당 디렉토리가 존재하는지 점검하세요.
  5. 재시작 정책:

    • restart: always는 서비스가 크래시될 경우 계속 재시작하게 만듭니다. 일시적인 문제 해결 시 주의하세요. 필요시 on-failure 등의 다른 정책을 고려할 수 있습니다.
  6. TTY 사용:

    • tty: true 설정이 필요한 상황인지 검토하세요. 일반적으로 서비스에 GUI 지원이 필요 없는 경우 불필요할 수 있습니다.
  7. 작업 디렉터리:

    • working_dir 설정이 올바른지 확인하고, 이 경로에 필요한 파일들이 존재하는지 검증하세요.

전반적으로 기본 구조가 잘 구성되어 있으나, 위의 사항들을 고려하여 보다 견고한 컨테이너 구성을 할 수 있습니다.

6 changes: 2 additions & 4 deletions src/modules/share/share.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { SemesterRepository } from 'src/prisma/repositories/semester.repository'
import { LecturesService } from '../lectures/lectures.service';
import { SemestersService } from '../semesters/semesters.service';
import { TimetablesService } from '../timetables/timetables.service';
import settings from '@src/settings';

interface RoundedRectangleOptions {
ctx: CanvasRenderingContext2D;
Expand Down Expand Up @@ -65,10 +66,7 @@ interface DrawTimetableDatas {

@Injectable()
export class ShareService {
private readonly file_path =
process.env.NODE_ENV === 'dev'
? 'static/'
: '/var/www/otlplus-server/static/';
private readonly file_path = settings().getStaticConfig().file_path;

constructor(
private readonly semesterRepository: SemesterRepository,

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 검토를 제공하겠습니다.

코드 리뷰

  1. 환경 설정 사용:

    • process.env.NODE_ENV를 직접 사용하는 대신 settings().getStaticConfig().file_path로 변경한 것은 유지보수성 측면에서 긍정적입니다. 환경변수의 하드코딩을 피하고 설정 파일을 통해 관리하는 것이 더 안전합니다.
  2. 버그 위험:

    • settings() 함수가 항상 올바른 값을 반환하는지 확인해야 합니다. 만약 해당 함수가 잘못된 경로를 반환하거나 에러를 발생시킬 경우, 전체 애플리케이션에 영향을 미칠 수 있습니다.
    • getStaticConfig() 메서드에서 file_path가 정의되지 않았을 경우 안정성을 고려한 예외 처리가 필요합니다.
  3. 성능:

    • settings() 호출이 빈번하게 이루어지지 않는다면, 비효율성 문제는 클 것 같지 않지만, 만약 여러 번 호출된다면 초기화 후 재사용하는 방식으로 개선할 수 있습니다.
  4. 타입 안전성:

    • file_path의 타입을 명시적으로 지정하면 코드 가독성을 높이고 의도하지 않은 값에 대해 경고를 받을 수 있습니다.
  5. 주석 및 문서화:

    • 코드 변경 부분에 대한 주석이 있으면 다른 개발자들이 이해하는 데 도움이 됩니다. 어떤 이유로 이 변경이 있었는지 설명하는 것이 좋습니다.

개선 제안

  • settings()와 관련된 오류 처리 로직을 추가하세요.
  • file_path의 타입을 명시하도록 하세요.
  • 변경 사항에 대한 문서화나 주석을 추가해 코드를 이해하기 쉽게 만드세요.

전체적으로 대체로 좋은 방향으로 진행되고 있지만, 몇 가지 점검이 필요합니다.

Expand Down
10 changes: 10 additions & 0 deletions src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default () => {
getSsoConfig: () => getSsoConfig(),
getCorsConfig: () => getCorsConfig(),
getVersion: () => getVersion(),
getStaticConfig: () => staticConfig(),
};
};

Expand Down Expand Up @@ -106,3 +107,12 @@ const getSsoConfig = (): any => {
const getVersion = () => {
return String(process.env.npm_package_version);
};

const staticConfig = (): any => {
return {
file_path:
process.env.DOCKER_DEPLOY === 'true'
? '/var/www/otlplus-server/static/'
: 'static/',
};
};

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

버그 리스크

  1. 타입 선언: getSsoConfig, getCorsConfig, getVersion와 같은 함수들은 명시적인 반환 타입을 갖고 있지만, 새로운 staticConfig 함수는 any 타입으로 정의되어 있습니다. 더 구체적인 타입 선언을 고려하여 타입 안정성을 향상시키는 것이 좋습니다.

  2. 환경 변수 검사: process.env.DOCKER_DEPLOY가 정의되어 있지 않을 경우 기본값('static/')이 그대로 사용됩니다. 만약 이 변수가 undefined라면, 의도하지 않은 값이 사용될 수 있으니 예외 처리 또는 기본값 설정에 관한 로직을 추가하는 것이 좋습니다.

개선 제안

  1. 코드 일관성: ::any와 같은 불필요한 타입 지정을 피하고, 가능한 한 정확한 타입을 사용하는 것이 좋습니다. 리턴 값을 명확히 하는 것이 좋습니다.

  2. 상수화: '/var/www/otlplus-server/static/'와 같은 문자열은 상수로 정의하여 나중에 유지 관리 및 변경이 용이하도록 할 수 있습니다.

  3. 가독성 개선: 2항 연산자를 사용할 때 코드가 적절하게 주석 처리되어 있지 않는 경우 가독성이 떨어질 수 있습니다. 복잡한 로직에 대해서는 주석을 추가하는 것이 도움이 될 수 있습니다.

  4. 테스트 추가: staticConfig 함수는 환경 변수가 올바르게 설정되었는지에 대한 테스트를 추가하면, 배포 이전에 누락된 문제를 사전에 확인할 수 있습니다.

종합적으로 보면, 코드는 전반적으로 잘 작성되어 있지만, 타입 안전성과 예외 처리를 더욱 강화할 기회가 있습니다.

Loading