Improve scaffolding of ServiceMonitor

[fgiloux]

What do you want to happen?

Currently the scaffolding of ServiceMonitor makes use of Prometheus in-cluster credentials for collecting metrics. In some Prometheus installation it is not allowed, i.e. the setting is ignored for security reason:
A problem with the approach is that Prometheus token is exposed to the user creating the ServiceMonitor. With this token the user would be able to query any metrics endpoint Prometheus has access to.

Another issue is that insecureSkipVerify is used, which deactivates an important mechanism to check that the server reached is what it pretends to be.

My use case is really about:

• increasing the security level of what is offered by default by Kubebuilder
• making the default configuration works with a larger estate

It is not related to a particular Kubernetes version.
After a quick search I am not aware of another issue associated with this.

I am proposing to create an additional service account and token secret, used for scrapping the operator metrics. This can then get referenced in the ServiceMonitor resource.
In a similar way the TLS certificates generated to secure the operator endpoint can be referenced in the ServiceMonitor to guarantee the authenticity of the server to Prometheus.

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  labels:
    control-plane: controller-manager
    app.kubernetes.io/name: servicemonitor
    app.kubernetes.io/instance: controller-manager-metrics-monitor
    app.kubernetes.io/component: metrics
    app.kubernetes.io/created-by: project-v4
    app.kubernetes.io/part-of: project-v4
    app.kubernetes.io/managed-by: kustomize
  name: controller-manager-metrics-monitor
  namespace: system
spec:
  endpoints:
    - path: /metrics
      port: https
      scheme: https
      bearerTokenSecret:
        key: token
        name: prometheus-token
      tlsConfig:
        ca:
          secret:
            key: ca.crt
            name: serving-cert
            # SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
        serverName: SERVICE_NAME.SERVICE_NAMESPACE.svc
  selector:
    matchLabels:
      control-plane: controller-manager

Extra Labels

No response

[camilamacedo86]

Hi @fgiloux,

Thank you for bringing attention to this. I have a couple of questions regarding the configuration snippet you provided:
Regards:

            key: ca.crt
            name: serving-cert 

• Could you specify what values we should use here? Our objective is to deploy the project without making cert-manager a prerequisite. Is this feasible, or would we need to consider cert-manager as mandatory?
• Ideally, we're hoping to bypass any additional manual configurations or steps. Do you see any streamlined solutions or recommendations on this front?

I'm raising these concerns because upcoming changes in the auth-proxy will also demand a certificate. It seems that the primary reason the auth-proxy no longer generates a default certificate when none is provided is due to security concerns. Given this, we might need to re-evaluate our approach.

Your insights on how these values should be provisioned would be immensely valuable as we move forward. Looking forward to hearing your thoughts on this.

Best regards,

[camilamacedo86]

Btw, if you have any ideas, or suggestions in mind and would like to push a pull request your collaboration is more than welcome.

[fgiloux]

Hi @camilamacedo86

I hope you are doing well.

            key: ca.crt
            name: serving-cert 

these are the key for the CA certificate and name of the secret containing it that are already scaffolded for kube-rbac-proxy. This is what is needed by Prometheus to check the server identity as part of the TLS hand-check.

Deploy the project without making cert-manager a pre-requisite.

There has to be something providing the certificates and currently Kubebuilder is using cert-manager.
The certificates are for the TLS port of kube-rbac-proxy.
This is all things that are already part of what Kubebuilder generates. This issue was not aiming at a redesign rather improving the current output.

Ideally, we're hoping to bypass any additional manual configurations or steps. Do you see any streamlined solutions or recommendations on this front?

I would argue that this change reduces manual configuration rather than increases it as it eliminates manual changes for properly handling TLS and avoiding Prometheus credentials being exposed.

It seems that the primary reason the auth-proxy no longer generates a default certificate when none is provided is due to security concerns. Given this, we might need to re-evaluate our approach.

I am not sure what you are referring to. The intent of this issue is an incremental improvement, not a redesign.

I am happy to look at creating a pull request but would like first that there is an agreement on the change.

[camilamacedo86]

Hi @fgiloux

Following are some comments that I hope bring some context.
Contributions are welcome 🎉 ; please feel free to submit a PR with your suggestions.
Also, please check the following info and feel free to reach out.

Webhooks & Cert-Manager

• If using webhooks, the use of cert-manager is mandatory.
• However, for those not using webhooks, cert-manager remains optional.

Considering Prometheus Implementation

• It seems reasonable to modify the Prometheus implementation to require cert-manager. (since it is required for webhooks. However, see the points below and why we might need to re-think/re-design the default layout. Either these ones came from the past when Kubebuilder was not based on plugins.)

Challenges with Controller-Runtime & Kube-Rbac-Proxy

• Controller-runtime has been introducing changes related and we might need to re-think. Newer versions of controller-runtime might offer alternatives for this configuration. Discussion Link. So, we need to understand how they recommend to setup webhooks nowadays and if we should still keep cert-manager as a pre-requirement in this case.
• Kube-rbac-proxy is evolving, with some features being deprecated. It will no longer scaffold those certs. it seems that is not safe to do this scaffold. Then, based on that would not make sense if we have in Kubebuilder a scaffold, config that is not recommended. More info: kube-rbac-proxy warn about deprecation and future breaking changes #3524 (comment)

Therefore, the above creates some overlap and challenges in determining our direction.

Kubebuilder cert-manager, auth-proxy and Prometheus options came from the legacy layout without plugins

In the past, we did have not a plugin ecosystem. Therefore, we have all scaffolded and the options to comment and uncomment them. So, we might need to ask: Shouldn't those become optional plugins?

I think we might need to move in this direction:

• auth-proxy to be an optional plugin. (i.e. kubebuilder edit --plugins=auth-proxy/v1 will scaffold and add this option to the default scaffold)
• ensure that when you use auth-proxy you must use cert-manager.
• Then, the same for Prometheus.

Making third-party be enabled by default

• Challenges:
  • Potential user resistance to use them (not all use case requires those).
  • Increased maintenance responsibility for us.
  • If to deploy the quick-start example we require to install any third party by default then it will be harder for new users and beginners. So, that does not seem the best approach.

[camilamacedo86]

Hi @fgiloux,

Your request here should be addressed in the Phase 2 of the plan to Discontinue Kube RBAC Proxy in Default Kubebuilder Scaffolding.

See: #3871

So, I am closing this one in favor of PEP and the above issue.