Update oauth proxy to v4.10 #1482
Conversation
lucferbux
requested review from
andrewballantyne,
manaswinidas and
pnaik1
and removed request for
Gkrumbach07 and
DaoDaoNoCode
|
/hold I'm checking if we need to modify the deployment in case something is deprecated, but so far it's working just fine. |
|
/unhold Ok, according to the docs, none of the parameters we use are deprecated. |
There was a problem hiding this comment.
Good first pass. We'll have to swing back and do proper sha references -- covered by #1470.
|
/hold Someone other than the author should test this on the cluster first :) |
|
I'd use digest notation instead, and a snapshot point in time, i.e. today, July 6 2023, digest value of v4.10. See also opendatahub-io/odh-manifests#869 Source: Manifest link digest from today of v4.10 tag https://catalog.redhat.com/software/containers/openshift4/ose-oauth-proxy/5cdb2133bed8bd5717d5ae64?architecture=amd64&tag=v4.10.0-202306170106.p0.g799d414.assembly.stream&push_date=1688610772000&container-tabs=overview alternative way with downloading, just to show it is in fact the most current v4.10 digest: |
| @@ -27,4 +27,4 @@ images: | |||
| newTag: main | |||
| - name: oauth-proxy | |||
| newName: registry.redhat.io/openshift4/ose-oauth-proxy | |||
| newTag: v4.8 | |||
| newTag: v4.10 | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
mmmm that's what @andrewballantyne commented above, we are planning to use digest in all our images, but we are going to do the migration all at once, we are going to keep the scope of this PR to just update the version of the oauth-proxy image.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
I wanted to do this in layers, because I want us to take the time to get the right sha -- compare it with everyone, hopefully get a constant or something going.
This PR is to correct a lossy effect of us not paying attention to our "lowest supported OCP version"... Not the final solution. I onboarded last year and wasn't even close to looking at this version change, so this is a "catch up" effort.
I do get your point though, @shalberd -- it is much better to use sha digests everywhere because it is very specific and helps a lot. But I think it's important that we do these two things in two steps because conversations and ideas can slow down this basic upgrade.
I'm suggesting we continue the path with this PR as-is, and I'll log something in odh-manifests to see if we can unify across the board.
There was a problem hiding this comment.
Good idea. Regarding odh.manifests, I've already initiated a unifying PR https://github.com/opendatahub-io/odh-manifests/pull/869/files#diff-100131d24ded424b92afb1f01bc131586ed11798e6d1f2011d1633bafaf7c94d
There was a problem hiding this comment.
I made some comments here: opendatahub-io/odh-manifests#869 to start things rolling that direction.
|
I have tested this via manually-modified odh-manifests, working well on an OCP 4.10 cluster |
|
@uidoyen @manaswinidas @DaoDaoNoCode @Gkrumbach07 can you guys check this out? |
|
/lgtm |
There was a problem hiding this comment.
auth-proxy container log looks good.
2023/07/07 17:25:01 provider.go:314: Delegation of authentication and authorization to OpenShift is enabled for bearer tokens and client certificates.
2023/07/07 17:25:01 oauthproxy.go:203: mapping path "/" => upstream "http://localhost:8080/"
2023/07/07 17:25:01 oauthproxy.go:224: compiled skip-auth-regex => "^/metrics"
2023/07/07 17:25:01 oauthproxy.go:230: OAuthProxy configured for Client ID: dashboard-oauth-client
2023/07/07 17:25:01 oauthproxy.go:240: Cookie settings: name:_oauth_proxy secure(https):true httponly:true expiry:23h0m0s domain:<default> samesite: refresh:disabled
2023/07/07 17:25:01 http.go:61: HTTP: listening on 127.0.0.1:4180
2023/07/07 17:25:01 http.go:107: HTTPS: listening on [::]:8443
I0707 17:25:01.082227 1 dynamic_serving_content.go:130] Starting serving::/etc/tls/private/tls.crt::/etc/tls/private/tls.key
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne, uidoyen 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 |
|
/unhold |
Description
Closes #1400
How Has This Been Tested?
kfdeffrom theOpen Data Hub Operatorif you had it deployed.make undeployif you already have a dashboard version deployedmake deployopendatahubnamespace, look for theodh-dashboarddeployment.registry.redhat.io/openshift4/ose-oauth-proxy:v4.10odh-dashboardpods, and check that the oauth-proxy container log is not raising any issueTest Impact
No testing coverage for infra update
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main