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.

Sign up for GitHub

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

Jump to bottom

feat(manifests): add securityContext to deployments #768

Merged
merged 2 commits into from
Feb 12, 2025
Merged

feat(manifests): add securityContext to deployments #768

merged 2 commits into from
Feb 12, 2025

Conversation

pboyd
Copy link
Contributor

@pboyd pboyd commented Feb 6, 2025

Description

Set seccompProfile, forbid containers to run as root, and disable
unnecessary system calls. This applies to:

  • Model registry itself
  • Example database (MySQL and PostgreSQL)
  • Model registry UI

Fixes #760

How Has This Been Tested?

Applying the manifests in a local cluster.

Merge criteria:

  • All the commits have been signed-off (To pass the DCO check)
  • The commits have meaningful messages; the author will squash them after approval or in case of manual merges will ask to merge with squash.
  • 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.
  • Code changes follow the kubeflow contribution guidelines.

If you have UI changes

  • The developer has added tests or explained why testing cannot be added.
  • Included any necessary screenshots or gifs if it was a UI change.
  • Verify that UI/UX changes conform the UX guidelines for Kubeflow.

@google-oss-prow google-oss-prow bot requested review from tarilabs and Tomcli February 6, 2025 16:52
lucferbux
lucferbux suggested changes Feb 7, 2025
View reviewed changes
manifests/kustomize/options/ui/overlays/standalone/kustomization.yaml Outdated
@@ -1,5 +1,6 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: kubeflow
Copy link
Contributor

Choose a reason for hiding this comment

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

Why adding namespace here? that would create an opinionated installation right? are we ok with having kubeflow as the default upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this line. It might be worth discussing using kubeflow as a default though. These manifests can't be applied without adding a namespace, and other manifests in this repo include already have namespace: kubeflow.

@pboyd
feat(manifests): add securityContext to deployments
83cee2e 
Set `seccompProfile`, forbid containers to run as root, and disable
unnecessary system calls. This applies to:

- Model registry itself
- Example database (MySQL and PostgreSQL)
- Model registry UI

Signed-off-by: Paul Boyd <pboyd@redhat.com>
dhirajsb
dhirajsb suggested changes Feb 7, 2025
View reviewed changes
manifests/kustomize/overlays/postgres/kustomization.yaml Outdated
@@ -39,7 +39,7 @@ vars:
- name: POSTGRES_PORT
objref:
kind: ConfigMap
name: model-registry-db-parameters
name: metadata-postgres-db-parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: metadata-postgres-db-parameters
name: metadata-registry-db-parameters

Let's keep it agnostic of the DB type if we can since we can't use multiple DBs and types at the same time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed another commit to rename the configmap and secret.

@pboyd
chore(manifests): rename metadata-postgres configmap and secret
386454c 
- Rename `metadata-postgres-db-parameters` to `metadata-registry-db-parameters`
- Rename `metadata-postgres-db-secrets` to `metadata-registry-db-secrets`

Signed-off-by: Paul Boyd <pboyd@redhat.com>
pboyd added a commit to opendatahub-io/model-registry-operator that referenced this pull request Feb 11, 2025
@pboyd
feat: add securityContext to manifests
085909f 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
@pboyd pboyd mentioned this pull request Feb 11, 2025
Closed
3 tasks
pboyd added a commit to opendatahub-io/model-registry-operator that referenced this pull request Feb 11, 2025
@pboyd
feat: add securityContext to manifests
d7a36be 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
@pboyd pboyd requested review from lucferbux and dhirajsb February 11, 2025 21:25
Al-Pragliola
Al-Pragliola approved these changes Feb 12, 2025
View reviewed changes
Copy link
Contributor

@Al-Pragliola Al-Pragliola left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Feb 12, 2025
tarilabs
tarilabs approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@tarilabs tarilabs left a comment

Choose a reason for hiding this comment

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

thank you @pboyd and all

/approve

@google-oss-prow Google OSS Prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Al-Pragliola, tarilabs

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 9198b2d into kubeflow:main Feb 12, 2025
16 checks passed
@pboyd pboyd deleted the issue-760 branch February 12, 2025 14:05
pboyd added a commit to opendatahub-io/model-registry-operator that referenced this pull request Feb 12, 2025
@pboyd
feat: add securityContext to manifests
21c986d 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
pboyd added a commit to opendatahub-io/model-registry-operator that referenced this pull request Feb 12, 2025
@pboyd
feat: add securityContext to manifests
15ccc12 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
pboyd added a commit to pboyd/model-registry-operator that referenced this pull request Feb 13, 2025
@pboyd
feat: add securityContext to manifests
15395aa 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
@pboyd pboyd mentioned this pull request Feb 13, 2025
Open
3 tasks
@tarilabs tarilabs mentioned this pull request Feb 14, 2025
Merged
pboyd added a commit to pboyd/model-registry-operator that referenced this pull request Feb 25, 2025
@pboyd
feat: add securityContext to manifests
bc25ea3 
Apply the `securityContext` settings from upstream PR
[#768](kubeflow/model-registry#768)

Signed-off-by: Paul Boyd <pboyd@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarilabs tarilabs tarilabs approved these changes

@Al-Pragliola Al-Pragliola Al-Pragliola approved these changes

@Tomcli Tomcli Awaiting requested review from Tomcli

@lucferbux lucferbux Awaiting requested review from lucferbux

@dhirajsb dhirajsb Awaiting requested review from dhirajsb

Assignees

@Al-Pragliola Al-Pragliola

Labels
approved Area/Manifests lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add security context to model-registry manifests
5 participants
@pboyd @dhirajsb @tarilabs @lucferbux @Al-Pragliola