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

Simplify policy structure and rename #947

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Simplify policy structure and rename #947

merged 1 commit into from
Sep 13, 2023

Conversation

JAORMX
Copy link
Contributor

@JAORMX JAORMX commented Sep 13, 2023

Only me and my dog understand why the policy had the concept of contextual rule sets.

This is not a feature we're actually using, and makes explaining these concepts to folks
quite confusing.

So... let's get rid of pluggable contexts within a policy and let's just declare the provider
at the top of the policy file. This way it's more simple and covers the use-cases we're currently working on.

When we do need another provider for a policy, we can then just create a separate policy file and
manage it that way.

While we're at it, this also renames PipelinePolicy to simply Policy.

Only me and my dog understand why the policy had the concept of contextual rule sets.

This is not a feature we're actually using, and makes explaining these concepts to folks
quite confusing.

So... let's get rid of pluggable contexts within a policy and let's just declare the provider
at the top of the policy file. This way it's more simple and covers the use-cases we're currently working on.

When we do need another provider for a policy, we can then just create a separate policy file and
manage it that way.

While we're at it, this also renames `PipelinePolicy` to simply `Policy`.
name: acme-github-policy
context:
group: Root Group
provider: github
repository:
- context: github
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know the original context behind the pipeline policy but the way I was always reading the policies was that the context was useful in case your policy would be testing entities around different contexts, e.g. repo check for GitHub, artifact checks for dockerhub and build environment checks for Jenkins.

With the policy pipeline going away would we have different policies for those cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jhrozek that's exactly the case, if you have rules to check for quay, dockerhub, or another provider, you'd need to create another policy directed towards that provider. Provider information would be set exclusively in the policy's global context. This doesn't mean that we can't mix and match providers, but that needs to be done in a rule type.

Copy link
Member

Choose a reason for hiding this comment

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

How would mixing work? If we set a policy per provider and also the option that there can be rule type declarations where they match more than one provider, how would that work if a rule that supports multiple providers is added to a provider-specific policy?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I guess this makes sense also for easier application of policies on lower levels of the hierarchy (repo policy that amends a group policy).

What about UX, do you think this is something the clients should then solve or did you think about wrapping this policy in a higher level object? With the pipeline policy it would be easier to see the overall status of policies regardless of the provider, with individual per-provider policies you need to iterate over them, which is doable, but we need to think about making the clients do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would mixing work? If we set a policy per provider and also the option that there can be rule-type declarations where they match more than one provider, how would that work if a rule that supports multiple providers is added to a provider-specific policy?

A policy that's applied to a GitHub package but that has to check an external provider (say sigstore) should call the provider explicitly in the rule type itself. It would work if the provider is registered in the database (work that I plan to start today). The global contextual provider setting is merely where the entities come from and how they're ingested, not necessarily how they get evaluated. We still need to get a better means for expressing how a rule type would apply to several providers.

OK, I guess this makes sense also for easier application of policies on lower levels of the hierarchy (repo policy that amends a group policy).

What about UX, do you think this is something the clients should then solve or did you think about wrapping this policy in a higher level object? With the pipeline policy it would be easier to see the overall status of policies regardless of the provider, with individual per-provider policies you need to iterate over them, which is doable, but we need to think about making the clients do that.

We can always roll back and make policies take several contexts. I figured that writing policies with the current scheme is a little tedious, and the benefits are not entirely clear right now. It could even be that we drop the global provider entirely from policies and fully rely on the rule types instead. We'll need to see how things progress as we evolve the engine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might want to draw some examples in a design doc. I'm wary of recommending people to modify rule_types, I guess they would treat them as something developers or power users only can modify. At the same time, I agree that the benefits of the pipeline now are unclear, maybe the contexts could be set in the global context:

contexts:
 - entity: artifact
    provider: quay.io
 - entity: repository
    provider: GitHub
    ....

but this is not the place to be solutioning :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's tackle this as soon as we start working on multi-provider policies. For now, I suggest we continue and get things simpler.

name: acme-github-policy
context:
group: Root Group
provider: github
repository:
- context: github
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might want to draw some examples in a design doc. I'm wary of recommending people to modify rule_types, I guess they would treat them as something developers or power users only can modify. At the same time, I agree that the benefits of the pipeline now are unclear, maybe the contexts could be set in the global context:

contexts:
 - entity: artifact
    provider: quay.io
 - entity: repository
    provider: GitHub
    ....

but this is not the place to be solutioning :-)

@JAORMX JAORMX merged commit a5dde0b into main Sep 13, 2023
13 checks passed
@JAORMX JAORMX deleted the policy-simple branch September 13, 2023 09:20
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.

3 participants