NETOBSERV-1999: remove kube-rbac-proxy #994
Conversation
|
@jotak: This pull request references NETOBSERV-1999 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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: 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 |
|
@jotak: This pull request references NETOBSERV-1999 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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. |
|
@jotak: This pull request references NETOBSERV-1999 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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. |
|
@jotak: This pull request references NETOBSERV-1999 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. 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. |
jotak
force-pushed
the
rm-kube-rbax-proxy
branch
from
82c26b4 to
47f19e5
Compare
|
hmm ... this is coming with a lot of dependencies, quite unexpected, I'll check if this is really needed |
| @@ -272,20 +272,24 @@ manifests: YQ controller-gen ## Generate WebhookConfiguration, ClusterRole and C | |||
| $(YQ) -i 'del(.spec.versions[].schema.openAPIV3Schema.properties.spec.properties.consolePlugin.properties.advanced.properties.affinity.properties | .. | select(has("description")) | .description)' config/crd/bases/flows.netobserv.io_flowcollectors.yaml | |||
|
|
|||
| gencode: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations. | |||
There was a problem hiding this comment.
Before you move forward, I suggest you add e2e tests to validate the metrics endpoint after the changes. Projects created with Kubebuilder's latest versions have comprehensive E2E tests, including code to validate the metrics endpoint. Example. See: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4/test/e2e/e2e_test.go#L166-L235 (Note that the Prometheus part is not required)
There was a problem hiding this comment.
So, I'll need to dig deeper on how this is all designed, but maybe not in this PR. Those new tests seem quite different from the ones that we have, it might be a bigger refactoring (and writing tests to check metrics with our current framework, which uses kube REST API instead of kubectl, might be tricky)
(I'm not too worried about not having those tests right now, because that should be covered by QE automated tests - the operator generates dashboards from those metrics, which are tested)
| # which protects your /metrics endpoint. | ||
| - auth_proxy_service.yaml | ||
| - auth_proxy_role.yaml | ||
| - auth_proxy_role_binding.yaml |
There was a problem hiding this comment.
You still needing RBAC permissions for the metrics
The Controller-Runtime feature will use it at the same away that kube-rbac-proxy
See the rules scaffold by default in the Kubebuilder Sample; https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v4/config/rbac
See that you will need to have:
There was a problem hiding this comment.
thanks, I've reintroduced that part
|
@jotak I am thinking regression testing is sufficient for this one, the sanity test e2e-operator is passing. QE e2e tests will be covered post-merge, wdyt? |
Replace with operator managing rbac and TLS for metrics directly, as described here: kubernetes-sigs/kubebuilder#3907
jotak
force-pushed
the
rm-kube-rbax-proxy
branch
from
47f19e5 to
7c138e2
Compare
|
New changes are detected. LGTM label has been removed. |
jotak
added
the
ok-to-test
Yes. As pointed out here, it should be tested that the health dashboard still shows the operator metrics, if I'm correct this is covered in your regression tests |
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:69b899d make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-69b899dOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-69b899d
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
|
health dashboard with operator metrics: |
Description
Replace with operator managing rbac and TLS for metrics directly, as described here: kubernetes-sigs/kubebuilder#3907
Fixes #915
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.