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

Add RFC 10, Rancher integration of policies #12

Merged
merged 11 commits into from
Sep 26, 2022

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented Aug 31, 2022

Description

Relates to #7

Rendered RFC.

@viccuad viccuad self-assigned this Aug 31, 2022
@viccuad viccuad linked an issue Aug 31, 2022 that may be closed by this pull request
@viccuad viccuad removed their assignment Aug 31, 2022
@viccuad viccuad changed the title Add rfc 10, Rancher integration of policies Add RFC 10, Rancher integration of policies Aug 31, 2022
@viccuad viccuad force-pushed the rancher-integration-policies branch from 60fb23d to e90cceb Compare August 31, 2022 14:47
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Great analysis 👏

I propose a variation on the alternative approach which is an hybrid between the core proposal and alternative "A":

  • Store the additional information inside of the Wasm metadata
  • Have a tool that reads the embedded metadata and produces as output all the Rancher helm chart files

The rest would work like the core proposal. What do you think?

rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Show resolved Hide resolved
@viccuad
Copy link
Member Author

viccuad commented Aug 31, 2022

As an example of a Helm chart for a policy, see kubewarden/allow-privilege-escalation-psp-policy#30.

This example is implemented directly in the main branch of the policy repo, instead of an orphan branch, and is reusing its readme, license, and files.

@flavio
Copy link
Member

flavio commented Aug 31, 2022

I propose a variation on the alternative approach which is an hybrid between the core proposal and alternative "A":

  • Store the additional information inside of the Wasm metadata

  • Have a tool that reads the embedded metadata and produces as output all the Rancher helm chart files

Looking at the contents created via kubewarden/allow-privilege-escalation-psp-policy#30, I think the only reasonable thing to include inside of the policy metadata would be the questions.yml stuff. I wonder if this is worth the effort...

@viccuad
Copy link
Member Author

viccuad commented Sep 1, 2022

Committed clarification of points, and reworded to use folder instead of branch.

Looking at the contents created via kubewarden/allow-privilege-escalation-psp-policy#30, I think the only reasonable thing to include inside of the policy metadata would be the questions.yml stuff. I wonder if this is worth the effort...

It also provides a simple airgap story for the metadata (download Helm charts, mirror OCI wasm modules).

I still favour Alternative B (use OCI's manifest.config). To me, feels like the correct implementation. It's a field that is present for normal container images, simplifies airgap, removes unneeded metadata from artifacthub-pkg.yml.

We just need a client that can pull the manifest of an artifact from an OCI registry (e.g: 30 secs search, https://github.com/Pixeladed/oci-registry-js), and to do so on kwctl push too. We aren't making much of a new promise, just substituting storing the metadata in artifacthub/helm chart for the OCI repo.

@viccuad viccuad requested a review from flavio September 1, 2022 08:25
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
rfc/0010-rancher-integration-policies.md Outdated Show resolved Hide resolved
Copy link
Contributor

@raulcabello raulcabello left a comment

Choose a reason for hiding this comment

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

Thanks @viccuad ! I don't think it is a good idea to place kubewarden policies in https://github.com/rancher/charts

In my opinion we should leave https://github.com/rancher/charts just for apps that can be deployed in your cluster (kubewarden for example)

We could create a separate chart repository for the policies something like https://github.com/rancher/kubewarden-policies-charts. Drawback is that users would need to add it manually, or would it possible to add this new repository when kubewarden is installed? (I don't think that's possible, but I'm not sure..)

Copy link
Contributor

@raulcabello raulcabello left a comment

Choose a reason for hiding this comment

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

LGTM. We might need to make some modifications in the future based on the feedback we get for the questions.yaml and where to place the policies. I'm happy merging this now.
Thanks for the hard work @viccuad!

Copy link
Member

@jvanz jvanz left a comment

Choose a reason for hiding this comment

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

LGTM! :D
Thank you @viccuad for this work!

@viccuad
Copy link
Member Author

viccuad commented Sep 26, 2022

Merging! we can make adjustments later on.

@viccuad viccuad merged commit 24c0471 into kubewarden:main Sep 26, 2022
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.

RFC: how to provide a list of policies to Rancher Manager
4 participants