Skip to content

genpolicy: Refactor tests to add different request types in testcases json #335

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 502 commits into
base: msft-main
Choose a base branch
from
Open

genpolicy: Refactor tests to add different request types in testcases json #335

wants to merge 502 commits into from

Conversation

@Sumynwa
Copy link

@Sumynwa Sumynwa commented Mar 20, 2025 •
edited by Redent0r
Loading

Merge Checklist
Summary

This commit introduces changes to add different request types in a single testcases.json file. This allows for testing request types, ex: ExecProcessRequest which depends on state from evaluation of CreateContainerRequest.

The changes include:

  • refactor tests/main.rs to allow different types in testcases.json
  • modify existing test cases data
  • add test for ExecProcessRequest
Test Methodology

Local tests using cargo test

manuelh-dev and others added 30 commits July 12, 2024 16:48
@manuelh-dev
Merge pull request #208 from microsoft/mahuber/incomplete_init
cc23bdb 
samples: introduce incomplete_init category
@manuelh-dev
Merge pull request #206 from microsoft/mahuber/cherry-pick-upstr
e7cb0d1 
Cherry-pick upstream PR kata-containers#9825: osbuilder: allow rootfs builds w/o git or version file deps
tools: Improve igvm-builder and node-builder/azure-linux scripting
2f672a1 
- Support for Mariner 3 builds using OS_VERSION variable
- Improvements to IGVM build process and flow as described in README
- Adoption of using only cloud-hypervisor-cvm on CBL-Mariner

Signed-off-by: Manuel Huber <[email protected]>
@manuelh-dev
Merge pull request #204 from microsoft/mahuber/uvm-spec-build-pr
dd57910 
tools: Improve igvm-builder and node-builder/azure-linux scripting
@miz060
tardev: update tardev-snapshotter.service
e6e6d34 
At the moment, we have circular dependencies between
tardev-snapshotter.service and containerd.service. Specifically,
containerd.service needs tardev-snapshotter.service to run any CC pods,
while tardev-snapshotter.service needs containerd.service to download
image layers. This dependency will be eliminated once we switch to
using remote-snapshotter. Currently, tardev-snapshotter.service's
binding to containerd.service gets delayed, and we won't be able to
run any CC pods until the boot process is completed. It doesn't matter
which service starts first. Based on the current logic, it makes more
sense to use WantedBy=kubelet.service in tardev-snapshotter.service, as
we won't be able to start any CC pods without kubelet. In the future,
once tardev-snapshotter becomes a remote snapshotter again, it will
make more sense to use WantedBy=containerd.service.

Signed-off-by: Mitch Zhu <[email protected]>
@miz060
Merge pull request #209 from microsoft/mitchzhu/tardev-service
1276d40 
tardev: update tardev-snapshotter.service
samples: reduce dependencies to docker hub
16523e2 
Use container image sources from ACR/MCR.

Signed-off-by: Manuel Huber <[email protected]>
@manuelh-dev
Merge pull request #210 from microsoft/tests/img-pull-src
386cab0 
samples: reduce dependencies to docker hub
@danmihai1
agent: fix the AllowRequestsFailingPolicy functionality
51b2fd2 
1. Use the new value of AllowRequestsFailingPolicy after setting up a
   new Policy. Before this change, the only way to enable
   AllowRequestsFailingPolicy was to change the default Policy file,
   built into the Guest rootfs image.

2. Ignore errors returned by regorus while evaluating Policy rules, if
   AllowRequestsFailingPolicy was enabled. For example, trying to
   evaluate the UpdateInterfaceRequest rules using a policy that didn't
   define any UpdateInterfaceRequest rules results in a "not found"
   error from regorus. Allow AllowRequestsFailingPolicy := true to
   bypass that error.

3. Add simple CI test for AllowRequestsFailingPolicy.

These changes are restoring functionality that was broken recently by
commmit 11f78ae.

Signed-off-by: Dan Mihai <[email protected]>
@danmihai1
Merge pull request #212 from microsoft/danmihai1/allow-failing-msft
80f047c 
agent: fix the AllowRequestsFailingPolicy functionality
@sprt
docs: add guide to install new CSI drivers
78e4962 
This adds a public guide on how to install and test the new storage CSI
drivers for AKS confidential pods.

Signed-off-by: Aurélien Bombo <[email protected]>
@sprt
Merge pull request #214 from microsoft/sprt/csi-instructions
4c89a18 
docs: add guide to install new CSI drivers
@Redent0r
genpolicy: add support for cron jobs
39e99dc 
Add support for cron jobs

Signed-off-by: Saul Paredes <[email protected]>
samples: update image references
71db681 
Use up-to-date AzL references

Signed-off-by: Manuel Huber <[email protected]>
@manuelh-dev
Merge pull request #217 from microsoft/tests/img-updates
c323d8a 
samples: update image references
@Redent0r
Merge pull request #218 from microsoft/saulparedes/support_cron_job
910bcd6 
genpolicy: add support for cron jobs
@danmihai1 @Redent0r
genpolicy: reject untested CreateContainer field values
bee89de 
Reject CreateContainerRequest field values that are not tested by
Kata CI and that might impact the confidentiality of CoCo Guests.

This change uses a "better safe than sorry" approach to untested
fields. It is very possible that in the future we'll encounter
reasonable use cases that will either:

- Show that some of these fields are benign and don't have to be
  verified by Policy, or
- Show that Policy should verify legitimate values of these fields

These are the new CreateContainerRequest Policy rules:

    count(input.shared_mounts) == 0
    is_null(input.string_user)

    i_oci := input.OCI
    is_null(i_oci.Hooks)
    is_null(i_oci.Linux.Seccomp)
    is_null(i_oci.Solaris)
    is_null(i_oci.Windows)

    i_linux := i_oci.Linux
    count(i_linux.GIDMappings) == 0
    count(i_linux.MountLabel) == 0
    count(i_linux.Resources.Devices) == 0
    count(i_linux.RootfsPropagation) == 0
    count(i_linux.UIDMappings) == 0
    is_null(i_linux.IntelRdt)
    is_null(i_linux.Resources.BlockIO)
    is_null(i_linux.Resources.Network)
    is_null(i_linux.Resources.Pids)
    is_null(i_linux.Seccomp)
    i_linux.Sysctl == {}

    i_process := i_oci.Process
    count(i_process.SelinuxLabel) == 0
    count(i_process.User.Username) == 0

Signed-off-by: Dan Mihai <[email protected]>
@Redent0r
samples: update annotations
8ad67ac 
Update annotations after reject untested CreateContainer field values

Signed-off-by: Saul Paredes <[email protected]>
@Redent0r
Merge pull request #219 from microsoft/saulparedes/reject_untested
89a658a 
genpolicy: reject untested CreateContainer field values
@Redent0r
samples: update ubuntu sample to use 18.04
32ac40e 
This is a more stable tag. Previous tag got updated yesterday and
requires to update the annotation.

Signed-off-by: Saul Paredes <[email protected]>
@Redent0r
Merge pull request #220 from microsoft/saulparedes/update_ubuntu_sample
f17f994 
samples: update ubuntu sample to use 18.04
@Redent0r
chore: bump release version
24423ed 
Bump release version to 3.2.0-azl1.genpolicy1

Signed-off-by: Saul Paredes <[email protected]>
@Redent0r
Merge pull request #221 from microsoft/saulparedes/bump_release_version
dc2d4de 
chore: bump release version
tools: Add package-tools-install functionality
db1787f 
- Add script to install kata-containers(-cc)-tools bits
- Minor improvements in README.md
- Minor fix in package_install
- Remove echo outputs in package_build

Signed-off-by: Manuel Huber <[email protected]>
@manuelh-dev
Merge pull request #215 from microsoft/mahuber/uvm-tools-pkg
f20dbb9 
tools: Add package-tools-install functionality
tools: Enable setting IGVM SVN
08333aa 
- Allow setting SVN parameter for IGVM build scripting

Signed-off-by: Manuel Huber <[email protected]>
@manuelh-dev
Merge pull request #224 from microsoft/mahuber/igvm-svn
d375a69 
tools: Enable setting IGVM SVN
@sprt
node-builder: introduce BUILD_TYPE variable
52a47b6 
This lets developers build and deploy Kata in debug mode without having to make
manual edits to the build scripts.

With BUILD_TYPE=debug (default is release):

 * The agent is built in debug mode.
 * The agent is built with a permissive policy (using allow-all.rego).
 * The shim debug config file is used, ie. we create the symlink
   configuration-clh-snp-debug.toml <- configuration-clh-snp.toml.

For example, building and deploying Kata-CC in debug mode is now as simple as:

   make BUILD_TYPE=debug all-confpods deploy-confpods

Also do note that make still lets you override the other variables even after
setting BUILD_TYPE. For example, you can use the production shim config with
BUILD_TYPE=debug:

   make BUILD_TYPE=debug SHIM_USE_DEBUG_CONFIG=no all-confpods deploy-confpods

Signed-off-by: Aurélien Bombo <[email protected]>
@sprt
Merge pull request #216 from microsoft/sprt/build-type-var
74cc478 
node-builder: introduce BUILD_TYPE variable
@Redent0r
samples: update samples
7e00f3d 
Update samples that use
image: "mcr.microsoft.com/azurelinux/base/nginx:1.25". Looks like
this image tag was updated in mcr on 8/27/2024.

