feat(*): automate policy generation #4197
Conversation
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Codecov Report
@@ Coverage Diff @@
## master #4197 +/- ##
==========================================
- Coverage 55.82% 55.65% -0.18%
==========================================
Files 931 938 +7
Lines 56485 56654 +169
==========================================
- Hits 31534 31531 -3
- Misses 22435 22606 +171
- Partials 2516 2517 +1
Continue to review full report at Codecov.
|
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
| ``` | ||
|
|
||
| 2. Create a proto file for new policy in `pkg/plugins/policies/donothingpolicy/api/v1alpha1`. For example | ||
| donothingpolicy.proto: |
There was a problem hiding this comment.
maybe best just to link to the file
|
|
||
| # we don't want expressions to be expanded with yq, that's why we're intentionally using single quotes | ||
| # shellcheck disable=SC2016 | ||
| yq e '.spec.versions[] | select (.name == "'"${VERSION}"'") | .schema.openAPIV3Schema.properties.spec | del(.type) | del(.description)' \ |
There was a problem hiding this comment.
May be my bash skills but this is an effort to read IMO, isn't ${VERSION} still expanded here since it's outside of the single quotes? Is this different from:
".spec.versions[] | select (.name == \"${VERSION}\") | .schema.openAPIV3Schema.properties.spec | del(.type) | del(.description)"
?
There was a problem hiding this comment.
I tried the option you proposed, but for some reason, it doesn't work. It just ignores escaping (debug mode output):
+ yq e '.spec.versions[] | select (.name == \"${VERSION}\") | .schema.openAPIV3Schema.properties.spec | del(.type) | del(.description)' pkg/plugins/policies/donothingpolicy/k8s/crd/kuma.io_donothingpolicies.yaml
+ yq eval-all -i '. as $item ireduce ({}; . * $item )' pkg/plugins/policies/donothingpolicy/api/v1alpha1/schema.yaml -
Error: parsing expression: Lexer error: could not match text starting at 1:37 failing at 1:38.
unmatched text: "\\"
I can leave a comment about why we need "'":
- the first double quote is needed because
yq selectexpects something likeselect (.name == "v1alpha1") - single quote is required to close the first single quote
- now when we outside of single quotes we simply use bash expression
"${VERSION}", that's why the last double quote is here
|
|
||
| cp "${SCHEMA_TEMPLATE}" "${POLICIES_API_DIR}"/schema.yaml | ||
|
|
||
| if [ "$(find "${POLICIES_CRD_DIR}" -type f | wc -l | xargs echo)" != 1 ]; then |
There was a problem hiding this comment.
| if [ "$(find "${POLICIES_CRD_DIR}" -type f | wc -l | xargs echo)" != 1 ]; then | |
| if [ "$(find "${POLICIES_CRD_DIR}" -type f | wc -l)" != 1 ]; then |
Doesn't wc -l already return one line with one number?
There was a problem hiding this comment.
I don't know why but it's \t1 instead of just 1, I didn't find another way to get rid of the tab
| ) | ||
|
|
||
| // CustomResourceTemplate for creating a Kubernetes CRD to wrap a Kuma resource. | ||
| var CustomResourceTemplate = template.Must(template.New("custom-resource").Parse(` |
There was a problem hiding this comment.
Dn't we already generate CRDs from Kuma resources? @parkanzky is that what you meant?
There was a problem hiding this comment.
Yes, we do in tools/resource-gen/main.go, but it doesn't make sense to reuse the template for 2 reasons:
- templates are different, because
Specfields are different - we want to get rid of
tools/resource-gen/main.goin the future because instead of accepting proto as an input, it imports all protos and generates resources for everything at once. It won't work well if you haveprotofiles inside plugins directories
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
There was a problem hiding this comment.
This is a great start IMO.
Things that are missing (but can be done in a follow up PR):
- script to ease bootstrapping a new polciy
- way to combine all openAPI spec with a root one with the other apis
- how to actually do something with this polciy (hooks)
- did we agree of having CRDs namespace bound?
- did we agree on moving
meshto be a label and not a field?
| description: Kuma API | ||
|
|
||
| paths: | ||
| /meshes/{mesh}/{{ WsPath }}/{name}: |
There was a problem hiding this comment.
Would it be possible to nest the new endpoints under /policies or something like this? We're now in a place where we can add a lot of different paths at the root. It seems dangerous to have something on /
There was a problem hiding this comment.
Maybe good time to start prefix it with version?
There was a problem hiding this comment.
You mean http API version?
There was a problem hiding this comment.
Maybe though we'd still need a policy prefix or equivalent right?
There was a problem hiding this comment.
btw, I didn't touch the code that generates API endpoints, so simple change of templates/endpoints.yaml is not enough. I'd keep this change until we implement first policy using generators
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com> Signed-off-by: Paul Parkanzky <paul.parkanzky@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Summary
Introduce
protoc-gen-kumapolicyand the number of Makefile targets to generate the following resources:Issues resolved
N/A
Documentation
Testing
Backwards compatibility
- [ ] UpdateUPGRADE.mdwith any steps users will need to take when upgrading.- [ ] Addbackport-to-stablelabel if the code follows our backporting policy