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

feat: 모니터링 서버 구축 #357

Merged
merged 4 commits into from
Sep 7, 2024
Merged

feat: 모니터링 서버 구축 #357

merged 4 commits into from
Sep 7, 2024

Conversation

mikekks
Copy link
Member

@mikekks mikekks commented Sep 4, 2024

👩‍💻 Contents

  • 모니터링 서버를 구축했습니다.
  • 일단 세팅만 해뒀고, 세부적인 내용은 해보면서 알아가봐야 할 것 같습니다!
  • AWS 계정은 1. 온보딩 인수인계 에 저장해뒀습니다!
  • k6(부하테스트 툴)도 붙여볼 예정입니다
  • 다른 의견 or 다른 툴 사용 생각하고 계시면 편하게 말씀주세요~~~!!
스크린샷 2024-09-04 오후 9 23 11

📝 Review Note

  • 액츄에이터 사용은 안전하게 사용하면 좋을 것 같다는 생각을 했습니다! 그래서 디폴트는 모두 disable 시키고, 필요한 것들만 enable 시켰습니다!
  • 참고자료 (https://techblog.woowahan.com/9232/)

📣 Related Issue

✅ 점검사항

  • docker-compose.yml 파일에 마이그레이션 한 API의 포워딩을 변경해줬나요?
  • Spring Secret 값을 수정하거나 추가했다면 Github Secret에서 수정을 해줬나요?
  • Nestjs Secret 값을 수정하거나 추가했다면 Docker-Compose.yml 파일 및 인스턴스 내부의 .env 파일을 수정했나요?

@mikekks mikekks added 🎁 feature 새로운 기능 🏭 environment 환경설정 변경 labels Sep 4, 2024
@mikekks mikekks requested a review from hoonyworld September 4, 2024 12:25
@mikekks mikekks self-assigned this Sep 4, 2024
Copy link

height bot commented Sep 4, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@hoonyworld hoonyworld left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 민규님!
민규님 덕분에 Actuator 의존성을 잘못 사용할 경우 보안 이슈가 생길 수도 있다는 사실을 알 수 있었습니다.
구현된 코드를 보면서 몇 가지 질문 사항이 떠올라서 아래에 적어보았는데요!!

민감한 정보의 경우 개인적으로 연락을 주셔서 답변을 주셔도 좋을 것 같습니다 ㅎㅎ

  1. 해당 아티클에서는 management.server.port 설정을 통해 운영하는 포트와 다른 포트를 Actuator로 사용할 것을 권장하는 것으로 보이는데 해당 설정을 하지 않으신 이유가 궁금합니다!

  2. 추가로 management.endpoints.web.base-path를 통해 변경된 경로를 Actuator로 사용할 것을 아티클에서 권장하는 것 같은데 해당 설정을 하지 않으신 이유도 궁금합니다!

  3. 스프링부트 3.x.x 버전의 Actuator 기본 경로 설정은 /actuator/health로 알고 있습니다. 그런데 제가 /actuator/health로 GET 테스트를 했을 때, Actuator가 작동하지 않았습니다. 로컬에서 작동해서 확인을 해본 결과 SecurityConfig 내 AUTH_WHITELIST에 "/actuator/health"가 없어서 발생하는 문제였고 추가해주니 정상적으로 Actuator가 작동하는 것을 확인했습니다. 따라서 화이트리스트에 해당 경로(endpoint가 바뀔 경우 다른 경로)를 추가해주어야 할 것 같습니다!

  4. 헬스체크 코드를 혼자서 파악해본 결과, 현재 /(루트 경로)로 NestJS 서버의 헬스 체크를 수행하는 것으로 보여지는데, 이 부분이 맞는지 궁금합니다!
    또한, 현재 Spring 서버의 헬스 체크는 어떤 방식으로 수행이 되고 있는지 궁금합니다

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 7, 2024
@mikekks
Copy link
Member Author

mikekks commented Sep 7, 2024

고생하셨습니다 민규님! 민규님 덕분에 Actuator 의존성을 잘못 사용할 경우 보안 이슈가 생길 수도 있다는 사실을 알 수 있었습니다. 구현된 코드를 보면서 몇 가지 질문 사항이 떠올라서 아래에 적어보았는데요!!

민감한 정보의 경우 개인적으로 연락을 주셔서 답변을 주셔도 좋을 것 같습니다 ㅎㅎ

  1. 해당 아티클에서는 management.server.port 설정을 통해 운영하는 포트와 다른 포트를 Actuator로 사용할 것을 권장하는 것으로 보이는데 해당 설정을 하지 않으신 이유가 궁금합니다!
  2. 추가로 management.endpoints.web.base-path를 통해 변경된 경로를 Actuator로 사용할 것을 아티클에서 권장하는 것 같은데 해당 설정을 하지 않으신 이유도 궁금합니다!
  3. 스프링부트 3.x.x 버전의 Actuator 기본 경로 설정은 /actuator/health로 알고 있습니다. 그런데 제가 /actuator/health로 GET 테스트를 했을 때, Actuator가 작동하지 않았습니다. 로컬에서 작동해서 확인을 해본 결과 SecurityConfig 내 AUTH_WHITELIST에 "/actuator/health"가 없어서 발생하는 문제였고 추가해주니 정상적으로 Actuator가 작동하는 것을 확인했습니다. 따라서 화이트리스트에 해당 경로(endpoint가 바뀔 경우 다른 경로)를 추가해주어야 할 것 같습니다!
  4. 헬스체크 코드를 혼자서 파악해본 결과, 현재 /(루트 경로)로 NestJS 서버의 헬스 체크를 수행하는 것으로 보여지는데, 이 부분이 맞는지 궁금합니다!
    또한, 현재 Spring 서버의 헬스 체크는 어떤 방식으로 수행이 되고 있는지 궁금합니다

답변 감사합니다 ㅎㅎ 제가 디테일을 놓친 부분이 있었네요 ㅎㅎ..

  1. 해당 부분은 사실 이렇게까지 해야하나 싶었는데 그래도 이왕하는거 적용하는게 좋을 것 같아서 적용해서 새로 커밋날렸습니다!!

  2. 이 부분도 동일하고, uuid로 path 제작했습니다! 설정 파일은 노션에 업데이트했습니다!

  3. 액츄에이터 관련 코드가 인증을 통해서 호출되었으면 좋겠다라는 생각으로 특별한 액션을 하지 않았는데요! 생각해보니 헬스체크는 열어두는게 맞는 거 같네요...! 필터 수정해서 한 번 확인 부탁드립니다 !!

  4. 기존에는 자체적으로 만든 헬스체크 API를 사용했습니다! 이 부분은 액츄에이터에 있는 헬스체크를 사용하는게 더 좋은 방식인 것 같습니다! 이 부분은 제가 새로 이슈파서 수정하도록 하겠습니다!

@mikekks
Copy link
Member Author

mikekks commented Sep 7, 2024

고생하셨습니다 민규님! 민규님 덕분에 Actuator 의존성을 잘못 사용할 경우 보안 이슈가 생길 수도 있다는 사실을 알 수 있었습니다. 구현된 코드를 보면서 몇 가지 질문 사항이 떠올라서 아래에 적어보았는데요!!

민감한 정보의 경우 개인적으로 연락을 주셔서 답변을 주셔도 좋을 것 같습니다 ㅎㅎ

  1. 해당 아티클에서는 management.server.port 설정을 통해 운영하는 포트와 다른 포트를 Actuator로 사용할 것을 권장하는 것으로 보이는데 해당 설정을 하지 않으신 이유가 궁금합니다!
  2. 추가로 management.endpoints.web.base-path를 통해 변경된 경로를 Actuator로 사용할 것을 아티클에서 권장하는 것 같은데 해당 설정을 하지 않으신 이유도 궁금합니다!
  3. 스프링부트 3.x.x 버전의 Actuator 기본 경로 설정은 /actuator/health로 알고 있습니다. 그런데 제가 /actuator/health로 GET 테스트를 했을 때, Actuator가 작동하지 않았습니다. 로컬에서 작동해서 확인을 해본 결과 SecurityConfig 내 AUTH_WHITELIST에 "/actuator/health"가 없어서 발생하는 문제였고 추가해주니 정상적으로 Actuator가 작동하는 것을 확인했습니다. 따라서 화이트리스트에 해당 경로(endpoint가 바뀔 경우 다른 경로)를 추가해주어야 할 것 같습니다!
  4. 헬스체크 코드를 혼자서 파악해본 결과, 현재 /(루트 경로)로 NestJS 서버의 헬스 체크를 수행하는 것으로 보여지는데, 이 부분이 맞는지 궁금합니다!
    또한, 현재 Spring 서버의 헬스 체크는 어떤 방식으로 수행이 되고 있는지 궁금합니다

추가로 SecurityConfig 파일이 환경마다 다르게 세팅되어있는데 중복되는 값이 너무 많은 것 같아서 이 부분은 보완해보도록 하겠습니다!

Copy link
Member

@hoonyworld hoonyworld left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 민규님 👍

port와 base-path 설정으로 좀 더 actuator를 안전하게 관리할 수 있게 된 것 같네요 ㅎㅎ
uuid로 base-path 설정하신거 노션에서 확인 완료 했습니다!

Comment on lines 48 to 54
private static final String[] AUTH_WHITELIST = {
"/health",
"/health/v2",
"/meeting/v2/org-user/**",
"/auth/v2",
"/auth/v2/**"
};
Copy link
Member

Choose a reason for hiding this comment

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

전에 말씀드렸던, Actuator의 endpoint를 GET하기 위해서는 AUTH_WHITELIST에 추가를 해야할 것 같다고 생각됩니다!

하지만 토큰을 이용해 actuator를 GET하도록 API를 설계한다면, 필요가 없는 작업이기도 합니다... 이 부분은 내일 대면회의 때 같이 정해보면 좋을 것 같아요 ㅎㅎ

토큰 없이 actuator를 접근 가능하도록 하기 위해서는 다음의 작업이 필요해 보입니다!

  1. actuatorEndPoint를 AUTH_WHITELIST에 추가합니다.(경로가 노출되지 않도록 변수로 추가 해야할 것으로 생각됨)
  2. 현재 AUTH_WHITELIST가 static 필드이므로 변수로 추가가 불가능한 상황입니다. 따라서 AUTH_WHITELIST를 인스턴스 필드로 변경하여야 합니다.

위 사항을 모두 반영하게 되면 수정사항은 다음과 같습니다.

AUTH_WHITELIST 인스턴스 필드로 변경

private String[] getAuthWhitelist() {
    return new String[] {
                "/health",
		"/health/v2",
		"/meeting/v2/org-user/**",
		"/auth/v2",
		"/auth/v2/**",
                 actuatorEndPoint + "/**"
    };
}

변경 전

requestMatchers(Stream.of(AUTH_WHITELIST)

변경 후

requestMatchers(Stream.of(getAuthWhitelist())

.of(AUTH_WHITELIST)
.map(AntPathRequestMatcher::antMatcher)
.toArray(AntPathRequestMatcher[]::new)).permitAll()
.requestMatchers(new AntPathRequestMatcher(actuatorEndPoint + "/health")).permitAll()
Copy link
Member

Choose a reason for hiding this comment

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

제가 위에 AUTH_WHITELIST에 관한 코맨트를 쓰고 나서야 해당 코드를 보게되었네요 ㅎㅎ
감안하고 봐주시면 감사하겠습니다!

혹시 새롭게 AntPathRequestMatcher(actuatorEndPoint + "/health")).permitAll()로 만들어주셨는데, 기존 화이트 리스트에 추가하는 방식이 아닌 새롭게 만들어 주신 이유가 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

리뷰 주신 내용으로 바꾸는게 좋을 것 같네요...! 가독성이랑 유지보수 측면에서 더 좋은 것 같습니다!! 리뷰감사합니다 !

.of(AUTH_WHITELIST)
.map(AntPathRequestMatcher::antMatcher)
.toArray(AntPathRequestMatcher[]::new)).permitAll()
.requestMatchers(new AntPathRequestMatcher(actuatorEndPoint + "/health")).permitAll()
Copy link
Member

