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.

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

Make operator-ca-tls handling GitOps friendly #1986

Closed
sebhoss opened this issue Feb 16, 2024 · 3 comments
Closed

Make operator-ca-tls handling GitOps friendly #1986

sebhoss opened this issue Feb 16, 2024 · 3 comments
Assignees

Comments

@sebhoss
Copy link

sebhoss commented Feb 16, 2024

Is your feature request related to a problem? Please describe.

The current documented approach to using cert-manager together with this operator lists manual steps in https://github.com/minio/operator/blob/master/docs/cert-manager.md#create-operator-ca-tls-secret. This is frustrating because I can only execute those steps after a tenant is deployed. Additionally it assumes that every tenant uses the same certificate issuer, even though that is not really required.

Describe the solution you'd like

The operator already has all the information it needs to get the certificate data (including CA) from the externalCertSecret field. I think it would be great if the operator just reads the mentioned secret from the namespace of the tenant and extract cert + CA itself without requiring manual steps by a user. Likewise, this would allow to define different issuers for different tenants (with different CAs)

Describe alternatives you've considered

None really - I'm not familiar with the workings of this operator and I'm unsure why the current approach was taken.

Additional context

Here is a trimmed down example of how the prometheus-operator handles certificates in a ServiceMonitor resource:

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: minio-tenant
  namespace: minio-tenant
spec:
  endpoints:
    - port: https-minio
      path: /minio/v2/metrics/resource
      scheme: https
      tlsConfig:
        serverName: minio.minio-tenant.svc.cluster.local
        ca:
          secret:
            name: tenant-certmanager-tls
            key: ca.crt
        keySecret:
          name: tenant-certmanager-tls
          key: tls.key
        cert:
          secret:
            name: tenant-certmanager-tls
            key: tls.crt
  namespaceSelector:
    matchNames:
      - minio-tenant
  selector:
    matchLabels:
      app.kubernetes.io/name: minio-tenant

Here we can define tlsConfig and be very explicit about which key from which secret we want to use. Since the minio-operator already knows which secret a tenant is using all this could be determined automatically by the minio-operator.

@ravindk89
Copy link
Contributor

ravindk89 commented Feb 20, 2024

#1971

This may be addressed, and we have to update the docs to correspond. Or we have the framework in place and we can fully address this in an upcoming release.

@cniackz can you confirm? I'm a little behind on the sum total of our TLS behavior w/ cert-manager involved as of 5.0.12

@cesnietor
Copy link
Contributor

we'll discuss this internally cc @cniackz

@cniackz
Copy link
Contributor

cniackz commented Nov 28, 2024

@sebhoss, I acknowledge that this manual procedure is both accurate and necessary. There was a time when we attempted to improve it by supporting multiple tenants in a less manual way, as Ravind pointed out in PR #1971. However, this approach was never thoroughly tested or widely adopted.

At this point, our plan is to keep the public operator as simple as possible for the community to use.

@cniackz cniackz removed the triage label Nov 28, 2024
@cniackz cniackz closed this as completed Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants