feat!: cli attributes normalization

* label is always required on config map to be picked up as policy or data * confusing polcies renamed to namespaces * introduce configuration for data label * enable-data is true by default Signed-off-by: Ievgenii Shepeliuk <[email protected]>
open-policy-agent · May 6, 2022 · 1aacf5f · 1aacf5f
1 parent fc5e99e
commit 1aacf5f
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 40 deletions.
diff --git a/charts/opa/templates/deployment.yaml b/charts/opa/templates/deployment.yaml
@@ -161,29 +161,26 @@ spec:
 {{ toYaml .Values.mgmt.extraEnv | indent 12 }}
 {{- end }}
           resources:
-{{ toYaml .Values.mgmt.resources | indent 12 }}
+            {{ toYaml .Values.mgmt.resources | indent 12 }}
           args:
-{{- if .Values.authz.enabled }}
+            {{- if .Values.authz.enabled }}
             - --opa-auth-token-file=/bootstrap/mgmt-token
-{{- end }}
+            {{- end }}
             - --opa-url={{ .Values.useHttps | ternary "https" "http" }}://127.0.0.1:{{ .Values.port }}/v1
             - --opa-allow-insecure
-            - --replicate-path={{ .Values.mgmt.replicate.path }}
+            - --namespaces={{ coalesce .Values.mgmt.namespaces (list .Release.Namespace) | join "," }}
             - --enable-data={{ .Values.mgmt.data.enabled }}
-            - --enable-policies={{ .Values.mgmt.configmapPolicies.enabled }}
-{{- if .Values.mgmt.configmapPolicies.enabled }}
-            - --policies={{ coalesce .Values.mgmt.configmapPolicies.namespaces (list .Release.Namespace) | join "," }}
-            - --require-policy-label={{ .Values.mgmt.configmapPolicies.requireLabel }}
-{{- end }}
-{{- range .Values.mgmt.replicate.namespace }}
+            - --enable-policies={{ .Values.mgmt.policies.enabled }}
+            - --replicate-path={{ .Values.mgmt.replicate.path }}
+            {{- range .Values.mgmt.replicate.namespace }}
             - --replicate={{ . }}
-{{- end }}
-{{- range .Values.mgmt.replicate.cluster }}
+            {{- end }}
+            {{- range .Values.mgmt.replicate.cluster }}
             - --replicate-cluster={{ . }}
-{{- end }}
-{{- range .Values.mgmt.extraArgs }}
+            {{- end }}
+            {{- range .Values.mgmt.extraArgs }}
             - {{ . }}
-{{- end }}
+            {{- end }}
 {{- if or .Values.authz.enabled .Values.bootstrapPolicies }}
           volumeMounts:
             - name: bootstrap

diff --git a/charts/opa/values.yaml b/charts/opa/values.yaml
@@ -146,17 +146,14 @@ mgmt:
   extraArgs: []
   extraEnv: []
   resources: {}
+  namespaces: []
   data:
     enabled: true
-  configmapPolicies:
-# NOTE IF you use these, remember to update the RBAC rules below to allow
-#      permissions to get, list, watch, patch and update configmaps
+  policies:
     enabled: true
-    namespaces: []
-    requireLabel: true
+  # NOTE IF you use these, remember to update the RBAC rules below to allow
+  #      permissions to replicate these things
   replicate:
-# NOTE IF you use these, remember to update the RBAC rules below to allow
-#      permissions to replicate these things
     cluster: []
 #     - [group/]version/resource
     namespace: []

