open-policy-agent / gatekeeper-library

📚 The OPA Gatekeeper policy library
https://open-policy-agent.github.io/gatekeeper-library
Apache License 2.0
648 stars 321 forks source link

Deprecation of any and all affects gatekeeper-library policies #225

Open erezo9 opened 2 years ago

erezo9 commented 2 years ago

Since OPA rego has deprecated any and all functions according to this issue https://github.com/open-policy-agent/opa/issues/2437 there should be a fix for using any in the constraint template for example is any/all replaced by some? or there is antoher solution? https://github.com/open-policy-agent/gatekeeper-library/blob/master/library/general/allowedrepos/template.yaml#L31

ritazh commented 2 years ago

@erezo9 Thanks for reporting this! you are right we should replace them. Feel free to open a PR if you are interested! @srenatus do you know which version of opa stops supporting any and all?

srenatus commented 2 years ago

@srenatus do you know which version of opa stops supporting any and all?

OPA "1.0", but it's not clear yet when that's going to happen.

That said, the linked template could use a do-over, I think. With the next release of OPA, we could do this:

not strings.any_prefix_match(input.parameters.repos, container.image)

Direct replacements could be (slightly updated for using in instead of [_] for clarity)

satisfied := { true | some repo in input.parameters.repos; startswith(container.image, repo) }
count(satisfied) == 0
maxsmythe commented 2 years ago

The breaking of backwards compatibility here is expensive.

We can rewrite library templates, but users will need to upgrade in order to benefit from that, and custom-written templates won't be touched by this.

It's tempting to re-implement these functions as custom built-ins to avoid this concern, I'm not sure how hard that would be.

srenatus commented 2 years ago

It's tempting to re-implement these functions as custom built-ins to avoid this concern, I'm not sure how hard that would be.

Not hard. I believe @anderseknert has a snippet for that to share, too.

anderseknert commented 2 years ago

I do somewhere, don't I? Can't find it now though 😅

But basically, any/every is replaced by the in and every keywords, which provide the additional benefit of being able to check for any type of value, and not just booleans. If that's what you want though, this would work the same way:

any / in

true in arr

all / every

every bool in bools {
    bool == true
}

FWIW, those functions have since long been removed from the OPA docs, so I'd be surprised to see them show up in any policy written in the last year or so.

maxsmythe commented 2 years ago

FWIW, those functions have since long been removed from the OPA docs, so I'd be surprised to see them show up in any policy written in the last year or so.

These policies do exist, and they will be broken without updating. Authorship recency is irrelevant to impact.

anderseknert commented 2 years ago

No one suggested otherwise :) What's relevant for impact is the number of policies in the wild that make use of those functions, and I just added that I doubt they're commonly found in policies from recent times. That existing ones that do would break is clear, and I would think an 1.0 release came with a few more breaking changes, so a release like that would need to be carefully coordinated.. but as @srenatus said, it's as far as I know not on the roadmap for now.

OPA provides a strict mode option for compilation, which will error when deprecated functions are encountered, and also add some strictness checks around e.g. unused or duplicate imports, unused variables, etc.. policies that pass the strict mode check should not be affected by a future 1.0 upgrade. Perhaps it would be a good idea to run that against the policy library? And maybe allow users of Gatekeeper to enable that check for their own policies?

maxsmythe commented 2 years ago

I'm glad there are ways to early-detect deprecated policies.

Part of my intent in pushing back on deprecation of language rules is that it is very expensive because of how many people it affects downstream, and that cost should be surfaced.

It looks like this language feature is being deprecated because the syntax/behavior is disfavored rather than due to any inherent security issue. I'd ask that the cost to the community be considered against the benefit of deprecation in the future. As the number of users grows, this cost will only ever increase and can lead to an erosion of trust in the long-term stability of the language.

stale[bot] commented 1 year ago

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.