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

🐛 (kustomize/v2): fix ServiceMonitor with TLS kustomize scaffolding #4536

Merged
merged 1 commit into from
Feb 8, 2025
Merged

🐛 (kustomize/v2): fix ServiceMonitor with TLS kustomize scaffolding #4536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 22 additions & 0 deletions docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ replacements:
delimiter: '.'
index: 0
create: true
- select:
kind: ServiceMonitor
group: monitoring.coreos.com
version: v1
name: controller-manager-metrics-monitor
fieldPaths:
- spec.endpoints.0.tlsConfig.serverName
options:
delimiter: '.'
index: 0
create: true

- source:
kind: Service
Expand All @@ -94,6 +105,17 @@ replacements:
delimiter: '.'
index: 1
create: true
- select:
kind: ServiceMonitor
group: monitoring.coreos.com
version: v1
name: controller-manager-metrics-monitor
fieldPaths:
- spec.endpoints.0.tlsConfig.serverName
options:
delimiter: '.'
index: 1
create: true

- source: # Uncomment the following block if you have any webhook
kind: Service
Expand Down
37 changes: 17 additions & 20 deletions docs/book/src/cronjob-tutorial/testdata/project/config/prometheus/monitor_tls_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
# Patch for Prometheus ServiceMonitor to enable secure TLS configuration
# using certificates managed by cert-manager
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: controller-manager-metrics-monitor
namespace: system
spec:
endpoints:
- tlsConfig:
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
- op: replace
path: /spec/endpoints/0/tlsConfig
value:
# SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
7 changes: 6 additions & 1 deletion docs/book/src/cronjob-tutorial/testdata/project/dist/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4276,7 +4276,11 @@ metadata:
namespace: project-system
spec:
endpoints:
- tlsConfig:
- bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
path: /metrics
port: https
scheme: https
tlsConfig:
ca:
secret:
key: ca.crt
Expand All @@ -4289,6 +4293,7 @@ spec:
keySecret:
key: tls.key
name: metrics-server-cert
serverName: project-controller-manager-metrics-service.project-system.svc
selector:
matchLabels:
app.kubernetes.io/name: project
Expand Down
22 changes: 22 additions & 0 deletions docs/book/src/getting-started/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ patches:
# delimiter: '.'
# index: 0
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 0
# create: true
#
# - source:
# kind: Service
Expand All @@ -94,6 +105,17 @@ patches:
# delimiter: '.'
# index: 1
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 1
# create: true
#
# - source: # Uncomment the following block if you have any webhook
# kind: Service
Expand Down
37 changes: 17 additions & 20 deletions docs/book/src/getting-started/testdata/project/config/prometheus/monitor_tls_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
# Patch for Prometheus ServiceMonitor to enable secure TLS configuration
# using certificates managed by cert-manager
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: controller-manager-metrics-monitor
namespace: system
spec:
endpoints:
- tlsConfig:
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
- op: replace
path: /spec/endpoints/0/tlsConfig
value:
# SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
22 changes: 22 additions & 0 deletions docs/book/src/multiversion-tutorial/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ replacements:
delimiter: '.'
index: 0
create: true
- select:
kind: ServiceMonitor
group: monitoring.coreos.com
version: v1
name: controller-manager-metrics-monitor
fieldPaths:
- spec.endpoints.0.tlsConfig.serverName
options:
delimiter: '.'
index: 0
create: true

- source:
kind: Service
Expand All @@ -94,6 +105,17 @@ replacements:
delimiter: '.'
index: 1
create: true
- select:
kind: ServiceMonitor
group: monitoring.coreos.com
version: v1
name: controller-manager-metrics-monitor
fieldPaths:
- spec.endpoints.0.tlsConfig.serverName
options:
delimiter: '.'
index: 1
create: true

