Repository: pingcap/chaos-mesh
Pull request #661 (2020-06-29 11:30:33)

Protected namespaces
### What problem does this PR solve? <!--add and issue link with summary if exists-->
#414 
### What is changed and how does it work?

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - E2E test
 - Manual test (add detailed scripts or steps below)
 - No code

Code changes

 - Has Go code change
 - Has CI related scripts change
 - Has Terraform scripts change

Side effects

 - Breaking backward compatibility

Related changes

 - Need to update the documentation

### Does this PR introduce a user-facing change?:
<!--
If no, just leave the release note block below as is.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
NONE
```


CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/pingcap/chaos-mesh?pullRequest=661) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/pingcap/chaos-mesh?pullRequest=661) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/pingcap/chaos-mesh?pullRequest=661) it.</sub>
codecov-commenter: # [Codecov](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=h1) Report
> Merging [#661](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=desc) into [master](https://codecov.io/gh/pingcap/chaos-mesh/commit/7e9ff3f2fc5cf08fe6bd94df6929cdf48eb08504&el=desc) will **decrease** coverage by `0.08%`.
> The diff coverage is `48.43%`.

[![Impacted file tree graph](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/graphs/tree.svg?width=650&height=150&src=pr&token=a3XIzuy5Dk)](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #661      +/-   ##
==========================================
- Coverage   55.78%   55.69%   -0.09%     
==========================================
  Files          68       67       -1     
  Lines        4383     4417      +34     
==========================================
+ Hits         2445     2460      +15     
- Misses       1768     1782      +14     
- Partials      170      175       +5     
```


