gitea/helm-chart
45
54
Fork 127
Code Issues 33 Pull Requests 9 Actions Releases 87 Activity

feat(service-monitor): support bearer token authentication on metrics endpoint #637

Open
hiteshnayak305 wants to merge 6 commits from hiteshnayak305/helm-chart:main into main
pull from: hiteshnayak305/helm-chart:main
merge into: gitea:main
Conversation 5 Commits 6 Files Changed 8 +164
hiteshnayak305 commented 2024-04-03 21:05:28 +00:00
First-time contributor
Copy Link

Description of the change

Benefits

Can protect metrics endpoint when monitoring using ServiceMonitor of prometheus-operator.

Possible drawbacks

No possible drawbacks

Applicable issues

Additional information

gitea:
  config:
    metrics:
      # ENABLED: true
      TOKEN: "somepassword"
  metrics:
    enabled: true
    serviceMonitor:
      enabled: true

Using above configuration will not give 401 authentication after changes.

⚠ BREAKING

No breaking changes

Checklist

  • Parameters are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Breaking changes are documented in the README.md
  • Templating unittests are added

Signed-off-by: Hitesh Nayak hiteshnayak305@gmail.com

<!-- Before you open the request please review the following guidelines and tips to help it be more easily integrated: - Describe the scope of your change - i.e. what the change does. - Describe any known limitations with your change. - Please run any tests or examples that can exercise your modified code. Thank you for contributing! We will try to review, test and integrate the change as soon as we can. --> ### Description of the change <!-- Describe the scope of your change - i.e. what the change does. --> ### Benefits <!-- What benefits will be realized by the code change? --> Can protect metrics endpoint when monitoring using ServiceMonitor of prometheus-operator. ### Possible drawbacks No possible drawbacks <!-- Describe any known limitations with your change --> ### Applicable issues <!-- Enter any applicable Issues here (You can reference an issue using #). Please remove this section if there is no referenced issue. --> - fixes #635 ### Additional information <!-- If there's anything else that's important and relevant to your pull request, mention that information here. Please remove this section if it remains empty. --> ``` gitea: config: metrics: # ENABLED: true TOKEN: "somepassword" metrics: enabled: true serviceMonitor: enabled: true ``` Using above configuration will not give 401 authentication after changes. ### ⚠ BREAKING <!-- If there's a breaking change, please shortly describe in which way users are affected and how they can mitigate it. If there are no breakings, please remove this section. --> No breaking changes ### Checklist <!-- [Place an '[X]' (no spaces) in all applicable fields. Please remove unrelated fields.] --> - ~~Parameters are documented in the `values.yaml` and added to the `README.md` using [readme-generator-for-helm](https://github.com/bitnami-labs/readme-generator-for-helm)~~ - ~~Breaking changes are documented in the `README.md`~~ - [X] Templating unittests are added Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
gitea-service-monitor.png
61 KiB
gitea-service-monitor.png
hiteshnayak305 added 1 commit 2024-04-03 21:05:28 +00:00
feat(service-monitor): support bearer token authentication on metrics endpoint 
Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
Some checks failed
check-and-test / check-and-test (pull_request) Has been cancelled
Details
0a433ab599
justusbunsi commented 2024-04-09 15:49:22 +00:00
Member
Copy Link

@hiteshnayak305 You checked the "unittests are added" checkbox in the checklist. I cannot find them. Please add meaningful ones. Think of edge cases, if any.
Unittests are required to get this PR merged. 🙂

@hiteshnayak305 You checked the "unittests are added" checkbox in the checklist. I cannot find them. Please add meaningful ones. Think of edge cases, if any. Unittests are required to get this PR merged. 🙂
hiteshnayak305 commented 2024-04-10 05:57:52 +00:00
Author
First-time contributor
Copy Link

👍 will try this weekend

👍 will try this weekend
hiteshnayak305 added 2 commits 2024-04-10 17:35:41 +00:00
fix: render secret when gitea.metrics.serviceMonitor is enabled 
Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
4548175824
fix: introduce unit tests for service monitor and metrics secret 
Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 35s
Details
a4c8fa19bf
hiteshnayak305 commented 2024-04-10 17:42:25 +00:00
Author
First-time contributor
Copy Link

In current configuration ServiceMonitor is created even if gitea.metrics.enabled = false and gitea.metrics.serviceMonitor.enabled = true. It will be in Prometheus targets but failing with 404. Is it alright ?

