From 1f6cd330acc1fce1bb4dd2b3ea9fd43e2364aa50 Mon Sep 17 00:00:00 2001 From: alex <8968914+acpana@users.noreply.github.com> Date: Mon, 23 Oct 2023 16:29:38 -0700 Subject: [PATCH 1/3] docs: examples, fix:improve gator err msg (#3079) Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> --- pkg/gator/errors.go | 3 -- pkg/gator/verify/runner.go | 3 +- pkg/gator/verify/runner_test.go | 2 +- pkg/util/request_validation.go | 2 +- website/docs/gator.md | 76 +++++++++++++++++++++++++++++++-- 5 files changed, 75 insertions(+), 11 deletions(-) diff --git a/pkg/gator/errors.go b/pkg/gator/errors.go index 101fa6958b4..43d84059085 100644 --- a/pkg/gator/errors.go +++ b/pkg/gator/errors.go @@ -51,9 +51,6 @@ var ( // object or oldObject for the underlying framework to review. // This mimicks the k8s api server behvaior. ErrNoObjectForReview = errors.New("no object or oldObject found to review") - // ErrNilOldObject indicates that the AdmissionRequest did not provide an oldObject. - // Gatekeeper expects oldObject to be non nil on DELETE operations. - ErrNilOldObject = errors.New("oldObject is nil") // ErrInvalidYAML indicates that a .yaml/.yml file was not parseable. ErrInvalidYAML = errors.New("invalid yaml") // ErrUnmarshallObject happens when the yaml defines an invalid object or oldObject. diff --git a/pkg/gator/verify/runner.go b/pkg/gator/verify/runner.go index 64255204036..74b29880420 100644 --- a/pkg/gator/verify/runner.go +++ b/pkg/gator/verify/runner.go @@ -358,10 +358,9 @@ func (r *Runner) validateAndReviewAdmissionReviewRequest(ctx context.Context, c } } - // parse into webhook/admission type req := &admission.Request{AdmissionRequest: *ar.Request} if err := util.SetObjectOnDelete(req); err != nil { - return nil, fmt.Errorf("%w: %w", gator.ErrNilOldObject, err) + return nil, fmt.Errorf("%w: %w", gator.ErrInvalidK8sAdmissionReview, err) } arr := target.AugmentedReview{ diff --git a/pkg/gator/verify/runner_test.go b/pkg/gator/verify/runner_test.go index ca55fb8e74a..ab7d8374904 100644 --- a/pkg/gator/verify/runner_test.go +++ b/pkg/gator/verify/runner_test.go @@ -1155,7 +1155,7 @@ func TestRunner_Run(t *testing.T) { {Name: "invalid admission review object", Error: gator.ErrInvalidK8sAdmissionReview}, {Name: "missing admission request object", Error: gator.ErrMissingK8sAdmissionRequest}, {Name: "no objects to review", Error: gator.ErrNoObjectForReview}, - {Name: "no oldObject on delete", Error: gator.ErrNilOldObject}, + {Name: "no oldObject on delete", Error: gator.ErrInvalidK8sAdmissionReview}, }, }, { diff --git a/pkg/util/request_validation.go b/pkg/util/request_validation.go index 38ab236d98f..48b89b69012 100644 --- a/pkg/util/request_validation.go +++ b/pkg/util/request_validation.go @@ -8,7 +8,7 @@ import ( ) // nolint: revive // Moved error out of pkg/webhook/admission; needs capitalization for backwards compat. -var ErrOldObjectIsNil = errors.New("For admission webhooks registered for DELETE operations, please use Kubernetes v1.15.0+.") +var ErrOldObjectIsNil = errors.New("oldObject cannot be nil for DELETE operations") // SetObjectOnDelete enforces that we use at least K8s API v1.15.0+ on DELETE operations // and copies over the oldObject into the Object field for the given AdmissionRequest. diff --git a/website/docs/gator.md b/website/docs/gator.md index a3c691aaf19..5a0ea19abc0 100644 --- a/website/docs/gator.md +++ b/website/docs/gator.md @@ -260,14 +260,12 @@ the `run` flag: gator verify path/to/suites/... --run "disallowed" ``` -#### Validating Metadata-Based Constraint Templates +### Validating Metadata-Based Constraint Templates `gator verify` may be used with an [`AdmissionReview`](https://pkg.go.dev/k8s.io/kubernetes/pkg/apis/admission#AdmissionReview) object to test your constraints. This can be helpful to simulate a certain operation (`CREATE`, `UPDATE`, `DELETE`, etc.) or [`UserInfo`](https://pkg.go.dev/k8s.io/kubernetes@v1.25.3/pkg/apis/authentication#UserInfo) metadata. -Recall that the `input.review.user` can be accessed in the Rego code (see [Input Review](howto.md#input-review) for more guidance). -A few examples for how to structure your yaml can be found [here](https://github.com/open-policy-agent/gatekeeper/blob/03e6adb74f1714242cf936fd27eee19a0eda2d52/pkg/gator/fixtures/fixtures.go#L506-L528). -The `AdmissionReview` object can be specified where you would specify the object under test above: +Recall that the `input.review.user` can be accessed in the Rego code (see [Input Review](howto.md#input-review) for more guidance). The `AdmissionReview` object can be specified where you would specify the object under test above: ```yaml - name: both-disallowed @@ -276,6 +274,76 @@ The `AdmissionReview` object can be specified where you would specify the object - violations: 1 ``` +Example for testing the `UserInfo` metadata: + +AdmissionReview, ConstraintTemplate, Constraint: +```yaml +kind: AdmissionReview +apiVersion: admission.k8s.io/v1beta1 +request: + operation: "UPDATE" + userInfo: + username: "system:foo" + object: + kind: Pod + labels: + - app: "bar" +--- +kind: ConstraintTemplate +apiVersion: templates.gatekeeper.sh/v1 +metadata: + name: validateuserinfo +spec: + crd: + spec: + names: + kind: ValidateUserInfo + targets: + - target: admission.k8s.gatekeeper.sh + rego: | + package k8svalidateuserinfo + violation[{"msg": msg}] { + username := input.review.userInfo.username + not startswith(username, "system:") + msg := sprintf("username is not allowed to perform this operation: %v", [username]) + } +--- +kind: ValidateUserInfo +apiVersion: constraints.gatekeeper.sh/v1 +metadata: + name: always-validate +``` + +Gator Suite: +```yaml +apiVersion: test.gatekeeper.sh/v1alpha1 +kind: Suite +tests: +- name: userinfo + template: template.yaml + constraint: constraint.yaml + cases: + - name: system-user + object: admission-review.yaml + assertions: + - violations: no +``` + +Note for `DELETE` operation, the `oldObject` should be the object being deleted: + +```yaml +kind: AdmissionReview +apiVersion: admission.k8s.io/v1beta1 +request: + operation: "DELETE" + userInfo: + username: "system:foo" + oldObject: + kind: Pod + labels: + - app: "bar" +``` + Note that [`audit`](audit.md) or `gator test` are different enforcement points and they don't have the `AdmissionReview` request metadata. Run `gator verify --help` for more information. From 9f2e69f963d714562dd6b4acaf55c2c7f899c056 Mon Sep 17 00:00:00 2001 From: Davis Haba <52938648+davis-haba@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:07:43 -0700 Subject: [PATCH 2/3] fix: limit length of ExpansionTemplate names to <64 (#3078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: davis-haba Signed-off-by: Rita Zhang Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Co-authored-by: alex <8968914+acpana@users.noreply.github.com> Co-authored-by: Rita Zhang Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> --- config/crd/kustomization.yaml | 6 ++++++ ...nsiontemplate-customresourcedefinition.yaml | 4 ++++ manifest_staging/deploy/gatekeeper.yaml | 4 ++++ pkg/expansion/system.go | 3 +++ pkg/expansion/system_test.go | 18 ++++++++++++++++++ 5 files changed, 35 insertions(+) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 9d6344716ec..25c4bf07e92 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -50,6 +50,12 @@ patchesJson6902: kind: CustomResourceDefinition name: assignimage.mutations.gatekeeper.sh path: patches/max_name_size.yaml +- target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: expansiontemplate.expansion.gatekeeper.sh + path: patches/max_name_size.yaml patchesStrategicMerge: #- patches/max_name_size_for_modifyset.yaml diff --git a/manifest_staging/charts/gatekeeper/crds/expansiontemplate-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/expansiontemplate-customresourcedefinition.yaml index 9d248f2ccd2..0452edb7761 100644 --- a/manifest_staging/charts/gatekeeper/crds/expansiontemplate-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/expansiontemplate-customresourcedefinition.yaml @@ -28,6 +28,10 @@ spec: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: + properties: + name: + maxLength: 63 + type: string type: object spec: description: ExpansionTemplateSpec defines the desired state of ExpansionTemplate. diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index b68e60d2c5f..8d7153264f4 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -2311,6 +2311,10 @@ spec: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: + properties: + name: + maxLength: 63 + type: string type: object spec: description: ExpansionTemplateSpec defines the desired state of ExpansionTemplate. diff --git a/pkg/expansion/system.go b/pkg/expansion/system.go index 5287b159726..cc0a45df175 100644 --- a/pkg/expansion/system.go +++ b/pkg/expansion/system.go @@ -85,6 +85,9 @@ func ValidateTemplate(template *expansionunversioned.ExpansionTemplate) error { if k == "" { return fmt.Errorf("ExpansionTemplate has empty name field") } + if len(k) >= 64 { + return fmt.Errorf("ExpansionTemplate name must be less than 64 characters") + } if template.Spec.TemplateSource == "" { return fmt.Errorf("ExpansionTemplate %s has empty source field", k) } diff --git a/pkg/expansion/system_test.go b/pkg/expansion/system_test.go index 9eaf394b6e6..726c915e3d1 100644 --- a/pkg/expansion/system_test.go +++ b/pkg/expansion/system_test.go @@ -336,6 +336,24 @@ func TestValidateTemplate(t *testing.T) { }), errFn: matchErr("empty name"), }, + { + name: "name too long", + temp: *fixtures.NewTemplate(&fixtures.TemplateData{ + Name: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Apply: []match.ApplyTo{{ + Groups: []string{"apps"}, + Kinds: []string{"Deployment"}, + Versions: []string{"v1"}, + }}, + Source: "spec.template", + GenGVK: expansionunversioned.GeneratedGVK{ + Group: "", + Version: "v1", + Kind: "Pod", + }, + }), + errFn: matchErr("less than 64"), + }, { name: "missing source", temp: *fixtures.NewTemplate(&fixtures.TemplateData{ From c5798c98cfdda2668d797d26294e2c96c9171e84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serta=C3=A7=20=C3=96zercan?= <852750+sozercan@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:59:09 -0700 Subject: [PATCH 3/3] chore: add codeowners (#3110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sertac Ozercan Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> --- .github/CODEOWNERS | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000000..676740d8d1c --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,2 @@ +# These owners are the maintainers and approvers of this repo +* @open-policy-agent/gatekeeper-maintainers