Expose metric endpoint on https #368

vikaschoudhary16 · 2019-08-03T08:29:34Z

This PR is adding changes to expose metrics endpoint on https and also changes at other objects for integration with openshift-monitoring at https.
This PR is also restricting http port access only on loopback ip.

Approach for https support:

kube-rbac-proxy container is being run alongside operator container which runs metrics server in the same pod. kube-rbac -proxy validates authz and authn on behalf of the metrics server and if success, proxies the incoming https request to the metrics server(which is listening on 127.0.0.1 and http port 8080).

How authn and authz is being validated by kube-rbac-proxy

It uses TokenReviews() and SubjectAccessReviews() apis internally for validation of token authentication and authorization respectively. These apis are used against a stub resource, namespace/metrics which is passed as config to the kube-rbac-proxy

apiVersion: v1
kind: ConfigMap
metadata:
  name: kube-rbac-proxy
  namespace: openshift-machine-api
data:
  config-file.yaml: |+
    authorization:
      resourceAttributes:
        apiVersion: v1
        resource: namespace
        subresource: metrics
        namespace: openshift-machine-api

RBAC permissions for this rule are defined same as that we expect for metrics endpoint.
So when a https request is received for metrics endpoint, token and rbac of the request user is validated against this stub resource. If it passes, proxied to the metrics endpoint.

How I tested it locally

launch a cluster using installer
make cluster-version deployment replica to 0
change mao image with the one with the PR changes
change/create all the api objects(like clusterrole, rbac-config etc) that are part of this PR
I had to make a curl command on the metrics service with service account, prometheus-k8s. I got this token using following command:

kubectl -n openshift-monitoring get secret $(kubectl -n openshift-monitoring get sa prometheus-k8s -o 'jsonpath={.secrets[0].name}') -o 'jsonpath={.data.token}' | base64 -d

Next, created a pod in the openshift-monitoring namespace to make curl call at service
Then copy token obtained above in the curl container and make the curl call

curl -v -s -k -H "Authorization: Bearer cat ./token2" https://machine-api-operator.openshift-machine-api.svc:8443/metrics

This should return metrics

vikaschoudhary16 · 2019-08-03T10:32:30Z

/test integration

vikaschoudhary16 · 2019-08-05T10:37:55Z

/test e2e-aws

vikaschoudhary16 · 2019-08-05T12:22:03Z

/test e2e-aws

vikaschoudhary16 · 2019-08-05T13:14:30Z

/test e2e-aws

vikaschoudhary16 · 2019-08-06T04:56:56Z

/test e2e-aws

vikaschoudhary16 · 2019-08-06T10:38:00Z

/test integration

vikaschoudhary16 · 2019-08-06T12:14:40Z

/test integration

vikaschoudhary16 · 2019-08-06T16:25:00Z

/test e2e-aws

vikaschoudhary16 · 2019-08-06T18:01:46Z

/test e2e-aws

frobware · 2019-08-07T07:06:56Z

config/machine-api-operator-deployment.yaml

+        - name: config
+          mountPath: /etc/kube-rbac-proxy
+      - name: machine-api-operator
+        image: docker.io/openshift/origin-machine-api-operator:v4.0.0


This image refers to v4.0 whereas the kube-rbac-proxy refers to 4.2.0. Is this intentional?

mao image version is same as it is today in https://github.com/openshift/machine-api-operator/blob/master/install/0000_30_machine-api-operator_11_deployment.yaml#L22

kube-rbac-proxy image 4.2.0 is taken just to be latest. Not sure if these two should be same. I see both image streams 4.0.0 and 4.2.0 in the install/image-references, so chose to follow the same pattern and use 4.2.0

frobware · 2019-08-07T09:23:59Z

Do we care that we listen on IPv4 only? Why not explicitly use localhost which would cover IPv6 too?

vikaschoudhary16 · 2019-08-07T11:31:42Z

/retest

enxebre · 2019-08-09T11:31:01Z

config/machine-api-operator-deployment.yaml

+kind: Deployment
+metadata:
+  name: machine-api-operator
+  namespace: openshift-machine-api


can you elaborate why we need this file?

This is required for the k8s-e2e ci job which runs test against the k8s. In the original deployment, we are mounting a secret into the pod. Based on the annotation in the mao metrics service object, this secret will get created automatically in the openshift cluster. With the minikube cluster, this secret will not be there and thus deployment will fail. To workaround this, in the k8s-e2e tests this deployment will be used which is not mounting secrets.

vikaschoudhary16 · 2019-08-09T15:30:43Z

/test e2e-aws

enxebre · 2019-09-03T07:44:52Z

/approve
@openshift/openshift-team-monitoring could anyone have a quick look? cc @simonpasquier @s-urbaniak @paulfantom

openshift-ci-robot · 2019-09-03T07:45:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [enxebre]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

brancz · 2019-09-09T09:55:27Z

config/machine-api-operator-deployment.yaml

