Skip to content

Comments

[refactor] domain 환경변수로 변경#98

Merged
chabinhwang merged 4 commits intomainfrom
domain-to-env
Aug 15, 2025
Merged

[refactor] domain 환경변수로 변경#98
chabinhwang merged 4 commits intomainfrom
domain-to-env

Conversation

@chabinhwang
Copy link
Collaborator

@chabinhwang chabinhwang commented Aug 13, 2025

변경한 부분

SPAContents 클래스 내부에 static final로 SPA_ADMIN관련 도메인 주소들을 @ConfigurationProperties 활용하여 수정하였습니다.

논의해볼 점

이미 GoogleOidcConfig클래스에 @ConfigurationProperties로 환경변수를 주입하는 방식을 사용하고 있어서, SPAContents에도 적용했습니다. 이로인해 클래스 로딩 시점에 초기화되던 static final 변수들이, 객체 생성 시점에 Spring이 주입하도록 순서의 변화가 발생하였습니다.

  1. 테스트 코드 동작에는 문제가 없어 보이는데, 추가적으로 신경써야 할 부분이 있을까요?
  2. (아직 미적용한 부분)PlatformConstants의 allowedOrigins 리스트도 static으로 활용되고 있어서, 이 부분에 변경한 SPAContents구조를 활용해 주입하는 구조로 바꾼다면 순서 변화가 발생하게 됩니다. 이부분도 괜찮을까요?
  3. PlatformConstants의 allowedOrigins 리스트에 많은 도메인 주소들이 있는데 이부분도 환경변수 처리를 할까요?

Copy link
Member

@sudo-Terry sudo-Terry left a comment

Choose a reason for hiding this comment

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

이미 GoogleOidcConfig클래스에 @ConfigurationProperties로 환경변수를 주입하는 방식을 사용하고 있어서, SPAContents에도 적용했습니다. 이로인해 클래스 로딩 시점에 초기화되던 static final 변수들이, 객체 생성 시점에 Spring이 주입하도록 순서의 변화가 발생하였습니다.
테스트 코드 동작에는 문제가 없어 보이는데, 추가적으로 신경써야 할 부분이 있을까요?

변수에 정적으로 접근하는 코드가 전부 제거된 것으로 보이니 문제는 없을 것 같습니다. 컴파일 시점 이전에 코드에서 참조하는 부분이 없다면 문제될 일은 없을 것 같아보여요.

PlatformConstants의 allowedOrigins 리스트도 static으로 활용되고 있어서, 이 부분에 변경한 SPAContents구조를 활용해 주입하는 구조로 바꾼다면 순서 변화가 발생하게 됩니다. 이부분도 괜찮을까요?

마찬가지로 정적으로 참조하는 부분이 없다면 문제가 없을 것 같아요. 서버 설정이 적용되는 시점은 환경변수 주입이 일어난 뒤이기 때문에, 컴파일 시점, 즉 빌드 시점에서 참조해버리지만 않으면 문제가 없지 않을까요?

PlatformConstants의 allowedOrigins 리스트에 많은 도메인 주소들이 있는데 이부분도 환경변수 처리를 할까요?

네 분리하는게 좋아 보입니다. 현재 허용된 origin에 개발용 url도 들어가 있는데 이런 것은 환경분리를 하는게 좋아보여요. 그리고 저희가 도메인 변경을 자주 하는 만큼 허용 목록은 숨기는게 좋겠습니다. 저희가 더 이상 사용하지 않는데 허용해둔 구 도메인을 3자가 구매해서 악성 웹사이트를 호스팅할 수도 있어서요 (물론 그럴 가치가 있는지는 모르겠지만 ㅎㅎ...)

- static과 주입받는 인스턴스 필드 같이 사용해서 테스트가 깨짐
- CorsProps를 따로 만들어, allowOrigins 주입 보장
@sonarqubecloud
Copy link

@chabinhwang chabinhwang merged commit eb911c1 into main Aug 15, 2025
3 checks passed
@chabinhwang chabinhwang deleted the domain-to-env branch August 15, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants