feat: Add get_attachment_details tool #75
Conversation
- Add get_attachmet_id that return attchement details - Tests are updated and passing
|
|
||
| attachment = conn.block_storage.get_attachment(attachment_id) | ||
|
|
||
| # NOTE: We exclude the auth_* fields for security reasons |
There was a problem hiding this comment.
auth 관련 필드정보가 필터링 되어야하는 이유가 있을까요?
There was a problem hiding this comment.
@halucinor
auth_* 부분( auth_mehtod, auth_username, auth_password)을 필터링한 이유는 cinder에 접근하기 위한 중요한 인증 정보이기 때문입니다.
auth_method=CHAP인 경우를 예로 들자면 Cinder에서 볼륨을 attach할때 컴퓨트 노드는 해당 계정을 활용하여 target에 대한 로그인을 수행하고, target은 이를 검증하여 허용된 initiator만 세션을 주고받게 됩니다. (참고)
하지만 이러한 인증 정보가 노출되면 cinder에 대한 무단 접근이 가능해집니다. 이는 데이터 탈취와 같은 심각한 보안 문제가 발생할 수 있습니다.
개인적인 이유로, mcp서버가 관리자 등 다양한 사용자를 대상으로 한다고 할지라도 인증 정보 필터링등 최소한의 안전 장치는 마련해야 한다고 생각합니다.
S0okJu
force-pushed
the
feat/volume-attachment
branch
from
5ffc0c7 to
0f071d4
Compare
|
|
||
| # Verify mcp.tool() was called for each method | ||
| assert mock_mcp.tool.call_count == 5 | ||
| assert mock_mcp.tool.call_count == 6 |
There was a problem hiding this comment.
mock_mcp.tool.call_count 갯수 확인이 어떤 의미를 주는지 와닿지 않는 상태로 말씀드립니다.
도구가 추가되거나 삭제되면 테스트 케이스를 매번 수정해줘야할 것 같아 유지보수 부담을 줄 수 있을 것 같네요
이런 테스트 케이스는 삭제하면 어떨까요?
요고 리뷰에서 논의되서 지우는 방향으로가면 제가 나머지 테스트 함수들 확인해보겠습니다.
(오늘 멘토링 내용중에 '의미있는 테스트를 작성하는가'에 대한 이야기가 지나가듯이 나와서 영감을 받아 코멘트남깁니다.)
There was a problem hiding this comment.
위의 코드에 대해서 비슷한 느낌을 받았습니다.
해당 테스트 케이스를 삭제하겠습니다!
There was a problem hiding this comment.
#77
관련해서 디스커션 생성해봤습니다! (디스커션 기능 처음 써 봄)
1~2 팀원분들 추가 의견들어볼게욥! ㅋㅋ
platanus-kr
added
the
feature
Overview
get_attachment_detailstoolKey Changes
get_attachmet_detailsthat return attchement detailsRelated Issues
Additional context