Skip to content

Configuration of trust stores for Prometheus and Apicurio clients#1280

Merged
MikeEdgar merged 3 commits intostreamshub:mainfrom
MikeEdgar:issue-1277
Dec 13, 2024
Merged

Configuration of trust stores for Prometheus and Apicurio clients#1280
MikeEdgar merged 3 commits intostreamshub:mainfrom
MikeEdgar:issue-1277

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.

Signed-off-by: Michael Edgar <medgar@redhat.com>
Signed-off-by: Michael Edgar <medgar@redhat.com>
Signed-off-by: Michael Edgar <medgar@redhat.com>
@sonarqubecloud
Copy link

@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
@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