Support operator native namespace creation.

[Gilbert88]

What would you like to be added:
Currently, KUDO has an assumption that namespace has to be pre-existed before installing an operator. However, when managing some cloud native applications (e.g., istio, kubeflow), users would expect KUDO to support a mechanism, which allows namespace to be created natively if the namespace information is defined along with the operator.

Basically, there are two tasks requested here:

1. Support a mechanism in Kudo operator to allow users configure ns resources and Kudo operator will create them.
2. Change the current k kudo install namespace overwriting semantic. KUDO should not always overwrite all resources to one namespace. It is more common today applications would manager multi-namespace.

Why is this needed:
This feature is blocking a couple of ongoing efforts:
-Build a Kudo operator for latest Istio controller
-Build a cluster addon for Kudo based kubeflow operator
-Multi namespace management in kudo operator

[porridge]

See also https://github.com/kudobuilder/kudo/issues?q=is%3Aopen+is%3Aissue+namespaced

[kensipe]

@Gilbert88 I'm trying to understand this request better...

1. It is confusing to refer to this as "blocking" a couple of things... as the prerequisite step of creating a namespace should unblock it... right?
2. Is this request for creation of namespace only during "install" or "deploy" plan of an operator? or are you expecting ongoing support for the operator past installation? if so, what does that use case look like?
3. we are left which some pretty interesting questions...
   a. there are some kudo devs which is the installation of Operator and OperatorVersion as a pre-condition of install of Instance... in this world, O and OV are installed by an admin... and an Instance is created by anyone else. It is also the world that the Dependency management has moved... that an Operator A dependent on Operator B will have O and OV of B added to the cluster before installing Operator A. So... is namespace creation part of the admin step... (which is likely the expectation or is it part of installation of the instance? The common reason that namespace is admin oriented is that it common has quota and security isolations associated with it. From a KUDO operator standpoint, it seems most logic that awareness of the needed namespace is at the time of Instance install... and it is currently allowed to have an instance in 2 different namespaces... pushing it in the direction of creation for the instance.
   b. If kudo created the namespace... should kudo delete it when an uninstall is invoked? (assuming all resources are removed).
   c. should kudo honor the CLI --namespace as an override or is the namespace defined in the operator the authority? does this create a situation where --namespace is honored in some situations but isn't in others? It seems like your request Upgrade Kubernetes version to support 1.13 #2 specifically is requesting this... it is confusing when to honor --namespace override... if we don't honor the flag based on operator configuration... how does a user install multiple instances of that operator? or are you expecting this type of operator is a single instance only?
   d. are you asking for kudo to auto-detect if the use of a namespace is present and if not create it? or are you asking to allow an operator to include a manifest for the namespace creation?

[GoelDeepak]

Here is a sample config that I tried in my operator:

1. namespace.yaml:

apiVersion: v1
kind: Namespace
metadata:
  name: istio-operator
  labels:
    istio-operator-managed: Reconcile
    istio-injection: disabled

2. deployment.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  namespace: {{.Params.operatorNamespace}}
  name: istio-operator
spec:
  replicas: 1
...

3. params.yaml

apiVersion: kudo.dev/v1beta1
parameters:
  - name: operatorNamespace
    default: "istio-operator"
    description: "Namespace in which istio-operator is deployed"
    displayName: "Namespace for the istio-operator"

4. operator.yaml

apiVersion: kudo.dev/v1beta1
appVersion: 1.5.1
kubernetesVersion: 1.16.0
kudoVersion: 0.10.1
name: "istio-operator"
operatorVersion: 0.1.0
url: https://istio.io/
tasks:
- kind: Apply
  name: crds
  spec:
    resources:
    - crd.yaml
- kind: Apply
  name: service-account
  spec:
    resources:
    - service_account.yaml
- kind: Apply
  name: namespace
  spec:
    resources:
    - namespace.yaml
- kind: Apply
  name: deployment
  spec:
    resources:
    - deployment.yaml
    - service.yaml
plans:
  deploy:
    phases:
    - name: preconditions
      steps:
      - name: crds
        tasks:
        - crds
      - name: service-account
        tasks:
        - service-account
      strategy: serial
    - name: deploy
      steps:
      - name: namespace
        tasks:
        - namespace
      - name: deployment
        tasks:
        - deployment
      strategy: serial
    strategy: serial

I didn't use --namespace parameter and expected my operator to get install in istio-operator namespace. However, kudo deployed it in default namespace. Ignoring the parameter

[kensipe]

We had a good zoom meeting... in it we reduced the scope of this Issue to KUDO creating 1 namespace (not multi-namespaces).
The request is:

1. kudo should create 1 namespace (with user control like --namespace
2. there is a strong desire to have that namespace creation have additional metadata (like labels)
3. kudo should honor namespaces annotated in the manifest files... so if there is No namespace defined in the manifest, then it goes to the --namespace or "default". If the manifest has a namespace, it should be deployed to that namespace. Example above provided by @GoelDeepak .

Some additional background:

1. helm v2 had the ability to create a namespace if specified... however helm v3 removed that ability with the same justification kudo has for forcing namespaces to be outside of kudo control as a pre-condition.
2. the history of this change AND a proposal for adding it back into helm is here: Enable Release Namespace Creation in Helm3 helm/helm#6794
3. there is already a tool to auto-create namespaces in k8s with https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#namespaceautoprovision however that doesn't have the extra metadata needed.
4. namespaces carry conflated meaning which needs to be understood. 1) logic separation 2) quota separation 3) security rights with serviceaccounts... it should be mentioned that the "preconditions" for kudo already exist in environments which require serviceaccounts for the operator. 4) additional meanings based on labels and controllers

