[refactor] #213 UID 관리 책임을 config-server로 이전 - WAS uid 완전 제거#214
Open
[refactor] #213 UID 관리 책임을 config-server로 이전 - WAS uid 완전 제거#214
Conversation
WAS가 uid를 직접 결정하던 구조에서 config-server가 단독 관리하는 구조로 변경. UID 충돌(동일 uid에 복수 계정 존재) 문제를 구조적으로 해결. 삭제: - UsedId entity / UsedIdRepository - CounterKey.UID - IdAllocationService: allocateFor(), allocateNewUid(), findReusableUidByUbuntuUsername(), releaseId(), AllocationResult - Request.ubuntuUid 필드 및 assignUbuntuUid() 메서드 - RequestRepository.findTopByUbuntuUsernameAndUbuntuUidIsNotNullOrderByApprovedAtDesc 수정: - UserCreationRequestDTO: uid/gid 필드 제거, passwd_sha512 → passwd_base64 - AdminRequestCommandService: idAllocationService 의존성 및 uid/primary group 로직 제거 - RequestExpiryService: releaseId() 호출 제거 - Group: usedId FK 필드 제거 - AcceptInfoResponseDTO: uid 필드 제거 - SaveRequestResponseDTO: ubuntuUid 필드 제거 - IdAllocationService: allocateNewGid()만 유지, usedIdRepository 의존성 제거 DB 마이그레이션 필요: - requests.ubuntuUid 컬럼 삭제 - groups.ubuntu_gid → used_ids.id_value FK 제약 해제 - used_ids 테이블 DROP - id_counter 테이블 UID row 삭제
- GroupService: UsedId/UsedIdRepository 의존성 제거 (used_ids 테이블 삭제에 따른 대응) - AdminRequestCommandService: Group/GroupRepository import 누락 복구 (approveModification GROUP 케이스에서 계속 사용) - 테스트 코드: UsedId/IdAllocationService 참조 제거, CounterKey.UID → SHARED_GID 교체 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
getUbuntuUid() 메서드 삭제로 항상 null을 반환하게 된 필드 정리 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough이 PR은 UID 관리 권한을 config-server로 이전하기 위해 WAS에서 UID 관련 모든 기능을 제거합니다. ChangesUID 관리 권한 이전 및 WAS에서 제거
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandServiceTest.java (1)
94-100: ⚡ Quick win
userCreation요청 바디도 검증해 두는 게 좋겠습니다.이번 PR의 핵심 변경이
uid/gid제거와passwd_base64전환인데, 현재는bodyValue(any())로 흘려보내서 DTO 매핑이 깨져도 테스트가 통과합니다.bodyValue(...)에 전달된UserCreationRequestDTO를 캡처해서username,passwordBase64,primaryGroupName까지 고정해 두면 이 변경의 회귀를 훨씬 빨리 잡을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandServiceTest.java` around lines 94 - 100, Update stubWebClientPut to capture and assert the actual body passed to bodyValue instead of using bodyValue(any()); specifically, in the stubWebClientPut method (which stubs mockWebClient.put(), putUriSpec.uri(...), and doReturn(putHeadersSpec).when(putBodySpec).bodyValue(...)), use an ArgumentCaptor or doAnswer on putBodySpec.bodyValue to capture the UserCreationRequestDTO and assert its username, passwordBase64 and primaryGroupName match expected values (and fail the test if not), so changes to uid/gid removal or passwd_base64 conversion are detected; reference the UserCreationRequestDTO object captured from the bodyValue call in your assertions.src/test/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationServiceTest.java (1)
36-55: ⚡ Quick win저장 호출 검증이 빠져서 회귀를 놓치기 쉽습니다.
지금 두 케이스는
IdCounter가 메모리에서 증가하기만 해도 통과해서, 서비스에서saveAndFlush(counter)가 빠져도 실패하지 않습니다. 성공/연속 할당 시 저장 호출까지 검증해 두는 편이 이 메서드의 실제 계약을 더 정확히 고정합니다.예시 수정
-import static org.mockito.Mockito.when; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @@ Long gid = idAllocationService.allocateNewGid(); assertThat(gid).isEqualTo(2000L); assertThat(counter.getNextValue()).isEqualTo(2001L); + verify(idCounterRepository).saveAndFlush(counter); @@ assertThat(first).isEqualTo(2000L); assertThat(second).isEqualTo(2001L); assertThat(third).isEqualTo(2002L); + verify(idCounterRepository, times(3)).saveAndFlush(counter);Also applies to: 88-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationServiceTest.java` around lines 36 - 55, The test IdAllocationServiceTest.success currently only verifies in-memory increment of IdCounter and misses asserting that the service persisted the change; add a Mockito verify for idCounterRepository.saveAndFlush(...) after calling idAllocationService.allocateNewGid() to ensure save is invoked with the updated IdCounter (and mirror this verification in the other test case(s) around lines 88-110); locate usages of idCounterRepository.saveAndFlush and the allocateNewGid invocation to add verify(idCounterRepository).saveAndFlush(argThat(counter -> counter.getKey()==CounterKey.SHARED_GID && counter.getNextValue()==2001L)) or equivalent to assert the repository save was called with the incremented counter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandService.java`:
- Around line 102-105: The code is passing a SHA-512 hex hash into
UserCreationRequestDTO.passwordBase64 via request.getUbuntuPassword(), but
Request.ubuntuPassword is already hashed in RequestCommandService using
PasswordUtil.encodePassword(); fix by supplying the original base64 password
that the config-server expects instead of the hashed value — either: (A) change
the Request model to store the original base64 password (e.g.,
ubuntuPasswordBase64) at creation time in RequestCommandService before calling
PasswordUtil.encodePassword(), and use request.getUbuntuPasswordBase64() when
constructing UserCreationRequestDTO in AdminRequestCommandService, or (B)
retrieve the original base64 credential from the secure source where it was kept
at creation/approval and pass that into UserCreationRequestDTO; update
references to PasswordUtil.encodePassword(), Request.ubuntuPassword, and
UserCreationRequestDTO(passwordBase64) accordingly.
---
Nitpick comments:
In
`@src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandServiceTest.java`:
- Around line 94-100: Update stubWebClientPut to capture and assert the actual
body passed to bodyValue instead of using bodyValue(any()); specifically, in the
stubWebClientPut method (which stubs mockWebClient.put(), putUriSpec.uri(...),
and doReturn(putHeadersSpec).when(putBodySpec).bodyValue(...)), use an
ArgumentCaptor or doAnswer on putBodySpec.bodyValue to capture the
UserCreationRequestDTO and assert its username, passwordBase64 and
primaryGroupName match expected values (and fail the test if not), so changes to
uid/gid removal or passwd_base64 conversion are detected; reference the
UserCreationRequestDTO object captured from the bodyValue call in your
assertions.
In
`@src/test/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationServiceTest.java`:
- Around line 36-55: The test IdAllocationServiceTest.success currently only
verifies in-memory increment of IdCounter and misses asserting that the service
persisted the change; add a Mockito verify for
idCounterRepository.saveAndFlush(...) after calling
idAllocationService.allocateNewGid() to ensure save is invoked with the updated
IdCounter (and mirror this verification in the other test case(s) around lines
88-110); locate usages of idCounterRepository.saveAndFlush and the
allocateNewGid invocation to add
verify(idCounterRepository).saveAndFlush(argThat(counter ->
counter.getKey()==CounterKey.SHARED_GID && counter.getNextValue()==2001L)) or
equivalent to assert the repository save was called with the incremented
counter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0dc85690-2296-4263-a532-0f2bafc08693
📒 Files selected for processing (20)
src/main/java/DGU_AI_LAB/admin_be/domain/groups/entity/Group.javasrc/main/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupService.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/request/UserCreationRequestDTO.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/AcceptInfoResponseDTO.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/ContainerInfoDTO.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/SaveRequestResponseDTO.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/entity/Request.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/repository/RequestRepository.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandService.javasrc/main/java/DGU_AI_LAB/admin_be/domain/requests/service/RequestExpiryService.javasrc/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/CounterKey.javasrc/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/UsedId.javasrc/main/java/DGU_AI_LAB/admin_be/domain/usedIds/repository/UsedIdRepository.javasrc/main/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationService.javasrc/test/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupServiceTest.javasrc/test/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandServiceTest.javasrc/test/java/DGU_AI_LAB/admin_be/domain/requests/service/ConfigRequestServiceTest.javasrc/test/java/DGU_AI_LAB/admin_be/domain/scheduler/RequestSchedulerServiceTest.javasrc/test/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/IdCounterTest.javasrc/test/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationServiceTest.java
💤 Files with no reviewable changes (12)
- src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/UsedId.java
- src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/CounterKey.java
- src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/repository/UsedIdRepository.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/SaveRequestResponseDTO.java
- src/test/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupServiceTest.java
- src/main/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupService.java
- src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/ConfigRequestServiceTest.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/AcceptInfoResponseDTO.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/entity/Request.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/RequestExpiryService.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/repository/RequestRepository.java
- src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/ContainerInfoDTO.java
Comment on lines
102
to
105
| UserCreationRequestDTO userCreationDto = new UserCreationRequestDTO( | ||
| request.getUbuntuUsername(), | ||
| allocation.getUid().getIdValue().intValue(), | ||
| allocation.getPrimaryGroup().getUbuntuGid().intValue(), | ||
| request.getUbuntuPassword(), | ||
| request.getUser().getName(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Request entity definition =="
fd 'Request.java' src/main -x sed -n '1,260p' {}
echo
echo "== Producers/consumers of ubuntuPassword / passwd_base64 =="
rg -n -C3 --type=java 'ubuntuPassword|passwd_base64|passwd_sha512|passwordBase64' src/main src/testRepository: CSID-DGU/admin_be
Length of output: 26602
🏁 Script executed:
fd 'AdminRequestCommandService.java' src/main -x cat -n {}Repository: CSID-DGU/admin_be
Length of output: 14874
passwd_base64 필드에 SHA-512 해시가 전달되고 있습니다. 이는 심각한 계약 위반입니다.
Line 104에서 request.getUbuntuPassword()를 UserCreationRequestDTO의 passwordBase64 필드로 전달하고 있습니다. 그러나 Request.ubuntuPassword는 저장 시점(RequestCommandService 라인 181)에서 이미 PasswordUtil.encodePassword()로 SHA-512 해싱되어 있습니다. 따라서 현재 request.getUbuntuPassword()는 base64 형식이 아닌 SHA-512 16진수 해시입니다. Config-server의 사용자 생성 API는 base64 형식의 패스워드를 기대하므로, 이 변경으로 인해 사용자 생성이 실패하거나 잘못된 패스워드 형식으로 생성될 것입니다.
원본 요청 생성 흐름에서 base64 패스워드를 별도로 저장하거나, 승인 흐름에서 해시 전 원본 패스워드를 별도 소스에서 가져오는 방식으로 수정해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandService.java`
around lines 102 - 105, The code is passing a SHA-512 hex hash into
UserCreationRequestDTO.passwordBase64 via request.getUbuntuPassword(), but
Request.ubuntuPassword is already hashed in RequestCommandService using
PasswordUtil.encodePassword(); fix by supplying the original base64 password
that the config-server expects instead of the hashed value — either: (A) change
the Request model to store the original base64 password (e.g.,
ubuntuPasswordBase64) at creation time in RequestCommandService before calling
PasswordUtil.encodePassword(), and use request.getUbuntuPasswordBase64() when
constructing UserCreationRequestDTO in AdminRequestCommandService, or (B)
retrieve the original base64 credential from the secure source where it was kept
at creation/approval and pass that into UserCreationRequestDTO; update
references to PasswordUtil.encodePassword(), Request.ubuntuPassword, and
UserCreationRequestDTO(passwordBase64) accordingly.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the account/group provisioning flow so the WAS no longer tracks Ubuntu UIDs locally and instead delegates user UID/primary-group ownership to the config-server, while keeping only local shared-GID allocation. In the broader codebase, this removes the old used_ids persistence model from request/group approval flows and trims the admin/config DTOs and tests to match the new contract.
Changes:
- Remove local UID tracking and
used_idspersistence (UsedId, repository, request field, release/reuse logic,CounterKey.UID). - Simplify approval/config DTOs so user creation no longer sends UID/GID and response payloads no longer expose UID.
- Update group/shared-GID allocation and related tests to work without
used_ids.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/test/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationServiceTest.java |
Updates allocator tests to cover shared-GID-only behavior. |
src/test/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/IdCounterTest.java |
Switches counter tests from UID to shared GID enum usage. |
src/test/java/DGU_AI_LAB/admin_be/domain/scheduler/RequestSchedulerServiceTest.java |
Removes UsedId setup/cleanup from scheduler integration test fixtures. |
src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/ConfigRequestServiceTest.java |
Adapts config accept-info tests after UID removal from responses. |
src/test/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandServiceTest.java |
Updates approval tests after removing local UID allocation dependency. |
src/test/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupServiceTest.java |
Simplifies group-service tests to no longer mock UsedId storage. |
src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/service/IdAllocationService.java |
Removes UID/release logic and leaves only shared-GID allocation. |
src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/repository/UsedIdRepository.java |
Deletes the obsolete used_ids repository layer. |
src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/UsedId.java |
Deletes the obsolete used_ids entity. |
src/main/java/DGU_AI_LAB/admin_be/domain/usedIds/entity/CounterKey.java |
Removes the UID counter key, keeping only SHARED_GID. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/RequestExpiryService.java |
Stops returning/releasing UIDs during expiry deletion. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandService.java |
Reworks approval flow to create users without local UID allocation. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/repository/RequestRepository.java |
Removes the username+UID lookup that supported UID reuse. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/entity/Request.java |
Drops the persisted Ubuntu UID association from requests. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/SaveRequestResponseDTO.java |
Removes UID from admin request detail responses. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/ContainerInfoDTO.java |
Removes UID from active-container response payloads. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/response/AcceptInfoResponseDTO.java |
Removes UID from config-server accept-info responses. |
src/main/java/DGU_AI_LAB/admin_be/domain/requests/dto/request/UserCreationRequestDTO.java |
Changes user-creation payload shape to omit UID/GID and send base64 password field. |
src/main/java/DGU_AI_LAB/admin_be/domain/groups/service/GroupService.java |
Removes used_ids persistence from shared-group creation flow. |
src/main/java/DGU_AI_LAB/admin_be/domain/groups/entity/Group.java |
Drops the UsedId one-to-one relationship from groups. |
Comments suppressed due to low confidence (1)
src/main/java/DGU_AI_LAB/admin_be/domain/requests/service/AdminRequestCommandService.java:153
- Removing the primary-group link here means approved requests no longer persist that group anywhere locally. Every response DTO that still exposes GIDs (
SaveRequestResponseDTO,ContainerInfoDTO,AcceptInfoResponseDTO) builds the list fromrequestGroups, so an approved request with no extra groups will now report an empty GID list even though config-server created a primary group for the user.
// 4. API 호출이 모두 성공한 후에 DB 업데이트
ContainerImage image = containerImageRepository.findById(dto.imageId())
.orElseThrow(() -> new BusinessException(ErrorCode.RESOURCE_NOT_FOUND));
ResourceGroup rg = resourceGroupRepository.findById(dto.resourceGroupId())
.orElseThrow(() -> new BusinessException(ErrorCode.RESOURCE_NOT_FOUND));
request.approve(image, rg, dto.volumeSizeGiB(), dto.adminComment());
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
25
to
27
| @Column(name = "ubuntu_gid", unique = true, nullable = false) | ||
| private Long ubuntuGid; | ||
|
|
Comment on lines
134
to
136
| stubWebClientPut(); | ||
| stubWebClientPost(); | ||
|
|
Comment on lines
102
to
+107
| UserCreationRequestDTO userCreationDto = new UserCreationRequestDTO( | ||
| request.getUbuntuUsername(), | ||
| allocation.getUid().getIdValue().intValue(), | ||
| allocation.getPrimaryGroup().getUbuntuGid().intValue(), | ||
| request.getUbuntuPassword(), | ||
| request.getUser().getName(), | ||
| allocation.getPrimaryGroup().getGroupName(), | ||
| false // sudo 권한은 기본값으로 false를 설정 | ||
| request.getUbuntuUsername(), // primary_group_name = username (Linux 관례) | ||
| false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UsedId엔티티 및UsedIdRepository삭제 (used_ids테이블 제거)IdAllocationService에서 UID 할당 로직(allocateFor,AllocationResult) 제거, GID 할당(allocateNewGid)만 유지CounterKeyenum에서UID제거,SHARED_GID만 유지UserCreationRequestDTO에서uid,gid필드 제거 (config-server가 uid 직접 부여)Request엔티티에서ubuntuUid필드 및assignUbuntuUid()제거Group엔티티에서UsedIdOneToOne 관계 제거AcceptInfoResponseDTO,SaveRequestResponseDTO,ContainerInfoDTO에서 uid 필드 제거GroupService.createGroup()에서used_ids중복 추적 제거RequestExpiryService에서 uid 반납 로직 제거 (config-server 소관)Test plan
IdAllocationServiceTest— GID 순차 할당, 범위 초과 예외 검증IdCounterTest—allocateOne()경계값 검증GroupServiceTest— 그룹 생성/조회 정상 동작AdminRequestCommandServiceTest— 승인 시 PodExternalPort 저장 검증RequestSchedulerServiceTest— 만료 삭제 및 알림 발송 통합 검증Closes #213
🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
버그 수정
리팩토링