OCPBUGS-55399: cluster-node-tuning-operator Pod of the hosted cluster is not listening on 60000 port which makes the target to be down #1346
Conversation
openshift-ci-robot
added
jira/severity-moderate
|
@georgelipceanu: This pull request references Jira Issue OCPBUGS-55399, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: georgelipceanu 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 |
|
/retest-required |
| select { | ||
| case server.caBundleCh <- caBundle: | ||
| default: | ||
| klog.Infof("Metrics server CA channel not ready, skipping CA bundle update") | ||
| } |
There was a problem hiding this comment.
this is welcome and seems correct, but why is it needed in the context of this PR to change the metrics server?
There was a problem hiding this comment.
I'm just after retesting this without the change and it seems to work without it. When trying out fixes for this bug, I was of the belief that the NTO was blocked on this line as it would be trying to receive a response that would never come due to the switch to HTTP but it seems that it works just fine without it. Either way, I'm willing to either keep this block in this PR or open a new one with just this little piece of code. Nice catch 🙌
There was a problem hiding this comment.
I would like to explore options before to resort to just disable tls and expose the port to everyone. I'm concerned about information leaking. How can we prevent it?
Regarding the switch to HTTP, from what I've found and experimented with, I found that the NTO needed RBAC permissions to get the ConfigMap from the hosted cluster's kube-system namespace while the Control-Plane-Operator in the hosted cluster, responsible for deploying the operator, did not have permission to create RBAC resources in the hosted cluster's kube-system namespace.
I personally wasn't able to find a way to keep HTTPS with the current configuration but I would be willing to take a deeper dive into it HTTP won't be secure enough.
There was a problem hiding this comment.
Thank you for the PR, IIRC, we had two issues with the metrics server reported to us by the Security team.
- not using https://
- authentication was required to use https:// endpoint
I'd be very reluctant to merge this without meeting both requirements.
There was a problem hiding this comment.
Makes sense, in that case I'll keep looking into keeping HTTPS with the metrics server for HyperShift. However if the new DumpCA() code looks good then I would be willing to repurpose this PR for this code only. Is this something that the team would be interested in?
georgelipceanu
force-pushed
the
OCPBUGS-55399
branch
2 times, most recently
from
b91c200 to
4a092c2
Compare
This is to accommodate for the change in the metrics port 60000 from HTTPS to HTTP, which allows the metrics to run without the need for additional permissions to RBAC resources in the kube-system namespace to the HyperShift operator. Signed-off-by: George Lipceanu <[email protected]>
georgelipceanu
force-pushed
the
OCPBUGS-55399
branch
from
4a092c2 to
df3314e
Compare
|
@georgelipceanu: all tests passed! 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. |
What this PR does / why we need it:
This PR changes the NTO to run NTO related metrics on port 60000 by changing the port protocol from HTTPS to HTTP, which allows port 60000 to run in HyperShift without the need for additional permissions to RBAC resources in the kube-system namespace to the HyperShift operator.
Which issue(s) this PR fixes
Fixes #OCPBUGS-55399
Currnetly Blocking HyperShift PR