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

add hardcoded authorizer to approve /metrics for metrics scraper #43

Merged
merged 2 commits into from
Mar 12, 2021

Conversation

deads2k
Copy link

@deads2k deads2k commented Feb 24, 2021

This is an opinionated authorizer that grants access for our metrics scraper without asking the kube-apiserver because on openshift, we know this identity should always have metrics access.

@s-urbaniak
Copy link

/cc @openshift/openshift-team-monitoring

@openshift-ci-robot openshift-ci-robot requested a review from a team February 24, 2021 15:42
@s-urbaniak
Copy link

fwiw this leaves the token review intact so we are not skipping verifying if the token is invalidated.

@deads2k
Copy link
Author

deads2k commented Feb 24, 2021

@s-urbaniak what did I do to vendor to piss it off?

@s-urbaniak
Copy link

a go mod vendor should do the trick.

@deads2k
Copy link
Author

deads2k commented Feb 25, 2021

a go mod vendor should do the trick.

fixed

@s-urbaniak
Copy link

/hold

I brought this back to team and the general consensus is to bring this upstream to reduce catch-parry maintenance tech debt. I can help in submitting a PR upstream short-term. This should give us enough headroom to bring this into 4.8.

@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, 2021
@s-urbaniak
Copy link

/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 11, 2021
@s-urbaniak
Copy link

As discussed with @simonpasquier we'll move forward for now with merging this and will try to contribute the functionality upstream in the 4.8 timeframe.

@s-urbaniak
Copy link

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, s-urbaniak

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2021
@s-urbaniak
Copy link

@deads2k not sure why, do you mind to go mod vendor again?

@deads2k
Copy link
Author

deads2k commented Mar 11, 2021

  • 1x kubelet: Failed to create pod sandbox: rpc error: code = Unknown desc = Kubelet may be retrying requests that are timing out in CRI-O due to system load: the

it was the CI outage

/retest

@simonpasquier
Copy link

/retest

1 similar comment
@s-urbaniak
Copy link

/retest

@openshift-bot
Copy link

/retest

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

1 similar comment
@openshift-bot
Copy link

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 794f9de into openshift:master Mar 12, 2021
prashbnair pushed a commit to prashbnair/kube-rbac-proxy that referenced this pull request Sep 20, 2021
This reverts commit 794f9de, reversing
changes made to 6ea3294.
prashbnair pushed a commit to prashbnair/kube-rbac-proxy that referenced this pull request Sep 20, 2021
This reverts commit 794f9de, reversing
changes made to 6ea3294.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants