unify all references to ose-oauth-proxy to v4.10 latest digest

[shalberd]

/closes opendatahub-io/opendatahub-operator#572

related to kubeflow odh notebook controller and oauth injected notebooks sidecar story, too

opendatahub-io/kubeflow#119

as well as odh dashboard PR opendatahub-io/odh-dashboard#1482

Description

Make ose-oauth-proxy container version or sha256 digest reference consistent across all components e.g. odh-dashboard.

Agree on v4.10

registry.redhat.io/openshift4/ose-oauth-proxy@sha256:ab112105ac37352a2a4916a39d6736f5db6ab4c29bad4467de8d613e80e9bb33

Since ose-oauth-proxy tag reference image behind, the digest, regardless which openshift 4 minor version, changes regularly. Thus, and to be compatible with restricted network environment ImageContentSourcePolicy as well with node image caching with imagePullPolicy: IfNotPresent, change to digest notation.

Latest manifest link digest for v4.10 from July 6 2023

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=gti

included in this PR.

alternative way with downloading, just to show it is in fact the most current v4.10 digest:

docker pull registry.redhat.io/openshift4/ose-oauth-proxy:v4.10
v4.10: Pulling from openshift4/ose-oauth-proxy
1df162fae087: Already exists 
d20a374ee8f7: Already exists 
e88a8f7a57fc: Pull complete 
dcbbfecd111e: Pull complete 
Digest: sha256:ab112105ac37352a2a4916a39d6736f5db6ab4c29bad4467de8d613e80e9bb33
Status: Downloaded newer image for registry.redhat.io/openshift4/ose-oauth-proxy:v4.10
registry.redhat.io/openshift4/ose-oauth-proxy:v4.10

How Has This Been Tested?

Used this image version in copied manifests, manually modified, zipped and tarred, running on an on-prem OCP 4.10 cluster with odh core components installed (except monitoring).

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vaishnavihire for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[openshift-ci]

Hi @shalberd. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

[shalberd]

@andrewballantyne let me know what you think about adding a yaml comment line for humans to read on the tag version used

i.e.

# tag v4.10 as of July 6 2023

mini-PR in odh-dashboard manifests directory, working and tested

opendatahub-io/odh-dashboard#1599

@Jooho same for model-mesh

[andrewballantyne]

@LaVLaS is there a way we can make a constant or something and refer to it throughout our multi-component manifests? It would be great for this to be unified in a singular spot rather than chasing after all the references.

[andrewballantyne]

@VaishnaviHire @etirelli Out of curiosity, could OAuth version be managed by the new Operator? Could it supply some value and inject containers when deploying things? Or is that out of scope of its goals?

[shalberd]

I am keeping the issue of container.name oauth-proxy and imagePullPolicy off this PR, but that you be done in a separate PR, too, setting each component's oauth container imagePullPolicy to IfNotPresent

[LaVLaS]

@LaVLaS is there a way we can make a constant or something and refer to it throughout our multi-component manifests? It would be great for this to be unified in a singular spot rather than chasing after all the references.

Yes but we would need to tightly couple all of the components manifests to a use kfctl parameters as a standard instead of the industry standard kustomize manifest. Manually updating all of the references is the simplest solution given that it would only occur a few times a year

@VaishnaviHire @etirelli Out of curiosity, could OAuth version be managed by the new Operator? Could it supply some value and inject containers when deploying things? Or is that out of scope of its goals?

I'll defer to @VaishnaviHire for the final answer but Yes the new operator design would allow us to include logic on top of the declaritive kustomize manifests to easily support this type of global override.

[shalberd]

@atheo89 @anishasthana @LaVLaS @andrewballantyne

What do you think of the PR as it is right now? I am using the manifest link sha256 digest value of v4.10 in this PR, has not changed since July 7.

Can you have a look, please and ok-to-test? This PR is also conceptually related to PR 868

