Skip to content

Conversation

@namsic
Copy link
Member

@namsic namsic commented Oct 27, 2025

🔗 Related Issue

  • jam2in/arcus-works#801

⌨️ What I did

@namsic namsic requested a review from jhpark816 October 27, 2025 11:33
@jhpark816 jhpark816 requested review from oliviarla and removed request for jhpark816 October 27, 2025 12:16
Copy link
Member

@oliviarla oliviarla left a comment

Choose a reason for hiding this comment

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

질문 1) 관리자는 비밀번호 변경이 불가능한지?
질문 2) 설정 파일에는 항상 zookeeper 정의만 포함되는지 아니면 다른게 추가될 가능성이 있는지? acl에서만 활용되는지?

궁금한 점입니다.

@namsic
Copy link
Member Author

namsic commented Oct 28, 2025

질문 1) 관리자는 비밀번호 변경이 불가능한지?

아래 인터페이스로 추가해두겠습니다. 혹시 다른 의견 있으면 주세요.

./arcusctl acl admin passwd <group>
# admin name:
# password:
# new admin password: 
# repeat new admin password:
# OK

질문 2) 설정 파일에는 항상 zookeeper 정의만 포함되는지 아니면 다른게 추가될 가능성이 있는지? acl에서만 활용되는지?

현재 구현 자체는 acl 외 다른 기능에서도 사용하는 것을 어느정도 고려한 구조입니다.
예를 들어, 아래와 같은 설정 파일을 작성해 두고 여러 명령에서 사용할 수 있습니다.

zookeeper: "zookeeper.example.com:2181"
memcached:
  path: "/home/arcus/.local"
./arcusctl acl group add # zookeeper만 사용
./arcusctl memcached start # zookeeper, memcached.path 사용

단, 이는 명령형 인터페이스를 기준으로 하는 구현이고 선언형 인터페이스와는 잘 맞지 않을 수 있어서
향후 인터페이스가 어떻게 결정되는지에 따라서 현재 구현(zookeeper 필드 하나 + acl에서만 사용)으로 그칠 가능성도 있습니다.

@oliviarla
Copy link
Member

이거 개발이 다 된 내용인건가요?

@namsic
Copy link
Member Author

namsic commented Oct 28, 2025

이거 개발이 다 된 내용인건가요?

acl 명령은 (admin passwd 제외하고) develop branch에 반영된 상태입니다.
acl과 무관한 기능은 초기 릴리즈에 포함하지 않을 계획입니다.

./arcusctl acl user list sample
# * app (kv,list,set,map,btree,attr,scan,flush)
# * *operator (attr,scan,flush,admin)
# Total 2
Copy link
Contributor

Choose a reason for hiding this comment

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

지난 번에 user list 보여줄 때의 형식이 계속 바뀌고 있는 것 같습니다.

logXXXXX 플래그로 표시된 형태를 보인 적이 있었는 데, 그 형태를 참고용으로 알려주세요. 그 형태를 명확하여 괜찮다고 생각하고 있었습니다. 다시 한번 비교해 보려고 합니다.

Copy link
Member Author

@namsic namsic Oct 28, 2025

Choose a reason for hiding this comment

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

아래와 같이 할 수 있습니다.

./arcusctl acl user list sample
#   * app (kv,list,set,map,btree,attr,scan,flush)
#   * operator (attr,scan,flush,admin) logAll
# Total 2

그러면, user 변경 명령에도 아래와 같이 logAll 인자 추가됩니다.

  • user add operator admin logAll로 사용자 추가한 상태에서,
    user add operator admin 사용자 다시 추가할 수 있습니다.
  • 위 두 사용자는 서로 다른 별개의 계정입니다.
  • 따라서, operator logAll 사용자 정보 변경 시 항상 logAll 인자를 함께 전달해야 합니다.
./arcusctl acl user add operator attr,scan,flush,admin logAll
./arcusctl acl user passwd operator logAll
./arcusctl acl user permission operator attr,scan,flush,admin logAll
./arcusctl acl user remove operator logAll

Copy link
Contributor

Choose a reason for hiding this comment

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

위 두 사용자는 서로 다른 별개의 계정입니다.

logAll 명시하는 방식이 나아 보이는 데요.

위의 문제는 단지 구현 문제로 보입니다.
하나의 사용자로 처리할 수 있지 않나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

위의 문제는 단지 구현 문제로 보입니다. 하나의 사용자로 처리할 수 있지 않나요?

아래와 같은 전제 하에 logAll 형태 구현을 사용할 수 있습니다.

  1. (arcusctl 구현 측면에서)
    alice라는 사용자를 생성/제거할 때, 단순히 znode create / delete 요청을 보내는 것이 아니라
    alice*alice에 대한 exists 요청을 보내어 기존 사용자의 존재 여부를 확인하고
    그 결과에 따라 보낼 연산을 분기하는 형태로 구현할 수 있습니다.

  2. 위와 같이 구현하더라도 캐시 서버 입장에서는 alice*alice를 서로 다른 계정으로 인식하며, 두 계정이 동시에 존재할 수도 있습니다.

  3. 마찬가지로, 클라이언트에서도 alice logAll로 생성한 계정을 사용할 때는 username을 *alice로 입력해야 하고,
    alice로 입력 시 존재하지 않는 사용자로 인식합니다.

캐시 서버 수준에서부터 하나의 사용자로 인식하도록 구현을 변경하려면,
캐시 서버가 username의 * 인디케이터를 통해 로그 기록 여부를 구분하는 것이 아니라,
logAll이라는 권한을 가진 사용자에 대해 로그 남기도록 처리할 수 있습니다.

