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

tools: add policy generation tool #8248

Merged
merged 11 commits into from Jan 11, 2024

Conversation

danmihai1
Copy link
Contributor

Add application that infers K8s user's intentions based on user's K8s YAML file, and generates a Rego/OPA based policy for that YAML.

Fixes: #7673

@danmihai1 danmihai1 requested a review from a team as a code owner October 18, 2023 21:23
@katacontainersbot katacontainersbot added the size/huge Largest and most complex task (probably needs breaking into small pieces) label Oct 18, 2023
@danmihai1
Copy link
Contributor Author

/test

@danmihai1
Copy link
Contributor Author

/test

Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

Hey Dan, thanks so much for the submission. There is a lot of code here and I'm not an expert, so I've focussed on the documentation and through to get the tool build and running myself primarily, and left some comments about how we might be able to make this a bit more accessible as there were a few hurdles I bumped into and the tool has a lot of power, so letting people understand the processing and output more feels important. Thanks!

src/tools/genpolicy/README.md Outdated Show resolved Hide resolved
src/tools/genpolicy/README.md Show resolved Hide resolved
src/tools/genpolicy/README.md Outdated Show resolved Hide resolved
@danmihai1
Copy link
Contributor Author

danmihai1 commented Nov 14, 2023

Hey Dan, thanks so much for the submission. There is a lot of code here and I'm not an expert, so I've focussed on the documentation and through to get the tool build and running myself primarily, and left some comments about how we might be able to make this a bit more accessible as there were a few hurdles I bumped into and the tool has a lot of power, so letting people understand the processing and output more feels important. Thanks!

Thanks for all the feedback @stevenhorsman. I have just pushed what I think are significant doc improvements in 2257a16

Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

Thanks very much for the extra documentation Dan - it really helps. Again the code is too much for me to do a sensible review, but I've kicked the tyres with testing, so I think it's fine to go in as the initial drop of some tooling.

Copy link

@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

I'm also if there are any requirements for the container images used. Not sure if that's written anywhere in the documentation and I've missed it. I've tested a bunch of images, some worked, some failed:

apiVersion: v1
kind: Pod
metadata:
  name: test
spec:
  containers:
    - name: test
      image: ${image}

image="ubuntu:latest"

Error: failed to generate policies: failed to generate policy for deployment/test.yml: genpolicy failed with exit code 101: thread 'main' panicked at src/registry.rs:111:17:
Failed to pull container image manifest and config - error: UnsupportedMediaTypeError(
    "application/vnd.oci.image.index.v1+json",
)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

image="nixery.dev/shell/socat"

Error: failed to generate policies: failed to generate policy for deployment/test.yml: genpolicy failed with exit code 101: thread 'main' panicked at src/registry.rs:91:61:
called `Result::unwrap()` on an `Err` value: Error("missing field `Env`", line: 1, column: 969)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


`genpolicy` downloads the `quay.io/prometheus/busybox:latest` container image.

Depending on the size of the container images and the speed of the network connection to the container registry, downloading these images might take several minutes. For testing scenarios where `genpolicy` gets executed several times, it can be useful to cache the container images after downloading them, in order to avoid most of the time needed to download the same container images multiple times. If a container image layer was already cached locally, `genpolicy` uses the local copy of that container layer. The application caches the image information under the `./layers_cache` directory.

Choose a reason for hiding this comment

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

It would be great to have a flag for specifying the cache directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that caching needs future improvements, but I don't think the initial merge of this code should wait for these improvements to become available. Please create github issues for such ideas for improvements.


# Use a custom path to `genpolicy` input files

By default, the `genpolicy` input files [`rules.rego`](rules.rego) and [`genpolicy-settings.json`](genpolicy-settings.json) must be present in the current directory - otherwise `genpolicy` returns an error. Users can specify a different path to these two files, using the `-i` command line parameter - e.g.,

Choose a reason for hiding this comment

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

I find the semantics of -i quite counter-intuitive. As a user, I would like to place these files independently from each other in the place I like. Would it be possible to implement this as -r flag instead to directly point to the rego policy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please log an issue for the future.

let mut resources = Vec::new();
let yaml_contents = yaml::get_input_yaml(&config.yaml_file)?;

for document in serde_yaml::Deserializer::from_str(&yaml_contents) {

Choose a reason for hiding this comment

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

Deployments can contain a mix of confidential and non-confidential resources. Parts that should deployed into confidential environment have a specific runtimeClassName. When testing these changes, the tool would just generate policies for all resources in the yaml files referenced, not respecting the (not) specified runtime class. I was wondering how we could solve this. I think we should either check for a set of well-known runtimeClassName values or let the user specify the runtimeClassName to target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@katexochen , thanks for the feedback! This tool doesn't solve all the Confidential K8s Policy problems (yet?). For example, if you search for TODO in its source code, you'll find a bunch of other problem that were not solved yet. I think we should start somewhere and keep improving. After working on this for several months, I think that waiting for the ideal tool before the first merge is impractical.

@danmihai1
Copy link
Contributor Author

I'm also if there are any requirements for the container images used. Not sure if that's written anywhere in the documentation and I've missed it. I've tested a bunch of images, some worked, some failed:

apiVersion: v1
kind: Pod
metadata:
  name: test
spec:
  containers:
    - name: test
      image: ${image}

image="ubuntu:latest"

Error: failed to generate policies: failed to generate policy for deployment/test.yml: genpolicy failed with exit code 101: thread 'main' panicked at src/registry.rs:111:17:
Failed to pull container image manifest and config - error: UnsupportedMediaTypeError(
    "application/vnd.oci.image.index.v1+json",
)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

image="nixery.dev/shell/socat"

Error: failed to generate policies: failed to generate policy for deployment/test.yml: genpolicy failed with exit code 101: thread 'main' panicked at src/registry.rs:91:61:
called `Result::unwrap()` on an `Err` value: Error("missing field `Env`", line: 1, column: 969)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Please log github issues for YAML files that are not working and attach the problematic YAML files to those issues. For these particular two problems:

  1. One of my colleagues at MSFT noticed the ubuntu:latest problem a few days ago, and we are working on a solution for it. I can share the gory details in case you are curious about the root cause. Meanwhile, creating your own container image on top of an Ubuntu base container image (but using a different manifest format) might be a good enough workaround.
  2. Please attach your test.yaml in github - the Env problem you mentioned sounds more related to the YAML contents, not to the container image.

@danmihai1
Copy link
Contributor Author

No code or doc changes in my latest push. Just squashed the doc changes into a single commit and rebased on top of the latest main code.

danmihai1 and others added 11 commits December 22, 2023 15:35
Add application that infers K8s user's intentions based on user's
K8s YAML file, and generates a Rego/OPA based policy for that YAML.

Just Pod YAML files are supported as input using this initial source
code. Support for other types of YAML files will come with upcoming
commits.

Fixes: kata-containers#7673

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s DaemonSet YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s Deployment YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s Job YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s List YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s ReplicaSet YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s ReplicationController YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Generate policy for K8s StatefulSet YAML.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Add docs for the Agent Policy and for the genpolicy tool.

Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Also support alternative media type and update samples

Signed-off-by: Saul Paredes <saulparedes@microsoft.com>
Signed-off-by: Dan Mihai <dmihai@microsoft.com>
Add SPDX license header to rules.rego.

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

@danmihai1 I want to report an issue while I am testing confidential containers from AKS, there is no place from microsoft/kata-containers to let me open an issue, so I put it here

It seems we not able to correctly handle mountPropagation: Bidirectional or mountPropagation: HostToContainer on emptyDir volume, as a consequence, the container won't be created with error "Error: failed to create containerd task: failed to create shim task: "CreateContainerRequest is blocked by policy": unknown"

To work around the issue, for HostToContainer, we need to replace rprivate to rslave in the generated policy.

      {
        "destination": "/mnt/remote",
        "source": "^$(cpath)/$(sandbox-id)/local/remotemounts$",
        "type_": "local",
        "options": [
          "rbind",
          "rslave",
          "rw"
        ]
      }

For Bidirectional, we need to replace rprivate to rshared in the generated policy.

      {
        "destination": "/mnt/remote",
        "source": "^$(cpath)/$(sandbox-id)/local/remotemounts$",
        "type_": "local",
        "options": [
          "rbind",
          "rshared",
          "rw"
        ]
      }

To reproduce the issue, you can use https://github.com/huangyingting/sev-snp/blob/main/encfs.yaml (ignore io.katacontainers.config.agent.policy, it was tweaked already based on above findings to make it work)

@danmihai1
Copy link
Contributor Author

@danmihai1 I want to report an issue while I am testing confidential containers from AKS, there is no place from microsoft/kata-containers to let me open an issue, so I put it here

It seems we not able to correctly handle mountPropagation: Bidirectional or mountPropagation: HostToContainer on emptyDir volume, as a consequence, the container won't be created with error "Error: failed to create containerd task: failed to create shim task: "CreateContainerRequest is blocked by policy": unknown"

Thanks! We are tracking other bugs applicable to this app under https://github.com/kata-containers/kata-containers/issues .

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

@danmihai1, lgtm, thanks!

I think this one should get in sooner than later, as we have been tracking the improvements in a different issue.

@fidencio
Copy link
Member

/test

@fidencio fidencio merged commit 86a6d13 into kata-containers:main Jan 11, 2024
168 of 175 checks passed
@danmihai1 danmihai1 deleted the danmihai1/genpolicy-main branch January 21, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/huge Largest and most complex task (probably needs breaking into small pieces)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add tool/application that generates CoCo policy based on user's Kubernetes YAML files
8 participants