Choose a reason for hiding this comment

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

아래 prod와 의견 동일합니다!

.of(AUTH_WHITELIST)
.map(AntPathRequestMatcher::antMatcher)
.toArray(AntPathRequestMatcher[]::new)).permitAll()
.requestMatchers(new AntPathRequestMatcher(actuatorEndPoint + "/health")).permitAll()
Copy link
Member

Choose a reason for hiding this comment

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

아래 prod와 의견 동일합니다!

Comment on lines +81 to +82
base-path: ${ACTUATOR_PATH}
Copy link
Member

Choose a reason for hiding this comment

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

아래 prod와 의견 동일합니다!

Comment on lines +79 to +82
exposure:
include: info, health, prometheus
base-path: ${ACTUATOR_PATH}
Copy link
Member

Choose a reason for hiding this comment

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

해당 설정으로 다음과 같이 세팅이 되는건지 궁금합니다 ㅎㅎ

애플리케이션 health check

  • ${ACTUATOR_PATH}/health

Prometheus 메트릭 수집

  • ${ACTUATOR_PATH}/prometheus

애플리케이션 정보

  • ${ACTUATOR_PATH}/info

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 그렇습니다!!

Copy link
Member

Choose a reason for hiding this comment

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

그렇군요!
그러면 화이트리스트에서 actuatorEndPoint + "/**" 방식으로 모두 접근 권한을 풀어줄 건지, 아니면 특정 endpoint만(ex. 프로메테우스는 화이트리스트에서 제외) 접근 권한을 풀어줄 건지도 얘기해보면 좋을 것 같아요.

