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

Improve RBAC backup / restore behavior #793

Closed
Slach opened this issue Dec 1, 2023 · 6 comments · Fixed by #843
Closed

Improve RBAC backup / restore behavior #793

Slach opened this issue Dec 1, 2023 · 6 comments · Fixed by #843
Assignees
Milestone

Comments

@Slach
Copy link
Collaborator

Slach commented Dec 1, 2023

Expected behavior

  1. 'rbac' should be always backed up by default (in new clickhouse-backup version)
  2. clickhouse-backup should not fail if it can not restore rbac, but it should log warnings/errors.

Actual notes

  • third item of list could be easy implements in next release
  • second will implement as a config option, with default true behavior
    this behavior is dangerous

restore rbac requires restart of clickhouse-server
and if clickhouse-server already contains RBAC object with the same name and different uuid, server will not start

Good point. Is it possible to check if there is any RBAC data on the cluster and do not restore in case of a conflict?

maybe it requires develop some sql parser

to get name+type+uuid from .sql and .jsonl files
need time to researh for this task

maybe, we can use system.users system.roles, system.role_granst came in modern versions of clickhouse
need to check when these tables is become

conflict resolution rules

restore without --rm
show error about RBAC object names conflict
restore --rm
show warning about RBAC object names conflict
delete old object
restore new object

@Slach Slach self-assigned this Dec 1, 2023
@Slach Slach added this to the 2.5.0 milestone Dec 1, 2023
Slach added a commit that referenced this issue Dec 1, 2023
… not present in backup, but it should log warnings/errors, look #793 for details
@jalavoy
Copy link

jalavoy commented Feb 8, 2024

Is there a rough timeline on this release? In the meantime is there a easy way to patch rbac into the existing release?

Looks like we could just throw something into the k8s cronjob command block no?

@Slach
Copy link
Collaborator Author

Slach commented Feb 9, 2024

@jalavoy
if you want to backup and restore RBAC related objects

just add to cronjob command --rbac parameter after create_remote and restore_remote

@jalavoy
Copy link

jalavoy commented Feb 9, 2024

@Slach
create_remote isn't called in the k8s cronjob. It's doing an insert into the table to trigger the backup. What's the proper way to add it to this insert?

                  for SERVER in $CLICKHOUSE_SERVICES; do
                    echo "create ${BACKUP_NAMES[$SERVER]} on $SERVER";
                    clickhouse-client --echo -mn -q "INSERT INTO system.backup_actions(command) VALUES('create ${SERVER}-${BACKUP_NAMES[$SERVER]}')" --host="$SERVER" --port="$CLICKHOUSE_PORT" --user="$BACKUP_USER" $BACKUP_PASSWORD;
                  done;

@Slach
Copy link
Collaborator Author

Slach commented Feb 9, 2024

VALUES('create --rbac

VALUES('restore --rbac

@jalavoy
Copy link

jalavoy commented Feb 9, 2024

that didn't work but I got it, need to add it into all the SELECTS too

Thank you for your help!

@Slach
Copy link
Collaborator Author

Slach commented Apr 1, 2024

decided to don't restore RBAC,only when --rbac excplicitly, to avoid overwrite exists users passwords or some other unexpected errors

@Slach Slach mentioned this issue Apr 3, 2024
@Slach Slach closed this as completed in #843 Apr 8, 2024
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 a pull request may close this issue.

2 participants