coreos / butane

Butane translates human-readable Butane Configs into machine-readable Ignition Configs.
https://coreos.github.io/butane/
Apache License 2.0
255 stars 70 forks source link

Import SElinux module #477 #486

Open yasminvalim opened 1 year ago

yasminvalim commented 1 year ago

Import SElinux module

We want to add sugar to butane which will allow users import costum SElinux module.


Related Issues

477

travier commented 1 year ago

Just a small note for your name in the Git config:

From: YASMIN VALIM <ydesouza@redhat.com>

You probably want Yasmin Valim <ydesouza@redhat.com> instead.

You'll have to modify it in you ~/.gitconfig and then "reset" the author for all your commits here with git rebase or git commit --amend.

yasminvalim commented 1 year ago

I'm now trying to test manually, and while reading the documentation, I had a question: How can I be sure that my code is running locally?

I understand that I can build it with custom files that I created and it will be translated from BU to Ignition. But I'm not sure how can I run the code I made instead of the butane version I installed and the latest image.

I think I am missing something.

prestist commented 1 year ago

I'm now trying to test manually, and while reading the documentation, I had a question: How can I be sure that my code is running locally?

I understand that I can build it with custom files that I created and it will be translated from BU to Ignition. But I'm not sure how can I run the code I made instead of the butane version I installed and the latest image.

Noooo worries there are a lot of layers here.

So, to test it locally we have to exercise your code which lives in butane, then we want to test that our expectation of what is required for the system to effectively implement the sugar is correct which is done by exercising existing code in ignition through first booting a fcos image.

The simplest step is directly exercising your code by creating a butane config which references the sugar you added, and using your locally built butane to create an .ign file.

Now to exercise your assumptions for enabling a semodule you would then need to first boot a fcos vm with your ignition config. The simpliest way to do this is by using cosa

  1. You can either download a qcow image from here or build your own using cosa int, fetch, build

  2. Then you can pass your ign file you built earlier using a command like so $ cosa kola qemuexec --ignition ./ignition.json --ignition-direct --qemu-image ./yourImage.qcow2

  3. If the ign config is understood and functional, you should be greeted by the console (with auto login) then you can do your probing of the os to see if the selinux module is enabled (as your sugar indicates)

travier commented 1 year ago

You can find an small example SELinux module (CIL) in https://github.com/coreos/fedora-coreos-tracker/issues/1447#issuecomment-1528146989

yasminvalim commented 1 year ago

Hey there,

I'm sharing my current approach for tackling this issue. I did have a few other ideas, but this one seems to be the most on point. I'm planning to commit the code I've been working on in the next few days, even though it might not be 100% ready, and it could potentially break my CI setup.

Here's the plan I've got in mind. If you've got any cool ideas or questions that could help me sharpen things up, please don't hold back.

This current approach would look like that in a butane file:

variant: fcos
version: 1.6.0-experimental
selinux_module:
  - contents:
      - |
        # setenforce 1
        # cat permissive-keyutils.cil
        (type keyutils_request_t)
        (typepermissive keyutils_request_t)
        # semodule -i permissive-keyutils.cil
  - path: /path/to/selinux/custom/file

Your input would be awesome and thanks for your attention.

travier commented 1 year ago

The example Butane config should likely only include the content of the SELinux module:

variant: fcos
version: 1.6.0-experimental
selinux:
  module:
    - name: "permissive-keyutils"
    - contents: |
        (type keyutils_request_t)
        (typepermissive keyutils_request_t)

Then Butane would generate an Ignition config that:

prestist commented 1 year ago

Your approach is sound. It might make sense to create tests before making the functions. In this case in particular could be well suited for TDD, and just create a test like TestTranslateBootDevice which would fail until you create the translate code to build the sugar.

Since you have a sound understanding of the input and the expected output. And as long as your spec changes are there you should be able to run the test with no compile errors.

yasminvalim commented 1 year ago

Hey folks, I am still having problem with unit tests. Currently, I am having this error and those are the logs. This error applies to each line in translations.

