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.

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

Ignore errors if label.Relabel returns ENOSUP #5197

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Nov 29, 2023

This is a common mistake by users and is ignored in some places but not everywhere. This change will help this to be ignored everwhere.

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

SELinux relabeling via :Z and :z on file systems which do not support labels like NFS are ignored.

[NO NEW TESTS NEEDED]

Copy link
Contributor

openshift-ci bot commented Nov 29, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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

@rhatdan
Copy link
Member Author

rhatdan commented Nov 29, 2023

@n1hility PTAL

@n1hility
Copy link
Member

Awesome! LGTM other than the conflict

This is a common mistake by users and is ignored in some places
but not everywhere. This change will help this to be ignored everwhere.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@nalind
Copy link
Member

nalind commented Nov 29, 2023

Most of the places where this PR suppresses that error are for files we're creating, where the user doesn't get to tell us whether or not to change the labels on them. The exceptions are the ones in Builder.runSetupVolumeMounts(), and I'm not sure we could hit this problem for the rest.

We're pretty inconsistent about where we put files we bind-mount in. An environment secret gets stored under define.TempDir, which looks like an unexplained inconsistency since a file secret is stored under the per-container directory, and SSH sockets and /run/.containerenv are placed under internal/tmpdir.GetTempDir().

@rhatdan
Copy link
Member Author

rhatdan commented Nov 30, 2023

I just wanted to be consistent. No reason to not call the same code everwhere. For most of these there will be no change as you point out. but if we spray around label.Relabel and relabel code, there is a decent chance a future contributor could do it wrong.

@TomSweeneyRedHat
Copy link
Member

I'm fine with the changes, but will let you and @nalind thumb-wrestle out the details.

@nalind
Copy link
Member

nalind commented Dec 13, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 13, 2023
@rhatdan rhatdan merged commit e089136 into containers:main Dec 13, 2023
33 of 36 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants