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.

Sign up for GitHub

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

chore: adding vap testing #618

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

chore: adding vap testing #618

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8a71fc6
adding vap testing
JaydipGabani Dec 27, 2024
0a8532a
fixing vap tests
JaydipGabani Dec 28, 2024
6b836e7
waiting before creating constraints
JaydipGabani Dec 31, 2024
74f4c13
fixing test path
JaydipGabani Dec 31, 2024
4830217
Merge branch 'master' into vap-test
JaydipGabani Jan 2, 2025
aa05fd8
separating vap tests for more clarity
JaydipGabani Jan 6, 2025
792e2c2
fixing vap deployment
JaydipGabani Jan 6, 2025
69035f3
debugging VAPB
JaydipGabani Jan 7, 2025
dbafae3
debugging VAP
JaydipGabani Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/workflow.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
strategy:
matrix:
gatekeeper: [ "3.17.1", "3.18.1" ]
engine: [ "cel", "rego" ]
engine: [ "cel", "rego", "vap" ]
name: "Integration test on Gatekeeper ${{ matrix.gatekeeper }} for ${{ matrix.engine }} policies"
steps:
- name: Harden Runner
Expand All @@ -86,7 +86,7 @@ jobs:

- name: Run integration test
run: |
make test-integration
make test-integration POLICY_ENGINE=${{ matrix.engine }}

- name: Save logs
run: |
Expand Down
8 changes: 4 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ KIND_VERSION ?= 0.23.0
# note: k8s version pinned since KIND image availability lags k8s releases
KUBERNETES_VERSION ?= 1.30.0
KUSTOMIZE_VERSION ?= 4.5.5
GATEKEEPER_VERSION ?= 3.16.3
GATEKEEPER_VERSION ?= 3.18.1
BATS_VERSION ?= 1.8.2
GATOR_VERSION ?= 3.17.0
GATOR_VERSION ?= 3.18.1
GOMPLATE_VERSION ?= 3.11.6
POLICY_ENGINE ?= rego

Expand Down Expand Up @@ -36,9 +36,9 @@ deploy:
ifeq ($(POLICY_ENGINE), rego)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here that only the first engine in the template gets used for evaluation UNLESS enableK8sNativeValidation=false which ensures the subsequent non-K8sNativeValidation engine gets used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I think because we are not actually testing the error message, it's hard to tell which engine caused the violation and which enforcement point caused the failure. Not sure how hard it is to add that, could be a follow up issue/PR if you want to track it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since GK does not have fallback between engines, violations are thrown through CEL engine if it is enabled and CT has CEL. Otherwise the source of violation is rego engine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since GK does not have fallback between engines, violations are thrown through CEL engine if it is enabled and CT has CEL. Otherwise the source of violation is rego engine.

yea thats how it works today but if we ever expose priority like the issue you opened, then we need to test the actual violation message. maybe open an issue to track for future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, we may want to modify the violation message to include engine information as well. Since as of now the violation message is the same regardless of the engine used to evaluate CTs. IMO, I don't think users apart from CT authors would care about which engine is being used to enforce policies. And CT authors would only care to verify the logic written for policy, which I think can be attained with gator to test CT.

helm install -n gatekeeper-system gatekeeper gatekeeper/gatekeeper --create-namespace --version $(GATEKEEPER_VERSION) --set enableK8sNativeValidation=false
else ifeq ($(POLICY_ENGINE), cel)
ifneq ($(GATEKEEPER_VERSION), 3.15.1)
helm install -n gatekeeper-system gatekeeper gatekeeper/gatekeeper --create-namespace --version $(GATEKEEPER_VERSION) --set enableK8sNativeValidation=true
endif
else ifeq ($(POLICY_ENGINE), vap)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these, since Gatekeeper webhook is a fallback for VAP, how do we ensure the failure resulted from VAP instead of the GK webhook?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check the violation string to make sure it includes ValidatingAdmissionPolicy - https://github.com/open-policy-agent/gatekeeper-library/pull/618/files#diff-3b0266d06db240d302bf9febd17c09cf104dccce0bb2ee8e799e49c85f7082ffR96.

helm install -n gatekeeper-system gatekeeper gatekeeper/gatekeeper --create-namespace --version $(GATEKEEPER_VERSION) --set enableK8sNativeValidation=true --set defaultCreateVAPForTemplates=true --set defaultCreateVAPBindingForConstraints=true
endif

uninstall:
Expand Down
16 changes: 13 additions & 3 deletions test/bats/test.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,27 @@ setup() {
if [ -d "$policy" ]; then
local policy_group=$(basename "$(dirname "$policy")")
local template_name=$(basename "$policy")
deny_substr="denied the request"
echo "running integration test against policy group: $policy_group, constraint template: $template_name"
# apply template
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -k $policy"
local kind=$(yq e .metadata.name "$policy"/template.yaml)
local kind=$(cat "$policy"/template.yaml | yq e .metadata.name)
if [ "$POLICY_ENGINE" == "vap" ] && grep -q "engine: K8sNativeValidation" "$policy"/template.yaml; then
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicy gatekeeper-$kind"
sleep 30
deny_substr="ValidatingAdmissionPolicy"
fi
for sample in "$policy"/samples/*; do
echo "testing sample constraint: $(basename "$sample")"
# apply constraint
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -f ${sample}/constraint.yaml"
local name=$(yq e .metadata.name "$sample"/constraint.yaml)
local name=$(cat "$sample"/constraint.yaml | yq e .metadata.name)
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "constraint_enforced $kind $name"

if [ "$POLICY_ENGINE" == "vap" ] && grep -q "engine: K8sNativeValidation" "$policy"/template.yaml; then
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl get ValidatingAdmissionPolicyBinding gatekeeper-$name"
fi

for inventory in "$sample"/example_inventory*.yaml; do
if [[ -e "$inventory" ]]; then
run kubectl apply -f "$inventory"
Expand Down Expand Up @@ -123,7 +133,7 @@ setup() {
echo "Applying ${disallowed} with contents:"
cat ${disallowed}
run kubectl apply -f "$disallowed"
assert_match_either 'denied the request' 'no matches for kind' "${output}"
assert_match_either "$deny_substr" 'no matches for kind' "${output}"
assert_failure
# delete resource
run kubectl delete --ignore-not-found -f "$disallowed"
Expand Down
Loading