In current configuration `ServiceMonitor` is created even if `gitea.metrics.enabled = false` and `gitea.metrics.serviceMonitor.enabled = true`. It will be in Prometheus targets but failing with 404. Is it alright ?
justusbunsi commented 2024-04-10 18:16:06 +00:00
Member
Copy Link

@hiteshnayak305

In current configuration ServiceMonitor is created even if gitea.metrics.enabled = false and gitea.metrics.serviceMonitor.enabled = true. It will be in Prometheus targets but failing with 404. Is it alright ?

Good catch. I guess that's a tricky one to fix, if possible right now. You can configure the metrics settings for app.ini within configmaps or secrets that are read when a release is already applied to the cluster. At that point you already had to make the decision to render the servicemonitor or not to render it.
I think the current situation is ok. If an admin intend to monitor Gitea, this will be noticed shortly after applying the release. 🙂

@hiteshnayak305 >In current configuration `ServiceMonitor` is created even if `gitea.metrics.enabled = false` and `gitea.metrics.serviceMonitor.enabled = true`. It will be in Prometheus targets but failing with 404. Is it alright ? Good catch. I guess that's a tricky one to fix, if possible right now. You can configure the metrics settings for app.ini within configmaps or secrets that are read when a release is already applied to the cluster. At that point you already had to make the decision to render the servicemonitor or not to render it. I think the current situation is ok. If an admin intend to monitor Gitea, this will be noticed shortly after applying the release. 🙂
hiteshnayak305 added 1 commit 2024-04-11 04:00:26 +00:00
Merge branch 'main' into main
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 36s
Details
6e37421445
pat-s commented 2024-04-16 08:22:20 +00:00
Member
Copy Link

Maybe worth adding a note into the README?

Maybe worth adding a note into the README?
👍 1
hiteshnayak305 added 1 commit 2024-04-19 10:26:22 +00:00
doc: add secure /metrics example in README 
Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 38s
Details
02f17782a5
pat-s reviewed 2024-04-23 07:15:42 +00:00
README.md Outdated
@ -727,1 +728,4 @@
### Secure Metrics Endpoint
Metrics endpoint `/metrics` can be secured using `Bearer` token authentication. Providing non-empty `TOKEN` value will also add authentication parameters to `ServiceMonitor`.
pat-s commented 2024-04-23 07:15:42 +00:00
Member
Copy Link

Metrics endpoint /metrics can be secured by using Bearer token authentication.
Note: Providing a non-empty TOKEN value will also require authentication for ServiceMonitor.

Metrics endpoint `/metrics` can be secured by using `Bearer` token authentication. **Note**: Providing a non-empty `TOKEN` value will also require authentication for `ServiceMonitor`.
hiteshnayak305 added 1 commit 2024-04-26 11:29:53 +00:00
doc: fix metrics secure endpoint documentation 
Signed-off-by: Hitesh Nayak <hiteshnayak305@gmail.com>
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 38s
Details
28cd2262a2
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 38s
Required
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u main:hiteshnayak305-main
git checkout hiteshnayak305-main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
has
backport

  
in progress

   
invalid
  
kind
breaking

  
kind
bug

  
kind
build

  
kind
dependency

  
kind
deployment

  
kind
docs

  
kind
enhancement

  
kind
feature

  
kind
lint

  
kind
proposal

  
kind
question

  
kind
refactor

  
kind
security

  
kind
testing

  
kind
translation

  
kind
ui

  
need
backport

  
priority
critical

  
priority
low

  
priority
maybe

  
priority
medium

  
reviewed
duplicate

  
reviewed
invalid

  
reviewed
wontfix

  
skip-changelog
  
status
blocked

  
status
needs-feedback

  
status
needs-reviews

  
status
wip

  
upstream
gitea

  
upstream
other

No Label
has
backport
in progress
invalid
kind
breaking
kind
bug
kind
build
kind
dependency
kind
deployment
kind
docs
kind
enhancement
kind
feature
kind
lint
kind
proposal
kind
question
kind
refactor
kind
security
kind
testing
kind
translation
kind
ui
need
backport
priority
critical
priority
low
priority
maybe
priority
medium
reviewed
duplicate
reviewed
invalid
reviewed
wontfix
skip-changelog
status
blocked
status
needs-feedback
status
needs-reviews
status
wip
upstream
gitea
upstream
other
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/helm-chart#637
No description provided.
Delete Branch "hiteshnayak305/helm-chart:main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?