Signed-off-by: Saul Paredes <[email protected]>
@Sumynwa Sumynwa force-pushed the sumsharma/genpolicy_tests branch from 790e195 to 5ce3cdf Compare March 20, 2025 11:33
@Redent0r Redent0r added the upstream/missing PRs that are yet to be upstreamed label Mar 20, 2025
manuelh-dev
manuelh-dev suggested changes Mar 21, 2025
View reviewed changes
Copy link

@manuelh-dev manuelh-dev left a comment

Choose a reason for hiding this comment

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

Can we please articulate why we can't upstream this before introducing downstream?

Camelron
Camelron reviewed Mar 21, 2025
View reviewed changes
@Redent0r
Copy link

Redent0r commented Mar 21, 2025

Can we please articulate why we can't upstream this before introducing downstream?

We can upstream it. I'd say there's 2 parts:

  • the test refactoring: this should (mostly) apply cleanly
  • adding the new test scenario: I believe this will require some massaging to produce an equivalent mock request due to differences in rules.rego, specially for CreateContainer.

I'd say let's try to upstream this first, at @Sumynwa 's convenience

@manuelh-dev
Copy link

manuelh-dev commented Mar 21, 2025

Can we please articulate why we can't upstream this before introducing downstream?

We can upstream it. I'd say there's 2 parts:

  • the test refactoring: this should (mostly) apply cleanly
  • adding the new test scenario: I believe this will require some massaging to produce an equivalent mock request due to differences in rules.rego, specially for CreateContainer.

I'd say let's try to upstream this first, at @Sumynwa 's convenience

Sounds good, in the meantime, assumption is that Sumynwa can use this commit to facilitate testing of his implementation work w/o too much pain?

@Sumynwa
Copy link
Author

Sumynwa commented Mar 22, 2025

Thanks.
Yes, upstreaming makes sense with Part 1: the test refactoring: this should (mostly) apply cleanly as Saul pointed out.
I will raise a PR.

danmihai1 and others added 2 commits March 25, 2025 14:22
@danmihai1
genpolicy: add support for BUILD_TYPE=debug
5737c1d 
Use "cargo build --release" when BUILD_TYPE was not specified, or when
BUILD_TYPE=release. The default "cargo build" behavior is to build in
debug mode.

Signed-off-by: Dan Mihai <[email protected]>
@sprt
Merge pull request #336 from microsoft/danmihai1/debug-build-genpolicy
cf1e7c2 
genpolicy: add support for BUILD_TYPE=debug
@Redent0r
Copy link

Redent0r commented Mar 25, 2025

Merged upstream kata-containers#11074 ! @Sumynwa if it makes sense to you, I would adapt this PR to just cherry pick the refactoring changes, and keep the added test cases in your hardening feature branch for now

@Redent0r Redent0r added upstream/merged PRs that have been merged upstream and removed upstream/missing PRs that are yet to be upstreamed labels Mar 25, 2025
@Redent0r Redent0r mentioned this pull request Mar 27, 2025
Closed
4 tasks
@Sumynwa Sumynwa force-pushed the sumsharma/genpolicy_tests branch from 5ce3cdf to 61afc56 Compare March 29, 2025 08:17
@Sumynwa
Copy link
Author

Sumynwa commented Mar 29, 2025

Modified PR changes to cherry pick only the up-streamed changes. @Redent0r

@Sumynwa Sumynwa changed the title policy: Refactor tests to allow different request types in a testcase. genpolicy: Refactor tests to add different request types in testcases json Mar 29, 2025
@Sumynwa Sumynwa force-pushed the sumsharma/genpolicy_tests branch 2 times, most recently from 131c2b8 to 82ab894 Compare March 29, 2025 08:24
@Sumynwa
genpolicy: Refactor tests to add different request types in testcases…
a42f0ab 
… json

Cherry-pick upstream changes in PR kata-containers#11074 to add test data for multiple request
type in a single testcases.json file. This allows for stateful testing,
for ex: enable testing ExecProcessRequest using policy state set after testing
a CreateContainerRequest.

Signed-off-by: Sumedh Sharma <[email protected]>
@Sumynwa Sumynwa force-pushed the sumsharma/genpolicy_tests branch from 82ab894 to a42f0ab Compare March 29, 2025 08:28
Redent0r
Redent0r approved these changes Mar 31, 2025
View reviewed changes
Copy link

@Redent0r Redent0r left a comment

Choose a reason for hiding this comment

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

Thanks!

@manuelh-dev
Copy link

manuelh-dev commented Mar 31, 2025

I vote for holding on with merging this PR into our fork and instead prioritizing upstreaming policy: improve args and env variables validation by Redent0r · Pull Request #308 · microsoft/kata-containers and along with this, adding the changes of this downstream PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Camelron Camelron Camelron approved these changes

@Redent0r Redent0r Redent0r approved these changes

+1 more reviewer

@manuelh-dev manuelh-dev manuelh-dev requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

upstream/merged PRs that have been merged upstream

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.