Proposal:

KUDO forces namespace management to be a precondition

For this proposal to work... all namespace creation is managed outside kudo. D2iQ would need to find or build automation for kubeaddons etc. This REQUIRES that kudo honor namespaces defined in manifests.. they can NOT go to "default" if there is a namespace defined.
this would result in the following:

1. manifest with namespace defined -> deployed to that namespace
2. manifest without namespace -> deploy to "default" or --namespace specified.

KUDO Creates Namespaces

For this it is proposed that we have by configuration or convention a namespace definition.
Example 1

# operator.yaml
name: "kubeflow"
operatorVersion: "0.2.0"
kudoVersion: "0.10.1"
kubernetesVersion: 1.15.0
appVersion: 1.0.0
namespaceManifest: templates/namespace.yaml

where namespaceManifest defines the manifest file for the namespace. I could also see having a naming convention... like namespace.yaml in the operator folder.

followed by the manifest:
example namespace.yaml

apiVersion: v1
kind: Namespace
metadata:
  labels:
    ca.istio.io/override: "true"
    istio-injection: enabled
    katib-metricscollector-injection: enabled
  name: {{ .Namespace }}

The following rules would apply:

1. no namespace.yaml, if --namespace provided and doesn't exist, create it, then all manifests that do not specify a namespace or have namespace: {{ .Namespace }} will go to that namespace... all other namespaces will be honored.
2. no namespace.yaml, no --namespace. All manifests without a namespace or have namespace: {{ .Namespace }} will go to "default"
3. namespace.yaml defined, no --namespace. If namespace.yaml is templated name: {{ .Namespace }}, this is an error. if the namespace.yaml is not templated name: foo, the foo namespace if not created will be. All manifests with undefined or namespace: {{ .Namespace }} namespaces will deploy to foo. All other namespaces will be honored.
4. namespace.yaml defined, with --namespace=bar. If the namespace manifest is templated, it will create bar namespace, if it is not templated it is an error. All namespaces in manifest will be honored as previously described.

[ANeumann82]

This looks interesting. I certainly see the need for some feature in this direction.

Some thoughts:

1. I think the ability to let KUDO create a namespace is good, i like the proposal for using a convention or configured namespaceManifest - I prefer the configuration, but that's personal opinion.
2. Honoring hardcoded namespaces is difficult, but probably necessary. I worry that we end up with operators that have resources which get overwritten if the operator is deployed twice: An operator with a resource that has namespace foo and name bar hardcoded wouldn't be able to installed twice. We could try to enforce that either the namespace or the name need to be templated.
3. Security: If we allow an operator to deploy to resources that differ from the defined namespace, we need to think about this. We could make this configurable on KUDO installation, so that cluster operators with strict namespace separation can prevent this. Or we may need to have a whitelist of namespaces where all operators are allowed to deploy resources in
4. Ownership: When we allow an instance to deploy resources in a different namespace, we can't set ownerReference. We need to ensure that we can provide correct cleanup and prevent accidental deletion of our resource in that case.
5. If we're talking only about a single namespace to be created - Do we need the option to make this templateable? What I mean is:
   When a namespace.yaml or a namespaceManifest is provided in the operator, the --namespace parameter becomes required - Not providing it would be an error when trying to install that operator.
   The namespace.yaml then would have to either have no name set, or have the name set to {{ .Namespace }}

In general, this looks like a good proposal.

Just throwing it out there, maybe this was discussed already - Having KUDO operators that can be cluster wide: Similar to Role, RoleBinding and ClusterRole, ClusterRoleBinding we could have ClusterOperatorVersion and ClusterInstance - This would allow the operator to create namespaces, deploy things into those namespaces. This also would solve OwnerReference problems, as the namespace-less instance could have ownership of resources in all namespaces.