제 생각에는 actuatorEndPoint의 value값도 uuid고, 환경변수로 관리하고 있기 때문에, 모두 접근 권한을 풀어줘도 될 것 같은데 민규님의 생각도 궁금합니다!

Comment on lines +83 to +86
exposure:
include: info, health, prometheus
base-path: ${ACTUATOR_PATH}
Copy link
Member

Choose a reason for hiding this comment

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

위 prod와 의견 동일합니다!

@mikekks mikekks merged commit 8c13e50 into develop Sep 7, 2024
1 check passed
@mikekks mikekks deleted the feat/#356 branch September 7, 2024 14:58
mikekks added a commit that referenced this pull request Sep 17, 2024
* �chore: 유저관련 temp API 추가 (#359)

* del(meeting): 모임 지원자 목록 csv 파일 다운로드 temp 삭제

* del(meeting): 모임 게시글 댓글 리스트 조회 temp 삭제

* chore(meeting): 유저 관련 API temp 버전 추가

* �chore: 모임 관련 API 스웨거에 보이는 응답값 수정 (#361)

* chore(meeting): 모임 활동 시간 Dto 반환 수정

* docs(meeting): status 스웨거상 enum 으로 보이게 수정

* chore(meeting): dto내 NotNull 오류 수정

* docs(meeting): 1,2,3 -> 0,1,2 로 수정

* docs(meeting): number -> integer 로 수정

* feat: 모니터링 서버 구축 (#357)

* add(setting): actuator 및 prometheus exporter 설정

* add(yml): 액츄에이터 관련하여 커스텀 포트 및 path 적용

* chore(config): 헬스체크 엔드포인트 화이트리스트에 추가

* chore(config): 화이트리스트 한 곳에서 관리할 수 있도록 코드 구조 변경

* chore(Advertisement): id, 게시 시작일 데이터 추가 (#365)

* refactor: 시큐리티 config 코드 개선 및 헬스 체크 방식 변경 (#363)

* refactor(config): 중복코드 제거

* del(config): 사용하지 않는 코드 제거

* fix(User): AppliedCount -> ApprovedCount 로 변경 (#374)

* feat(application-local.yml): application-local.yml 세팅 (#375)

* fix(Post): desc 데이터 추가 (#378)

* �chore: 플그 db 연결 작업 (#376)

* feat(MemberBlock): 플그의 MemberBlock 테이블 등록

* infra(Playground): 플그 db 연결

* add(Playground): 차단 관련 repository 추가

* feat(Playground): 기능 테스트를 위한 임시 API 개발 (삭제 필요)

* fix(config): 테스트 db는 기존대로 유지

* chore(config): local db 설정을 위한 수정

* chore(yml): prod yml 수정

* chore(yml): 테스트를 위한 API 수정

* fix(config): validate 추가

* del(config): 사용하지 않는 어노테이션 삭제

* chore(local,config): local-yml 수정

* refactor(ci&cd): docker hub 푸시할 때 태그 추가 (#380)

---------

Co-authored-by: DongHoon Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏭 environment 환경설정 변경 🎁 feature 새로운 기능 size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: 모니터링 서버 구축
2 participants