-
Notifications
You must be signed in to change notification settings - Fork 1
ROX-35435: Support differing versions of Central and SecuredCluster #246
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
base: main
Are you sure you want to change the base?
Changes from all commits
85ebd0d
05bd699
c89ec22
ca14c02
d63b2af
e96547a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think before diving deeper, we should align on the changes to the config structure. I guess there are many ways we could implement this. What I consider a limitation of your approach here is that there is no way to define per-operator settings (there is just one Just thinking loud here, what about we allow users to have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to think a bit about this PR from a conceptual viewpoint. But, looking at the concrete changes, I can already provide limited feedback.
For example, I don't think we need these flags. Direct CLI flags (IMHO) are remnants from the time before we had the unified roxie config. Everything can be set there and these settings specifically are not settings that we need every day (outside of automation), hence I think it's perfectly fine to drop these and let users configure those in the roxie config if they need to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a roxie user I'd actually like to have them.
To give you an example, for cert refresh and CA rotation I had to do a lot of mixed version testing. It was quite tedious to do and because of that I didn't do as much compatibility testing as I should have. And these flags would make it quite easy to be able to switch around versions easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they are important for you, fine.
But you do know that you can do the same with
--set, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, didn't think about that. Then I'm inclined to agree with you.
Having a flag is still somewhat useful because it makes the feature more easily discoverable, that's the only argument I have for it now.