envoyproxy / gateway

Manages Envoy Proxy as a Standalone or Kubernetes-based Application Gateway
https://gateway.envoyproxy.io
Apache License 2.0
1.46k stars 305 forks source link

Add Gateway API webhook validation functions to egctl translate #1295

Open arkodg opened 1 year ago

arkodg commented 1 year ago

Description:

Run the upstream Gateway API validation functions such as https://github.com/kubernetes-sigs/gateway-api/blob/a78e09220feec7947eb1a08b658b45f3b4182d7f/apis/v1beta1/validation/gateway.go#L54 on the inputted resources in egctl translate https://github.com/envoyproxy/gateway/blob/5019d8b598e54fddeece07bbf6de666ec0875780/internal/cmd/egctl/translate.go#L221 to catch client side validation errors and mimic the same experience the user would have had if they were applying these resources using kubectl apply

Xunzhuo commented 1 year ago

Can we have a egctl validate cmd for validating resources ?

arkodg commented 1 year ago

Can we have a hgctl validate cmd for validating resources ?

we currently solve above case using egctl translate --from gateway-api --to gateway-api but we could also create a validate alias, if users prefer validate

liangyuanpeng commented 1 year ago

I want to working for it.

/assign

Xunzhuo commented 1 year ago

Thanks @liangyuanpeng !

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

Xunzhuo commented 1 year ago

@liangyuanpeng hi, what is the status?

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

shawnh2 commented 3 months ago

This would be very helpful if we bring validations into translate, which will also help the validations in file-resources provider.

For now, some validations are implemented via CEL, maybe what we need to do here is to find a way convert all the CEL rules into validations code.

assign myself this one since I've been doing some works around file-resources provider.

zirain commented 3 months ago

For now, some validations are implemented via CEL, maybe what we need to do here is to find a way convert all the CEL rules into validations code.

can we run CEL validation locally like what we did in cel validation tests?

shawnh2 commented 3 months ago

For now, some validations are implemented via CEL, maybe what we need to do here is to find a way convert all the CEL rules into validations code.

can we run CEL validation locally like what we did in cel validation tests?

Have a discussion about this today in the meeting, it seems we cannot run CEL validation tests locally without kube-api-server support, so may need more investigation.

shawnh2 commented 3 months ago

It seems we can take https://github.com/google/cel-go as a example, and input our CEL validation rules like:

ast, issues := env.Compile(`OUR-CEL-RULE`)

and then validate the input object like:

out, details, err := prg.Eval(XXX)

So the only thing left unresolved now is how we retrieve our current CEL rules from comments.

arkodg commented 3 months ago

oh nice @shawnh2 ! we can look into kube-builder converts CEL into x-kubernetes-validations fields https://github.com/envoyproxy/gateway/blob/cc8a86e0961e9dec5f5c56422127a8950931efb3/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml#L218

shawnh2 commented 2 months ago

oh nice @shawnh2 ! we can look into kube-builder converts CEL into x-kubernetes-validations fields

https://github.com/envoyproxy/gateway/blob/cc8a86e0961e9dec5f5c56422127a8950931efb3/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml#L218

sure we can read CEL rules from x-kubernetes-validations fields in all these CRD yaml file. but if we do so, does it mean we need to include all these yaml file ?

maybe we can introduce a knob like --crd-path to indicate all the path for these files.

arkodg commented 2 months ago

we'll need to codify this, if we need access to the YAML file we can use embed to make it part of the binary

shawnh2 commented 2 months ago

Find a more suitable way to validate k8s CRD object using cel lib provided by apiextensions-apiserver. Here is an example code that works:


import (
    "context"
    "math"
    "testing"

    apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
    "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
    celschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
    "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model"
    "k8s.io/apimachinery/pkg/util/validation/field"
    "k8s.io/apimachinery/pkg/util/version"
    celconfig "k8s.io/apiserver/pkg/apis/cel"
    "k8s.io/apiserver/pkg/cel/environment"
)

func TestExprEval(t *testing.T) {
        // Here is going to be our CEL validation schema for each CRD
    objectSchema := schema.Structural{
        Generic: schema.Generic{
            Type: "object",
        },
        Properties: map[string]schema.Structural{
            "nestedObj": {
                Generic: schema.Generic{
                    Type: "object",
                },
                Properties: map[string]schema.Structural{
                    "val": {
                        Generic: schema.Generic{
                            Type: "integer",
                        },
                    },
                },
            },
        },
        Extensions: schema.Extensions{
            XValidations: apiextensions.ValidationRules{
                {
                    Rule:    "self.nestedObj.val == 10",
                    Message: "val should be equal to 10",
                },
            },
        },
    }

    env, err := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()).Extend(
        environment.VersionedOptions{
            IntroducedVersion: version.MajorMinor(1, 999),
        })
    if err != nil {
        t.Fatal(err)
    }
    loader := celschema.NewExpressionsEnvLoader()

    compilationResults, err := celschema.Compile(&objectSchema, model.SchemaDeclType(&objectSchema, false), celconfig.PerCallLimit, env, loader)
    if err != nil {
        t.Fatalf("Expected no error, but got: %v", err)
    }
    t.Log("compile ret", compilationResults)

    if len(compilationResults) != len(objectSchema.XValidations) {
        t.Fatalf("complie ret must equal xvalidations size")
    }

    celValidator := celschema.NewValidator(&objectSchema, true, celconfig.PerCallLimit)
    if celValidator == nil {
        t.Fatalf("unexpected nil cel validator")
    }

        // Here is going to be the object resource that we'd like to perform validations on
    validateObj := map[string]interface{}{
        "nestedObj": map[string]interface{}{
            "val": 10,
        },
    }

    errs, _ := celValidator.Validate(context.Background(), field.NewPath("root"), &objectSchema, validateObj, nil, math.MaxInt)
    t.Log(errs)
}

// The test pass.

// Changing the 'val' of validateObj to 6, the test fail with reason: "val should be equal to 10" (which is exactly our CEL rule msg)

BTW:


FYI: The whole implementation work will based on the idea above, post here first just for better understanding.


Refs:

  1. https://github.com/kubernetes/apiextensions-apiserver/blob/v0.30.1/pkg/apiserver/schema/cel/compilation_test.go
  2. https://github.com/kubernetes/apiextensions-apiserver/blob/v0.30.1/pkg/apiserver/schema/cel/validation_test.go
github-actions[bot] commented 3 weeks ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days.