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

test: Add modifications in SELinux disabled mode #201

Merged

Conversation

bachradsusi
Copy link
Member

@bachradsusi bachradsusi commented Oct 4, 2023

When targeted SELinux policy is installed it should be possible to setup SELinux while disabled and before it's changed to permissive/enforcing.

Related to #188

This test is supposed to fail until #194 is merged.

Enhancement:

Reason:

Result:

Issue Tracker Tickets (Jira or BZ if any):
#188

selinux_logins_purge: true

tasks:
- name: Ensure SELinux tool semanage
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use

    - name: Ensure SELinux tool semanage
      include_tasks: set_selinux_variables.yml

to ensure semanage is present?

Copy link
Member Author

Choose a reason for hiding this comment

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

set_selinux_variables.yml runs 4 time semanage command which can take 2 seconds on my vm and information collected by this task is not necessary for the test. As the test is now it doesn't even need semanage so it could be dropped completely.

Copy link
Contributor

@richm richm Oct 4, 2023

Choose a reason for hiding this comment

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

set_selinux_variables.yml runs 4 time semanage command which can take 2 seconds on my vm

ok - I guess we need a separate task file (e.g. tests/ensure_semanage.yml) which just installs semanage, and another task file to populate those test variables (or just move the semanage stuff out of set_selinux_variables.yml). We can do that in a subsequent PR. I don't like having multiple copy/paste of the code to install semanage - it makes things like ANSIBLE_GATHERING=explicit, rpm-ostree support, snapshot creation, new test development, etc. harder.

and information collected by this task is not necessary for the test. As the test is now it doesn't even need semanage so it could be dropped completely.

ack

@bachradsusi
Copy link
Member Author

I guess I should also test whether modifications were applied.

@bachradsusi bachradsusi force-pushed the test-modifications-in-disabled branch from 867123f to 7bb42de Compare October 5, 2023 15:33
@bachradsusi
Copy link
Member Author

[citest]

@bachradsusi
Copy link
Member Author

[citest]

changed_when: true
when: selinux_mountpoint.stdout != ""

- name: Execute the role and catch errors
Copy link
Contributor

Choose a reason for hiding this comment

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

The test isn't catching errors, it is "Execute the role and cleanup" - not sure if you are expecting any errors, but the test is failing:

TASK [linux-system-roles.selinux : Set SELinux file contexts] ******************
task path: /WORKDIR/git-test-modifications-in-disabled37f0mfqq/tests/roles/linux-system-roles.selinux/tasks/main.yml:74
Friday 06 October 2023  21:18:41 +0000 (0:00:00.013)       0:00:12.398 ******** 
failed: [sut] (item={'target': '/tmp/test_dir(/.*)?', 'setype': 'user_home_dir_t', 'ftype': 'd'}) => {
    "ansible_loop_var": "item",
    "changed": false,
    "item": {
        "ftype": "d",
        "setype": "user_home_dir_t",
        "target": "/tmp/test_dir(/.*)?"
    }
}

MSG:

SELinux is disabled on this host.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's expected because #194 is not merged yet. The test demonstrates the current problem and should start passing when #194 is merged

Copy link
Member Author

Choose a reason for hiding this comment

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

- name: Execute the role and catch errors should be just - name: Execute the role

@bachradsusi bachradsusi force-pushed the test-modifications-in-disabled branch from 6e007b7 to d7d9b6b Compare October 10, 2023 08:32
@bachradsusi
Copy link
Member Author

#194 is approved.

I could add this test to #194 or wait until it's merged and then [citest] again.

@richm
Copy link
Contributor

richm commented Oct 10, 2023

#194 is approved.

I could add this test to #194 or wait until it's merged and then [citest] again.

Yes. I just merged that one - please rebase this one and we can retest

When targeted SELinux policy is installed it should be possible to setup
SELinux while disabled and before it's changed to permissive/enforcing.

Related to linux-system-roles#188

Signed-off-by: Petr Lautrbach <[email protected]>
@bachradsusi bachradsusi force-pushed the test-modifications-in-disabled branch from d7d9b6b to 887d1b2 Compare October 10, 2023 14:29
@bachradsusi
Copy link
Member Author

[citest]

@spetrosi
Copy link
Contributor

[citest bad]

@richm
Copy link
Contributor

richm commented Oct 11, 2023

I wouldn't worry about the rhel 6 issues, and the rhel 7 issue with the all transitions test and reboot seems to be transient - not easy to reproduce - might be an artifact of the CI system, unless you can consistently reproduce it with qemu locally.

@richm
Copy link
Contributor

richm commented Oct 12, 2023

Is this PR ready to merge? It conflicts with #206, and it would be easier for me to rebase on top of this one, than for you to rebase on top of 206

@bachradsusi
Copy link
Member Author

It's ready to be merged. I don't plan any changes in this PR. Thanks!

@richm richm merged commit 55edea4 into linux-system-roles:main Oct 13, 2023
16 checks passed
@bachradsusi bachradsusi deleted the test-modifications-in-disabled branch October 13, 2023 15:17
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.

3 participants