diff --git a/cmd/kube-mgmt/main.go b/cmd/kube-mgmt/main.go
@@ -38,12 +38,13 @@ type params struct {
 	opaAllowInsecure   bool
 	policyLabel        string
 	policyValue        string
+	dataLabel           string
+	dataValue           string
 	podName            string
 	podNamespace       string
 	enablePolicies     bool
 	enableData         bool
-	policies           []string
-	requirePolicyLabel bool
+	namespaces          []string
 	replicateCluster   gvkFlag
 	replicateNamespace gvkFlag
 	replicatePath      string
@@ -82,12 +83,13 @@ func main() {
 	rootCmd.Flags().StringVarP(&params.podNamespace, "pod-namespace", "", "", "set pod namespace (required for admission registration ownership)")
 	rootCmd.Flags().StringVarP(&params.policyLabel, "policy-label", "", "openpolicyagent.org/policy", "replace label openpolicyagent.org/policy")
 	rootCmd.Flags().StringVarP(&params.policyValue, "policy-value", "", "rego", "replace value rego")
+	rootCmd.Flags().StringVarP(&params.dataLabel, "data-label", "", "openpolicyagent.org/data", "replace label openpolicyagent.org/data")
+	rootCmd.Flags().StringVarP(&params.dataValue, "data-value", "", "opa", "replace value opa")
 
 	// Replication options.
-	rootCmd.Flags().BoolVarP(&params.enablePolicies, "enable-policies", "", true, "whether to automatically discover policies from ConfigMaps")
-	rootCmd.Flags().BoolVarP(&params.enableData, "enable-data", "", false, "whether to automatically discover data from correctly labelled ConfigMaps")
-	rootCmd.Flags().StringSliceVarP(&params.policies, "policies", "", []string{"opa", "kube-federation-scheduling-policy"}, "automatically load policies from these namespaces")
-	rootCmd.Flags().BoolVarP(&params.requirePolicyLabel, "require-policy-label", "", false, "only load policies out of labelled configmaps")
+	rootCmd.Flags().BoolVarP(&params.enablePolicies, "enable-policies", "", true, "whether to automatically discover policies from labelled ConfigMaps")
+	rootCmd.Flags().BoolVarP(&params.enableData, "enable-data", "", true, "whether to automatically discover data from labelled ConfigMaps")
+	rootCmd.Flags().StringSliceVarP(&params.namespaces, "namespaces", "", []string{"opa"}, "namespaces to load policies and data from")
 	rootCmd.Flags().VarP(&params.replicateNamespace, "replicate", "", "replicate namespace-level resources")
 	rootCmd.Flags().VarP(&params.replicateCluster, "replicate-cluster", "", "replicate cluster-level resources")
 	rootCmd.Flags().StringVarP(&params.replicatePath, "replicate-path", "", "kubernetes", "set path to replicate data into")
@@ -163,15 +165,16 @@ func run(params *params) {
 			kubeconfig,
 			opa.New(params.opaURL, params.opaAuth),
 			configmap.DefaultConfigMapMatcher(
-				params.policies,
-				params.requirePolicyLabel,
+				params.namespaces,
 				params.enablePolicies,
 				params.enableData,
 				params.policyLabel,
 				params.policyValue,
+				params.dataLabel,
+				params.dataValue,
 			),
 		)
-		_, err = sync.Run(params.policies)
+		_, err = sync.Run(params.namespaces)
 		if err != nil {
 			logrus.Fatalf("Failed to start configmap sync: %v", err)
 		}

diff --git a/pkg/configmap/configmap.go b/pkg/configmap/configmap.go
@@ -64,23 +64,16 @@ func CustomPolicyLabel(key, value string) (string, error) {
 // specified namespaces and/or with a policy or data label. The first bool return
 // value specifies a policy/data match and the second bool indicates if the configmap
 // contains a policy.
-func DefaultConfigMapMatcher(namespaces []string, requirePolicyLabel, enablePolicies, enableData bool, policyLabelKey, policyLabelValue string) func(*v1.ConfigMap) (bool, bool) {
+func DefaultConfigMapMatcher(namespaces []string, enablePolicies, enableData bool, policyLabelKey, policyLabelValue, dataLabelKey, dataLabelValue string) func(*v1.ConfigMap) (bool, bool) {
 	return func(cm *v1.ConfigMap) (bool, bool) {
 		var match, isPolicy bool
 
-		// Check for data label. This label needs to be set on any
-		// configmap that contains JSON data to be loaded into OPA.
 		if enableData {
 			match = matchesNamespace(cm, namespaces) && matchesLabel(cm, dataLabelKey, dataLabelValue)
 		}
 
-		// Check for explicit policy label or match on any policy namespace.
 		if !match && enablePolicies {
-			if requirePolicyLabel {
-				match = matchesNamespace(cm, namespaces) && matchesLabel(cm, policyLabelKey, policyLabelValue)
-			} else {
-				match = matchesNamespace(cm, namespaces) || matchesLabel(cm, policyLabelKey, policyLabelValue)
-			}
+			match = matchesNamespace(cm, namespaces) && matchesLabel(cm, policyLabelKey, policyLabelValue)
 
 			if match {
 				isPolicy = true

diff --git a/test/e2e/fixture-labels.yaml b/test/e2e/fixture-labels.yaml
@@ -0,0 +1,46 @@
+---
+kind: ConfigMap
+metadata:
+  name: policy-include
+  labels:
+    kube-mgmt/e2e: "true"
+    qweqwe/policy: "111"
+apiVersion: v1
+data:
+  include.rego: |
+    package example.include
+    allow := true
+--- 
+kind: ConfigMap
+metadata:
+  name: policy-exclude
+  labels:
+    kube-mgmt/e2e: "true"
+    openpolicyagent.org/policy: rego
+apiVersion: v1
+data:
+  exclude.rego: |
+    package example.exclude
+    allow := true
+---
+kind: ConfigMap
+metadata:
+  name: data-include
+  labels:
+    kube-mgmt/e2e: "true"
+    asdasd/data: "222"
+apiVersion: v1
+data:
+  include.json: |
+    {"inKey": "inValue"}
+--- 
+kind: ConfigMap
+metadata:
+  name: data-exclude
+  labels:
+    kube-mgmt/e2e: "true"
+    openpolicyagent.org/data: opa
+apiVersion: v1
+data:
+  exclude.json: |
+    {"exKey": "exValue"}
diff --git a/test/e2e/labels/test.sh b/test/e2e/labels/test.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+set -e
+set -x
+
+OPA="http :8080/v1"
+
+${OPA}/data | jq -e '.result.default//{}|keys|length==0'
+
+kubectl apply -f "$(dirname $0)/../fixture-labels.yaml"
+
+${OPA}/policies | jq -e '.result[].id=="default/policy-include/include.rego"'
+${OPA}/data/example/include/allow | jq -e '.result|true'
+
+${OPA}/data/default | jq -e '.result|keys==["data-include"]'
+${OPA}/data/default/data-include | jq -e '.result["include.json"].inKey=="inValue"'
diff --git a/test/e2e/labels/values.yaml b/test/e2e/labels/values.yaml
@@ -0,0 +1,13 @@
+useHttps: false
+
+opa: null
+
+authz:
+  enabled: false
+
+mgmt:
+  extraArgs:
+    - "--policy-label=qweqwe/policy"
+    - "--policy-value=111"
+    - "--data-label=asdasd/data"
+    - "--data-value=222"