Guardrails policies (M1) (#2970)

* Revert "temporarily remove policies other than the machine one as the example and test policy to create a base code pr" This reverts commit 08d377d. * extracted shared rego resources to a separate lib * improvement: rego unit test and gator test polishing (#2767) * rego unit test and gator test polishing * lint fix * rego lint fix * adjusted user id related judgement plus match kinds for resources other than pod * added test cases for priv'd ns to cover pull-secret deletion * add new policy for machine config modification (#2879) * add new policy for machine config modification * reformat yaml * revise api group logic * added pod host path policy * dont run guardrails if a standard gatekeeper instance is already started * comment out corresponding gator tests as r/w PV check is temporarily removed * satisfy mega linter * temporarily backoff the standard gatekeeper check * enable standard gatekeeper check with proper test case modifications * comment out non-namespaced resources * add k8s specific namespaces to the priv'd list * update README plus add two SA to allowed list * update Guardrails README * a typo in README * allow policies to enforce on openshift-azure-guardrails namespace * added group support for user validation * update: Guardrail policy scripts and doc updates (#2941) * update generate.sh to support single dir gen * update scripts to support params * update README * added usage print for scripts * change to flexible mode for username, group and SA name validation * update get func to print more debug info * rely solely on userInfo for user authentication * extend audit-interval to slow down the audit run, plus display more violations * roll back a temp change for local test * dont allow updates for machine and machineset * removed MachineSet * unified the constraint filename and resource name to make the config easier * adjust constraint and template name and kind as per convention * update gatekeeper params, affinity and tolerations * log violations * white list more user and group * extend priv'd ns protection to ns itself * add guardrails policy generate entry in makefile * make gator in README lower cased to keep consistent with official doc --------- Co-authored-by: Arris Li <[email protected]>
Azure · Jul 18, 2023 · b06512a · b06512a
1 parent a37c544
commit b06512a
Show file tree

Hide file tree

Showing 53 changed files with 2,556 additions and 72 deletions.
diff --git a/Makefile b/Makefile
@@ -90,6 +90,9 @@ discoverycache:
 generate:
 	go generate ./...
 
+generate-guardrails:
+	cd pkg/operator/controllers/guardrails/policies && ./scripts/generate.sh > /dev/null
+
 image-aro: aro e2e.test
 	docker pull $(REGISTRY)/ubi8/ubi-minimal
 	docker build --platform=linux/amd64 --network=host --no-cache -f Dockerfile.aro -t $(ARO_IMAGE) --build-arg REGISTRY=$(REGISTRY) .

diff --git a/pkg/operator/controllers/guardrails/guardrails_controller.go b/pkg/operator/controllers/guardrails/guardrails_controller.go
@@ -74,14 +74,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	// If enabled and managed=false, remove the GuardRails deployment
 	// If enabled and managed is missing, do nothing
 	if strings.EqualFold(managed, "true") {
+		// Check if standard GateKeeper is already running
+		if running, err := r.deployer.IsReady(ctx, "gatekeeper-system", "gatekeeper-audit"); err != nil && running {
+			r.log.Warn("standard GateKeeper is running, skipping Guardrails deployment")
+			return reconcile.Result{}, nil
+		}
+
 		// Deploy the GateKeeper manifests and config
 		deployConfig := r.getDefaultDeployConfig(ctx, instance)
 		err = r.deployer.CreateOrUpdate(ctx, instance, deployConfig)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 
-		// Check gatekeeper has become ready, wait up to readinessTimeout (default 5min)
+		// Check Gatekeeper has become ready, wait up to readinessTimeout (default 5min)
 		timeoutCtx, cancel := context.WithTimeout(ctx, r.readinessTimeout)
 		defer cancel()
 

diff --git a/pkg/operator/controllers/guardrails/guardrails_controller_test.go b/pkg/operator/controllers/guardrails/guardrails_controller_test.go
@@ -70,6 +70,7 @@ func TestGuardRailsReconciler(t *testing.T) {
 					MutatingWebhookFailurePolicy:   "Ignore",
 				}
 				md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, expectedConfig).Return(nil)
+				md.EXPECT().IsReady(gomock.Any(), "gatekeeper-system", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().IsReady(gomock.Any(), "wonderful-namespace", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().IsReady(gomock.Any(), "wonderful-namespace", "gatekeeper-controller-manager").Return(true, nil)
 			},
@@ -98,6 +99,7 @@ func TestGuardRailsReconciler(t *testing.T) {
 					MutatingWebhookFailurePolicy:   "Ignore",
 				}
 				md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, expectedConfig).Return(nil)
+				md.EXPECT().IsReady(gomock.Any(), "gatekeeper-system", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().IsReady(gomock.Any(), "openshift-azure-guardrails", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().IsReady(gomock.Any(), "openshift-azure-guardrails", "gatekeeper-controller-manager").Return(true, nil)
 			},
@@ -127,6 +129,7 @@ func TestGuardRailsReconciler(t *testing.T) {
 					MutatingWebhookFailurePolicy:   "Ignore",
 				}
 				md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, expectedConfig).Return(nil)
+				md.EXPECT().IsReady(gomock.Any(), "gatekeeper-system", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().IsReady(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil)
 			},
 			wantErr: "GateKeeper deployment timed out on Ready: timed out waiting for the condition",
@@ -139,6 +142,7 @@ func TestGuardRailsReconciler(t *testing.T) {
 				controllerPullSpec: "wonderfulPullspec",
 			},
 			mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) {
+				md.EXPECT().IsReady(gomock.Any(), "gatekeeper-system", "gatekeeper-audit").Return(true, nil)
 				md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, gomock.AssignableToTypeOf(&config.GuardRailsDeploymentConfig{})).Return(errors.New("failed ensure"))
 			},
 			wantErr: "failed ensure",

