standard ose-oauth-proxy image in digest format as used in all other …

[shalberd]

Description

related to odh-manifests PR opendatahub-io/odh-manifests#869

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]

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.

[DaoDaoNoCode]

/ok-to-test

[lucferbux]

I'm gonna ping @andrewballantyne here too, this is something we currently have in our roadmap, we want to convert both images in the releases into sha-265, I don't see an issue on getting the sha image for oauth proxy right now, except committing ourselves to keep it updated.

Let's start a conversation around it.

[shalberd]

@lucferbux I also noticed manifests is not the only place to ose-oauth-proxy is used here

https://github.com/search?q=repo%3Aopendatahub-io%2Fodh-dashboard%20ose-oauth-proxy&type=code

Are the frontend src mocks functional? That is, would it be necessary to update to the common version digest there, too?

[andrewballantyne]

I thought we had a Dashboard issue for this -- we'll not be able to use sha values on the dashboard version -- and I'm not sure that's hugely a problem. The "tag" main is fine for now. We could look at each release to replace the dashboard tag with the latest sha from quay... but I don't expect that really to be done now as we are still mid-way to release.

[shalberd]

@andrewballantyne yes, the issue for sha256 is this one here

#1470

Your odh-dashboard:main image is indeed the same a lot as ose-oauth:v4.10, in terms of the concept of an image behind a fixed tag changing.

Both tags have changing digests / images behind them, similar to the tag :latest.
However, even in the main branch, pinning down ose-oauth-proxy to digest is uncritical, as the idea is to precisely pin down to a really specific image across the board for ose-oauth-proxy.

we'll not be able to use sha values on the dashboard version -- and I'm not sure that's hugely a problem.

It is a problem for airgapped / disconnected restricted network environment installed with image mirroring and ImageContentSourcePolicy. Those need sha256 digest format, so at least for the release branches, once you set a tag for odh-dashboard, i.e. v2.67, you will have to change that to digest format in your deployment yaml.

Side note:

What will also have to be considered: If you ever change your deployment.yaml recipe's imagePullPolicy to IfNotPresent, which in itself is a good idea, then that can only be either in odh-manifests where you have release images that do not change given a certain tag or in odh-dashboard release branches, where you'll have sha256 digests, too.
That is the reason that in PR-800, when assembling the notebook yaml, using imagePullPolicy is ok cause the images in the imagestreams, more specifically in the container image-field, are in sha256 digest format, even if imagestream tag from.name external ref url is in tag format. The standard imagestreams that come with odh-manifests are already pinned down to sha256 digest format by default even in master branch. They created a Github action for that, I think @harshad16 did that.
That might be an avenue for your main branch, optionally, too.

So yes, the two are related, but this PR is uncritical and independent of all that.

You will be able to get a lot of input for your non-main branches and digest looking-up within github actions with I think skopeo for getting the digest of an image by tag from i.e. the ds-pipelines and model-mesh project people, some have already implemented Github actions.

[andrewballantyne]

Sorry, let me clarify my statement...

we'll not be able to use sha values on the dashboard version -- and I'm not sure that's hugely a problem. The "tag" main is fine for now

This is fine for development mid-sprint/release. We can look at changing this per release, we just gotta be careful that anyone deploying the Dashboard directly can get the latest image and not the one from the last release. It's possible we'll need to have an overlay or something like that for releases or something for development. But I don't want to break the "direct to dashboard" to get the latest UI. odh-manifests has our tagged release v2.yy.z -- which is a singular build. We can change that from the tag to a sha value if we want.

[shalberd]

odh-manifests has our tagged release v2.yy.z -- which is a singular build. We can change that from the tag to a sha value if we want.

We are on the same page definitely then, thank you very much. You could also think about changing the deployment.yaml imagePullPolicy to IfNotPresent in odh-manifests then, also. That way, node-level cache is used for the image, increasing stability (startup frequency and time is less of an issue for dashboard, when compared to notebook pods and images).

But I don't want to break the "direct to dashboard" to get the latest UI

Yes, I understand that from a devops and testing perspective, your main tag is similar to :latest

[andrewballantyne]

You could also think about changing the deployment.yaml imagePullPolicy to IfNotPresent in odh-manifests then, also. That way, node-level cache is used for the image, increasing stability (startup frequency and time is less of an issue for dashboard, when compared to notebook pods and images).

Is this a real concern? The Dashboard pods almost never restart. they are not like Notebook Images 🤔 I am not against the improvement, but it seems like a minor improvement, no?

[shalberd]

Is this a real concern?

not so much from a startup time or frequency perspective, but imagine the, albeit unlikely scenario, of the internal openshift registry or the mirror external docker registry not being reachable due to network problems or whatever. When imagePullPolicy is set to Always, that'll prevent startup of the container. While with IfNotPresent, the node cache can be used, increasing availability and so on.

but it seems like a minor improvement

yep, but doesn't hurt in odh-manifests

[shalberd]

@lucferbux since this is manifests-only related to ose-oauth-proxy docker image, can you merge this, please?

I assume the frontend mocks are non-functional, so I did not change the version / digest there

https://github.com/search?q=repo%3Aopendatahub-io%2Fodh-dashboard%20ose-oauth&type=code

[lucferbux]

@lucferbux since this is manifests-only related to ose-oauth-proxy docker image, can you merge this, please?

I assume the frontend mocks are non-functional, so I did not change the version / digest there

https://github.com/search?q=repo%3Aopendatahub-io%2Fodh-dashboard%20ose-oauth&type=code

@shalberd Don't worry about the mocks, we don't need to change them since that value is not part of the coverage.
I'm fine with this change as it is right now. Can you create an issue next time to associate with the pr? not needed for today, but just for the sake of traceability would be nice to have it.

Thanks again.

[lucferbux]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: lucferbux

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:

• OWNERS

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