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

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

Open
erezo9 opened this issue Aug 27, 2022 · 9 comments
Open

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

erezo9 opened this issue Aug 27, 2022 · 9 comments
Labels
bug Something isn't working triaged

Comments

@erezo9
Copy link

erezo9 commented Aug 27, 2022

Since OPA rego has deprecated any and all functions according to this issue open-policy-agent/opa#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
Copy link
Member

ritazh commented Aug 29, 2022

@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?

@ritazh ritazh added the bug Something isn't working label Aug 29, 2022
@srenatus
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

maxsmythe commented Sep 1, 2022

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
Copy link

stale bot commented Jan 31, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged
Projects
None yet
Development

No branches or pull requests

5 participants