-
Notifications
You must be signed in to change notification settings - Fork 730
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
Rego "import future.keywords.in" causes template failure #1827
Comments
@srenatus Do you know off the top of your head whether this is due to needing to upgrade G8r's version of OPA? Or is there some bootstrapping that needs doing to enable these imports? |
GK is at OPA 0.35.0 right now, I think. The future keywords machinery was introduced with "in" in 0.34.0 (https://github.com/open-policy-agent/opa/releases/tag/v0.34.0). Not sure where the problem comes from, I'd need a closer look... |
Generally there are two ways to enable the future keywords, the import alone should be sufficient. In situations where that isn't available, you can provide the parser options. But one or the other is enough. |
Hmm looks like 3.7.0 and 3.7.1 of GK use a version of OPA that's too old, 0.29.4. |
Opa upgrade now comes from framework. Can you try latest commit in master? that is referencing framework that’s using opa v0.35 https://github.com/open-policy-agent/frameworks/pull/167/files#diff-45830b37d8fe6cfa6317672df498ec856ff48ffd48cd2537d18337e6281d6618R14 +1 on showing opa version badge in master, and perhaps for each release doc. thoughts? |
I am trialing gatekeeper currently and I'm super glad this wasn't just me having this issue. I'm not clear on the above fixes things. I have pulled master for open-policy-agent/gatekeeper and have re-run |
@ritazh When I try to build Gatekeeper locally it fails. My boss doesn't want me spending time trying to debug the local build. I'm installing from the published Helm charts. If there were a snapshot or bugfix Helm chart I could use I could test that. Or a bugfix or snapshot Docker image that I could specify in my "helm install". Thanks, |
IIRC we build an image from HEAD for every commit, tagged with the shortened commit hash: https://github.com/open-policy-agent/gatekeeper/tree/master/charts/gatekeeper It looks like you can change the image tag used in the Helm chart via https://hub.docker.com/r/openpolicyagent/gatekeeper/tags Mixing Helm chart versions and image versions probably isn't a great long term idea, since the required config for an image can change over time, but should work. The Helm chart that is built from HEAD for a specific commit lives here: https://github.com/open-policy-agent/gatekeeper/tree/master/manifest_staging/charts/gatekeeper @ritazh If we need to upgrade OPA before we're comfortable upgrading the constraint framework, we could cut a new release that overrides the version of OPA used. |
also, @srenatus thanks for digging in! |
I think that makes sense. But this is technically not a patch since we are upgrading opa. So we would need to cut v3.8.0. Are we ready for that? |
Do we have specific features we want in 3.8? Sharding? |
yes, sharding |
Okay, let's see how the timelines go |
I just tested this again. I used the command "helm install gatekeeper gatekeeper/gatekeeper --set image.release=$1 --set postInstall.labelNamespace.image.tag=$1" where "$1" was every Docker image tag (from https://hub.docker.com/r/openpolicyagent/gatekeeper/tags) "dev" back to "e47b31c". All of them failed with the error "Error: admission webhook "validation.gatekeeper.sh" denied the request: invalid ConstraintTemplate: invalid import: bad import" That's a different error than I was getting before. Maybe I'm not getting the right Docker image? Is there a specific image tag you'd like me to test? Here are the Helm charts I'm using to test: http://cakewalk.menlo.com/gatekeeper/; the raw REGO file is also there. |
Anything tagged with a hash in the past little while ( I think the issue might be our locked down importing that we use to enforce sandboxing: It'd need to be updated to allow @willbeason, it looks like the config path for passing in allowed data fields has changed as part of sharding. Right now it doesn't look like the client option |
@maxsmythe Yes - I'm fixing this and moving it to be an option on Driver. Client shouldn't care about this, and it makes sense as a Driver configuration. There's never a reason to change this at runtime, so it will be an immutable part of Driver specified via an Arg in the constructor. |
@willbeason, @maxsmythe: Any idea when this change will make it into a release (3.7.x or 3.8)? Please understand that I'm not trying to rush you or put pressure on you, I just need to schedule some other work so it would help me if I had an idea of when this might be ready? Thanks. |
Yes - 3.8 is pushed back to March 14th, but this change will be included. |
I just tested this in v3.8.0-rc.1 by doing the following:
followed by "helm install" of my Gatekeeper templates and constraints. Unfortunately I'm still getting the "bad import" error when I try to use a template that contains "import future.keywords.in". Do I need to do something more to get the latest release candidate installed? I thought this issue was going to be fixed in the 3.8 release; was the fixed pushed out to a later release? Thanks. |
@asm2git This was added to Frameworks in open-policy-agent/frameworks#207, and updated in Gatekeeper in #1968, which is included in Can you provide an example ConstraintTemplate which exhibits this behavior? My best guess is that either there is a problem with the ConstraintTemplate, or we aren't testing this properly. With the ConstraintTemplate I'll be able to narrow this down. |
@willbeason here's what I'm, using; sorry it's so long
|
@willbeason just in case you need it, here's the constraint that goes along with it
Let me know if you need the full Helm Charts I'm using. |
That's perfect! I'll try to replicate and check back in the next couple days. |
@willbeason I just tried my test again with rc2, unfortunately it still fails. I don't know if this is expected or not. Just to recap, here's how I'm installing Gatekeeper:
Please let me know if there's something I need to do differently. Thanks. |
As an aside, @asm2git you should always upgrade the helm chart release to make sure you get all the chart/yaml updates in addition to just updating the image tag. e.g CRD spec updates. Though I don’t think that’s the issue here. |
@ritazh Absolutely, although it looks like there's no chart for rc2, just rc1. I'll try my tests again tomorrow with "helm install ... --version 3.8.0-rc.2" (and fall back to "rc.1" if that doesn't find a newer chart). I'll post the results. |
admission webhook "validation.gatekeeper.sh" denied the request: invalid ConstraintTemplate: invalid import: bad import
opa gatekeeper install today |
@maxsmythe On a freshly-installed Debian 11 droplet at DigitalOcean, wholly disconnected from anything to do with my company, I copied your exact commands and got the same error. On a new KinD cluster, I tried "helm install gatekeeper gatekeeper/gatekeeper --namespace gatekeeper-system --version 3.8.0"; same error. On yet another KinD cluster, I tried "helm install gatekeeper gatekeeper/gatekeeper --set 'image.repository=openpolicyagent/gatekeeper' --set 'image.release=v3.8.0'"; same error. @asahnovskiy-deloitte have you tried it with the v3.8.0 release and if so, did you get the same error I'm getting or does it now work for you? @maxsmythe It sure seems to me there's something different between your environment and mine. If you look back in this thread at the comments from @srenatus and @ritazh there was some discussion of which version of OPA was being included (pulled? loaded?) in the latest release of Gatekeeper. Is it possible that you already have a new version of OPA somehow "available" that Gatekeeper is using, and that's why it's working for you and not for me? Anyway, I still can't get this to work. Should we re-open this issue? Thanks, |
Adding @ritazh as an assignee since this could be a helm issue |
@asm2git This is weird. Can you post the image sha running on the pod? It should be in the pod status. That will be less ambiguous than label. For me, kind should be pulling directly from Docker (but it should also be doing that for you), so I'm not sure why we'd be getting different images. |
apply.gatekeeper-controller-manager.pod.yaml.txt
I confirmed that I get the same image and imageID whether I use "helm install" or "kubectl apply -f https://raw.githubusercontent.com/..." I also confirmed that I am still getting the same failure as I posted last week. Please let me know if there's any additional information you need or if you need me to run any other experiments. If necessary I might be able to arrange for you to get access to my external instance. |
This is very weird. I have the exact same image sha. Everything points to an image mismatch, however. e.g. if I have a "bad import" error, here is what happens for me:
but if I make it a verbose request, I get this:
It might be worth setting up a video call to see this in real time. Not sure why there is a version mismatch (Rego version is hardlinked in to Gatekeeper), but everything is pointing to that. |
I'm happy to do a video call. I'm in US/Eastern, typically working 0830 to 1600 (UTC 1230 to 2000) but I can do a call as late as 2000 (UTC 0000) if that's better for you. Monday, Wednesday, and Friday are better for me. It might be better if we do this on my personal instance (Debian 11). I'll work on recreating that, then I can snapshot it and just bring it up when we're ready for the call. |
I've started getting the same errors as you. Not sure why I wasn't before. Looking into it, the "from future" stuff doesn't work as of 3.8.0, but on newer commits. If you wanted to play around with it, this image should work:
Sorry for the confusion, not sure why I wasn't seeing the errors before. Maybe the webhook hadn't spun up before I created the constraint template the first time? In any case, we'd need to cut a release if you want it on a non-commit-hash-based tag. |
@maxsmythe Still no joy :-( Installed with: helm install gatekeeper gatekeeper/gatekeeper --namespace gatekeeper-system --set "image.release=ecf6092" --set "postInstall.labelNamespace.image.tag=ecf6092" Error when applying bad ConstraintTemplate: Error from server: error when creating "policy/test-import/template.yaml": admission webhook "validation.gatekeeper.sh" denied the request: invalid ConstraintTemplate: invalid import: bad import Image check: "kubectl get deployment -n gatekeeper-system gatekeeper-controller-manager -oyaml | grep image" gives "image: openpolicyagent/gatekeeper:ecf6092" I haven't tried this yet on my outside instance but I have less reason to believe this is due to any of our proxying or caching. Please let me know what you'd like me to try next, or if you want to go ahead with setting up a call. |
Sigh, my version struggle bus continues. I think the dev tag was impacted by: This time I've verified the following image should work. If it doesn't work, we should set up a call for Monday (giving us Friday to find a time):
|
@maxsmythe My tests pass with ffea423. Any chance of getting a 3.8.2 release that includes this fix? If so, roughly how long would that take? Thanks again, |
Whew, glad we sorted this out! @sozercan Would we cut this as 3.8.2? What would be the barriers to releasing this? |
@sozercan @maxsmythe Any word on a new release? |
@sozercan any barriers to starting a new release chain incorporating everything in main? |
Per community call, we have decided to cut v3.9.0-beta.1 to include this change. |
Release v3.9.0-beta.1 cut! https://github.com/open-policy-agent/gatekeeper/releases/tag/v3.9.0-beta.1 |
And 3.8.2 will be created as well? |
Looking at v3.8.0 and v3.8.0-rc.1, both include #1968, @maxsmythe from your tests do you know why v3.8.0 and v3.8.0-rc.1 are missing the OPA upgrade? Or was the change introduced in a different commit that came after #1968? |
There was a frameworks change to regorewriter that was necessary in order to recognize the new keyword |
Do you mind point me to the PR/commit where this change was made? |
@ritazh open-policy-agent/frameworks#217 GK 3.8.0 pins frameworks to https://github.com/open-policy-agent/frameworks/releases/tag/v0.5.0 |
As discussed on 8/3/3022 community call, this has been addressed by open-policy-agent/frameworks#217 and is part of GK v3.9.0. |
What steps did you take and what happened:
After using konstraint to create a ConstraintTemplate YAML file, then doing "helm install" to install the file, I got the following error:
What did you expect to happen:
I expected there not to be any errors. https://www.openpolicyagent.org/docs/latest/policy-language/#membership-and-iteration-in documents that Opa uses the notation "import future.keywords" to allow new constructs, in this case, the "in" keyword. That link also shows a sample Rego file that uses the "in" keyword.
Anything else you would like to add:
In vendor/github.com/open-policy-agent/opa/ast/parser.go I see code for WithFutureKeywords(), WithAllFutureKeywords(), futureParser(), and "var futureKeywords" so maybe this is just missing documentation, in that there's a Helm value or a command line arg that enables "import future..."? I've searched all the code and the whole Helm chart and if such a flag or option or parameter is there, I can't find it (maybe because I don't know exactly what to look for?).
Environment:
kubectl version
): 1.20.2The text was updated successfully, but these errors were encountered: