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

tests: add test for watching files labeled kubernetes_file_t #1064

Merged
merged 1 commit into from
Jul 17, 2021

Conversation

miabbott
Copy link
Member

@miabbott miabbott commented Jun 18, 2021

Adds a test for the ability of systemd to watch files
labeled with kubernetes_file_t.

See: coreos/fedora-coreos-tracker#861
See: containers/container-selinux#135

@miabbott
Copy link
Member Author

This might be overkill but I was thinking we might want to expand our SELinux coverage in the future with tests like this.

Maybe even moving the existing SELinux tests from kola/mantle into the ext.config format

@dustymabe
Copy link
Member

Seems reasonable to me.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

I think having a test for this here is totally fine, but personally think the cost of starting a whole VM for such a simple check is high. This is why I've been pushing for https://github.com/coreos/fedora-coreos-config/tree/testing-devel/tests/kola/misc-ign-ro -- WDYT about folding that in there?

@miabbott
Copy link
Member Author

I think having a test for this here is totally fine, but personally think the cost of starting a whole VM for such a simple check is high. This is why I've been pushing for https://github.com/coreos/fedora-coreos-config/tree/testing-devel/tests/kola/misc-ign-ro -- WDYT about folding that in there?

Yup, I was concerned about the cost of a VM for a simple check. I can fold this into that location.

@dustymabe
Copy link
Member

I think having a test for this here is totally fine, but personally think the cost of starting a whole VM for such a simple check is high. This is why I've been pushing for https://github.com/coreos/fedora-coreos-config/tree/testing-devel/tests/kola/misc-ign-ro -- WDYT about folding that in there?

This triggered a memory of mine and I found the old discussion and started a new feature request to bring the best of both worlds together: https://github.com/coreos/fedora-coreos-config/issues/1067

@miabbott miabbott changed the title tests: add a test for systemd reading kubernetes_file_t tests: add tests for systemd + kubernetes_file_t labels Jun 21, 2021
@miabbott
Copy link
Member Author

Moved the original test for reading kubernetes_file_t and pulled in another test for the issue in coreos/fedora-coreos-tracker#861

@cgwalters
Copy link
Member

Looks sane to me but the test is failing.

@miabbott
Copy link
Member Author

Looks sane to me but the test is failing.

Yeah, these will both fail until we have a new container-selinux with an updated policy.

@miabbott
Copy link
Member Author

miabbott commented Jul 15, 2021

The kube-env part of the test is still failing with an SELinux denial.

@dustymabe if you want, I can split that test out into another PR if you would like to get the "watch" part of the test merged in.

@dustymabe
Copy link
Member

ahh - sorry for the noise. don't worry about splitting it out (unless you want to 🙂)

@miabbott
Copy link
Member Author

ahh - sorry for the noise. don't worry about splitting it out (unless you want to slightly_smiling_face)

split out the kube-env test...this passes with latest from testing-devel now

@miabbott miabbott changed the title tests: add tests for systemd + kubernetes_file_t labels tests: add test for watching files labeled kubernetes_file_t Jul 15, 2021
@miabbott miabbott marked this pull request as ready for review July 15, 2021 20:53
fi
ok "successfully created /etc/kubernetes/kubeconfig"

if [ "$(systemctl is-active kube-watch.service)" != "active" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

what's the point of running systemctl is-active kube-watch.service twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

The first systemctl is-active is checking kube-watch.path

Adds a test for the ability of `systemd` to watch files
labeled with `kubernetes_file_t`.

See: coreos/fedora-coreos-tracker#861
See: containers/container-selinux#135

Co-authored-by: Dusty Mabe <dusty@dustymabe.com>
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@dustymabe dustymabe merged commit 589866f into coreos:testing-devel Jul 17, 2021
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