-
Notifications
You must be signed in to change notification settings - Fork 164
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
Allow optional entry to /ruler/rule_groups endpoint #476
base: master
Are you sure you want to change the base?
Allow optional entry to /ruler/rule_groups endpoint #476
Conversation
1249853
to
48942d1
Compare
Signed-off-by: AlexDHoffer <alexdchoffer@gmail.com>
48942d1
to
1125b62
Compare
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.
Custom route "injection" is already supported by setting .Values.nginx.config.serverSnippet
I'm inclined to reject this PR on the basis that this would just add more spaghetti templating
The custom route injection for this is kind of hideous:
Or is there a way to call that $rootDomain function the other routes use from the values file? |
I don't know, have you tried setting $rootDomain? I assume the context might be different |
This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
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.
To be honest I can't really follow my reasoning before as to why this not worth doing.
We can definitely get this in.
@@ -722,6 +722,8 @@ ruler: | |||
enabled: true | |||
readOnlyRootFilesystem: true | |||
|
|||
list_all_rule_groups_api: false |
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.
Can we move this under nginx.config
? Considering that the docs say it's not part of the ruler api, this should be a bit better suited there. Also might I suggest a rename to something like exposeRulerRulesAPIEndpoint
?
Whatever you choose please no snake_case
. You can also add a doc string
What this PR does: Allow optional entry into /ruler/rule_groups endpoint via nginx proxy
Which issue(s) this PR fixes:
Fixes #475
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]