STOR-2884: Add service to generate a TLS cert for EFS operator#522
STOR-2884: Add service to generate a TLS cert for EFS operator#522jsafrane wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jsafrane: This pull request references STOR-2884 which is a valid jira issue. DetailsIn response to this:
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new ClusterIP Service manifest at config/aws-efs/manifests/stable/aws-efs-csi-driver-operator-metrics-service.yaml exposing port 443 (targetPort 8443) with an annotation to request a serving certificate. Updates the operator CSV to add two terminate-on-files arguments, add a secret-backed volume named metrics-serving-cert sourced from 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/aws-efs/manifests/stable/aws-efs-csi-driver-operator-metrics-service.yaml (1)
1-21: Service configuration looks correct for certificate generation purpose.The manifest correctly uses the
service.beta.openshift.io/serving-cert-secret-nameannotation to trigger service-ca-operator certificate generation, and the secret name matches what's referenced in the ClusterServiceVersion.A few observations:
- The selector
app: aws-efs-csi-driver-operatorcorrectly matches the Deployment's pod labels.- The inline comments clearly document this is a placeholder for certificate generation.
The TODO on lines 12-13 acknowledges that the operator metrics endpoint needs to be exposed later. Ensure this follow-up work is tracked.
Would you like me to open an issue to track the follow-up work for exposing the operator metrics port and creating a ServiceMonitor?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/aws-efs/manifests/stable/aws-efs-csi-driver-operator-metrics-service.yaml` around lines 1 - 21, Create tracking work to implement the TODO: open an issue (or Jira ticket) titled e.g. "Expose operator metrics and add ServiceMonitor for aws-efs-csi-driver-operator" that describes adding a real metrics port to the operator Deployment and updating the Service aws-efs-csi-driver-operator-metrics to point to that port (replace the fake port mapping to target the operator metrics port), plus creating a ServiceMonitor resource to scrape the endpoint; reference the Service name aws-efs-csi-driver-operator-metrics, the serving-cert annotation service.beta.openshift.io/serving-cert-secret-name, and the TODO comment in this manifest so the future PR updates config/manifests and CSV accordingly.
🤖 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/aws-efs/manifests/stable/aws-efs-csi-driver-operator-metrics-service.yaml`:
- Around line 1-21: Create tracking work to implement the TODO: open an issue
(or Jira ticket) titled e.g. "Expose operator metrics and add ServiceMonitor for
aws-efs-csi-driver-operator" that describes adding a real metrics port to the
operator Deployment and updating the Service aws-efs-csi-driver-operator-metrics
to point to that port (replace the fake port mapping to target the operator
metrics port), plus creating a ServiceMonitor resource to scrape the endpoint;
reference the Service name aws-efs-csi-driver-operator-metrics, the serving-cert
annotation service.beta.openshift.io/serving-cert-secret-name, and the TODO
comment in this manifest so the future PR updates config/manifests and CSV
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98152978-dd32-40c8-aec5-83885f889b9e
📒 Files selected for processing (3)
config/aws-efs/bundle.Dockerfileconfig/aws-efs/manifests/stable/aws-efs-csi-driver-operator-metrics-service.yamlconfig/aws-efs/manifests/stable/aws-efs-csi-driver-operator.clusterserviceversion.yaml
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
jsafrane
changed the title
Add service aws-efs-csi-driver-operator-metrics to the EFS 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.
jsafrane
force-pushed
the
add-service-to-generate-a-tls
branch
from
bb442b7 to
1474830
Compare
|
@jsafrane: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Add service aws-efs-csi-driver-operator-metrics to the EFS 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.
cc @openshift/storage