Add inject support to namespaces

[mayankshah1607]

Fixes #3255

This PR adds support for linkerd inject to annotate namespaces as follows :
kubectl get ns/default -o yaml | linkerd inject -
Anything added to the said namespace gets linkerd support.

• Added method AppendNsAnnotation() for ResourceConfig objects
• Added method IsNamespace() for ResourceConfig objects - checks if a given config is of Kind namespace
• Updated linkerd install golden files
• Updated inject.go to make namespace annotations come in through patching - Pothulapati@b49fcdc
• Relevant unit tests have been added with overrideAnnotations as well

[Pothulapati]

This looks good.

[Pothulapati]

This Looks good 💯

@ihcsim can you take a look?

[ihcsim]

@Pothulapati @mayankshah1607 Hey guys, thanks for putting time and effort into this. I think the number of new exported functions being introduced in this PR to the pkg/inject package is a bit much. In addition to the existing ones, the ResourceConfig struct now have these functions: WithNsAnnotation(), AppendAnnotations(), AppendAnnotation(), AppendNsAnnotation(), AppendPodAnnotation(). I am very sure I will forget what are their differences in a few weeks time.

I think ultimately, all we needed to do is:

1. Unmarshal the namespace YAML into an object
2. So that we can add the linkerd.io/inject annotation to it
3. Marshal the object back into YAML
4. Return any errors and report mssage
5. Then profit!

I propose that we don't bother with calling the inject.GetPatch(bool) function, because there are many logic in there there are relevant only to pods. WDYT of the idea of just introduce one new exported function to do the 5 steps outlined above?

diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go
index 8e701344..5c54718c 100644
--- a/cli/cmd/inject.go
+++ b/cli/cmd/inject.go
@@ -147,8 +147,8 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 
 	//Add auto-proxy support for namespace configs
 	if conf.IsNamespace() {
-		// Namespace config annotations get appended here
-		conf.AppendNsAnnotation(k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled)
+		b, err := conf.InjectNamespace()
+		return b, reports, err
 	}
 
 	if rt.injectProxy {
diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go
index 2f0f60a7..de39ea2c 100644
@@ -647,6 +637,15 @@ func (conf *ResourceConfig) injectPodAnnotations(values *patch) {
 	}
 }
 
+func (conf *ResourceConfig) InjectNamespace() ([]byte, error) {
+	ns, ok := conf.workload.obj.(*corev1.Namespace)
+	if !ok {
+		return nil, errors.New("can't inject namespace. Type assertion failed")
+	}
+	ns.Annotations[k8s.ProxyInjectAnnotation] = k8s.ProxyInjectEnabled
+	return yaml.Marshal(ns)
+}
+
 func (conf *ResourceConfig) injectNsAnnotations(values *patch) {
 	values.AddRootAnnotations = len(conf.pod.meta.Annotations) == 0

With this approach, we don't need the additional exported functions and conditionals to get around the pod-relevant injection logic.

Here's the result of my quick test:

⚡  k get ns default -oyaml                                                                                
apiVersion: v1
kind: Namespace
metadata:
  creationTimestamp: "2019-10-16T22:15:44Z"
  name: default
  resourceVersion: "151"
  selfLink: /api/v1/namespaces/default
  uid: f094e0ca-af97-4072-a658-25d9ffd2c531
spec:
  finalizers:
  - kubernetes
status:
  phase: Active

 ⚡ k get ns default -oyaml| bin/go-run cli inject -                                                         
apiVersion: v1
kind: Namespace
metadata:
  annotations:
    linkerd.io/inject: enabled
  creationTimestamp: "2019-10-16T22:15:44Z"
  name: default
  resourceVersion: "151"
  selfLink: /api/v1/namespaces/default
  uid: f094e0ca-af97-4072-a658-25d9ffd2c531
spec:
  finalizers:
  - kubernetes
status:
  phase: Active
---

namespace "default" injected

[Pothulapati]

Hmm, @ihcsim that would make the logic simple,
but a command like linkerd inject --proxy-Image XYZ namespace.yaml would not add the config annoations.
This may create confusion now that the config annotations are honoured by the proxy-injector during injection.

[ihcsim]

I think those options are stored in the rt.overrideAnnotations map, which can be passed to the proposed InjectNamespace() function, where the namespace can be annotated accordingly. We just need to annotate the namespace, without needing to generate any patches for it.

[mayankshah1607]

I'll work on these changes and open another PR.

[mayankshah1607]

Hey, @ihcsim
I followed your exact approach, and I seem to be getting this error :
Error transforming resources: can't inject namespace. Type assertion failed

On logging ns, it seems to be nil.

[cpretzer]

@mayankshah1607 do you mind pushing your commits so that we can take a look at the code where the namespace is nil?

[mayankshah1607]

Hi @cpretzer . I have pushed my commits - mayankshah1607@24f0c0c

[mayankshah1607]

Fixed it. The problem was that conf.workload.obj itself was nil because conf.ParseMetaYAML() was not configured to return a namespace kind ResourceConfig.
Everything works as expected now.

Considering all the factors discussed above, I have opened a new PR as this one has too many unnecessary file changes dealing with the GetPatch() method. Hence, I shall be closing this one.

Please review my new PR - #3607