diff --git a/pkg/operator/controllers/guardrails/policies/README.md b/pkg/operator/controllers/guardrails/policies/README.md
@@ -31,54 +31,53 @@ Constraint is manually created
 apiVersion: templates.gatekeeper.sh/v1
 kind: ConstraintTemplate
 metadata:
-  name: aroprivilegednamespace
+  name: arodenyprivilegednamespace
   annotations:
     metadata.gatekeeper.sh/title: "Privileged Namespace"
     metadata.gatekeeper.sh/version: 1.0.0
     description: >-
       Disallows creating, updating or deleting resources in privileged namespaces.
-      including, ["^kube.*|^openshift.*|^default$|^redhat.*|^com$|^io$|^in$"]
 spec:
   crd:
     spec:
       names:
-        kind: AROPrivilegedNamespace
+        kind: ARODenyPrivilegedNamespace
       validation:
         # Schema for the `parameters` field
         openAPIV3Schema:
           type: object
           description: >-
             Disallows creating, updating or deleting resources in privileged namespaces.
-            including, ["^kube.*|^openshift.*|^default$|^redhat.*|^com$|^io$|^in$"]
   targets:
     - target: admission.k8s.gatekeeper.sh
       rego: |
 {{ file.Read "gktemplates-src/aro-deny-privileged-namespace/src.rego" | strings.Indent 8 | strings.TrimSuffix "\n" }}
+      libs:
+        - |
+{{ file.Read "gktemplates-src/library/common.rego" | strings.Indent 10 | strings.TrimSuffix "\n" }}
+
 ```
 
 
 * Create the src.rego file in the same folder, howto https://www.openpolicyagent.org/docs/latest/policy-language/, example:
 ```