I assume this idea would have a couple of downsides as well - not the least that we'd need a duplicated set of CRDs for the namespace-less resources.

[GoelDeepak]

@kensipe I like the proposal. One question: Will the namespace be removed on uninstall?

[zen-dog]

I like the proposal. More thoughts:

1. I'd prefer namespaceManifest referencing a templates/some.yaml to top-level namespace.yaml. If it can be templated, I'd rather keep in the templates folder and not on the top level. This way it is automatically part of the OV which leads to the next point (mentioned by GoelDeepak)

2. Namespace removal: now, that we create them, we need to think about cleaning them up too. Having the original namespaceManifest will make it easier.

3. Cross-namespaced ownership is ugly but we had multiple requests about it in the past so I'd say we should allow it 🤷 I'd also advocate for making it part of the package verify command to point out that x-namespaced resources are dangerous and as ANeuman mentioned above, can be overridden by another operator installation.

4. 👍 on making it backward-compatible.

[Gilbert88]

Thanks for the proposal, @kensipe ! LGTM in general. A couple of tweaks that I would like to suggest inline.

I read through the Helm v3 issue helm/helm#6794, and I think they made a good move from Helm v2 -> v3. Namespace, as an admin/user-defined resources boundary, should not be created IMPLICITLY if not pre-existed.

However, it is a different story in our case - we want an optional for admin/user to tell KUDO EXPLICITLY that a namespace needs to be created before installation, if specified. Also, this aligns with the followup Helm v3 fix, which gives it a namespace creation configurable option to users.

1. no namespace.yaml, if --namespace provided and doesn't exist, create it, then all manifests that do not specify a namespace or have namespace: {{ .Namespace }} will go to that namespace... all other namespaces will be honored.

Let's remove this one. We should avoid creating any namespace implicitly. If any user expects a namespace to be created by KUDO, they MUST provide the namespace.yaml.

Also, any implicitly-created namespace may not meet what users need. For example, a namespace may need some Istio labels, in order to make sure the resources under this namespace can communicate through the network within an istio-enabled cluster.

2. no namespace.yaml, no --namespace. All manifests without a namespace or have namespace: {{ .Namespace }} will go to "default"
3. namespace.yaml defined, no --namespace. If namespace.yaml is templated name: {{ .Namespace }}, this is an error. if the namespace.yaml is not templated name: foo, the foo namespace if not created will be. All manifests with undefined or namespace: {{ .Namespace }} namespaces will deploy to foo. All other namespaces will be honored.
4. namespace.yaml defined, with --namespace=bar. If the namespace manifest is templated, it will create bar namespace, if it is not templated it is an error. All namespaces in manifest will be honored as previously described.

Although an optional namespaceManifest is sufficient for defining our logics, we can introduce another flag --create-namespace, like what Helm v3 did recently. More explicit for users, and users have to pick either this flag or --namespace, or None.

+1 for delete the namespace specified by --create-namespace when the operator is deleted.

Regarding the namespace overwriting and multi namespaces support, we can split them out from this namespace creation scope. Created #1503 as a separate feature request.

[Gilbert88]

In addition, we need to handle the case if a specific namespace from --create-namespace has already pre-existed in the cluster. If we expect the admin to chim in, probably just return an error, because it is hard to merge namespace labels if any idential label conflicts in true or false.

[zmalik]

Thanks @kensipe! I like the proposal. I think I fully agree on use cases where a namespace exists already.

For the use case where namespace doesn't exist and multi-ns/cross-ns use cases. I would like to see a more straight-forward approach, which keeps in mind the ownerReference. This will also push operator developers towards a more clean approach when developing/designing their operators.

If we decide to create namespaces through KUDO, in that case:

1. KUDO should create a cluster scope instance to own the namespaces, and we shouldn't rely on cleanup plans or labels
2. Cluster-Scope Instance can be composed of:
   1. Cluster-Scoped objects like namespaces/clusterrole/clusterrolebindings
   2. Namespaced Instances (existing KUDO Instances) that are created in a Namespace
   3. Cluster-scoped objects and Namespaced Instances are owned by Cluster-Scope Instance
   4. Namespaced objects other than Namespaced Instance are owned by the Namespaced Instance
3. Each Namespaced Instance should be able to create objects only in its namespace, created by KUDO or not.

If we introduce the concept of namespace creation without clear ownership, we would be diverging from our best practices model of ownerReference defined in each object. And this will create more complexity and anti-patterns adopted by KUDO Operators.

[zen-dog]

I believe this issue is done, right @kensipe ?