| [Impacted Files](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [controllers/networkchaos/ipset/ipset.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvbmV0d29ya2NoYW9zL2lwc2V0L2lwc2V0Lmdv) | `37.14% <0.00%> (ø)` | |
| [pkg/store/experiment/experiment.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-cGtnL3N0b3JlL2V4cGVyaW1lbnQvZXhwZXJpbWVudC5nbw==) | `32.20% <0.00%> (-6.58%)` | :arrow_down: |
| [pkg/utils/chaosdaemon.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-cGtnL3V0aWxzL2NoYW9zZGFlbW9uLmdv) | `51.21% <0.00%> (ø)` | |
| [pkg/utils/grpc.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-cGtnL3V0aWxzL2dycGMuZ28=) | `0.00% <0.00%> (ø)` | |
| [pkg/webhook/inject/inject.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-cGtnL3dlYmhvb2svaW5qZWN0L2luamVjdC5nbw==) | `73.49% <0.00%> (-0.68%)` | :arrow_down: |
| [controllers/twophase/types.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvdHdvcGhhc2UvdHlwZXMuZ28=) | `54.20% <40.00%> (-1.68%)` | :arrow_down: |
| [controllers/networkchaos/netem/types.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvbmV0d29ya2NoYW9zL25ldGVtL3R5cGVzLmdv) | `47.27% <50.00%> (ø)` | |
| [pkg/utils/selector.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-cGtnL3V0aWxzL3NlbGVjdG9yLmdv) | `53.15% <66.66%> (+0.91%)` | :arrow_up: |
| [controllers/podchaos/containerkill/types.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcG9kY2hhb3MvY29udGFpbmVya2lsbC90eXBlcy5nbw==) | `64.38% <100.00%> (ø)` | |
| [controllers/timechaos/types.go](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvdGltZWNoYW9zL3R5cGVzLmdv) | `61.44% <100.00%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/pingcap/chaos-mesh/pull/661/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=footer). Last update [699993e...f4ac05a](https://codecov.io/gh/pingcap/chaos-mesh/pull/661?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Gallardot: > The code looks good. Just curious about the design, do we only need one regex instead of a list of regex?

@yeya24 I think one regular expression is enough.

For example, you want chaos tasks to be performed on a namespace with either the `DEV-` prefix or the `TEST-` suffix. The `allowedNamespaces ` can be set to `TEST-\w+|\w+-DEV`.
Gallardot: /merge
zhouqiang-cl: /merge
ti-srebot: /run-all-tests

Patch:
diff --git a/helm/chaos-mesh/README.md b/helm/chaos-mesh/README.md
index 6eeff40e..5b993339 100644
--- a/helm/chaos-mesh/README.md
+++ b/helm/chaos-mesh/README.md
@@ -31,6 +31,8 @@ The following tables list the configurable parameters of the Chaos Mesh chart an
 | `controllerManager.tolerations` |  Toleration labels for chaos-controller-manager pod assignment | `[]` |
 | `controllerManager.affinity` |  Map of chaos-controller-manager node/pod affinities | `{}` |
 | `controllerManager.podAnnotations` |  Pod annotations of chaos-controller-manager | `{}`|
+| `controllerManager.allowedNamespaces` |  A regular expression, and matching namespace will allow the chaos task to be performed | ``|
+| `controllerManager.ignoredNamespaces` |  A regular expression, and the chaos task will be ignored by a matching namespace. Configuring `allowedNamespaces` at the same time will ignore this configuration. | ``|
 | `chaosDaemon.image` | docker image for chaos-daemon | `pingcap/chaos-mesh:latest` |
 | `chaosDaemon.imagePullPolicy` | image pull policy | `Always` |
 | `chaosDaemon.grpcPort` | The port which grpc server listens on | `31767` |
diff --git a/helm/chaos-mesh/templates/controller-manager-deployment.yaml b/helm/chaos-mesh/templates/controller-manager-deployment.yaml
index 6183f2db..1172f584 100644
--- a/helm/chaos-mesh/templates/controller-manager-deployment.yaml
+++ b/helm/chaos-mesh/templates/controller-manager-deployment.yaml
@@ -53,6 +53,14 @@ spec:
             value: "app.kubernetes.io/component:template"
           - name: CONFIGMAP_LABELS
             value: "app.kubernetes.io/component:webhook"
+          {{- if .Values.controllerManager.allowedNamespaces }}
+          - name: ALLOWED_NAMESPACES
+            value: {{ .Values.controllerManager.allowedNamespaces }}
+          {{- end }}
+          {{- if .Values.controllerManager.ignoredNamespaces }}
+          - name: IGNORED_NAMESPACES
+            value: {{ .Values.controllerManager.ignoredNamespaces }}
+          {{- end }}
           {{- if .Values.enableProfiling }}
           - name: PPROF_ADDR
             value: ":10081"
diff --git a/helm/chaos-mesh/values.yaml b/helm/chaos-mesh/values.yaml
index 07cca65e..e24d44b2 100644
--- a/helm/chaos-mesh/values.yaml
+++ b/helm/chaos-mesh/values.yaml
@@ -26,6 +26,9 @@ controllerManager:
   nameOverride: ""
   fullnameOverride: ""
 
+  allowedNamespaces: ""
+  ignoredNamespaces: ""
+
   service:
     type: ClusterIP
 
diff --git a/pkg/config/controller.go b/pkg/config/controller.go
index b71822dc..be23e6aa 100644
--- a/pkg/config/controller.go
+++ b/pkg/config/controller.go
@@ -36,6 +36,10 @@ type ChaosControllerConfig struct {
 	EnableLeaderElection bool `envconfig:"ENABLE_LEADER_ELECTION" default:"false"`
 	// CertsDir is the directory for storing certs key file and cert file
 	CertsDir string `envconfig:"CERTS_DIR" default:"/etc/webhook/certs"`
+	// AllowedNamespaces is a regular expression, and matching namespace will allow the chaos task to be performed
+	AllowedNamespaces string `envconfig:"ALLOWED_NAMESPACES" default:""`
+	// AllowedNamespaces is a regular expression, and the chaos task will be ignored by a matching namespace
+	IgnoredNamespaces string `envconfig:"IGNORED_NAMESPACES" default:""`
 	// RPCTimeout is timeout of RPC between controllers and chaos-operator
 	RPCTimeout    time.Duration `envconfig:"RPC_TIMEOUT" default:"1m"`
 	WatcherConfig *watcher.Config
diff --git a/pkg/utils/consts.go b/pkg/utils/consts.go
deleted file mode 100644
index 01a5266a..00000000
--- a/pkg/utils/consts.go
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright 2019 PingCAP, Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package utils
-
-import (
-	"fmt"
-	"os"
-
-	ctrl "sigs.k8s.io/controller-runtime"
-)
-
-var (
-	InitLog            = ctrl.Log.WithName("setup")
-	DashboardNamespace string
-	DataSource         string
-)
-
-func init() {
-	var ok bool
-
-	DashboardNamespace, ok = os.LookupEnv("NAMESPACE")
-	if !ok {
-		InitLog.Error(nil, "cannot find NAMESPACE")
-		DashboardNamespace = "chaos"
-	}
-
-	DataSource = fmt.Sprintf("root:@tcp(chaos-collector-database.%s:3306)/chaos_operator", DashboardNamespace)
-}
diff --git a/pkg/utils/selector.go b/pkg/utils/selector.go
index 824c3ca8..e6bc4502 100644
--- a/pkg/utils/selector.go
+++ b/pkg/utils/selector.go
@@ -19,10 +19,12 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
+	"regexp"
 	"strconv"
 	"strings"
 
 	"github.com/pingcap/chaos-mesh/api/v1alpha1"
+	"github.com/pingcap/chaos-mesh/controllers/common"
 	"github.com/pingcap/chaos-mesh/pkg/label"
 	"github.com/pingcap/chaos-mesh/pkg/mock"
 
@@ -82,6 +84,9 @@ func SelectPods(ctx context.Context, c client.Client, selector v1alpha1.Selector
 	// pods are specifically specified
 	if len(selector.Pods) > 0 {
 		for ns, names := range selector.Pods {
+			if !IsAllowedNamespaces(ns) {
+				log.Info("filter pod by namespaces", "namespace", ns)
+			}
 			for _, name := range names {
 				var pod v1.Pod
 				err := c.Get(ctx, types.NamespacedName{
@@ -120,11 +125,13 @@ func SelectPods(ctx context.Context, c client.Client, selector v1alpha1.Selector
 
 	pods = append(pods, podList.Items...)
 
+	pods = filterByNamespaces(pods)
+
 	namespaceSelector, err := parseSelector(strings.Join(selector.Namespaces, ","))
 	if err != nil {
 		return nil, err
 	}
-	pods, err = filterByNamespaces(pods, namespaceSelector)
+	pods, err = filterByNamespaceSelector(pods, namespaceSelector)
 	if err != nil {
 		return nil, err
 	}
@@ -193,7 +200,7 @@ func CheckPodMeetSelector(pod v1.Pod, selector v1alpha1.SelectorSpec) (bool, err
 		return false, err
 	}
 
-	pods, err = filterByNamespaces(pods, namespaceSelector)
+	pods, err = filterByNamespaceSelector(pods, namespaceSelector)
 	if err != nil {
 		return false, err
 	}
@@ -365,8 +372,43 @@ func filterByPhaseSelector(pods []v1.Pod, phases labels.Selector) ([]v1.Pod, err
 	return filteredList, nil
 }
 
-// filterByNamespaces filters a list of pods by a given namespace selector.
-func filterByNamespaces(pods []v1.Pod, namespaces labels.Selector) ([]v1.Pod, error) {
+func filterByNamespaces(pods []v1.Pod) []v1.Pod {
+	var filteredList []v1.Pod
+
+	for _, pod := range pods {
+		if IsAllowedNamespaces(pod.Namespace) {
+			filteredList = append(filteredList, pod)
+		} else {
+			log.Info("filter pod by namespaces",
+				"pod", pod.Name, "namespace", pod.Namespace)
+		}
+	}
+	return filteredList
+}
+
+// IsAllowedNamespaces returns whether namespace allows the execution of a chaos task
+func IsAllowedNamespaces(namespace string) bool {
+	if common.ControllerCfg.AllowedNamespaces != "" {
+		matched, err := regexp.MatchString(common.ControllerCfg.AllowedNamespaces, namespace)
+		if err != nil {
+			return false
+		}
+		return matched
+	}
+
+	if common.ControllerCfg.IgnoredNamespaces != "" {
+		matched, err := regexp.MatchString(common.ControllerCfg.IgnoredNamespaces, namespace)
+		if err != nil {
+			return false
+		}
+		return !matched
+	}
+
+	return true
+}
+
+// filterByNamespaceSelector filters a list of pods by a given namespace selector.
+func filterByNamespaceSelector(pods []v1.Pod, namespaces labels.Selector) ([]v1.Pod, error) {
 	// empty filter returns original list
 	if namespaces.Empty() {
 		return pods, nil
diff --git a/pkg/webhook/inject/inject.go b/pkg/webhook/inject/inject.go
index 769c3bfd..94ee7471 100644
--- a/pkg/webhook/inject/inject.go
+++ b/pkg/webhook/inject/inject.go
@@ -140,6 +140,10 @@ func injectRequired(metadata *metav1.ObjectMeta, cli client.Client, cfg *config.
 			return "", false
 		}
 	}
+	if !utils.IsAllowedNamespaces(metadata.Namespace) {
+		log.Info("Skip mutation for it' in special namespace", "name", metadata.Name, "namespace", metadata.Namespace)
+		return "", false
+	}
 
 	log.V(4).Info("meta", "meta", metadata)
 


Test patch:
diff --git a/pkg/utils/selector_test.go b/pkg/utils/selector_test.go
index 0707a7c6..691a6775 100644
--- a/pkg/utils/selector_test.go
+++ b/pkg/utils/selector_test.go
@@ -21,6 +21,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	"github.com/pingcap/chaos-mesh/api/v1alpha1"
+	"github.com/pingcap/chaos-mesh/controllers/common"
 	"github.com/pingcap/chaos-mesh/pkg/label"
 
 	v1 "k8s.io/api/core/v1"
@@ -405,7 +406,69 @@ func TestFilterByAnnotations(t *testing.T) {
 	}
 }
 
-func TestFilterNamespace(t *testing.T) {
+func TestIsAllowedNamespaces(t *testing.T) {
+	g := NewGomegaWithT(t)
+	type TestCase struct {
+		name   string
+		pods   []v1.Pod
+		ret    []bool
+		allow  string
+		ignore string
+	}
+	pods := []v1.Pod{
+		newPod("p1", v1.PodRunning, "allow", nil, nil),
+		newPod("p1", v1.PodRunning, "allow-app", nil, nil),
+		newPod("p1", v1.PodRunning, "app-allow", nil, nil),
+		newPod("p1", v1.PodRunning, "ignore", nil, nil),
+		newPod("p1", v1.PodRunning, "ignore-app", nil, nil),
+		newPod("p1", v1.PodRunning, "app-ignore", nil, nil),
+	}
+
+	allowRet := []bool{true, true, true, false, false, false}
+
+	var tcs []TestCase
+	tcs = append(tcs, TestCase{
+		name:  "only set allow",
+		pods:  pods,
+		ret:   allowRet,
+		allow: "allow",
+	})
+
+	tcs = append(tcs, TestCase{
+		name:   "only set ignore",
+		pods:   pods,
+		ret:    allowRet,
+		ignore: "ignore",
+	})
+
+	tcs = append(tcs, TestCase{
+		name:   "only set allow",
+		pods:   pods,
+		ret:    allowRet,
+		allow:  "allow",
+		ignore: "ignore",
+	})
+
+	setRule := func(allow string, ignore string) {
+		common.ControllerCfg.AllowedNamespaces = allow
+		common.ControllerCfg.IgnoredNamespaces = ignore
+	}
+
+	clean := func() {
+		common.ControllerCfg.AllowedNamespaces = ""
+		common.ControllerCfg.IgnoredNamespaces = ""
+	}
+
+	for _, tc := range tcs {
+		setRule(tc.allow, tc.ignore)
+		for index, pod := range tc.pods {
+			g.Expect(IsAllowedNamespaces(pod.Namespace)).Should(Equal(tc.ret[index]))
+		}
+		clean()
+	}
+}
+
+func TestFilterNamespaceSelector(t *testing.T) {
 	g := NewGomegaWithT(t)
 
 	type TestCase struct {
@@ -470,7 +533,7 @@ func TestFilterNamespace(t *testing.T) {
 	})
 
 	for _, tc := range tcs {
-		g.Expect(filterByNamespaces(tc.pods, tc.filterSelector)).To(Equal(tc.filteredPods), tc.name)
+		g.Expect(filterByNamespaceSelector(tc.pods, tc.filterSelector)).To(Equal(tc.filteredPods), tc.name)
 	}
 }