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

Conversation

LarryKwon
Copy link
Contributor

  1. DockerFile을 환경에 맞게 분리하였습니다.
    1-1. multistage build를 적용해서 파일 크기를 줄였습니다.
    1-2. 쉬운 배포를 위해 deploy.sh를 작성하였습니다.

  2. share.service의 file path를 Docker 환경인지 아닌지에 따라 /static과 /var/www/otlplus-server로 구성하게끔 바꾸었습니다.

@LarryKwon LarryKwon linked an issue Aug 21, 2024 that may be closed by this pull request
@@ -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. 기능 테스트: 이 변경사항이 실제로 동작하는지, 의도한 대로 작동하는지 확인할 필요가 있습니다.

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


RUN npm prune --production

EXPOSE 8000 No newline at end of file

Choose a reason for hiding this comment

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

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

버그 위험

  1. npm install --force 제거: --force 플래그가 제거되면 의존성 충돌 문제가 발생할 수 있습니다. 충분히 테스트하여 필요한 경우 'force' 옵션을 다시 고려해야 합니다.
  2. openssl 설치: apt-get install -y openssl이 필요한지 확인해야 합니다. 사용되지 않는 패키지 설치는 컨테이너 크기를 불필요하게 증가시킬 수 있습니다.

개선 제안

  1. 멀티 스테이지 빌드: 멀티 스테이지 빌드를 사용한 점은 좋습니다. 하지만 빌드 스펙을 명확히 하기 위해 별도의 스테이지 이름을 추가하는 것도 고려해보세요.
  2. 의존성 정리: npm prune --production 명령어는 잘 작성되었습니다. 다만, 이 과정을 좀 더 자동화하려면 Dockerfile에 직접 개발 의존성 제거를 포함하는 것도 좋습니다.
  3. CMD 및 ENTRYPOINT: 현재 CMD에서 dev 모드로 실행되는 부분은 운영 환경에는 적합하지 않을 수 있습니다. 프로덕션 환경에 맞는 명령어로 변경하여 배포 과정을 명확히 하는 것이 좋겠습니다.

기타

  • 소스 코드 끝에 줄바꿈이 없으므로, 이는 관행상 수정하는 것이 좋습니다 (예: 코드 형식 일관성 유지).

전반적으로 변화가 긍정적입니다. 다만, 몇 가지 사항을 더욱 세심하게 고려하면 좋겠습니다.

@@ -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. 버전 관리:

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

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

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 정책을 재검토하여, 원격 서버에서의 안정성을 높일 수 있는지 고려하세요.

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

- '/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. 주석 추가: 각 설정에 대한 주석을 추가하면 팀원들이 이해하는 데 도움이 됩니다.

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

deploy.sh Outdated
echo "Error: Environment option is required."
usage
exit 1
fi

Choose a reason for hiding this comment

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

전체적으로 이 스크립트는 잘 작성된 것 같습니다. 그러나 몇 가지 개선 사항과 잠재적인 버그 위험이 있습니다.

  1. 변수 초기화: NODE_ENV 변수가 명시적으로 초기화되어 있지만, 초기 값을 설정하지 않으면 조건부 검사가 실패할 수 있습니다. 현재 구조에서는 문제가 없지만, 올바르게 이해하기 위해 변수 초기화를 명확히 하면 좋습니다.

  2. 옵션 체크: getopts의 사용은 적절합니다. 하지만 추가적인 오류 처리를 통해 더 많은 사용자 친화적인 메시지를 제공하는 것도 고려해볼 수 있습니다.

  3. 파일 존재 여부 체크: docker-compose 파일이 존재하는지 여부를 확인하여, 항상 특정 환경의 배포 파일이 있을 것이라 가정하지 않는 것이 좋습니다. -f 뒤에 지정된 파일이 존재하지 않을 경우 적절한 오류 메시지를 반환하도록 추가하여야 합니다.

  4. 전역 쉘 변수 사용 지양: 전역 변수 NODE_ENV 대신 지역 변수를 사용하는 것이 좋습니다. 이는 스크립트가 다른 것들과 혼동되지 않게 합니다.

  5. 스트링 비교: 문자열 비교에서 == 대신 =를 사용해 Bash 스타일에 바람직하게 맞추는 것이 좋습니다. ([[ "$NODE_ENV" = "prod" ]])

  6. 옵션 구문 오류 처리 추가: \?)에 대한 처리가 넓은 범위의 옵션을 처리하지 않습니다. 더 많은 유연성을 제공하기 위해 기본적으로 모든 잘못된 옵션에 대해 경고하고 도움 메시지를 표시할 수 있습니다.

  7. 사용자 경험 개선: 환경 입력 시기를 좀 더 분명하게 하여 사용자가 인식을 쉽게 할 수 있도록 하는 것이 도움이 됩니다.