user add alice kv,logAll,list

@jhpark816
Copy link
Contributor

jhpark816 commented Oct 29, 2025

질문입니다.
아래와 같은 제약을 두어야 하지 않는지 ? 제약을 둔다면 문서에도 명시하는 것이 좋겠습니다.

  • 최대 생성 가능한 acl group 개수
  • 1개 acl group에 생성할 수 있는 최대 user 수

@namsic
Copy link
Member Author

namsic commented Oct 29, 2025

아래와 같은 제약을 두어야 하지 않는지 ? 제약을 둔다면 문서에도 명시하는 것이 좋겠습니다.

  • 최대 생성 가능한 acl group 개수
  • 1개 acl group에 생성할 수 있는 최대 user 수
  • 제약을 두더라도 arcusctl 수준에서의 로직이고, zkCli 또는 기타 도구를 사용하여 znode를 다수 생성하는 것은 막을 수 없습니다.
  • znode 조회하여 validation하는 작업을 추가로 수행하기 때문에 arcusctl에서 ZK에 전송하는 요청 수는 오히려 증가합니다.

제약을 두면 어떤 점에서 유용한지 충분히 이해한 것은 아니지만, 각 제약에 대한 예시 값을 알려주시면 구현 및 문서 추가할 수 있습니다.

Copy link
Contributor

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

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

리뷰 완료


### 0) 사전 준비

[설정 파일 또는 환경 변수](./config-file.md)를 통해 zookeeper 주소를 지정합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

아래 문장으로 변경하시죠.


Arcus ACL 정보를 저장, 관리하는 ZooKeeper에 연결하기 위하여,
설정 파일 또는 환경 변수를 통해 ZooKeeper 주소를 지정합니다.

./arcusctl acl user list sample
# * app (kv,list,set,map,btree,attr,scan,flush)
# * *operator (attr,scan,flush,admin)
# Total 2
Copy link
Contributor

Choose a reason for hiding this comment

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

위 두 사용자는 서로 다른 별개의 계정입니다.

logAll 명시하는 방식이 나아 보이는 데요.

위의 문제는 단지 구현 문제로 보입니다.
하나의 사용자로 처리할 수 있지 않나요?


```sh
./arcusctl acl user list sample
# * app (kv,list,set,map,btree,attr,scan,flush)
Copy link
Contributor

Choose a reason for hiding this comment

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

권한 표현 방식이 다음과 같습니다.

  • user add 경우 : kv,list,set,map,btree,attr,scan,flush
  • user list 경우 : (kv,list,set,map,btree,attr,scan,flush)

user add 하는 경우에도 아래 표현을 사용하는 것은 어떤가요?
의견만 들었으면 합니다.

  • (kv,list,set,map,btree,attr,scan,flush)

Copy link
Member Author

Choose a reason for hiding this comment

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

user add 하는 경우에도 아래 표현을 사용하는 것은 어떤가요? 의견만 들었으면 합니다.
(kv,list,set,map,btree,attr,scan,flush)

괄호는 의미가 없는 표현임에도 파싱 로직이 추가되어야 해서 좋지 않아 보입니다.
통일성 측면에서 한 쪽으로 맞춘다면 list 결과에서 괄호 제거하는 편이 낫겠습니다.

  * app kv,list,set,map,btree,attr,scan,flush
  * admin attr,scan,flush,admin

# repeat user password:
# OK
```
- 기존 사용자의 비밀번호를 변경합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

일반 사용자는 자신의 비밀번호를 변경하도록 제공해야 하지 않는 지?

Copy link
Member Author

Choose a reason for hiding this comment

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

이전 논의에서 ZooKeeper ACL의 특성에 따른 구현 복잡성 때문에 admin에 의한 변경만 지원하는 것으로 하였는데,
arcus user가 자신의 비밀번호를 변경할 수 있도록 할까요?

이 경우, "admin이 user 비밀번호 변경" / "user가 자신 비밀번호 변경"하는 경우 인터페이스를 결정해야 합니다.

# admin password:
# OK
```
- 기존 사용자의 권한을 변경합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

admin 사용자만 수행할 수 있다고 명시 필요

Copy link
Contributor

Choose a reason for hiding this comment

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

아래 문구를 추가합시다.

  • logAll 설정은 변경할 수 없으며, 사용자 생성 시에만 설정할 수 있다.

@namsic
Copy link
Member Author

namsic commented Oct 29, 2025

논의 결과

  1. 서버 수준에서 로그 기록 여부 판단 시 znode key(username) 대신 znode value(permission) 사용하도록 변경
  2. arcusctl에서 logAll 인터페이스 적용
  3. ZK 내 그룹 수, 그룹 내 유저 수 제한하지 않음
  4. user 자신 비밀번호 변경 기능 제공하지 않음

@namsic namsic force-pushed the docs branch 2 times, most recently from cfa39d5 to 53f094d Compare October 29, 2025 12:33
Copy link
Contributor

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

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

리뷰 완료

# admin password:
# OK
```
- 기존 사용자의 권한을 변경합니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

아래 문구를 추가합시다.

  • logAll 설정은 변경할 수 없으며, 사용자 생성 시에만 설정할 수 있다.

@jhpark816 jhpark816 merged commit 6e477af into develop Oct 29, 2025
1 check passed
@namsic namsic deleted the docs branch October 29, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants