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

Disable insecure parameters by default #263

Merged
merged 2 commits into from
Jan 13, 2025
Merged

Disable insecure parameters by default #263

merged 2 commits into from
Jan 13, 2025

Conversation

lpiwowar
Copy link
Collaborator

@lpiwowar lpiwowar commented Dec 9, 2024 •
edited
Loading

This patch sets by default for all test pods:

  • runAsNonRoot: true
  • automountServiceAccountToken: false

These two parameters shouldn't be enabled unless necessary for any
pods in the OCP cluster. The runAsNonRoot parameter ensures that
the container process does not run as root nor does it switch to
root user after it has been spawned.

And the automountServiceAccountToken makes sure that we are not
mounting service account tokens into the test pods. This would
allow them to modify the state of the OCP cluster. As of now the
service account tokens are not required by any of the test pods.

This behavior can be overridden by setting privileged: true.

@openshift-ci openshift-ci bot requested review from kstrenkova and stuggi December 9, 2024 13:21
@openshift-ci openshift-ci bot added the approved label Dec 9, 2024
@lpiwowar lpiwowar force-pushed the bugfix/runAsNonroot-and-automountServiceAccountToken branch from b20e5cb to 8d0607b Compare December 9, 2024 13:24
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/1511969714454c25b9f8679ed3f6acee

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 57m 07s
❌ podified-multinode-edpm-deployment-crc-test-operator FAILURE in 1h 43m 48s

@lpiwowar
Copy link
Collaborator Author

lpiwowar commented Dec 9, 2024

recheck

@lpiwowar lpiwowar force-pushed the bugfix/runAsNonroot-and-automountServiceAccountToken branch from 8d0607b to 038565e Compare December 10, 2024 09:12
@lpiwowar lpiwowar requested a review from kopecmartin December 10, 2024 09:20
@lpiwowar
Copy link
Collaborator Author

/hold

Let's first merge this one -> #256

@lpiwowar
Copy link
Collaborator Author

/test test-operator-build-deploy

3 similar comments
@lpiwowar
Copy link
Collaborator Author

/test test-operator-build-deploy

@lpiwowar
Copy link
Collaborator Author

/test test-operator-build-deploy

@lpiwowar
Copy link
Collaborator Author

/test test-operator-build-deploy

@lpiwowar lpiwowar force-pushed the bugfix/runAsNonroot-and-automountServiceAccountToken branch from 038565e to 66556b3 Compare December 18, 2024 12:42
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/63a494005974489c8906dbd188e0957d

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 47m 05s
❌ podified-multinode-edpm-deployment-crc-test-operator FAILURE in 2h 03m 20s

@lpiwowar lpiwowar force-pushed the bugfix/runAsNonroot-and-automountServiceAccountToken branch from 66556b3 to 1827a7f Compare December 19, 2024 09:04
kstrenkova
kstrenkova approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@kstrenkova kstrenkova left a comment

Choose a reason for hiding this comment

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

Looking through the patch, I don't see any obvious mistakes or typos. But I did not try to run these changes locally.

@openshift-ci openshift-ci bot added the lgtm label Jan 7, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 7, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kstrenkova, lpiwowar

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

@lpiwowar lpiwowar removed the request for review from kopecmartin January 8, 2025 09:40
@lpiwowar
Copy link
Collaborator Author

lpiwowar commented Jan 8, 2025

@kstrenkova thank you for the review! 🎉 Can you please run the test-operator locally with this change and execute some smoke tests with it?:) I'm going to remove the /hold after that.

@kstrenkova
Copy link
Contributor

I have run this pr locally for Tempest and Tobiko. The changes seem to be working, so in my opinion lgtm.

@lpiwowar
Disable insecure parameters by default
bdf97df 
This patch sets by default for all test pods:

  - runAsNonRoot: true
  - automountServiceAccountToken: false

These two parameters shouldn't be enabled unless necessary for any
pods in the OCP cluster. The runAsNonRoot parameter ensures that
the container process does not run as root nor does it switch to
root user after it has been spawned.

And the automountServiceAccountToken makes sure that we are not
mounting service account tokens into the test pods. This would
allow them to modify the state of the OCP cluster. As of now the
service account tokens are not required by any of the test pods.

This behavior can be overridden by setting privileged: true.
@lpiwowar
Add expectedFailuresList to CSV
0a23722 
The epectedFailuresList parameter was added as part of this PR [1] but
unfortunatel it was not added to the CSV. This commit fixes that.

[1] openstack-k8s-operators#233
@lpiwowar lpiwowar force-pushed the bugfix/runAsNonroot-and-automountServiceAccountToken branch from 1827a7f to 0a23722 Compare January 10, 2025 10:03
@openshift-ci openshift-ci bot removed the lgtm label Jan 10, 2025
@kstrenkova
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 10, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit 0da95da into openstack-k8s-operators:main Jan 13, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kstrenkova kstrenkova kstrenkova approved these changes

Assignees

@kstrenkova kstrenkova

Labels
approved lgtm Ready For Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lpiwowar @kstrenkova @openshift-merge-robot