- source: # Uncomment the following block if you have any webhook
kind: Service
Expand Down
37 changes: 17 additions & 20 deletions .../book/src/multiversion-tutorial/testdata/project/config/prometheus/monitor_tls_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
# Patch for Prometheus ServiceMonitor to enable secure TLS configuration
# using certificates managed by cert-manager
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: controller-manager-metrics-monitor
namespace: system
spec:
endpoints:
- tlsConfig:
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
- op: replace
path: /spec/endpoints/0/tlsConfig
value:
# SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
7 changes: 6 additions & 1 deletion docs/book/src/multiversion-tutorial/testdata/project/dist/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8122,7 +8122,11 @@ metadata:
namespace: project-system
spec:
endpoints:
- tlsConfig:
- bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
path: /metrics
port: https
scheme: https
tlsConfig:
ca:
secret:
key: ca.crt
Expand All @@ -8135,6 +8139,7 @@ spec:
keySecret:
key: tls.key
name: metrics-server-cert
serverName: project-controller-manager-metrics-service.project-system.svc
selector:
matchLabels:
app.kubernetes.io/name: project
Expand Down
22 changes: 22 additions & 0 deletions hack/docs/internal/cronjob-tutorial/sample.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ const certManagerForMetricsAndWebhooks = `#replacements:
# delimiter: '.'
# index: 0
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 0
# create: true
#
# - source:
# kind: Service
Expand All @@ -71,6 +82,17 @@ const certManagerForMetricsAndWebhooks = `#replacements:
# delimiter: '.'
# index: 1
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 1
# create: true
#
# - source: # Uncomment the following block if you have any webhook
# kind: Service
Expand Down
22 changes: 22 additions & 0 deletions ...plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ patches:
# delimiter: '.'
# index: 0
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 0
# create: true
#
# - source:
# kind: Service
Expand All @@ -139,6 +150,17 @@ patches:
# delimiter: '.'
# index: 1
# create: true
# - select:
# kind: ServiceMonitor
# group: monitoring.coreos.com
# version: v1
# name: controller-manager-metrics-monitor
# fieldPaths:
# - spec.endpoints.0.tlsConfig.serverName
# options:
# delimiter: '.'
# index: 1
# create: true
#
# - source: # Uncomment the following block if you have any webhook
# kind: Service
Expand Down
37 changes: 17 additions & 20 deletions ...s/common/kustomize/v2/scaffolds/internal/templates/config/prometheus/monitor_tls_patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,21 @@ func (f *ServiceMonitorPatch) SetTemplateDefaults() error {

const serviceMonitorPatchTemplate = `# Patch for Prometheus ServiceMonitor to enable secure TLS configuration
# using certificates managed by cert-manager
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: controller-manager-metrics-monitor
namespace: system
spec:
endpoints:
- tlsConfig:
insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
- op: replace
path: /spec/endpoints/0/tlsConfig
value:
# SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
Copy link
Member

Choose a reason for hiding this comment

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

Did we only need to change the kustomize default file just to pass the serviceName right?
But that is fixed. It will always be the same.
Could we not simplify the changes and the default kustomize by adding here the name

Suggested change
serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
serverName: controller-manager-metrics-monitor.SERVICE_NAMESPACE.svc

Instead?

Copy link
Member

Choose a reason for hiding this comment

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

I am trying to see if we can avoid do changes at: config/default/kustomization.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to follow the existing convention followed in kubebuilder kustomize files.
For other scenarios where the service name is referred, we follow similar convention of replacements. See certificate
I'm not sure if this serverName replacement is any different. Let me know if my understanding is wrong

Copy link
Member

Choose a reason for hiding this comment

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

I will need a few time to check/think about this one.
But I will be back to you ASAP

Copy link
Member

@camilamacedo86 camilamacedo86 Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am not too happy with the complexities in the kustomize, but I think that is not part of the scope, and it makes sense you follow the same approach. Thank you.

insecureSkipVerify: false
ca:
secret:
name: metrics-server-cert
key: ca.crt
cert:
secret:
name: metrics-server-cert
key: tls.crt
keySecret:
name: metrics-server-cert
key: tls.key
`
Loading