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

feature/add-etcd-vm-node-scrape #614

Merged
merged 12 commits into from
Feb 6, 2025
Merged

feature/add-etcd-vm-node-scrape #614

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9de782e
fix
klinch0 Jan 16, 2025
191c8b4
Merge branch 'aenix-io:main' into main
klinch0 Jan 16, 2025
2f0d092
Merge branch 'aenix-io:main' into main
klinch0 Jan 20, 2025
8181966
Merge branch 'aenix-io:main' into main
klinch0 Jan 21, 2025
469d1e9
Merge branch 'aenix-io:main' into main
klinch0 Jan 23, 2025
719cedd
Merge branch 'aenix-io:main' into main
klinch0 Jan 27, 2025
e5e1472
Merge branch 'aenix-io:main' into main
klinch0 Jan 29, 2025
f67816e
Merge branch 'aenix-io:main' into main
klinch0 Feb 5, 2025
4655657
add-managed-etcd-monitoring
klinch0 Feb 5, 2025
dfce1b0
Update packages/system/monitoring-agents/templates/etcd-proxy-scrape.…
klinch0 Feb 6, 2025
c5b89cb
Update packages/system/monitoring-agents/templates/etcd-proxy-scrape.…
klinch0 Feb 6, 2025
86b6861
del nodescrape
klinch0 Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/core/platform/bundles/distro-full.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ releases:
privileged: true
optional: true
dependsOn: [cilium,victoria-metrics-operator]
values:
scrapeRules:
etcd:
enabled: true

- name: metallb
releaseName: metallb
Expand Down
4 changes: 4 additions & 0 deletions packages/core/platform/bundles/distro-hosted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ releases:
privileged: true
optional: true
dependsOn: [victoria-metrics-operator]
values:
scrapeRules:
etcd:
enabled: true

- name: etcd-operator
releaseName: etcd-operator
Expand Down
4 changes: 4 additions & 0 deletions packages/core/platform/bundles/paas-full.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ releases:
namespace: cozy-monitoring
privileged: true
dependsOn: [cilium,kubeovn,victoria-metrics-operator]
values:
scrapeRules:
etcd:
enabled: true

- name: kubevirt-operator
releaseName: kubevirt-operator
Expand Down
4 changes: 4 additions & 0 deletions packages/core/platform/bundles/paas-hosted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ releases:
namespace: cozy-monitoring
privileged: true
dependsOn: [victoria-metrics-operator]
values:
scrapeRules:
etcd:
enabled: true

- name: etcd-operator
releaseName: etcd-operator
Expand Down
133 changes: 133 additions & 0 deletions packages/system/monitoring-agents/templates/etcd-proxy-scrape.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
{{- if not .Values.scrapeRules.etcd.enabled }}
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: kube-rbac-proxy
namespace: cozy-monitoring
labels:
app: kube-rbac-proxy
spec:
selector:
matchLabels:
app: kube-rbac-proxy
template:
metadata:
labels:
app: kube-rbac-proxy
spec:
serviceAccountName: kube-rbac-proxy
hostNetwork: true
nodeSelector:
node-role.kubernetes.io/control-plane: ""
containers:
- name: kube-rbac-proxy
image: quay.io/brancz/kube-rbac-proxy:v0.11.0
args:
- "--secure-listen-address=0.0.0.0:9443"
- "--upstream=http://127.0.0.1:2381/"
ports:
- containerPort: 9443
name: etcd-metrics
securityContext:
runAsUser: 1000
runAsNonRoot: true

---

apiVersion: v1
kind: ServiceAccount
metadata:
name: kube-rbac-proxy
namespace: cozy-monitoring

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: kube-rbac-proxy-auth
rules:
- apiGroups: ["authentication.k8s.io"]
resources: ["tokenreviews"]
verbs: ["create"]
- apiGroups: ["authorization.k8s.io"]
resources: ["subjectaccessreviews"]
verbs: ["create"]

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: kube-rbac-proxy-auth-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: kube-rbac-proxy-auth
subjects:
- kind: ServiceAccount
name: kube-rbac-proxy
namespace: cozy-monitoring

---

apiVersion: v1
kind: ServiceAccount
metadata:
name: vm-scrape
namespace: cozy-monitoring

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: etcd-metrics-reader
rules:
- nonResourceURLs: ["/metrics"]
verbs: ["get"]

---

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: etcd-metrics-reader
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: etcd-metrics-reader
subjects:
- kind: ServiceAccount
name: vm-scrape
namespace: cozy-monitoring

---

apiVersion: v1
kind: Secret
type: kubernetes.io/service-account-token
metadata:
name: vm-token
annotations:
kubernetes.io/service-account.name: vm-scrape

---

