WIP: OCPBUGS-59768: Authenticated metrics endpoints using controller-runtime auth filters #1111
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: anik120 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test unit-olm |
| certPoolStore, err := filemonitor.NewCertPoolStore(*sc.clientCAPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("certificate monitoring for client-ca failed: %v", err) | ||
| } | ||
| cpsw, err := filemonitor.NewWatch(sc.logger, []string{filepath.Dir(*sc.clientCAPath)}, certPoolStore.HandleCABundleUpdate) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error creating cert file watcher: %v", err) | ||
| } | ||
| cpsw.Run(context.Background()) | ||
|
|
||
| s.TLSConfig = &tls.Config{ | ||
| GetCertificate: func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { | ||
| return certStore.GetCertificate(), nil | ||
| }, | ||
| GetConfigForClient: func(_ *tls.ClientHelloInfo) (*tls.Config, error) { | ||
| var certs []tls.Certificate | ||
| if cert := certStore.GetCertificate(); cert != nil { | ||
| certs = append(certs, *cert) | ||
| } | ||
| return &tls.Config{ | ||
| Certificates: certs, | ||
| ClientCAs: certPoolStore.GetCertPool(), | ||
| ClientAuth: tls.VerifyClientCertIfGiven, | ||
| }, nil | ||
| }, |
There was a problem hiding this comment.
It isn't obvious to me what the connection is between use of filters.WithAuthenticationAndAuthorization and removing the certPoolStore/GetConfigForClient. What's the story here?
|
@joelanford doesn't look like this set up is working for this operator. $ oc get pods -n openshift-operator-lifecycle-manager
NAME READY STATUS RESTARTS AGE
catalog-operator-5dbb86796b-r25vk 1/1 Running 0 133m
olm-operator-d98db4949-l4bcg 1/1 Running 0 133m
package-server-manager-79658c858-r2k5p 2/2 Running 0 133m
packageserver-7b7957b98-9s2g7 1/1 Running 0 130m
packageserver-7b7957b98-lw9c7 1/1 Running 0 130m $ oc get svc -n openshift-operator-lifecycle-manager
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
catalog-operator-metrics ClusterIP 172.30.186.209 <none> 8443/TCP 137m
olm-operator-metrics ClusterIP 172.30.78.175 <none> 8443/TCP 137m
package-server-manager-metrics ClusterIP 172.30.37.240 <none> 8443/TCP 136m
packageserver-service ClusterIP 172.30.8.255 <none> 5443/TCP 131m TLS Configuration VerificationBoth operators are correctly configured with TLS arguments: OLM Operator: args:
- --tls-cert
- /srv-cert/tls.crt
- --tls-key
- /srv-cert/tls.key
- --client-ca
- /profile-collector-cert/tls.crt Catalog Operator: args:
- --tls-cert
- /srv-cert/tls.crt
- --tls-key
- /srv-cert/tls.key
- --client-ca
- /profile-collector-cert/tls.crt Both services correctly expose HTTPS on port 8443 with proper health checks: ports:
- containerPort: 8443
name: metrics
protocol: TCP
livenessProbe:
httpGet:
path: /healthz
port: 8443
scheme: HTTPS Unauthenticated Access Test:$ oc run test-pod --image=curlimages/curl:latest --rm -it --restart=Never -- \
curl -k https://olm-operator-metrics.openshift-operator-lifecycle-manager.svc.cluster.local:8443/metrics
# HELP csv_count Number of CSVs successfully registered
# TYPE csv_count gauge
csv_count 1
# HELP csv_succeeded Successful CSV install
# TYPE csv_succeeded gauge
csv_succeeded{name="packageserver",namespace="openshift-operator-lifecycle-manager",version="0.0.1-snapshot"} 1
[... full metrics output returned without authentication ...] Port Forward Test:$ kubectl port-forward -n openshift-operator-lifecycle-manager svc/olm-operator-metrics 8443:8443
$ curl -k https://localhost:8443/metrics
# Full metrics data returned without any authentication |
|
/assign @Xia-Zhao-rh |
|
Okay so this worked 🎉 I'll start the plumbing from upstream and update this PR once the upstream bits are downstreamed. |
|
@anik120: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
anik120
changed the title
openshift-ci-robot
added
jira/severity-moderate
|
@anik120: This pull request references Jira Issue OCPBUGS-59768, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In 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. |
|
Still waiting for upstream PR to merge: operator-framework/operator-lifecycle-manager#3660 |
No description provided.