-package aroprivilegednamespace
-
-violation[{"msg": msg, "details": {}}] {
-  input_priv_namespace(input.review.object.metadata.namespace)
-  msg := sprintf("Operation in privileged namespace %v is not allowed", [input.review.object.metadata.namespace])
-}
-
-input_priv_namespace(ns) {
-  any([ns == "default",
-  ns == "com",
-  ns == "io",
-  ns == "in",
-  startswith(ns, "openshift"),
-  startswith(ns, "kube"),
-  startswith(ns, "redhat")])
+package arodenyprivilegednamespace
+
+import data.lib.common.is_priv_namespace
+import data.lib.common.is_exempted_account
+import data.lib.common.get_username
+
+violation[{"msg": msg}] {
+  ns := input.review.object.metadata.namespace
+  is_priv_namespace(ns)
+  not is_exempted_account(input.review)
+  username := get_username(input.review)
+  msg := sprintf("user %v not allowed to operate in namespace %v", [username, ns])
 }
 ```
 * Create src_test.rego for unit tests in the same foler, which will be called by test.sh, howto https://www.openpolicyagent.org/docs/latest/policy-testing/, example:
 ```
-package aroprivilegednamespace
+package arodenyprivilegednamespace
 
 test_input_allowed_ns {
   input := { "review": input_ns(input_allowed_ns) }
@@ -87,7 +86,7 @@ test_input_allowed_ns {
 }
 
 test_input_disallowed_ns1 {
-  input := { "review": input_review(input_disallowed_ns1) }
+  input := { "review": input_ns(input_disallowed_ns1) }
   results := violation with input as input
   count(results) == 1
 }
@@ -111,7 +110,7 @@ input_disallowed_ns1 = "openshift-config"
 
 ```yaml
 apiVersion: constraints.gatekeeper.sh/v1beta1
-kind: AROPrivilegedNamespace
+kind: ARODenyPrivilegedNamespace
 metadata:
   name: aro-privileged-namespace-deny
 spec:
@@ -137,7 +136,9 @@ spec:
         kinds: ["PodDisruptionBudget"]
 ```
 
-## Test the rego
+Make sure the filename of constraint is the same as the .metadata.name of the Constraint object, as it is the feature flag name that will be used to turn on / off the policy.
+
+## Test Rego source code
 
 * install opa cli, refer https://github.com/open-policy-agent/opa/releases/
 
@@ -150,16 +151,24 @@ spec:
 
 * install gomplate which is used by generate.sh, see https://docs.gomplate.ca/installing/
 
-* execute generate.sh under policies, which will generate the acutal Constraint Templates under gktemplates folder, example:
+* execute generate.sh under policies, which will generate the acutal Constraint Templates to gktemplates folder, example:
+
 
   ```sh
+  # Generate all the Constraint Templates
   ARO-RP/pkg/operator/controllers/guardrails/policies$ ./scripts/generate.sh 
   Generating gktemplates/aro-deny-delete.yaml from gktemplates-src/aro-deny-delete/aro-deny-delete.tmpl
   Generating gktemplates/aro-deny-privileged-namespace.yaml from gktemplates-src/aro-deny-privileged-namespace/aro-deny-privileged-namespace.tmpl
   Generating gktemplates/aro-deny-labels.yaml from gktemplates-src/aro-deny-labels/aro-deny-labels.tmpl
   ```
 
-## gator test
+  ```sh
+  # Generate a specific Constraint Template by providing the specific policy directory under gktemplates-src folder
+  ARO-RP/pkg/operator/controllers/guardrails/policies$ ./scripts/generate.sh aro-deny-machine-config
+  Generating gktemplates/aro-deny-machine-config.yaml from gktemplates-src/aro-deny-machine-config/aro-deny-machine-config.tmpl
+  ```
+
+## Test policy with gator
 
 Create suite.yaml and testcases in gator-test folder under the folder created for the new policy, refer example below:
 
@@ -171,7 +180,7 @@ metadata:
 tests:
 - name: privileged-namespace
   template: ../../gktemplates/aro-deny-privileged-namespace.yaml
-  constraint: ../../gkconstraints-test/aro-priv-ns-operations.yaml
+  constraint: ../../gkconstraints-test/aro-privileged-namespace-deny.yaml
   cases:
   - name: ns-allowed-pod
     object: gator-test/ns_allowed_pod.yaml
@@ -212,25 +221,54 @@ spec:
           cpu: "100m"
           memory: "30Mi"
 ```
-the assertions section is the expected result
+the `assertions` section is the expected result
 
-gator test is done via cmd:
+gator test can be done via cmd:
 
 test.sh executes both opa test and gator verify
 ```sh
+# Run tests for all the policies
 ARO-RP/pkg/operator/controllers/guardrails/policies$ ./scripts/test.sh
 ```
+```sh
+# Run tests for a specific policy
+# Providing the policy directory under gktemplates-src folder and the correspondent Constraint file under gkconstraints folder
+ARO-RP/pkg/operator/controllers/guardrails/policies$ ./scripts/test.sh aro-deny-machine-config aro-machine-config-deny.yaml
+```
 
 or below cmd after test.sh has been executed:
 ```sh
 gator verify . [-v] #-v for verbose
 ```
 
-It is now good to test your policy on a real cluster.
+Sometimes we need to mock kube admission review request especially as gator test inputs when verifying policies that check specific operations (e.g., CREATE, DELETE or UPDATE).
 
+Please refer the yaml file below as a sample of kube admission review request:
 
+```yaml
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1
+request:
+  uid: d700ab7f-8f42-45ff-83f5-782c739806d9
+  operation: UPDATE
+  userInfo:
+    username: kube-review
+    uid: 45884572-1cab-49e5-be4c-1d2eb0299776
+  object:
+    kind: MachineConfig
+    apiVersion: machineconfiguration.openshift.io/v1
+    metadata:
+      name: 99-worker-generated-crio-fake
+  oldObject:
+    kind: MachineConfig
+    apiVersion: machineconfiguration.openshift.io/v1
+    metadata:
+      name: 99-worker-generated-crio-seccomp-use-default
+  dryRun: true
+```
+A tool [admr-gen](https://github.com/ArrisLee/admr-gen) has been created and can be utilized to generate mocked kube admission review requests in an easier way.
 
-## Enable your policy on a dev cluster
+## Enable and test your policy on a dev cluster
 
 Set up local dev env following “Deploy development RP” section if not already: https://github.com/Azure/ARO-RP/blob/master/docs/deploy-development-rp.md
 
@@ -284,7 +322,6 @@ Verify ConstraintTemplate is created
 $ oc get constrainttemplate
 NAME            AGE
 arodenylabels   20h
-$ oc get constraint
 ```
 
 
@@ -302,4 +339,4 @@ NAME                ENFORCEMENT-ACTION   TOTAL-VIOLATIONS
 aro-machines-deny   deny
 ```
 
-Once the constraint is created, you are all good to rock with your policy!
+Once the constraint is created, you are all good to rock with your policy!
diff --git a/pkg/operator/controllers/guardrails/policies/gkconstraints/aro-machine-config-deny.yaml b/pkg/operator/controllers/guardrails/policies/gkconstraints/aro-machine-config-deny.yaml
@@ -0,0 +1,10 @@
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: ARODenyMachineConfig
+metadata:
+  name: aro-machine-config-deny
+spec:
+  enforcementAction: {{.Enforcement}}
+  match:
+    kinds:
+      - apiGroups: ["machineconfiguration.openshift.io"]
+        kinds: ["MachineConfig"]
diff --git a/...perator/controllers/guardrails/policies/gkconstraints/aro-master-toleration-pod-deny.yaml b/...perator/controllers/guardrails/policies/gkconstraints/aro-master-toleration-pod-deny.yaml
@@ -0,0 +1,10 @@
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: ARODenyMasterTolerationTaints
+metadata:
+  name: aro-master-toleration-pod-deny
+spec:
+  enforcementAction: {{.Enforcement}}
+  match:
+    kinds:
+      - apiGroups: [""]
+        kinds: ["Pod"]
diff --git a/...operator/controllers/guardrails/policies/gkconstraints/aro-privileged-namespace-deny.yaml b/...operator/controllers/guardrails/policies/gkconstraints/aro-privileged-namespace-deny.yaml
@@ -0,0 +1,35 @@
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: ARODenyPrivilegedNamespace
+metadata:
+  name: aro-privileged-namespace-deny
+spec:
+  enforcementAction: {{.Enforcement}}
+  match:
+    kinds:
+      - apiGroups: [""]
+        kinds: [
+        "Pod",
+        "Secret",
+        "Service",
+        "ServiceAccount",
+        "ReplicationController",
+        "ResourceQuota",
+        "Namespace",
+        ]
+      - apiGroups: ["apps"]
+        kinds: ["Deployment", "ReplicaSet", "StatefulSet", "DaemonSet"]
+      - apiGroups: ["batch"]
+        kinds: ["Job", "CronJob"]
+      - apiGroups: ["rbac.authorization.k8s.io"]
+        kinds: ["Role", "RoleBinding"]
+      - apiGroups: ["policy"]
+        kinds: ["PodDisruptionBudget"]
+      - apiGroups: ["machine.openshift.io"]
+        kinds: ["Machine"]
+      # non-namespaced resources
+      # - apiGroups: [""]
+      #   kinds: ["PersistentVolume", "PersistentVolumeClaim"]
+      # - apiGroups: ["rbac.authorization.k8s.io"]
+      #   kinds: ["ClusterRole", "ClusterRoleBinding"]
+      # - apiGroups: ["apiextensions"]
+      #   kinds: ["CustomResourceDefinition"]
diff --git a/pkg/operator/controllers/guardrails/policies/gkconstraints/aro-rw-host-mount-deny.yaml b/pkg/operator/controllers/guardrails/policies/gkconstraints/aro-rw-host-mount-deny.yaml
@@ -0,0 +1,11 @@
+apiVersion: constraints.gatekeeper.sh/v1beta1
+kind: ARODenyHostMount
+metadata:
+  name: aro-rw-host-mount-deny
+spec:
+  enforcementAction: {{.Enforcement}}
+  match:
+    kinds:
+      - apiGroups: [""]
+        # kinds: ["Pod", "PersistentVolume"]
+        kinds: ["Pod"] # disable PV check as it is not agreed, need revise
diff --git a/...trollers/guardrails/policies/gktemplates-src/aro-deny-host-mount/aro-deny-host-mount.tmpl b/...trollers/guardrails/policies/gktemplates-src/aro-deny-host-mount/aro-deny-host-mount.tmpl
@@ -0,0 +1,27 @@
+apiVersion: templates.gatekeeper.sh/v1
+kind: ConstraintTemplate
+metadata:
+  name: arodenyhostmount
+  annotations:
+    metadata.gatekeeper.sh/title: "Host Mount"
+    metadata.gatekeeper.sh/version: 1.0.0
+    description: >-
+      To prevent the creation of non-OpenShift pods with dangerous read/write mounts
+spec:
+  crd:
+    spec:
+      names:
+        kind: ARODenyHostMount
+      validation:
+        # Schema for the `parameters` field
+        openAPIV3Schema:
+          type: object
+          description: >-
+            To prevent the creation of non-OpenShift pods with dangerous read/write mounts
+  targets:
+    - target: admission.k8s.gatekeeper.sh
+      rego: |
+{{ file.Read "gktemplates-src/aro-deny-host-mount/src.rego" | strings.Indent 8 | strings.TrimSuffix "\n" }}
+      libs:
+        - |
+{{ file.Read "gktemplates-src/library/common.rego" | strings.Indent 10 | strings.TrimSuffix "\n" }}