+      serviceAccountName: machine-api-operator
+      containers:
+      - name: kube-rbac-proxy
+        image: quay.io/openshift/origin-kube-rbac-proxy:4.2.0


this should be an image stream no?

nevermind, it is

brancz · 2019-09-09T10:00:23Z

config/machine-api-operator-deployment.yaml

+      - name: kube-rbac-proxy
+        image: quay.io/openshift/origin-kube-rbac-proxy:4.2.0
+        args:
+        - "--secure-listen-address=0.0.0.0:8443"


this will use an at startup generated self-signed cert, is that intended?

yes, and this is related, https://github.com/openshift/machine-api-operator/pull/368/files/5d2b38bc7a0e838f2032a29c6efb7c966c256912#diff-56c9657f199776b5f04119428ca26c17R8

spangenberg · 2019-09-25T14:15:09Z

/lgtm

openshift-bot · 2019-09-25T16:47:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-09-25T17:43:17Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2019-09-25T18:03:50Z

@vikaschoudhary16: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-scaleup-rhel7	`5d2b38b`	link	`/test e2e-aws-scaleup-rhel7`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot requested review from ingvagabund and spangenberg August 3, 2019 08:29

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 3, 2019

vikaschoudhary16 force-pushed the metrics-on-https branch 2 times, most recently from 91455eb to d9b290d Compare August 3, 2019 09:34

vikaschoudhary16 force-pushed the metrics-on-https branch 3 times, most recently from 00c9a1c to cfacd9d Compare August 5, 2019 08:34

vikaschoudhary16 force-pushed the metrics-on-https branch from cfacd9d to 188be58 Compare August 6, 2019 09:34

vikaschoudhary16 force-pushed the metrics-on-https branch from 188be58 to 095437c Compare August 6, 2019 11:40

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 6, 2019

vikaschoudhary16 force-pushed the metrics-on-https branch from 095437c to 480ef2b Compare August 6, 2019 14:57

frobware reviewed Aug 7, 2019

View reviewed changes

vikaschoudhary16 force-pushed the metrics-on-https branch from 480ef2b to f8fb691 Compare August 7, 2019 09:43

vikaschoudhary16 force-pushed the metrics-on-https branch 2 times, most recently from 134014d to e0ef43a Compare August 8, 2019 04:49

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2019

Listen only on loopback IP at http port

9ec6aa2

Integrate with openshift-monitoring at secure(https) metrics endpoint

5d2b38b

vikaschoudhary16 force-pushed the metrics-on-https branch from e0ef43a to 5d2b38b Compare August 9, 2019 10:58

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2019

enxebre reviewed Aug 9, 2019

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2019

brancz reviewed Sep 9, 2019

View reviewed changes

openshift-ci-robot assigned spangenberg Sep 25, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2019

openshift-merge-robot merged commit 57d57d8 into openshift:master Sep 25, 2019

Expose metric endpoint on https #368

Expose metric endpoint on https #368

Uh oh!

Conversation

vikaschoudhary16 commented Aug 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approach for https support:

How authn and authz is being validated by kube-rbac-proxy

How I tested it locally

Uh oh!

vikaschoudhary16 commented Aug 3, 2019

Uh oh!

vikaschoudhary16 commented Aug 5, 2019

Uh oh!

vikaschoudhary16 commented Aug 5, 2019

Uh oh!

vikaschoudhary16 commented Aug 5, 2019

Uh oh!

vikaschoudhary16 commented Aug 6, 2019

Uh oh!

vikaschoudhary16 commented Aug 6, 2019

Uh oh!

vikaschoudhary16 commented Aug 6, 2019

Uh oh!

vikaschoudhary16 commented Aug 6, 2019

Uh oh!

vikaschoudhary16 commented Aug 6, 2019

Uh oh!

frobware Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

vikaschoudhary16 Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

frobware commented Aug 7, 2019

Uh oh!

vikaschoudhary16 commented Aug 7, 2019

Uh oh!

enxebre Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

vikaschoudhary16 Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

vikaschoudhary16 commented Aug 9, 2019

Uh oh!

enxebre commented Sep 3, 2019

Uh oh!

openshift-ci-robot commented Sep 3, 2019

Uh oh!

brancz Sep 9, 2019

Choose a reason for hiding this comment

Uh oh!

brancz Sep 9, 2019

Choose a reason for hiding this comment

Uh oh!

brancz Sep 9, 2019

Choose a reason for hiding this comment

Uh oh!

vikaschoudhary16 Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spangenberg commented Sep 25, 2019

Uh oh!

openshift-bot commented Sep 25, 2019

Uh oh!

openshift-bot commented Sep 25, 2019

Uh oh!

openshift-ci-robot commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vikaschoudhary16 commented Aug 3, 2019 •

edited

Loading

vikaschoudhary16 Sep 11, 2019 •

edited

Loading

openshift-ci-robot commented Sep 25, 2019 •

edited

Loading