Update kubeflow/model-registry manifests from 5d8ed91

[tarilabs]

Pull Request Template for Kubeflow manifests Issues

✏️ A brief description of the changes

Updated KF manifests for MR following most recent release update

📦 List any dependencies that are required for this change

none

🐛 If this PR is related to an issue, please put the link to the issue here.

for KF 1.10 RC builds

✅ Contributor checklist

• Make sure you have tested with kustomize. See Installation Prerequisites
• All the commits have been signed-off (To pass the DCO check)

---

You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

[tarilabs]

/assign @juliusvonkohout

as discussed in today's meeting, thank you again

[juliusvonkohout]

Hello, you are missing a security context for most of your deployments. Please add

manifests/common/dex/base/deployment.yaml

Lines 25 to 32 in 550e0b3

	securityContext:
	allowPrivilegeEscalation: false
	seccompProfile:
	type: RuntimeDefault
	runAsNonRoot: true
	capabilities:
	drop:
	- ALL

to all of your deployments.

[juliusvonkohout]

Hello, where is the Kubeflow userid header checked ?

[ederign]

https://github.com/kubeflow/model-registry/blob/main/clients/ui/bff/internal/api/middleware.go#L33-L68
https://github.com/kubeflow/model-registry/blob/main/clients/ui/bff/internal/api/middleware.go#L183-L215

@juliusvonkohout is this info that you need?

[lucferbux]

Hi @juliusvonkohout I'm assuming you are asking something similar as what pipelines has implemented here, we weren't told when we asked for feedback it was an option, if we need it, I'm happy to add it right away.

[juliusvonkohout]

Do we need to update https://github.com/kubeflow/manifests/blob/master/common/networkpolicies/base/model-registry.yaml as well ?

[juliusvonkohout]

do we have tests for the UI ?

[lucferbux]

Hi @juliusvonkohout, yes, we have a stack of tests run before merging both for unit testing and integration, everything runs in an action before every PR, not sure if there's extra test for kubeflow integration.

[ederign]

Also, @juliusvonkohout, as this is still not integrated with Central Dashboard by default, we have not added the integration tests on Kubeflow Central Dashboard yet. (but as @lucferbux said, we test it on the model registry repo)

[juliusvonkohout]

But do we have some basic tests in kubeflow/manifests for the UI before we enable it? See https://github.com/kubeflow/manifests/blob/a21afee95a7129d5374eebe84ef9b0983ec3adb5/.github/workflows/model_registry_test.yaml. We need some integration tests.

[lucferbux]

Can we add the tests in a follow up PR? I can raise a new bug to cover it and add it right away.

[juliusvonkohout]

@lucferbux then just please create a tracking issue for the next release and link it here.

[lucferbux]

I'm sorry @juliusvonkohout I didn't follow up on this, here's #2984 and I currently have a local draft PR that I'm testing, probably I'll push it later today, first thing monday.

[juliusvonkohout]

Hello, you are missing a security context for most of your deployments. Please add

manifests/common/dex/base/deployment.yaml

Lines 25 to 32 in 550e0b3

	securityContext:
	allowPrivilegeEscalation: false
	seccompProfile:
	type: RuntimeDefault
	runAsNonRoot: true
	capabilities:
	drop:
	- ALL

to all of your deployments.

[juliusvonkohout]

https://github.com/kubeflow/manifests/blob/master/apps/model-registry/upstream/base/model-registry-deployment.yaml is also missing the securitycontext

[tarilabs]

hi @juliusvonkohout @tarekabouzeid wrt to :

• Update kubeflow/model-registry manifests from 5d8ed91 #2973 (review)

this was not raised as a blocker since:

• https://github.com/kubeflow/manifests/commits/master/apps/model-registry/upstream

I'd prefer to defer this new requirement on the roadmap for the next release(s).

for this release, what we received in Release team meeting, was to implement K8s matrix check, which we did promptly and as requested with

• build: use also different K8s versions for E2E testing in GHA model-registry#659 (review)

[juliusvonkohout]

hi @juliusvonkohout @tarekabouzeid wrt to :

* [Update kubeflow/model-registry manifests from 5d8ed91 #2973 (review)](https://github.com/kubeflow/manifests/pull/2973#pullrequestreview-2592150562)

this was not raised as a blocker since:

* https://github.com/kubeflow/manifests/commits/master/apps/model-registry/upstream

I'd prefer to defer this new requirement on the roadmap for the next release(s).

for this release, what we received in Release team meeting, was to implement K8s matrix check, which we did promptly and as requested with

* [build: use also different K8s versions for E2E testing in GHA model-registry#659 (review)](https://github.com/kubeflow/model-registry/pull/659#pullrequestreview-2512327021)

Can you then create and link here a tracking issue for the security context in the next model-registry release?

[tarilabs]

The link has been created and for the next roadmap (ie KF) release(s) accordingly.
It is my understanding this is not a blocker like it's not been since #2973 (comment) and neither for other KF components.

[juliusvonkohout]

I assume that kubeflow/model-registry#760 is the issue. For KFP and manifests we already have the securitycontext upstream :-)

I think we should have one for the integration tests as well, see #2973 (comment) and then i just add the two issues here as well #2763 and merge this PR.

[lucferbux]

I assume that kubeflow/model-registry#760 is the issue. For KFP and manifests we already have the securitycontext upstream :-)

I think we should have one for the integration tests as well, see #2973 (comment) and then i just add the two issues here as well #2763 and merge this PR.

Ok, so for the Integration tests, already created #2984 so we are good there, I'll raise the PR asap.

[juliusvonkohout]

I assume that kubeflow/model-registry#760 is the issue. For KFP and manifests we already have the securitycontext upstream :-)

I think we should have one for the integration tests as well, see #2973 (comment) and then i just add the two issues here as well #2763 and merge this PR.

Ok, so for the Integration tests, already created #2984 so we are good there, I'll raise the PR asap.

Yes please the securitycontext and the integration tests in BOTH (also kubeflow/manifests) repositories for the next release. They don't need to be exhaustive, but at least basic API and UI functionality must be tested and then we can review PRs to kubeflow/manifests way faster.

/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

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 [juliusvonkohout]

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

[juliusvonkohout]

@tarilabs it seems that

manifests/README.md

Line 60 in 71ea0ee

| Kubeflow Model Registry | apps/model-registry/upstream | [v0.2.12](https://github.com/kubeflow/model-registry/tree/v0.2.12/manifests/kustomize) |

has not been updated automatically by the script. Can you check that for the next manifest synchronization?

[tarilabs]

@tarilabs it seems that

manifests/README.md

Line 60 in 71ea0ee

| Kubeflow Model Registry | apps/model-registry/upstream | [v0.2.12](https://github.com/kubeflow/model-registry/tree/v0.2.12/manifests/kustomize) |

has not been updated automatically by the script. Can you check that for the next manifest synchronization?

ack, thank you for raising it