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

genpolicy: mount source for non-confidential guest #9029

Merged
merged 1 commit into from Feb 7, 2024

Conversation

danmihai1
Copy link
Contributor

The emergent Kata CI tests for Policy use confidential_guest = false in genpolicy-settings.json. That value is inconsistent with the following mount settings:

    "emptyDir": {
        "mount_type": "local",
        "mount_source": "^$(cpath)/$(sandbox-id)/local/",
        "mount_point": "^$(cpath)/$(sandbox-id)/local/",
        "driver": "local",
        "source": "local",
        "fstype": "local",
        "options": [
            "mode=0777"
        ]
    },

We need to keep those settings for confidential_guest = true, and change confidential_guest = false to use:

    "emptyDir": {
        "mount_type": "local",
        "mount_source": "^$(cpath)/$(sandbox-id)/rootfs/local/",
        "mount_point": "^$(cpath)/$(sandbox-id)/local/",
        "driver": "local",
        "source": "local",
        "fstype": "local",
        "options": [
            "mode=0777"
        ]
    },

The value of the mount_source field is different.

This change unblocks testing using Kata CI's pod-empty-dir.yaml:

genpolicy -u -y pod-empty-dir.yaml

kubectl apply -f pod-empty-dir.yaml

k get pod sharevol-kata
NAME READY STATUS RESTARTS AGE
sharevol-kata 1/1 Running 0 53s

Fixes: #8887

The emergent Kata CI tests for Policy use confidential_guest = false
in genpolicy-settings.json. That value is inconsistent with the
following mount settings:

        "emptyDir": {
            "mount_type": "local",
            "mount_source": "^$(cpath)/$(sandbox-id)/local/",
            "mount_point": "^$(cpath)/$(sandbox-id)/local/",
            "driver": "local",
            "source": "local",
            "fstype": "local",
            "options": [
                "mode=0777"
            ]
        },

We need to keep those settings for confidential_guest = true, and
change confidential_guest = false to use:

        "emptyDir": {
            "mount_type": "local",
            "mount_source": "^$(cpath)/$(sandbox-id)/rootfs/local/",
            "mount_point": "^$(cpath)/$(sandbox-id)/local/",
            "driver": "local",
            "source": "local",
            "fstype": "local",
            "options": [
                "mode=0777"
            ]
        },

The value of the mount_source field is different.

This change unblocks testing using Kata CI's pod-empty-dir.yaml:

genpolicy -u -y pod-empty-dir.yaml

kubectl apply -f pod-empty-dir.yaml

k get pod sharevol-kata
NAME            READY   STATUS    RESTARTS   AGE
sharevol-kata   1/1     Running   0          53s

Fixes: kata-containers#8887

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
@wainersm
Copy link
Contributor

wainersm commented Feb 6, 2024

Hi @danmihai1 !

I didn't follow the initial CI implementation for testing the agent policy so I'm trying to understand it now.

If I got it right, the yamls for the k8s tests are transformed (i.e. added annotations) for certain $KATA_HYPERVISOR (i.e. qemu-sev, qemu-tdx, qemu-snp) or $KATA_HOST_OS is cb-lmariner (see code).

The following jobs met those conditions:

The aforementioned "yamls transformation" don't use the genpolicy at the moment. Instead the default kata's repo policies are applied "manually". So here is what you doing: improving genpolicy and configuration files so that it could be used instead?

@danmihai1
Copy link
Contributor Author

The aforementioned "yamls transformation" don't use the genpolicy at the moment. Instead the default kata's repo policies are applied "manually". So here is what you doing: improving genpolicy and configuration files so that it could be used instead?

Yes:

  1. The older way of testing Policy (

    add_policy_to_yaml "${K8S_TEST_YAML}"
    ) uses a very basic, hand-crafted policy - insufficient for real world use cases.

  2. A proper CoCo Policy would be significantly more complex - therefore typical customers will need to use an automatically generated Policy, such as a Policy generated by the genpolicy tool. So, the next goal is to test using genpolicy, as shown for one test in tests: k8s-attach-handlers auto-generated policy #8922

@wainersm
Copy link
Contributor

wainersm commented Feb 6, 2024

The aforementioned "yamls transformation" don't use the genpolicy at the moment. Instead the default kata's repo policies are applied "manually". So here is what you doing: improving genpolicy and configuration files so that it could be used instead?

Yes:

1. The older way of testing Policy ( https://github.com/kata-containers/kata-containers/blob/f1ca5d1563be3e704acd10ccf2c44a975f26bf8b/tests/integration/kubernetes/run_kubernetes_tests.sh#L133
    ) uses a very basic, hand-crafted policy - insufficient for real world use cases.

2. A proper CoCo Policy would be significantly more complex - therefore typical customers will need to use an automatically generated Policy, such as a Policy generated by the _genpolicy_ tool. So, the next goal is to test using genpolicy, as shown for one test in [tests: k8s-attach-handlers auto-generated policy #8922](https://github.com/kata-containers/kata-containers/pull/8922)

Perfect. Got it!

@danmihai1
Copy link
Contributor Author

/test

@wainersm
Copy link
Contributor

wainersm commented Feb 6, 2024

Ok, I think I understood how the confidential_guest setting means. If set to true then genpolicy will pick up the confidential rules confidential_emptyDir, confidential_configMap, etc...

I was thinking on how to remove the conditionals if confidential then do this from the code (if that´s desirable):

  • having two settings files (e.g. genpolicy-settings.json and coco-genpolicy-settings.json), user specify which he/she wants to use. Problem: changes on common rules should be applied on both files (i.e. maintenance gets worse)
  • still haveing two settings but the specialized one (i.e. coco-genpolicy-settings.json) points to the base_settings_file, overwriting existing rules from the base and/or adding new rules. User still need to switch the settings file with the -j option.

Copy link
Contributor

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

Based on my limited knowledge of genpolicy: LGTM!

@danmihai1
Copy link
Contributor Author

Ok, I think I understood how the confidential_guest setting means. If set to true then genpolicy will pick up the confidential rules confidential_emptyDir, confidential_configMap, etc...

I was thinking on how to remove the conditionals if confidential then do this from the code (if that´s desirable):

  • having two settings files (e.g. genpolicy-settings.json and coco-genpolicy-settings.json), user specify which he/she wants to use. Problem: changes on common rules should be applied on both files (i.e. maintenance gets worse)
  • still haveing two settings but the specialized one (i.e. coco-genpolicy-settings.json) points to the base_settings_file, overwriting existing rules from the base and/or adding new rules. User still need to switch the settings file with the -j option.

That is correct. Sorry these settings are confusing - I agree we need to clean them up.

@danmihai1 danmihai1 merged commit 0174568 into kata-containers:main Feb 7, 2024
540 of 560 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/small Small and simple task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: genpolicy: k8s-empty-dirs.bats doesn't work with automatically-generated Policy
4 participants