STOR-2884: Add service to generate a TLS cert for SMB operator

[jsafrane]

Add service smb-csi-driver-operator-metrics to the SMB CSI driver OLM manifests. OLM will instantiate it together with the operator Deployment.

This service causes service-ca-operator to generate a TLS key + certificate for the operator. As result, the operator stops generating a self-signed cert and uses the provided one instead.

The service has no other use than to generate the TLS material. The operator itself does not (yet) expose a port with metrics.

[openshift-ci-robot]

@jsafrane: This pull request references STOR-2884 which is a valid jira issue.

Details

In response to this:

Add service smb-csi-driver-operator-metrics to the SMB CSI driver OLM manifests. OLM will instantiate it together with the operator Deployment.

This service causes service-ca-operator to generate a TLS key + certificate for the operator. As result, the operator stops generating a self-signed cert and uses the provided one instead.

The service has no other use than to generate the TLS material. The operator itself does not (yet) expose a port with metrics.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a3f21284-7f42-4134-afa2-3f3d88dc38a0

📥 Commits

Reviewing files that changed from the base of the PR and between f978b43 and 729d796.

📒 Files selected for processing (3)

• config/samba/bundle.Dockerfile
• config/samba/manifests/stable/smb-csi-driver-operator-metrics-service.yaml
• config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml

🚧 Files skipped from review as they are similar to previous changes (3)

• config/samba/bundle.Dockerfile
• config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml
• config/samba/manifests/stable/smb-csi-driver-operator-metrics-service.yaml

---

📝 Walkthrough

Walkthrough

Adds a new Kubernetes Service manifest smb-csi-driver-operator-metrics under config/samba/manifests/stable/, annotated for serving certificate provisioning and configured to expose HTTPS on port 443 targeting container port 8443. Updates config/samba/bundle.Dockerfile to include the new manifest in the bundle. Modifies the ClusterServiceVersion to add --terminate-on-files arguments, add a metrics-serving-cert secret volume, and mount it at /var/run/secrets/serving-cert in the operator container.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly describes the main change: adding a service to generate a TLS certificate for the SMB operator.
Description check	✅ Passed	The description is directly related to the changeset, explaining the purpose of the new service, how it works, and why it was added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [jsafrane]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (1)

config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml (1)

369-371: Consider adding optional: true to handle startup race condition.

The secret smb-csi-driver-operator-metrics-serving-cert is created by service-ca-operator after it observes the Service. Without optional: true, the pod may fail to start if the Deployment is scheduled before the secret exists.

♻️ Proposed fix

                  - name: metrics-serving-cert
                    secret:
                      secretName: smb-csi-driver-operator-metrics-serving-cert
+                     optional: true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml`
around lines 369 - 371, The secret volume entry for "metrics-serving-cert"
currently references secretName "smb-csi-driver-operator-metrics-serving-cert"
but lacks optional handling; update the secret spec under the
"metrics-serving-cert" volume (where secretName is defined) to include optional:
true so the pod won't fail if the service-ca-operator hasn't created the secret
yet; modify the secret block that contains secretName
smb-csi-driver-operator-metrics-serving-cert to add optional: true.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml`:
- Around line 369-371: The secret volume entry for "metrics-serving-cert"
currently references secretName "smb-csi-driver-operator-metrics-serving-cert"
but lacks optional handling; update the secret spec under the
"metrics-serving-cert" volume (where secretName is defined) to include optional:
true so the pod won't fail if the service-ca-operator hasn't created the secret
yet; modify the secret block that contains secretName
smb-csi-driver-operator-metrics-serving-cert to add optional: true.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3fbc25df-b247-4e31-8930-cbd074546ae1

📥 Commits

Reviewing files that changed from the base of the PR and between 735a1ea and f978b43.

📒 Files selected for processing (3)

• config/samba/bundle.Dockerfile
• config/samba/manifests/stable/smb-csi-driver-operator-metrics-service.yaml
• config/samba/manifests/stable/smb-csi-driver-operator.clusterserviceversion.yaml

[jsafrane]

/test help

[jsafrane]

/test smb-operator-e2e

[mpatlasov]

/retest-required

[mpatlasov]

@jsafrane , if you compare your new Service for smb with existing one for CSO, you can see a few discrepancies:

1. A bunch of extra annotations in CSO Service:

     include.release.openshift.io/hypershift: "true"
     include.release.openshift.io/ibm-cloud-managed: "true"
     include.release.openshift.io/self-managed-high-availability: "true"
     include.release.openshift.io/single-node-developer: "true"
     capability.openshift.io/name: Storage

2. The naming scheme for the cert: CSO Service doesn't have "metrics" in the name:

     service.alpha.openshift.io/serving-cert-secret-name: cluster-storage-operator-serving-cert

while SMB one has:

    service.beta.openshift.io/serving-cert-secret-name: smb-csi-driver-operator-metrics-serving-cert

3. .spec.ports.name is https in CSO and metrics for SMB Service.

4. .spec.ports.targetPort is numeric 8443 in CSO, but metrics in SMB Service

5. .spec.selector is name: <something> in CSO, but app: <something> in SMB Service

Do you think it's good idea to have some uniformity for metrics Service-s among storage components? (we could fix CSO one in a NO-JIRA PR if you think new SMB Service is better). I think it will be hard exercise for any future code-reader to understand why two similar Service-s differ so much :)

[jsafrane]

1. A bunch of extra annotations in CSO Service:

     include.release.openshift.io/hypershift: "true"
     include.release.openshift.io/ibm-cloud-managed: "true"
     include.release.openshift.io/self-managed-high-availability: "true"
     include.release.openshift.io/single-node-developer: "true"
     capability.openshift.io/name: Storage

These annotations are interpreted by CVO. SMB does not go through CVO and have no effect in OLM, so I removed them.

2. The naming scheme for the cert: CSO Service doesn't have "metrics" in the name:

     service.alpha.openshift.io/serving-cert-secret-name: cluster-storage-operator-serving-cert

while SMB one has:

    service.beta.openshift.io/serving-cert-secret-name: smb-csi-driver-operator-metrics-serving-cert

I copied the secret name from vSphere, which is the only (?) CSI operator one that has configured metrics collection. And it has vmware-vsphere-csi-driver-operator-metrics-serving-cert.

I agree the names are not consistent, I've chosen to be consistent with the only CSI driver operator. Without much thinking about other options, to be honest.

3. .spec.ports.name is https in CSO and metrics for SMB Service.
4. .spec.ports.targetPort is numeric 8443 in CSO, but metrics in SMB Service

I changed SMB to https / numeric 8443, as in CSO. BTW, the vSphere driver operator has a completely different port name (port: vsphere-omp, what does it even mean?)

5. .spec.selector is name: <something> in CSO, but app: <something> in SMB Service

The selector must correspond to a label on Pods. And the operator Pod(s) have app: <something> already. I think this PR is not a good place to change that, as it affects also the Deployment and I am not exactly sure it's actually safe to do that by just renaming the label.

[mpatlasov]

/lgtm
/retest-required

[mpatlasov]

BTW, the vSphere driver operator has a completely different port name (port: vsphere-omp, what does it even mean?)

@jsafrane , it was introduced four years ago by this PR: Bug 2043132: Add metrics for vsphere operator. If I understand Hemant's logic correctly, the PR introduced a new port 8445, and a name for this port: vsphere-omp. It might stand for: vsphere operator metrics port.

[openshift-ci]

@jsafrane: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.