*: remove cluster-scoped flag for operator-sdk new (operator-framework#1434)

hasbro17 · web-flow · commit 2e1c2bc3cdbe · 2019-05-17T10:59:54.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@
 
 - The SDK will no longer run `defaulter-gen` on running `operator-sdk generate k8s`. Defaulting for CRDs should be handled with mutating admission webhooks. ([#1288](https://github.com/operator-framework/operator-sdk/pull/1288))
 - **Breaking Change**: The `test cluster` subcommand and the corresponding `--enable-tests` flag for the `build` subcommand have been removed ([#1414](https://github.com/operator-framework/operator-sdk/pull/1414))
+- **Breaking Change**: The `--cluster-scoped` flag for `operator-sdk new` has been removed so it won't scaffold a cluster-scoped operator. Read the [operator scope](https://github.com/operator-framework/operator-sdk/blob/master/doc/operator-scope.md) documentation on the changes needed to run a cluster-scoped operator. ([#1434](https://github.com/operator-framework/operator-sdk/pull/1434))
 
 ### Bug Fixes
 
diff --git a/cmd/operator-sdk/new/cmd.go b/cmd/operator-sdk/new/cmd.go
@@ -58,7 +58,6 @@ generates a skeletal app-operator application in $GOPATH/src/github.com/example.
 	newCmd.Flags().BoolVar(&skipGit, "skip-git-init", false, "Do not init the directory as a git repository")
 	newCmd.Flags().StringVar(&headerFile, "header-file", "", "Path to file containing headers for generated Go files. Copied to hack/boilerplate.go.txt")
 	newCmd.Flags().BoolVar(&generatePlaybook, "generate-playbook", false, "Generate a playbook skeleton. (Only used for --type ansible)")
-	newCmd.Flags().BoolVar(&isClusterScoped, "cluster-scoped", false, "Generate cluster-scoped resources instead of namespace-scoped")
 
 	newCmd.Flags().StringVar(&helmChartRef, "helm-chart", "", "Initialize helm operator with existing helm chart (<URL>, <repo>/<name>, or local path)")
 	newCmd.Flags().StringVar(&helmChartVersion, "helm-chart-version", "", "Specific version of the helm chart (default is latest version)")
@@ -76,7 +75,6 @@ var (
 	headerFile       string
 	skipGit          bool
 	generatePlaybook bool
-	isClusterScoped  bool
 
 	helmChartRef     string
 	helmChartVersion string
@@ -187,9 +185,9 @@ func doGoScaffold() error {
 		&scaffold.Entrypoint{},
 		&scaffold.UserSetup{},
 		&scaffold.ServiceAccount{},
-		&scaffold.Role{IsClusterScoped: isClusterScoped},
-		&scaffold.RoleBinding{IsClusterScoped: isClusterScoped},
-		&scaffold.Operator{IsClusterScoped: isClusterScoped},
+		&scaffold.Role{},
+		&scaffold.RoleBinding{},
+		&scaffold.Operator{},
 		&scaffold.Apis{},
 		&scaffold.Controller{},
 		&scaffold.Version{},
@@ -218,8 +216,8 @@ func doAnsibleScaffold() error {
 	s := &scaffold.Scaffold{}
 	err = s.Execute(cfg,
 		&scaffold.ServiceAccount{},
-		&scaffold.Role{IsClusterScoped: isClusterScoped},
-		&scaffold.RoleBinding{IsClusterScoped: isClusterScoped},
+		&scaffold.Role{},
+		&scaffold.RoleBinding{},
 		&scaffold.CRD{Resource: resource},
 		&scaffold.CR{Resource: resource},
 		&ansible.BuildDockerfile{GeneratePlaybook: generatePlaybook},
@@ -247,7 +245,7 @@ func doAnsibleScaffold() error {
 			GeneratePlaybook: generatePlaybook,
 			Resource:         *resource,
 		},
-		&ansible.DeployOperator{IsClusterScoped: isClusterScoped},
+		&ansible.DeployOperator{},
 		&ansible.Travis{},
 		&ansible.MoleculeTestLocalMolecule{},
 		&ansible.MoleculeTestLocalPrepare{Resource: *resource},
@@ -311,7 +309,7 @@ func doHelmScaffold() error {
 	if err != nil {
 		return fmt.Errorf("failed to get kubernetes config: %s", err)
 	}
-	roleScaffold, err := helm.CreateRoleScaffold(k8sCfg, chart, isClusterScoped)
+	roleScaffold, err := helm.CreateRoleScaffold(k8sCfg, chart)
 	if err != nil {
 		return fmt.Errorf("failed to generate role scaffold: %s", err)
 	}
@@ -325,8 +323,8 @@ func doHelmScaffold() error {
 		},
 		&scaffold.ServiceAccount{},
 		roleScaffold,
-		&scaffold.RoleBinding{IsClusterScoped: isClusterScoped},
-		&helm.Operator{IsClusterScoped: isClusterScoped},
+		&scaffold.RoleBinding{IsClusterScoped: roleScaffold.IsClusterScoped},
+		&helm.Operator{},
 		&scaffold.CRD{Resource: resource},
 		&scaffold.CR{
 			Resource: resource,
diff --git a/doc/ansible/user-guide.md b/doc/ansible/user-guide.md
@@ -39,20 +39,7 @@ layout][layout_doc] doc.
 
 #### Operator scope
 
-A namespace-scoped operator (the default) watches and manages resources in a single namespace, whereas a cluster-scoped operator watches and manages resources cluster-wide. Namespace-scoped operators are preferred because of their flexibility. They enable decoupled upgrades, namespace isolation for failures and monitoring, and differing API definitions. However, there are use cases where a cluster-scoped operator may make sense. For example, the [cert-manager](https://github.com/jetstack/cert-manager) operator is often deployed with cluster-scoped permissions and watches so that it can manage issuing certificates for an entire cluster.
-
-If you'd like to create your memcached-operator project to be cluster-scoped use the following `operator-sdk new` command instead:
-```
-$ operator-sdk new memcached-operator --cluster-scoped --api-version=cache.example.com/v1alpha1 --kind=Memcached --type=ansible
-```
-
-Using `--cluster-scoped` will scaffold the new operator with the following modifications:
-* `deploy/operator.yaml` - Set `WATCH_NAMESPACE=""` instead of setting it to the pod's namespace
-* `deploy/role.yaml` - Use `ClusterRole` instead of `Role`
-* `deploy/role_binding.yaml`:
-  * Use `ClusterRoleBinding` instead of `RoleBinding`
-  * Use `ClusterRole` instead of `Role` for roleRef
-  * Set the subject namespace to `REPLACE_NAMESPACE`. This must be changed to the namespace in which the operator is deployed.
+Read the [operator scope][operator_scope] documentation on how to run your operator as namespace-scoped vs cluster-scoped.
 
 ### Watches file
 
@@ -423,6 +410,7 @@ $ kubectl delete -f deploy/service_account.yaml
 $ kubectl delete -f deploy/crds/cache_v1alpha1_memcached_crd.yaml
 ```
 
+[operator_scope]:./../operator-scope.md
 [install_guide]: ../user/install-operator-sdk.md
 [layout_doc]:./project_layout.md
 [homebrew_tool]:https://brew.sh/
diff --git a/doc/helm/user-guide.md b/doc/helm/user-guide.md
@@ -66,21 +66,8 @@ If `--helm-chart-version` is not set, the SDK will fetch the latest available ve
 
 ### Operator scope
 
-A namespace-scoped operator (the default) watches and manages resources in a single namespace, whereas a cluster-scoped operator watches and manages resources cluster-wide. Namespace-scoped operators are preferred because of their flexibility. They enable decoupled upgrades, namespace isolation for failures and monitoring, and differing API definitions. However, there are use cases where a cluster-scoped operator may make sense. For example, the [cert-manager](https://github.com/jetstack/cert-manager) operator is often deployed with cluster-scoped permissions and watches so that it can manage issuing certificates for an entire cluster.
+Read the [operator scope][operator_scope] documentation on how to run your operator as namespace-scoped vs cluster-scoped.
 
-If you'd like to create your nginx-operator project to be cluster-scoped use the following `operator-sdk new` command instead:
-
-```sh
-operator-sdk new nginx-operator --cluster-scoped --api-version=example.com/v1alpha1 --kind=Nginx --type=helm
-```
-
-Using `--cluster-scoped` will scaffold the new operator with the following modifications:
-* `deploy/operator.yaml` - Set `WATCH_NAMESPACE=""` instead of setting it to the pod's namespace
-* `deploy/role.yaml` - Use `ClusterRole` instead of `Role`
-* `deploy/role_binding.yaml`:
-  * Use `ClusterRoleBinding` instead of `RoleBinding`
-  * Use `ClusterRole` instead of `Role` for roleRef
-  * Set the subject namespace to `REPLACE_NAMESPACE`. This must be changed to the namespace in which the operator is deployed.
 
 ## Customize the operator logic
 
@@ -342,6 +329,7 @@ kubectl delete -f deploy/service_account.yaml
 kubectl delete -f deploy/crds/example_v1alpha1_nginx_crd.yaml
 ```
 
+[operator_scope]:./../operator-scope.md
 [install_guide]: ../user/install-operator-sdk.md
 [layout_doc]:./project_layout.md
 [homebrew_tool]:https://brew.sh/
diff --git a/doc/operator-scope.md b/doc/operator-scope.md
@@ -0,0 +1,25 @@
+## Operator scope
+
+A namespace-scoped operator watches and manages resources in a single namespace, whereas a cluster-scoped operator watches and manages resources cluster-wide. Namespace-scoped operators are preferred because of their flexibility. They enable decoupled upgrades, namespace isolation for failures and monitoring, and differing API definitions.
+
+However, there are use cases where a cluster-scoped operator may make sense. For example, the [cert-manager](https://github.com/jetstack/cert-manager) operator is often deployed with cluster-scoped permissions and watches so that it can manage issuing certificates for an entire cluster.
+
+The SDK scaffolds operators to be namespaced by default but with a few modifications to the default manifests the operator can be run as cluster-scoped.
+
+* `deploy/operator.yaml`:
+  * Set `WATCH_NAMESPACE=""` to watch all namespaces instead of setting it to the pod's namespace
+* `deploy/role.yaml`:
+  * Use `ClusterRole` instead of `Role`
+* `deploy/role_binding.yaml`:
+  * Use `ClusterRoleBinding` instead of `RoleBinding`
+  * Use `ClusterRole` instead of `Role` for `roleRef`
+  * Set the subject namespace to the namespace in which the operator is deployed.
+
+### CRD scope
+
+Additionally the CustomResourceDefinition (CRD) scope can also be changed for cluster-scoped operators so that there is only a single instance (for a given name) of the CRD to manage across the cluster.
+
+For each CRD that needs to be cluster-scoped, update its manifest to be cluster-scoped.
+
+* `deploy/crds/<group>_<version>_<kind>_crd.yaml`
+  * Set `spec.scope: Cluster`
diff --git a/doc/sdk-cli-reference.md b/doc/sdk-cli-reference.md
@@ -259,7 +259,6 @@ Scaffolds a new operator project.
 * `--api-version` string - CRD APIVersion in the format `$GROUP_NAME/$VERSION` (e.g app.example.com/v1alpha1)
 * `--kind` string - CRD Kind. (e.g AppService)
 * `--generate-playbook` - Generate a playbook skeleton. (Only used for `--type ansible`)
-* `--cluster-scoped` - Initialize the operator to be cluster-scoped instead of namespace-scoped
 * `--helm-chart` string - Initialize helm operator with existing helm chart (`<URL>`, `<repo>/<name>`, or local path)
 * `--helm-chart-repo` string - Chart repository URL for the requested helm chart
 * `--helm-chart-version` string - Specific version of the helm chart (default is latest version)
diff --git a/doc/user-guide.md b/doc/user-guide.md
@@ -57,20 +57,7 @@ The Operator SDK uses [vendoring][go_vendoring] to supply dependencies to operat
 
 #### Operator scope
 
-A namespace-scoped operator (the default) watches and manages resources in a single namespace, whereas a cluster-scoped operator watches and manages resources cluster-wide. Namespace-scoped operators are preferred because of their flexibility. They enable decoupled upgrades, namespace isolation for failures and monitoring, and differing API definitions. However, there are use cases where a cluster-scoped operator may make sense. For example, the [cert-manager](https://github.com/jetstack/cert-manager) operator is often deployed with cluster-scoped permissions and watches so that it can manage issuing certificates for an entire cluster.
-
-If you'd like to create your memcached-operator project to be cluster-scoped use the following `operator-sdk new` command instead:
-```sh
-$ operator-sdk new memcached-operator --cluster-scoped
-```
-
-Using `--cluster-scoped` will scaffold the new operator with the following modifications:
-* `deploy/operator.yaml` - Set `WATCH_NAMESPACE=""` instead of setting it to the pod's namespace
-* `deploy/role.yaml` - Use `ClusterRole` instead of `Role`
-* `deploy/role_binding.yaml`:
-  * Use `ClusterRoleBinding` instead of `RoleBinding`
-  * Use `ClusterRole` instead of `Role` for roleRef
-  * Set the subject namespace to `REPLACE_NAMESPACE`. This must be changed to the namespace in which the operator is deployed.
+Read the [operator scope][operator_scope] documentation on how to run your operator as namespace-scoped vs cluster-scoped.
 
 ### Manager
 The main program for the operator `cmd/manager/main.go` initializes and runs the [Manager][manager_go_doc].
@@ -557,7 +544,7 @@ func main() {
 
 When the operator is not running in a cluster, the Manager will return an error on starting since it can't detect the operator's namespace in order to create the configmap for leader election. You can override this namespace by setting the Manager's `LeaderElectionNamespace` option.
 
-
+[operator_scope]:./operator-scope.md
 [install_guide]: ./user/install-operator-sdk.md
 [pod_eviction_timeout]: https://kubernetes.io/docs/reference/command-line-tools-reference/kube-controller-manager/#options
 [manager_options]: https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/manager#Options
diff --git a/internal/pkg/scaffold/ansible/deploy_operator.go b/internal/pkg/scaffold/ansible/deploy_operator.go
@@ -25,7 +25,6 @@ const DeployOperatorFile = "operator.yaml"
 
 type DeployOperator struct {
 	input.Input
-	IsClusterScoped bool
 }
 
 // GetInput - gets the input
@@ -76,13 +75,9 @@ spec:
             name: runner
           env:
             - name: WATCH_NAMESPACE
-              [[- if .IsClusterScoped ]]
-              value: ""
-              [[- else ]]
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-              [[- end]]
             - name: POD_NAME
               valueFrom:
                 fieldRef:
diff --git a/internal/pkg/scaffold/helm/operator.go b/internal/pkg/scaffold/helm/operator.go
@@ -24,8 +24,6 @@ import (
 // Operator specifies the Helm operator.yaml manifest scaffold
 type Operator struct {
 	input.Input
-
-	IsClusterScoped bool
 }
 
 // GetInput gets the scaffold execution input
@@ -59,13 +57,9 @@ spec:
           imagePullPolicy: Always
           env:
             - name: WATCH_NAMESPACE
-              {{- if .IsClusterScoped }}
-              value: ""
-              {{- else }}
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-              {{- end}}
             - name: POD_NAME
               valueFrom:
                 fieldRef:
diff --git a/internal/pkg/scaffold/helm/role.go b/internal/pkg/scaffold/helm/role.go
@@ -15,7 +15,6 @@
 package helm
 
 import (
-	"errors"
 	"fmt"
 	"path/filepath"
 	"sort"
@@ -41,11 +40,12 @@ import (
 // CreateRoleScaffold generates a role scaffold from the provided helm chart. It
 // renders a release manifest using the chart's default values and uses the Kubernetes
 // discovery API to lookup each resource in the resulting manifest.
-func CreateRoleScaffold(cfg *rest.Config, chart *chart.Chart, isClusterScoped bool) (*scaffold.Role, error) {
+// The role scaffold will have IsClusterScoped=true if the chart lists cluster scoped resources
+func CreateRoleScaffold(cfg *rest.Config, chart *chart.Chart) (*scaffold.Role, error) {
 	log.Info("Generating RBAC rules")
 
 	roleScaffold := &scaffold.Role{
-		IsClusterScoped:  isClusterScoped,
+		IsClusterScoped:  false,
 		SkipDefaultRules: true,
 		// TODO: enable metrics in helm operator
 		SkipMetricsRules: true,
@@ -72,22 +72,10 @@ func CreateRoleScaffold(cfg *rest.Config, chart *chart.Chart, isClusterScoped bo
 		roleScaffold.SkipDefaultRules = false
 	}
 
-	if !isClusterScoped {
-		// If there are cluster-scoped resources, but we're creating a namespace-scoped operator,
-		// log all of the cluster-scoped resources, and return a helpful error message.
-		for _, rule := range clusterResourceRules {
-			for _, resource := range rule.Resources {
-				log.Errorf("Resource %s.%s is cluster-scoped, but --cluster-scoped was not set.", resource, rule.APIGroups[0])
-			}
-		}
-		if len(clusterResourceRules) > 0 {
-			return nil, errors.New("must use --cluster-scoped with chart containing cluster-scoped resources")
-		}
-
-		// If we're here, there are no cluster-scoped resources, so add just the rules for namespaced resources
-		roleScaffold.CustomRules = append(roleScaffold.CustomRules, namespacedResourceRules...)
-	} else {
-		// For a cluster-scoped operator, add all of the rules
+	// Use a ClusterRole if cluster scoped resources are listed in the chart
+	if len(clusterResourceRules) > 0 {
+		log.Info("Scaffolding ClusterRole and ClusterRolebinding for cluster scoped resources in the helm chart")
+		roleScaffold.IsClusterScoped = true
 		roleScaffold.CustomRules = append(roleScaffold.CustomRules, append(clusterResourceRules, namespacedResourceRules...)...)
 	}
 
diff --git a/internal/pkg/scaffold/operator.go b/internal/pkg/scaffold/operator.go
@@ -24,8 +24,6 @@ const OperatorYamlFile = "operator.yaml"
 
 type Operator struct {
 	input.Input
-
-	IsClusterScoped bool
 }
 
 func (s *Operator) GetInput() (input.Input, error) {
@@ -60,13 +58,9 @@ spec:
           imagePullPolicy: Always
           env:
             - name: WATCH_NAMESPACE
-              {{- if .IsClusterScoped }}
-              value: ""
-              {{- else }}
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-              {{- end}}
             - name: POD_NAME
               valueFrom:
                 fieldRef:
diff --git a/internal/pkg/scaffold/operator_test.go b/internal/pkg/scaffold/operator_test.go
@@ -33,19 +33,6 @@ func TestOperator(t *testing.T) {
 	}
 }
 
-func TestOperatorClusterScoped(t *testing.T) {
-	s, buf := setupScaffoldAndWriter()
-	err := s.Execute(appConfig, &Operator{IsClusterScoped: true})
-	if err != nil {
-		t.Fatalf("Failed to execute the scaffold: (%v)", err)
-	}
-
-	if operatorClusterScopedExp != buf.String() {
-		diffs := diffutil.Diff(operatorClusterScopedExp, buf.String())
-		t.Fatalf("Expected vs actual differs.\n%v", diffs)
-	}
-}
-
 const operatorExp = `apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -80,36 +67,3 @@ spec:
             - name: OPERATOR_NAME
               value: "app-operator"
 `
-
-const operatorClusterScopedExp = `apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: app-operator
-spec:
-  replicas: 1
-  selector:
-    matchLabels:
-      name: app-operator
-  template:
-    metadata:
-      labels:
-        name: app-operator
-    spec:
-      serviceAccountName: app-operator
-      containers:
-        - name: app-operator
-          # Replace this with the built image name
-          image: REPLACE_IMAGE
-          command:
-          - app-operator
-          imagePullPolicy: Always
-          env:
-            - name: WATCH_NAMESPACE
-              value: ""
-            - name: POD_NAME
-              valueFrom:
-                fieldRef:
-                  fieldPath: metadata.name
-            - name: OPERATOR_NAME
-              value: "app-operator"
-`