apiVersion: operator.victoriametrics.com/v1beta1
kind: VMPodScrape
metadata:
name: etcd-managment-scrape
spec:
podMetricsEndpoints:
- port: etcd-metrics
scheme: https
tlsConfig:
insecureSkipVerify: true
bearerTokenSecret:
name: vm-token
key: token
selector:
matchLabels:
app: kube-rbac-proxy
{{- end }}
69 changes: 35 additions & 34 deletions packages/system/monitoring-agents/templates/etcd-scrape.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
#---
#apiVersion: operator.victoriametrics.com/v1beta1
#kind: VMNodeScrape
#metadata:
# name: kube-etcd
# namespace: cozy-monitoring
#spec:
# selector:
# node-role.kubernetes.io/control-plane: ""
# bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
# honorLabels: true
# metricRelabelConfigs:
# - action: labeldrop
# regex: (uid)
# - action: labeldrop
# regex: (id|name)
# - action: drop
# regex: (rest_client_request_duration_seconds_bucket|rest_client_request_duration_seconds_sum|rest_client_request_duration_seconds_count)
# source_labels:
# - __name__
# port: "2379"
# relabelConfigs:
# - action: labelmap
# regex: __meta_kubernetes_node_label_(.+)
# - sourceLabels:
# - __metrics_path__
# targetLabel: metrics_path
# - replacement: etcd
# targetLabel: job
# scheme: https
# scrapeTimeout: 5s
# tlsConfig:
# caFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
# insecureSkipVerify: true
{{- if .Values.scrapeRules.etcd.enabled }}
apiVersion: operator.victoriametrics.com/v1beta1
kind: VMNodeScrape
metadata:
name: kube-etcd
namespace: cozy-monitoring
spec:
selector:
node-role.kubernetes.io/control-plane: ""
bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
honorLabels: true
metricRelabelConfigs:
- action: labeldrop
regex: (uid)
- action: labeldrop
regex: (id|name)
- action: drop
regex: (rest_client_request_duration_seconds_bucket|rest_client_request_duration_seconds_sum|rest_client_request_duration_seconds_count)
source_labels:
- __name__
port: "2379"
relabelConfigs:
- action: labelmap
regex: __meta_kubernetes_node_label_(.+)
- sourceLabels:
- __metrics_path__
targetLabel: metrics_path
- replacement: etcd
targetLabel: job
scheme: http
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Protocol Scheme vs. TLS Configuration Inconsistency
The scheme is explicitly set to http, while a TLS configuration block (lines 32–34) is still provided. TLS settings are generally applicable with HTTPS endpoints. Please verify your intent:

  • If secure communication is desired, update the scheme to https and adjust TLS settings accordingly, or
  • If HTTP is intended, consider removing the TLS block to reduce confusion.

scrapeTimeout: 5s
tlsConfig:
caFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
insecureSkipVerify: true
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Review TLS configuration for security concerns.

The configuration uses HTTP scheme while also specifying TLS config with insecureSkipVerify: true. This combination raises security concerns:

  1. HTTP scheme with TLS config is inconsistent.
  2. insecureSkipVerify: true bypasses certificate validation.

Consider using HTTPS scheme if TLS is required, or verify if TLS can be safely disabled for this internal communication.

🏁 Script executed:

#!/bin/bash
# Check if etcd is configured to use TLS for metrics endpoint
rg -A 5 "metrics.*cert" packages/extra/etcd/

Length of output: 46

🏁 Script executed:

#!/bin/bash
# Display the TLS settings and surrounding context from the etcd-scrape.yaml file for manual verification.
rg -C 5 "tlsConfig:" packages/system/monitoring-agents/templates/etcd-scrape.yaml

Length of output: 307

Security Misconfiguration in TLS Settings

The file packages/system/monitoring-agents/templates/etcd-scrape.yaml is configured with an HTTP scheme yet also provides TLS settings with insecureSkipVerify: true. This combination is problematic because:

  • HTTP scheme vs. TLS config: Using HTTP alongside a TLS configuration is inconsistent. If encryption is desired, the scheme should be HTTPS.
  • Bypassing Certificate Validation: The use of insecureSkipVerify: true disables proper certificate validation, exposing the communication to potential security risks.

It is recommended to either:

  • Update the scheme to https and remove insecureSkipVerify: true so that certificate validation is enforced, or
  • Eliminate the TLS configuration entirely if secure communication is not needed for this internal endpoint.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Redundant TLS Configuration for HTTP Endpoint
The TLS configuration (including insecureSkipVerify: true) appears redundant when using http as the protocol. Removing these settings—if TLS is not required—can simplify the configuration and avoid potential misinterpretation.

{{- end }}
4 changes: 4 additions & 0 deletions packages/system/monitoring-agents/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,3 +359,7 @@ fluent-bit:
Name modify
Match *
Add cluster root-cluster

scrapeRules:
etcd:
enabled: false