--- FAIL: TestTranslateSelinux (0.00s)
    --- FAIL: TestTranslateSelinux/translate_0 (0.00s)
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:45: 
                Error Trace:    /home/ydesouza/butane/base/util/test.go:45
                                            /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1717
                Error:          Not equal: 
                                expected: translate.Translation{From:path.ContextPath{Path:[]interface {}{"selinux", "module", 0}, Tag:"yaml"}, To:path.ContextPath{Path:[]interface {}{"storage"}, Tag:"json"}}
                                actual  : translate.Translation{From:path.ContextPath{Path:[]interface {}{"selinux", "module"}, Tag:"yaml"}, To:path.ContextPath{Path:[]interface {}{"storage"}, Tag:"json"}}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -2,6 +2,5 @@
                                  From: (path.ContextPath) {
                                -  Path: ([]interface {}) (len=3) {
                                +  Path: ([]interface {}) (len=2) {
                                    (string) (len=7) "selinux",
                                -   (string) (len=6) "module",
                                -   (int) 0
                                +   (string) (len=6) "module"
                                   },
                Test:           TestTranslateSelinux/translate_0
                Messages:       non-identity translation with unexpected From

What I am missing here? Thanks! :smile_cat:

yasminvalim commented 12 months ago

After spending time debugging, I'm still encountering similar errors to those I had before, despite making various changes. If anyone has any ideas about what might be causing this, I would appreciate. Thanks!

--- FAIL: TestTranslateSelinux (0.00s)
    --- FAIL: TestTranslateSelinux/translate_0 (0.00s)
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module → $.storage
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.systemd.units.0.dropins
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.systemd.units.0.dropins.0
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.path
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append.0
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append.0.source
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:55: 
                Error Trace:    /home/ydesouza/butane/base/util/test.go:55
                                            /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1726
                Error:          Not equal: 
                                expected: []interface {}{"selinux", "module", 0}
                                actual  : []interface {}{"systemd", "units", 0, "contents"}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,5 +1,6 @@
                                -([]interface {}) (len=3) {
                                - (string) (len=7) "selinux",
                                - (string) (len=6) "module",
                                - (int) 0
                                +([]interface {}) (len=4) {
                                + (string) (len=7) "systemd",
                                + (string) (len=5) "units",
                                + (int) 0,
                                + (string) (len=8) "contents"
                                 }
                Test:           TestTranslateSelinux/translate_0
                Messages:       translation is not identity
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:55: 
                Error Trace:    /home/ydesouza/butane/base/util/test.go:55
                                            /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1726
                Error:          Not equal: 
                                expected: []interface {}{"selinux", "module", 0}
                                actual  : []interface {}{"systemd", "units", 0, "enabled"}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,5 +1,6 @@
                                -([]interface {}) (len=3) {
                                - (string) (len=7) "selinux",
                                - (string) (len=6) "module",
                                - (int) 0
                                +([]interface {}) (len=4) {
                                + (string) (len=7) "systemd",
                                + (string) (len=5) "units",
                                + (int) 0,
                                + (string) (len=7) "enabled"
                                 }
                Test:           TestTranslateSelinux/translate_0
                Messages:       translation is not identity
        /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1727: 
                Error Trace:    /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1727
                Error:          Received unexpected error:
                                Missing paths in TranslationSet:
                                $.storage.files.0.path
                                $.storage.files.0.append.0.compression
                                $.storage.files.0.append.0.source
                                $.storage.files.0.append.0
                                $.storage.files.0.append
                                $.storage.files.0
                                $.storage.files
                                $.storage
                                $.systemd.units
                                $.systemd
                Test:           TestTranslateSelinux/translate_0
                Messages:       incomplete TranslationSet coverage
FAIL
FAIL    github.com/coreos/butane/config/fcos/v1_6_exp   0.003s
yasminvalim commented 11 months ago

I've made few changes based on the review and debugged the code. However, I'm still uncertain about what's causing the issue in either the code or the test itself. Below is the current error.

Running tool: /usr/bin/go test -timeout 30s -run ^TestTranslateSelinux$ github.com/coreos/butane/config/fcos/v1_6_exp

--- FAIL: TestTranslateSelinux (0.00s)
    --- FAIL: TestTranslateSelinux/translate_0 (0.00s)
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module → $.storage
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.path
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append.0
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append.0.source
        /home/ydesouza/butane/config/fcos/v1_6_exp/test.go:47: missing non-identity translation $.selinux.module.0 → $.storage.files.0.append.0.compression
        /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1737: 
                Error Trace:    /home/ydesouza/butane/config/fcos/v1_6_exp/translate_test.go:1737
                Error:          Received unexpected error:
                                Missing paths in TranslationSet:
                                $.storage.files.0.path
                                $.storage.files.0.append.0.compression
                                $.storage.files.0.append.0.source
                                $.storage.files.0.append.0
                                $.storage.files.0.append
                                $.storage.files.0
                                $.storage.files
                                $.storage
                                $.systemd.units
                                $.systemd
                Test:           TestTranslateSelinux/translate_0
                Messages:       incomplete TranslationSet coverage
FAIL
FAIL    github.com/coreos/butane/config/fcos/v1_6_exp   0.003s
FAIL

Please share any insights if you have them. :smile_cat:

prestist commented 10 months ago

@yasminvalim the go 1.21 test should be fixed by https://github.com/coreos/butane/pull/505.

steps to update branch assuming origin is your fork and upstream is ``` git fetch upstream git merge upstream/main git push -f ```
yasminvalim commented 10 months ago

@yasminvalim the go 1.21 test should be fixed by #505.

steps to update branch

Thanks, I updated my branch and I will squash to have just one commit. :smile_cat:

prestist commented 10 months ago

@yasminvalim the go 1.21 test should be fixed by #505. steps to update branch assuming origin is your fork and upstream is

Sorry I led you astray, the merge commit is nooooo good, the best thing to do is likely to undo your merge and just rebase

git revert -m 1 a419de4df7286bb307f87df55763db153fa418ef git fetch upstream git rebase upstream/main

that way your fast-forwarded rather then merged into.

yasminvalim commented 10 months ago

@yasminvalim the go 1.21 test should be fixed by #505. steps to update branch assuming origin is your fork and upstream is

Sorry I led you astray, the merge commit is nooooo good, the best thing to do is likely to undo your merge and just rebase

git revert -m 1 a419de4 git fetch upstream git rebase upstream/main

that way your fast-forwarded rather then merged into.

I think it's fine now, but not entirely sure because it's showing all the files updated in the old merge. Can you take a look, please?

Also, can I use rebase as a pattern when I need to update a branch?

prestist commented 10 months ago

@yasminvalim I am a bit confused because this https://github.com/coreos/butane/pull/486/commits/36f71f1e0d0660796212b1ab0dc50b1b91e3379c should not be a thing. after reverting your merge it should have put you in a place with only your other commit.

Once you revert the merge you can update your branch with the rebase pattern instead.