아래는 제안된 일부 수정을 반영한 코드 예시입니다:

#!/bin/bash

usage() {
  echo "Usage: $0 -e value"
  echo "  -e: 현재 배포 환경을 입력하세요. (prod, dev, local)"
}

NODE_ENV=""

while getopts "e:" opt; do
  case $opt in
    e)
      NODE_ENV=$OPTARG
      echo "NODE_ENV: $NODE_ENV"
      if [[ "$NODE_ENV" = "prod" ]]; then
         COMPOSE_FILE="docker/docker-compose.prod.yml"
      elif [[ "$NODE_ENV" = "dev" ]]; then
          COMPOSE_FILE="docker/docker-compose.dev.yml"
      elif [[ "$NODE_ENV" = "local" ]]; then
          COMPOSE_FILE="docker/docker-compose.local.yml"
      else
        echo "Invalid environment: $NODE_ENV" 1>&2
        usage
        exit 1
      fi
      
      if [[ ! -f "$COMPOSE_FILE" ]]; then
        echo "Error: Compose file does not exist: $COMPOSE_FILE" 1>&2
        exit 1
      fi
      
      docker-compose -f "$COMPOSE_FILE" up -d
      ;;
    \?)
      echo "Invalid option: -$OPTARG" 1>&2
      usage
      exit 1
      ;;
  esac
done

if [ -z "$NODE_ENV" ]; then
  echo "Error: Environment option is required."
  usage
  exit 1
fi

이런 점들을 고려하면 코드를 더욱 견고하고 사용자 친화적으로 만들 수 있습니다.

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 설정이 올바른지 확인하고, 이 경로에 필요한 파일들이 존재하는지 검증하세요.

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

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의 타입을 명시하도록 하세요.
  • 변경 사항에 대한 문서화나 주석을 추가해 코드를 이해하기 쉽게 만드세요.

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

? '/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 함수는 환경 변수가 올바르게 설정되었는지에 대한 테스트를 추가하면, 배포 이전에 누락된 문제를 사전에 확인할 수 있습니다.

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

echo "Error: Environment option is required."
usage
exit 1
fi No newline at end of file

Choose a reason for hiding this comment

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

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

버그 리스크

  1. 명령어 오류: docker-compose 명령이 실패하는 경우에 대한 처리가 없습니다. 예를 들어, docker-compose가 제대로 실행되지 않으면 사용자에게 알리지 않습니다.
  2. 옵션 파싱: 잘못된 옵션을 제공할 때 동작은 적절하지만, 전체 스크립트 실행 흐름에서 처리하는 방법이 조금 일관성이 없어 보입니다.

개선 제안

  1. 기본 값 설정: NODE_ENV 변수가 비어있을 때의 처리를 코드 시작 부분으로 이동시켜 사용성을 향상시킬 수 있습니다.
  2. 함수화: 배포 환경에 따라 Compose 파일을 결정하는 로직을 별도의 함수로 분리하면 가독성과 유지보수성이 향상됩니다.
  3. 예외 처리: docker-compose 실행 후 성공 여부를 체크하고, 실패 시 에러 메시지를 출력하도록 추가합니다.
  4. 사용자 피드백: 환경 선택 시, 사용할 수 있는 인수(prod, dev, local)에 대한 설명을 더하면 좋습니다. 사용자가 어떤 값이 유효한지 명확히 알 수 있습니다.

정리

전체적으로 구조는 깔끔하지만, 몇 가지 버그 리스크와 함께 개선점이 있습니다. 사용자의 잘못된 입력이나 docker-compose 실행 실패에 대한 처리를 강화하면 더 안정적인 스크립트가 될 것입니다.

@LarryKwon LarryKwon merged commit 0befbd5 into dev Aug 22, 2024
4 checks passed
@LarryKwon LarryKwon deleted the issue/126/production-env branch August 22, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

production 환경 설정 문제
2 participants