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

Configuration of trust stores for Prometheus and Apicurio clients #1280

Merged
merged 3 commits into from
Dec 13, 2024

Conversation

MikeEdgar
Copy link
Member

@MikeEdgar MikeEdgar commented Dec 5, 2024

Closes #1277


Approach

This change allows for a trust store to be configured for metrics source (Prometheus) and schema registry (Apicurio) clients. The trust store may be set in the Console CR along with the URLs for each connection type.

For example:

...
spec:
  metricsSources:
    - name: ocp-prometheus
      type: openshift-monitoring
      url: https://thanos-querier.mycluster.example.com
      truststore:
        type: PEM  # also supports JKS or PKCS12
        alias: "" # optional, only for JKS or PKCS12
        password: {} # optional, only for JKS or PKCS12. Same structure as `content` below
        content:
          # only one of `value` or `valueFrom` is used
          value: |
            -----BEGIN CERTIFICATE-----
            MIIFazCCA1OgAwIBAgIRAIIQz7DSQONZRGPgu2OCiwAwDQYJKoZIhvcNAQELBQAw
            ...
            emyPxgcYxn/eR44/KJ4EBs+lVDR3veyJm+kXQ99b21/+jh5Xos1AnX5iItreGCc=
            -----END CERTIFICATE-----
          # May use a ConfigMap or Secret key when no literal value is given
          valueFrom:
            configMapKeyRef: {}
            secretKeyRef: {}
...

The operator will then read the trust store content either directly from the CR for literal values or from the ConfigMap or Secret, copy to the Console secret being reconciled, and create a volume/mount and appropriate Quarkus environment vars for a TLS configuration bucket [1] specific to the connection.

The clients in the API server will then obtain the trust store/SSL context from the TLS bucket named by convention and use for the connection to the remote service.

[1] https://quarkus.io/guides/tls-registry-reference#using-the-tls-registry

@MikeEdgar MikeEdgar force-pushed the issue-1277 branch 2 times, most recently from f224aed to f737625 Compare December 5, 2024 11:50
@MikeEdgar
Copy link
Member Author

@katheris , @k-wall this PR still needs some unit/integration tests, but I am curious to get your thoughts on the general approach/pattern to implement this.

@k-wall
Copy link

k-wall commented Dec 5, 2024

    alias: "" # optional, only for JKS or PKCS12

what's the idea here? I'm familiar with alias on the keystore side - to select a single key from potentially many in the store, but in the truststore side you normally trust all the certificates in the truststore. In Java TrustManagerFactory accepts a truststore and gives you a TrustManager which trusts them all. So, what's the role of the alias?

@k-wall
Copy link

k-wall commented Dec 5, 2024

    password: {} # optional, only for JKS or PKCS12. Same structure as `content` below

is this a ref to a secret? you would normally try to avoid an inline secret.

EDIT: answered my own question.

@MikeEdgar
Copy link
Member Author

    alias: "" # optional, only for JKS or PKCS12

what's the idea here? I'm familiar with alias on the keystore side - to select a single key from potentially many in the store, but in the truststore side you normally trust all the certificates in the truststore. In Java TrustManagerFactory accepts a truststore and gives you a TrustManager which trusts them all. So, what's the role of the alias?

That's what I've typically seen also, but the TLS registry supports configuration of an alias for JKS and P12 [1]. I figured better to align to the target on it.

[1] https://quarkus.io/guides/tls-registry-reference#trust-stores

@MikeEdgar
Copy link
Member Author

    password: {} # optional, only for JKS or PKCS12. Same structure as `content` below

is this a ref to a secret? you would normally try to avoid an inline secret.

Yes, it can be. The field allows for value or valueFrom like the truststore content. I kept the structure the same to allow security, but also convenience for development and testing, etc.

@k-wall
Copy link

k-wall commented Dec 6, 2024

On the whole, the approach looks good. I'm concerned about some of the coding in the ApicurioClient but will be content to overlook providing there's a plan to address the debt.

@MikeEdgar MikeEdgar marked this pull request as ready for review December 12, 2024 14:48
@MikeEdgar
Copy link
Member Author

@k-wall I'll go ahead and merge this if no objections. The config option in the CR will be used also for #1248 to support a TLS truststore for the OIDC provider (building on #1126 once that's merged)

@MikeEdgar MikeEdgar added this to the 0.6.0 milestone Dec 12, 2024
@MikeEdgar MikeEdgar merged commit b0f5a99 into streamshub:main Dec 13, 2024
6 checks passed
@MikeEdgar MikeEdgar deleted the issue-1277 branch December 13, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support trusted certificate configuration for metrics sources and schema registries
2 participants