-
Notifications
You must be signed in to change notification settings - Fork 1
infra: pinpoint 구축 (Fitrun 183) #73
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
The head ref may contain hidden characters: "FITRUN-183-Server-pinpoint-\uAD6C\uCD95"
Conversation
Walkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deployment/staging/docker-compose.yml (1)
21-22
: 호스트 절대 경로 볼륨 바인딩 – 이식성·권한 이슈 확인
/home/yapp/pinpoint-agent
는 호스트 특정 경로입니다.
• 스테이징 서버 외 다른 노드에서도 동일 경로가 보장되는지,
• 디렉터리에 agent 실행에 필요한 읽기 + 실행 권한이 부여돼 있는지 확인해주세요.
CI/CD 파이프라인에서 경로 준비 단계를 명시하거나, 이미지 빌드 시 agent 를 포함시키는 방법도 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deployment/staging/docker-compose.yml
(1 hunks)src/main/resources/application-staging.yml
(1 hunks)
🔇 Additional comments (1)
src/main/resources/application-staging.yml (1)
11-14
: JWT 만료 기간이 지나치게 김 – 보안 팀과 재검토 필요
access-expiration-millis
/refresh-expiration-millis
가 모두 31,536,000,000 ms(1년)로 설정돼 있습니다. 일반적으로 Access-Token 은 수분수시간, Refresh-Token 도 수주수개월이 권장됩니다.
장기 토큰은 유출 시 피해 규모가 커지므로,
- 만료 값을 단축하거나
- 최소한 운영·스테이징을 환경 변수로 분리해 즉시 조정 가능하도록 하는 방안을 검토해 주세요.
코드(필터, 인증 모듈)에서 해당 프로퍼티 키를 그대로 읽고 있는지도 함께 확인 부탁드립니다.
- /home/yapp/pinpoint-agent:/pinpoint-agent | ||
entrypoint: | ||
- java | ||
- -javaagent:/pinpoint-agent/pinpoint-bootstrap.jar | ||
- -Dpinpoint.agentId=$${HOSTNAME} | ||
- -Dpinpoint.applicationName=FITRUN-STAGING | ||
- -Duser.timezone=Asia/Seoul | ||
- org.springframework.boot.loader.launch.JarLauncher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EntryPoint 클래스 경로 오타 및 실행 방식 오류 – 컨테이너 부팅 실패 가능성
org.springframework.boot.loader.launch.JarLauncher
→ 실제 패키지는 org.springframework.boot.loader.JarLauncher
(launch 패키지 없음)입니다.
또한 JarLauncher 방식은 -cp <jar>
혹은 -jar <jar>
옵션을 따로 지정해야 하는데 현재 누락돼 있어 애플리케이션이 시작되지 않을 위험이 큽니다.
예시 수정안:
entrypoint:
- java
- -javaagent:/pinpoint-agent/pinpoint-bootstrap.jar
- -Dpinpoint.agentId=$${HOSTNAME}
- -Dpinpoint.applicationName=FITRUN-STAGING
- -Duser.timezone=Asia/Seoul
- - org.springframework.boot.loader.launch.JarLauncher
+ - -jar
+ - /app/fitrun.jar # 실제 JAR 경로로 교체
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- /home/yapp/pinpoint-agent:/pinpoint-agent | |
entrypoint: | |
- java | |
- -javaagent:/pinpoint-agent/pinpoint-bootstrap.jar | |
- -Dpinpoint.agentId=$${HOSTNAME} | |
- -Dpinpoint.applicationName=FITRUN-STAGING | |
- -Duser.timezone=Asia/Seoul | |
- org.springframework.boot.loader.launch.JarLauncher | |
- /home/yapp/pinpoint-agent:/pinpoint-agent | |
entrypoint: | |
- java | |
- -javaagent:/pinpoint-agent/pinpoint-bootstrap.jar | |
- -Dpinpoint.agentId=$${HOSTNAME} | |
- -Dpinpoint.applicationName=FITRUN-STAGING | |
- -Duser.timezone=Asia/Seoul | |
- -jar | |
- /app/fitrun.jar |
🤖 Prompt for AI Agents
In deployment/staging/docker-compose.yml lines 21 to 28, the entrypoint
incorrectly references the class path as
org.springframework.boot.loader.launch.JarLauncher, which should be
org.springframework.boot.loader.JarLauncher without the 'launch' package. Also,
the current command lacks the necessary -cp or -jar option to specify the jar
file for JarLauncher, causing startup failure. Fix this by correcting the class
path and adding the appropriate -cp or -jar option with the jar file path to the
java command in the entrypoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deployment/staging/docker-compose.yml (1)
21-21
: Pinpoint 에이전트 볼륨은 읽기 전용으로 마운트하는 편이 안전합니다
에이전트 바이너리는 변경될 일이 없으므로:ro
옵션을 추가해 우발적 수정‧삭제를 방지하는 것을 권장합니다.- - /home/yapp/pinpoint-agent:/pinpoint-agent + - /home/yapp/pinpoint-agent:/pinpoint-agent:ro
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deployment/staging/docker-compose.yml
(1 hunks)src/main/resources/application-staging.yml
(1 hunks)
🔇 Additional comments (2)
src/main/resources/application-staging.yml (1)
12-14
: 프로퍼티 키가 애플리케이션 코드와 일치하는지 확인 필요
jwt.access-expiration-millis / jwt.refresh-expiration-millis
형태가 서비스 코드에서 참조하는 키와 정확히 일치해야 합니다. 오타나 네이밍 차이로 설정이 무시될 수 있으니 grep 등으로 호출부 확인 부탁드립니다.deployment/staging/docker-compose.yml (1)
22-28
: Jar 실행 엔트리포인트에 대상 JAR 지정/클래스패스 누락 가능성 확인 요망
org.springframework.boot.loader.launch.JarLauncher
를 직접 호출할 경우 보통-cp /app/fitrun-staging.jar
와 같이 클래스패스를 지정하거나-jar
옵션을 사용합니다. 현재 이미지 내부에 CLASSPATH 지정 로직이 없다면 컨테이너가 부팅되지 않을 수 있습니다. 기존 이미지의ENTRYPOINT
를 완전히 대체하므로,
- 실제 기동 여부를 로컬에서 확인,
- 필요 시
-cp
또는-jar
인자 추가
를 검토해 주세요.
jwt: | ||
access-expiration-millis: 31536000000 | ||
refresh-expiration-millis: 31536000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
액세스‧리프레시 토큰 만료 시간이 1년으로 설정되어 있어 보안 위험이 큽니다
일반적으로 Access Token은 분‧시간 단위(짧게), Refresh Token은 일‧주 단위(중간 길이)로 설정해 세션 하이재킹 리스크를 줄입니다. staging 환경이라도 실사용 계정이 들어오면 문제될 수 있으니 값 재검토를 권고합니다.
🤖 Prompt for AI Agents
In src/main/resources/application-staging.yml around lines 12 to 14, the access
and refresh token expiration times are both set to 1 year, which poses a
security risk. Adjust the access token expiration to a shorter duration such as
minutes or hours, and set the refresh token expiration to a moderate duration
like days or weeks to reduce session hijacking risks, even in the staging
environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
노션에 내용 잘 작성해 주셔서 별도 코멘트는 없습니다!!
댓글 남긴 부분만 확인해 주시고 머지 하면 될 것 같습니다~
|
||
jwt: | ||
access-expiration-millis: 31536000000 | ||
refresh-expiration-millis: 31536000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
질문)
제거 깜빡하신거죠??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실 의도했었습니다. 스테이징 환경이 부하 테스트 목적으로 사용할 것인데 매번 토큰 발급하는것도 리소스라고 생각했습니다!
그런데 생각해보면 로그인 플로우대로 부하 테스트하는게 맞을 것 같아서 해당 부분은 지우겠습니다!
34e4981
to
b7ae236
Compare
📎 관련 이슈
📄 추가 작업 내용
✅ Check List
💬 기타 참고 사항
Summary by CodeRabbit