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.

Sign up for GitHub

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

Jump to bottom

(ruka/loki) Add Loki user to Ruka Ceph object store #637

Merged
merged 1 commit into from
Oct 22, 2024
Merged

(ruka/loki) Add Loki user to Ruka Ceph object store #637

merged 1 commit into from
Oct 22, 2024

Conversation

gseriche
Copy link
Contributor

No description provided.

@gseriche gseriche added the enhancement New feature or request label Oct 21, 2024
@gseriche gseriche self-assigned this Oct 21, 2024
cbarria
cbarria requested changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@cbarria cbarria left a comment

Choose a reason for hiding this comment

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

  • Commit message is not Present Imperative.
  • This user doesn't push credentials to 1password like the other users in the folder? is this correct?

dtapiacl
dtapiacl requested changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@dtapiacl dtapiacl left a comment

Choose a reason for hiding this comment

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

Where are the credentials of this user being pushed to ?

cbarria
cbarria requested changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@cbarria cbarria left a comment

Choose a reason for hiding this comment

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

dunno why i got re-requested when i already reviewed this, but it is still missing lots of stuff.

@csilva-cl
Copy link
Contributor

dunno why i got re-requested when i already reviewed this, but it is still missing lots of stuff.

I added you the review list, didn't mean to trigger a review, given the comment you left.

@gseriche
Copy link
Contributor Author

  • Commit message is not Present Imperative.
  • This user doesn't push credentials to 1password like the other users in the folder? is this correct?

Yes, We talked with @jhoblitt about that; the credentials don't need to be stored in 1Password:

The end users do not need access to the Loki bucket(s). Also, Loki will be deployed in the same cluster as RGW, so pushing the credentials out to 1Pass is unnecessary.

@gseriche gseriche changed the title (ruka/loki) loki user added into ruka ceph object store user (ruka/loki) Add Loki user to Ruka Ceph object store Oct 21, 2024
@gseriche gseriche requested a review from cbarria October 21, 2024 21:03
@gseriche gseriche mentioned this pull request Oct 22, 2024
Open
@gseriche gseriche force-pushed the IT-5699_setup_loki_user_on_ruka_cephstorage branch 3 times, most recently from 7bc9bbc to 66a5f02 Compare October 22, 2024 14:28
@gseriche
(ruka/loki) add user to loki into ruka ceph object store user
07cf9e4 
(ruka/loki) Push loki user to 1Password

(ruka/loki) Push loki user to 1Password ruka.dev

(ruka/loki) Push loki user to 1Password

(ruka/loki) Push updated loki user to 1Password
@gseriche gseriche force-pushed the IT-5699_setup_loki_user_on_ruka_cephstorage branch from 66a5f02 to 07cf9e4 Compare October 22, 2024 14:30
cbarria
cbarria approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@cbarria cbarria left a comment

Choose a reason for hiding this comment

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

changes requested were made and code looks good.

dtapiacl
dtapiacl approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@dtapiacl dtapiacl left a comment

Choose a reason for hiding this comment

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

LGTM.

@gseriche gseriche merged commit 9bfd6d3 into master Oct 22, 2024
8 checks passed
@gseriche gseriche deleted the IT-5699_setup_loki_user_on_ruka_cephstorage branch October 22, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtapiacl dtapiacl dtapiacl approved these changes

@cbarria cbarria cbarria approved these changes

@jhoblitt jhoblitt Awaiting requested review from jhoblitt

Assignees

@gseriche gseriche

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gseriche @csilva-cl @cbarria @dtapiacl