Also, @HumairAK I could not trace back which version of ose-oauth-proxy you are using, but if the intent is to use v4.10 across the board, then this PR does just that. I noticed the CI tests here spawn up an Openshift with v4.10 as well ..

[HumairAK]

@shalberd dsp is using 4.12 oauth proxy, so this change is not needed on the dsp front since the primary reason for the issue is to stop using old oauth proxy images afaik

also, typically we make changes in the dsp manifests within the component repo before trickling the changes down to odh-manifests (as these need to be kept in sync)

[shalberd]

@HumairAK no, the idea of this PR is to unify the version of ose-oauth-proxy across all components. I know some people use latest versions of Openshift i.e. 4.12 or 4.13, but the standard here is really OCP 4.10. Thus the unified reference to v4.10 of ose-oauth-proxy.

We use digest format for disconnected support. You are right about also having to do this in your repo as well. ODH Dashboard has already unified to v4.10 from old 4.8 in their repo, for example. In a second step, they will now include the image in digest format.

Do you have a hard requirement / reason for using v4.12 oauth proxy image?

[HumairAK]

I see, thanks for the clarification. There's no hard requirement for 4.12, we can go with 4.10, but may we get this merged in the component repo first (at least for dsp)? we can keep this in the pr here in the meantime, would you be willing to make the same adjustment here?

[andrewballantyne]

I don't want to prescribe solutions on this kind of stuff -- but when I see the same value copy and pasted, my mind goes "why is that not a variable"... can we not variable it up in one spot?

[VaishnaviHire]

Sorry for the delay here, yes we could definitely create a global configuration that can be used by all components. For this specific change, we should be able to make use of imageTagPlugin or some other alternatives to maintain unified tags across components.

Support for kustomize plugins in new operator can be found here.

I would not hold up this PR, but will add an issue in the operator repo.

[andrewballantyne]

I would not hold up this PR, but will add an issue in the operator repo.

@VaishnaviHire begs the question here -- would we want to lean into the operator supplying the value to every component it mounts and letting them use it? Is that possible? A default being "whatever the component wants to do" and the component could just ignore it. I imagine there are components delivered by the component that are not delivered by the operator...

This is all to say, it would be cool to unify all on the same underlying components... like OAuth Proxy.

[VaishnaviHire]

I would not hold up this PR, but will add an issue in the operator repo.

@VaishnaviHire begs the question here -- would we want to lean into the operator supplying the value to every component it mounts and letting them use it? Is that possible? A default being "whatever the component wants to do" and the component could just ignore it. I imagine there are components delivered by the component that are not delivered by the operator...

This is all to say, it would be cool to unify all on the same underlying components... like OAuth Proxy.

I think we can selectively apply the plugin i.e component can opt-in to use the default value of oauth-proxy provide by operator or can go ahead with their pre-defined value. Is that something that will cover this requirement?

[andrewballantyne]

I think we can selectively apply the plugin i.e component can opt-in to use the default value of oauth-proxy provide by operator or can go ahead with their pre-defined value. Is that something that will cover this requirement?

Sounds like it to me... is this something we can do with a label/annotation or do we need to get into variables in the manifests?

Again, this shouldn't hold up this PR if we want to sync up what we have here.

[andrewballantyne]

/ok-to-test

[shalberd]

agreed on finding a more centralized solution later on, as @andrewballantyne and @VaishnaviHire discussed.

The intent of this PR was to raise awareness, and, it shows how I standardized on a corporate OCP cluster with ODH slightly custom manifests in them (using sha256 notation across the board, for all images, using imagePullPolicy: IfNotPresent across the board ....).

For all components, the manifests are also in their respective repos, so PRs need to be done there, too. That already happened for data-science-pipelines (merged in) and odh-dashboard (pending), for the others, PRs still to come.

[openshift-merge-robot]

PR needs rebase.

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/test-infra repository.

[VaishnaviHire]

Repo archived. Refer to individual component manifests for any changes