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

Document change to K3s SELinux option #2686

Merged
merged 5 commits into from
Sep 16, 2020

Conversation

catherineluse
Copy link
Contributor

This PR addresses this issue k3s-io/k3s#2247 and it's related to this issue k3s-io/k3s#2058

@davidnuzik
Copy link
Contributor

@ShylajaDevadiga can you review this for k3s v1.19.1 release? I think we may need to call out k3s-io/k3s#2058 (comment) do you have information on this and if so could briefly post what this needs in docs?

```
{{%/tab%}}
{{% tab "K3s prior to v1.19.1+k3s1" %}}

You can turn off SELinux enforcement in the embedded containerd by launching K3s with the `--disable-selinux` flag.

Choose a reason for hiding this comment

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

We do not have --disable-selinux flag instead use --selinux=false or skip selinux flag which does not enable selinux

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tab is saying that the --disable-selinux flag was used in older versions than v1.19.x. This tab won't show by default. is that OK?

Choose a reason for hiding this comment

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

Sounds good @catherineluse Thanks

{{% tabs %}}
{{% tab "K3s v1.19.1+k3s1" %}}

To leverage experimental SELinux, specify the `--selinux` flag when starting K3s servers and agents.

Choose a reason for hiding this comment

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

Can we explicitly say, not passing the flag would have selinux in disabled mode

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. In my opinion this is probably fine as it is.

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'll leave this sentence as is because @dweomer clarified it in his suggestion as well

```
{{%/tab%}}
{{% tab "K3s prior to v1.19.1+k3s1" %}}

Copy link
Contributor

Choose a reason for hiding this comment

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

As suggested, https://github.com/rancher/docs/pull/2686/files#r489578963, mentioning the --selinux flag without mentioning the conflict with --disable-selinux will likely lead to confusion and error. Additionally we should note for users that --disable-selinux is deprecated and will be either ignored or simply unrecognized, resulting in an error, in future minor releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the info to the K3s v1.19+ tab:

The --disable-selinux option should not be used. It is deprecated and will be either ignored or will be unrecognized, resulting in an error, in future minor releases.

@catherineluse
Copy link
Contributor Author

This seems somewhat related. A community member made this change to the same section: #2538 Should that change be included here?

@catherineluse
Copy link
Contributor Author

I also updated it to say a custom --data-dir shouldn't be used.

Using a custom --data-dir under SELinux is not supported. To customize it, you would most likely need to write your own custom policy. For guidance, you could refer to the containers/container-selinux repository, which contains the SELinux policy files for Container Runtimes, and the rancher/k3s-selinux repository, which contains the SELinux policy for K3s .

@ShylajaDevadiga
Copy link

LGTM

Copy link
Contributor

@dweomer dweomer left a comment

Choose a reason for hiding this comment

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

@catherineluse catherineluse merged commit 55d29ed into rancher:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants