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.

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

docs: add Policy lifecycle RFC #44

Merged
merged 24 commits into from
Jan 23, 2025
Merged

docs: add Policy lifecycle RFC #44

merged 24 commits into from
Jan 23, 2025

Conversation

fabriziosestito
Copy link
Contributor

@fabriziosestito fabriziosestito commented Dec 19, 2024 •
edited by viccuad
Loading

Description

Adds Policy lifecycle RFC

Fixes #43

Rendered RFC

flavio
flavio reviewed Dec 20, 2024
View reviewed changes
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 job, I think we are finally on the right path

rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
@fabriziosestito fabriziosestito self-assigned this Jan 7, 2025
@fabriziosestito fabriziosestito added the documentation Improvements or additions to documentation label Jan 7, 2025
flavio
flavio reviewed Jan 10, 2025
View reviewed changes
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.

looks really good to me

rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from a9c0b86 to 86ca64b Compare January 13, 2025 07:53
@fabriziosestito
docs: add Policy lifecycle RFC
a1f8f29 
Signed-off-by: Fabrizio Sestito <[email protected]>
@fabriziosestito
review fixes
bfb9545 
Signed-off-by: Fabrizio Sestito <[email protected]>
@fabriziosestito
add policy version to cache path
af80bde 
Signed-off-by: Fabrizio Sestito <[email protected]>
@fabriziosestito
add policy server lifecycle
712c466 
Signed-off-by: Fabrizio Sestito <[email protected]>
@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from 86ca64b to fd5b9b4 Compare January 13, 2025 07:58
@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from fd5b9b4 to e0706e0 Compare January 13, 2025 07:58
fabriziosestito
fabriziosestito commented Jan 13, 2025
View reviewed changes
rfc/0022-policy-lifecycle.md Outdated
| Feature Name | [Name] |
| Start Date | [Today] |
| Category | [Category] |
| RFC PR | [fill this in after opening PR] |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

memo to self: fix this

jhkrug
jhkrug requested changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@jhkrug jhkrug left a comment

Choose a reason for hiding this comment

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

A few suggestions and occasional typo for your consideration.

rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch 2 times, most recently from 8147609 to 2e35667 Compare January 14, 2025 12:53
@fabriziosestito
Apply suggestions
0840d68 
Signed-off-by: Fabrizio Sestito <[email protected]>
@fabriziosestito
generation as url parameter
141f2ac 
Signed-off-by: Fabrizio Sestito <[email protected]>
jvanz
jvanz approved these changes Jan 16, 2025
View reviewed changes
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
flavio
flavio reviewed Jan 17, 2025
View reviewed changes
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.

Looks really good! We're getting there

rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Show resolved Hide resolved
jvanz
jvanz requested changes Jan 17, 2025
View reviewed changes
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.

We should mention in the RFC that we need to change the audit-scanner to properly select the policy generation when building the endpoint used to send the request to validate a resource in the cluster. Because the validation endpoint in the policy server has generation in the path. As well as mention the policy generation in the reports.

I would say that the audit-scanner should select the totally ready policy generation when building the request. Ignoring the generation that under the process to be active.

@flavio
Copy link
Member

flavio commented Jan 20, 2025 •
edited
Loading

We should mention in the RFC that we need to change the audit-scanner to properly select the policy generation when building the endpoint used to send the request to validate a resource in the cluster. Because the validation endpoint in the policy server has generation in the path. As well as mention the policy generation in the reports.

These are some good points, good catch!

@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from 5407d80 to 5276c3c Compare January 21, 2025 13:45
@fabriziosestito fabriziosestito marked this pull request as ready for review January 22, 2025 06:53
viccuad
viccuad reviewed Jan 22, 2025
View reviewed changes
Copy link
Member

@viccuad viccuad left a comment

Choose a reason for hiding this comment

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

Looks quite good to me, but before approving I would like to agree on if we will be using annotations to link the Policy and the actual PolicyRevision in use (open comment thread up).

viccuad
viccuad approved these changes Jan 22, 2025
View reviewed changes
Copy link
Member

@viccuad viccuad left a comment

Choose a reason for hiding this comment

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

LGTM!

@fabriziosestito
Copy link
Contributor Author

@jhkrug, ready for another review pass

jvanz
jvanz approved these changes Jan 22, 2025
View reviewed changes
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

rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
jhkrug
jhkrug approved these changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@jhkrug jhkrug left a comment

Choose a reason for hiding this comment

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

Two tiny suggestions but lgtm.

rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md Outdated Show resolved Hide resolved
rfc/0022-policy-lifecycle.md

### Sequence diagram

```mermaid
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice sequence diagrams!

@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from 1c47758 to 5800d98 Compare January 23, 2025 15:15
@fabriziosestito fabriziosestito force-pushed the docs/add-policy-lifecycle-rfc branch from 5800d98 to f1636a3 Compare January 23, 2025 15:18
@fabriziosestito fabriziosestito merged commit fcdcfef into kubewarden:main Jan 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhkrug jhkrug jhkrug approved these changes

@viccuad viccuad viccuad approved these changes

@jvanz jvanz jvanz approved these changes

@flavio flavio Awaiting requested review from flavio

Assignees

@fabriziosestito fabriziosestito

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RFC: PolicyServer should report the status of a Policy
5 participants
@fabriziosestito @flavio @jvanz @viccuad @jhkrug