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

Remove kube-rbac-proxy #556

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

dgn
Copy link
Collaborator

@dgn dgn commented Jan 10, 2025

Replaces proxy with controller-runtime functionality. Fixes #502.

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 75.33%. Comparing base (9159944) to head (7a07d4e).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
cmd/main.go 0.00% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #556      +/-   ##
==========================================
- Coverage   75.65%   75.33%   -0.33%     
==========================================
  Files          40       40              
  Lines        2481     2497      +16     
==========================================
+ Hits         1877     1881       +4     
- Misses        514      527      +13     
+ Partials       90       89       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@FilipB
Copy link
Collaborator

FilipB commented Jan 13, 2025

@dgn could you please confirm that following can be removed after this?

@dgn
Copy link
Collaborator Author

dgn commented Jan 13, 2025

@dgn could you please confirm that following can be removed after this?

Yep!

@dgn dgn force-pushed the remove-kube-rbac-proxy branch from 289e91d to e616548 Compare January 13, 2025 11:00
@dgn dgn force-pushed the remove-kube-rbac-proxy branch from e616548 to b98d470 Compare January 13, 2025 11:04
@dgn dgn force-pushed the remove-kube-rbac-proxy branch from b98d470 to 6c48a2e Compare January 13, 2025 11:35
@dgn
Copy link
Collaborator Author

dgn commented Jan 13, 2025

/retest

@dgn dgn force-pushed the remove-kube-rbac-proxy branch from 6c48a2e to da32304 Compare January 14, 2025 19:39
@dgn dgn changed the title [WIP] Remove kube-rbac-proxy Remove kube-rbac-proxy Jan 14, 2025
@dgn dgn force-pushed the remove-kube-rbac-proxy branch from da32304 to 4d85108 Compare January 15, 2025 10:26
@dgn
Copy link
Collaborator Author

dgn commented Jan 15, 2025

/retest

1 similar comment
@dgn
Copy link
Collaborator Author

dgn commented Jan 15, 2025

/retest

@dgn dgn force-pushed the remove-kube-rbac-proxy branch from 4d85108 to e349524 Compare January 17, 2025 10:21
@dgn
Copy link
Collaborator Author

dgn commented Jan 21, 2025

/cherry-pick release-1.0

@istio-testing
Copy link
Collaborator

@dgn: once the present PR merges, I will cherry-pick it on top of release-1.0 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.0

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.

@dgn dgn force-pushed the remove-kube-rbac-proxy branch from e349524 to f9e75ed Compare January 21, 2025 14:36
subjects:
- kind: ServiceAccount
name: %s
namespace: %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work Daniel. I verified the PR in both v4 and dualStack clusters and it works fine.

$ curl -4 -s -k -H "Authorization: Bearer `cat /var/run/secrets/kubernetes.io/serviceaccount/token`" https://sail-operator-metrics-service.sail-operator.svc.cluster.local:8443/metrics
# HELP certwatcher_read_certificate_errors_total Total number of certificate read errors
# TYPE certwatcher_read_certificate_errors_total counter
certwatcher_read_certificate_errors_total 0
# HELP certwatcher_read_certificate_total Total number of certificate reads
# TYPE certwatcher_read_certificate_total counter
certwatcher_read_certificate_total 0
$ curl -6 -s -k -H "Authorization: Bearer `cat /var/run/secrets/kubernetes.io/serviceaccount/token`" https://sail-operator-metrics-service.sail-operator.svc.cluster.local:8443/metrics
# HELP certwatcher_read_certificate_errors_total Total number of certificate read errors
# TYPE certwatcher_read_certificate_errors_total counter
certwatcher_read_certificate_errors_total 0
# HELP certwatcher_read_certificate_total Total number of certificate reads
# TYPE certwatcher_read_certificate_total counter
certwatcher_read_certificate_total 0

Suggestion:

  1. Instead of spawning the "curl-metrics" pod in the "sail-operator" namespace, probably it would be good to create a new namespace and run the curl from that namespace.

  2. In the current state after running the e2e test, the curl-metrics pod continues to stay in "completed" state. If we use a different namespace, we can even delete the namespace after the validation (i.e., in Line 178) to remove all the test artifacts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Sridhar! Good idea to use a different namespace. Will push a change

@dgn dgn force-pushed the remove-kube-rbac-proxy branch from f9e75ed to 906c1da Compare January 22, 2025 13:47
Replaces proxy with controller-runtime functionality. Fixes istio-ecosystem#502.

Signed-off-by: Daniel Grimm <[email protected]>
@dgn dgn force-pushed the remove-kube-rbac-proxy branch from 906c1da to 7a07d4e Compare January 22, 2025 13:52
@istio-testing istio-testing merged commit 7c54546 into istio-ecosystem:main Jan 22, 2025
14 of 16 checks passed
@istio-testing
Copy link
Collaborator

@dgn: new pull request created: #577

In response to this:

/cherry-pick release-1.0

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.

openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Jan 22, 2025
* upstream/main:
  Remove kube-rbac-proxy (istio-ecosystem#556)
  Remove automatic channel prefix detection (istio-ecosystem#574)
  Create IstioRevisionTag documentation (istio-ecosystem#511)

# Conflicts:
#	bundle/manifests/sail-operator-metrics-service_v1_service.yaml
#	bundle/manifests/sailoperator.clusterserviceversion.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⚠️ Action Required: Replace Deprecated gcr.io/kubebuilder/kube-rbac-proxy
4 participants