-
Notifications
You must be signed in to change notification settings - Fork 26
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
Upgrade operator-sdk to 1.38.0 #1040
Changes from 1 commit
8d3abdd
d66b9f9
7445bc8
cb5192d
f22c54a
7af03ba
e0e4315
f8ac1de
40d0c0b
9412cda
4676133
8f40158
1839fc4
85ee265
d831f4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Added | ||
body: Added "EnableWithTLS" option to Helm parameter "prometheus.expose", allowing secure access | ||
to metrics from outside the cluster | ||
time: 2025-01-31T22:14:06.675326382Z | ||
custom: | ||
Issue: "1040" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
kind: Removed | ||
body: Removed Helm parameter "prometheus.createServiceMonitor" | ||
time: 2025-01-31T22:12:20.085253713Z | ||
custom: | ||
Issue: "1040" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,10 @@ package main | |
import ( | ||
"context" | ||
"crypto/tls" | ||
"crypto/x509" | ||
"log" | ||
"os" | ||
"strings" | ||
"time" | ||
|
||
// Allows us to pull in things generated from `go generate` | ||
|
@@ -286,20 +288,42 @@ func main() { | |
TLSOpts: webhookTLSOpts, | ||
}) | ||
|
||
secureMetrics := opcfg.GetMetricsAddr() == "127.0.0.1:8443" | ||
secureMetrics := strings.EqualFold(opcfg.GetMetricsExposeMode(), "EnableWithAuth") | ||
secureByTLS := strings.EqualFold(opcfg.GetMetricsExposeMode(), "EnableWithTLS") | ||
var metricCertDir string | ||
if opcfg.GetMetricsTLSSecret() != "" { | ||
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. Isn't it better to complain if 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. That is fine. Normally, the token is good enough to verify the user's identity. No need to use tls secret for "EnableWithTLS". |
||
metricCertDir = "/cert" | ||
metricsTLSOpts = append(metricsTLSOpts, func(c *tls.Config) { | ||
// Load the CA certificate | ||
caCert, err := os.ReadFile("/cert/ca.crt") | ||
if err != nil { | ||
log.Fatalf("failed to read CA cert: %v", err) | ||
} | ||
// Create a CertPool and add the CA certificate to it | ||
caCertPool := x509.NewCertPool() | ||
ok := caCertPool.AppendCertsFromPEM(caCert) | ||
if !ok { | ||
log.Fatal("failed to append CA cert to CertPool") | ||
} | ||
c.ClientCAs = caCertPool | ||
// If we enabled authorization, then no client certs are really needed. | ||
// Otherwise, we need the client certs. | ||
if secureMetrics { | ||
c.ClientAuth = tls.VerifyClientCertIfGiven | ||
} else if secureByTLS { | ||
c.ClientAuth = tls.RequireAndVerifyClientCert | ||
} | ||
}) | ||
} | ||
|
||
// Metrics endpoint is enabled in 'config/default/kustomization.yaml'. The Metrics options configure the server. | ||
// More info: | ||
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/server | ||
// - https://book.kubebuilder.io/reference/metrics.html | ||
metricsServerOptions := metricsserver.Options{ | ||
BindAddress: ":8443", | ||
SecureServing: secureMetrics, | ||
// TODO(user): TLSOpts is used to allow configuring the TLS config used for the server. If certificates are | ||
BindAddress: opcfg.GetMetricsAddr(), | ||
SecureServing: secureMetrics || secureByTLS, | ||
// TLSOpts is used to allow configuring the TLS config used for the server. If certificates are | ||
// not provided, self-signed certificates will be generated by default. This option is not recommended for | ||
// production environments as self-signed certificates do not offer the same level of trust and security | ||
// as certificates issued by a trusted Certificate Authority (CA). The primary risk is potentially allowing | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
apiVersion: rbac.authorization.k8s.io/v1 | ||
kind: ClusterRoleBinding | ||
metadata: | ||
name: metrics-reader | ||
roleRef: | ||
apiGroup: rbac.authorization.k8s.io | ||
kind: ClusterRole | ||
name: metrics-reader | ||
subjects: | ||
- kind: ServiceAccount | ||
name: manager | ||
namespace: system | ||
- apiGroup: rbac.authorization.k8s.io | ||
kind: Group | ||
name: system:authenticated | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,8 +13,8 @@ tests: | |||||
- it: should cotain ip if expose is with auth | ||||||
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.
Suggested change
|
||||||
set: | ||||||
prometheus: | ||||||
expose: EnableWithAuthProxy | ||||||
expose: EnableWithAuth | ||||||
asserts: | ||||||
- equal: | ||||||
path: data.METRICS_ADDR | ||||||
value: 127.0.0.1:8443 | ||||||
value: 0.0.0.0:8443 |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,30 +151,32 @@ serviceAccountAnnotations: {} | |
prometheus: | ||
# Controls exposing of the prometheus metrics endpoint. Valid options are: | ||
# | ||
# EnableWithAuthProxy: A new service object will be created that exposes the | ||
# EnableWithAuth: A new service object will be created that exposes the | ||
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. does 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. No, EnableWithAuth could not provide tls certs when accessing the metrics url. |
||
# metrics endpoint. Access to the metrics are controlled by rbac rules. | ||
# The metrics endpoint will use the https scheme. | ||
# EnableWithoutAuth: Like EnableWithAuthProxy, this will create a service | ||
# EnableWithoutAuth: Like EnableWithAuth, this will create a service | ||
# object to expose the metrics endpoint. However, there is no authority | ||
# checking when using the endpoint. Anyone who had network access | ||
# endpoint (i.e. any pod in k8s) will be able to read the metrics. The | ||
# metrics endpoint will use the http scheme. | ||
# EnableWithTLS: Like EnableWithAuth, this will create a service | ||
# object to expose the metrics endpoint. However, there is no authority | ||
# checking when using the endpoint. People with network access to the | ||
# endpoint (i.e. any pod in k8s) and the correct certs can read the metrics. | ||
# The metrics endpoint will use the https scheme. | ||
# It needs to be used with tlsSecret. If tlsSecret is not set, the behavior | ||
# will be similar to EnableWithoutAuth, except that the endpoint will use | ||
# https schema. | ||
# Disable: Prometheus metrics are not exposed at all. | ||
expose: Disable | ||
|
||
# If prometheus is exposed with an auth proxy (EnableWithAuthProxy), use this | ||
# If prometheus is exposed with an auth proxy (EnableWithAuth), use this | ||
# parameter to control what certificates are used for the https endpoint. If | ||
# this is empty, the operator will use a generated self-signed cert. When | ||
# provided, the certificates can be used to authenticate with the metrics | ||
# endpoint. | ||
tlsSecret: "" | ||
|
||
# ** This parameter is deprecated and will be removed in a future release. | ||
# Set this to true if you want to create a ServiceMonitor. This object is a | ||
# CR provided by the prometheus operator to allow for easy service discovery. | ||
# https://github.com/prometheus-operator/prometheus-operator | ||
createServiceMonitor: false | ||
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. You removed this but still kept monitor.yaml in config/prometheus 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. monitor.yaml in config/prometheus is generated by new operator-sdk. I leave it there in case we want to put service monitor back. In kustomization.yaml, we can uncomment the line contains "prometheus" to generate the monitor manifest. |
||
|
||
# This controls the creation of ClusterRole/ClusterRoleBinding to access | ||
# the metrics endpoint. | ||
createProxyRBAC: true |
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.
Let's move these to opcfg package: add 2 methods that will do the comparison there.
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.
Sure, I can add that.