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.

Sign up for GitHub

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

Jump to bottom

Secure metrics endpoint with oauth-proxy #76

Merged
merged 1 commit into from Mar 1, 2019
Merged

Secure metrics endpoint with oauth-proxy #76

merged 1 commit into from Mar 1, 2019

Conversation

josefkarasek
Copy link

Prometheus server can't access ES metrics, because ES uses certs
signed by a self-signed CA, which isn't recognized.

Instead a proxy is used, which presents certs signed by a CA
which can be accessed through serving-certs-ca-bundle

@josefkarasek josefkarasek added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2019
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 30, 2019
@josefkarasek
Copy link
Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2019
@openshift-ci-robot 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 Jan 31, 2019
jcantrill
jcantrill reviewed Jan 31, 2019
View reviewed changes
Copy link
Contributor

@jcantrill jcantrill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an open BZ or issue to link this to?

pkg/k8shandler/desirednodestate.go Outdated Show resolved Hide resolved
pkg/k8shandler/desirednodestate.go Outdated Show resolved Hide resolved
@ewolinetz
Copy link
Contributor

@jcantrill yes, https://jira.coreos.com/browse/LOG-329

@richm
Copy link
Contributor

richm commented Feb 6, 2019

/test e2e-operator

@josefkarasek josefkarasek force-pushed the log324-name-service branch 2 times, most recently from 82127e2 to 796e3c7 Compare February 12, 2019 22:23
@ewolinetz
Copy link
Contributor

formatting nits otherwise lgtm

@ewolinetz
Copy link
Contributor

Will there be a CLO pr to go along with this to populate this ENV var and would it be useful to have a default value here in the case where it is omitted?

@ewolinetz
Copy link
Contributor

Would it make sense to add CI tests here as well?

@richm
Copy link
Contributor

richm commented Feb 13, 2019

/test e2e-aws

@josefkarasek josefkarasek changed the title Secure metrics endpoint with kube-rbac-proxy Secure metrics endpoint with oauth-proxy Feb 22, 2019
@richm
Copy link
Contributor

richm commented Feb 22, 2019

/test e2e-aws

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2019
ewolinetz
ewolinetz reviewed Feb 27, 2019
View reviewed changes
pkg/k8shandler/rbac.go Outdated Show resolved Hide resolved
ewolinetz
ewolinetz reviewed Feb 27, 2019
View reviewed changes
pkg/k8shandler/rbac.go Outdated Show resolved Hide resolved
@jcantrill
Copy link
Contributor

@josefkarasek please update the commit message to be something like:

bug 1662273. Fixes metrics by ...

Bug 1662273. Fixes secure access to elasticseach metrics endpoint.
7dab408 
Prometheus server can't access ES metrics, because ES uses certs
signed by a self-signed CA, which isn't recognized.

Instead a proxy is used, which presents certs signed by a CA
which can be accessed through serving-certs-ca-bundle
@josefkarasek
Copy link
Author

/test e2e-operator

lukas-vlcek
lukas-vlcek approved these changes Mar 1, 2019
View reviewed changes
Copy link
Contributor

@lukas-vlcek lukas-vlcek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lukas-vlcek
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: josefkarasek, lukas-vlcek

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 [josefkarasek,lukas-vlcek]

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

@jcantrill
Copy link
Contributor

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2019
@josefkarasek
Copy link
Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2019
@ewolinetz
Copy link
Contributor

/hold
have a few change requests

@openshift-merge-robot openshift-merge-robot merged commit 7f19c6a into openshift:master Mar 1, 2019
@jcantrill
Copy link
Contributor

@josefkarasek you will need to address @ewolinetz changes in follow pr

This was referenced Mar 6, 2019
Merged
Merged
@jcantrill jcantrill mentioned this pull request Jan 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richm richm richm left review comments

@ewolinetz ewolinetz ewolinetz left review comments

@jcantrill jcantrill jcantrill left review comments

@lukas-vlcek lukas-vlcek lukas-vlcek approved these changes

Assignees

@lukas-vlcek lukas-vlcek

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@josefkarasek @ewolinetz @richm @jcantrill @lukas-vlcek @openshift-ci-robot @openshift-merge-robot