Linkerd2: "linkerd inject --manual" changes types of label values (string becomes float64)

[KIVagant]

Bug Report

What is the issue?

After linkerd inject --manual is applied, all Deployment labels lose double quotes around label values which causes problems with unobvious types.

How can it be reproduced?

1. Create the deployment:

# tmp-2.yaml
---
apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: "1493e89"
    commit: "1493e89"
  name: "1493e89"
spec:
  replicas: 1
  selector:
    matchLabels:
      app: "1493e89"
  template:
    metadata:
      labels:
        app: "1493e89"
        commit: "1493e89"
      name: "1493e89"
    spec:
      containers:
        - image: nginx
          name: "1493e89"

1. Apply it using kubectl apply -f tmp-2.yaml and make sure it works. New pods will start, for example:

NAME                                       READY   STATUS    RESTARTS   AGE
1493e89-7946985fdc-7zx2w                   1/1     Running   0          2m31s

1. Inject Linkerd Proxy linkerd inject --manual tmp-2.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  labels:
    app: 1493e89
    commit: 1493e89
  name: 1493e89
spec:
  replicas: 1
  selector:
    matchLabels:
      app: 1493e89
  strategy: {}
  template:
    metadata:
      annotations:
        linkerd.io/created-by: linkerd/cli edge-19.9.3 # actually, there's edge-20.1.2 on server side which behaves the same
        linkerd.io/identity-mode: default
        linkerd.io/proxy-version: edge-20.1.2
      creationTimestamp: null
      labels:
        app: 1493e89
        commit: 1493e89
        linkerd.io/control-plane-ns: linkerd
        linkerd.io/proxy-deployment: 1493e89
      name: 1493e89
    spec:
      containers:
      - image: nginx
        name: 1493e89
        resources: {}
...

As you can see here, all labels (and not only labels) lost double quotes around values. Now try to apply it:

linkerd inject --manual tmp-2.yaml | kubectl apply -f -

Logs, error output, etc

deployment "1493e89" injected

error: error when retrieving current configuration of:
...
Object: &{map["apiVersion":"apps/v1" "kind":"Deployment" ...
:map["app":%!q(float64=+1.493e+92) "commit":%!q(float64=+1.493e+92)] "name":%!q(float64=+1.493e+92)
...
from server for: "STDIN": resource name may not be empty

// or even more unclear error:

for: "./tmp-2.yaml": unrecognized type: string

Environment

• Kubernetes Version: v1.14.9-eks-c0eccc
• Cluster Environment: EKS
• Host OS: Amazon Linux
• Linkerd version: edge-20.1.2

Possible solution

Add quotes in yaml templates everywhere by default when possible.

Workaround:

Update the commit id in any way, if it is a commit id causing the problem.

[ihcsim]

This might also be problematic for labels that has "true" / "false" values, which when stripped turn into boolean, causing deployment to fail completely (labels only takes string values.)

[adrijshikhar]

@grampelberg , i would like to work on this.
i have already setup the repo

[grampelberg]

@adrijshikhar awesome! Go for it!

[adrijshikhar]

What does inject do basically?

[grampelberg]

@adrijshikhar I would recommend going through the getting started tutorials and spending some time in the docs to answer that question =)

[Vineet-Sharma29]

@adrijshikhar Are you still working on this issue? If not, I would like to work on it.

[adrijshikhar]

no i am not

[saurav-malani]

@grampelberg is this assigned to anyone? If not, then please assign this to me, I would like to give this issue a try.

[grampelberg]

@Vineet-Sharma29 are you still working on this?

[Vineet-Sharma29]

@grampelberg No

[saurav-malani]

@grampelberg @ihcsim According to me there is somethings wrong with the "transform" function in cli/cmd/inject.go, specifically here: https://github.com/linkerd/linkerd2/blob/master/cli/cmd/inject.go#L193-L208. i.e. when interconversion occurs from yaml to json and then back to yaml.
ie. maybe when yamltojson conversion is done then every string is under double quote (" "), so conversion method fails to keep track of it. So, doule quotes disappears when when jsonToYaml conversion is done. And, hence the values in final injected yaml files do not have double quotes.

[Pothulapati]

As @saurav-malani mentioned, It's not straight forward from what we discussed.
but I do think its an important use-case, I can take this up and see what I can do there.

@ihcsim Do you have some ideas on how to fix this?

[ihcsim]

@Pothulapati @saurav-malani Can you help me to extract out that block of code into an unexported function and add some unit test to help identify which library is acting weird? Maybe something like:

func applyPatch(original, patchJSON []byte) ([]byte, error) {
    patch, err := jsonpatch.DecodePatch(patchJSON)
    if err != nil {
        return nil, err
    }

    origJSON, err := yaml.YAMLToJSON(original)
    if err != nil {
        return nil, err
    }

    injectedJSON, err := patch.Apply(original)
    if err != nil {
        return nil, err
    }

    return conf.JSONToYAML(injectedJSON)
}

There is a good example on how to set up your unit test with simple inputs in the json-patch library README. Thanks!

[saurav-malani]

@ihcsim I commented out this portion and returned injectedJSON instead of injectedYAML. And, the output corresponding to it is exactly same as expected i.e. integers are not under double quotes and rest under double quotes. So, I believe there is nothing wrong till L208 and probably something goes wrong afterwards, specifically here: injectedYAML, err := conf.JSONToYAML(injectedJSON).
PS: complete json output can be viewed here

[ihcsim]

Are you able to track down what's going on in conf.JSONToYAML(injectedJSON)? Adding some unit tests might help to flush out the bugs.

[adnxn]

@ihcsim: cant seem to repro this using example above with build from latest commit on master. i wonder if it was fixed with a dependency upgrade or something. haven't actually sifted through the PRs.

$ linkerd version
Client version: stable-2.7.0
Server version: stable-2.7.0

$ linkerd inject --manual tmp.yml | k apply -f -

deployment "1493e89" injected

error: unable to decode "STDIN": resource.metadataOnlyObject.ObjectMeta: v1.ObjectMeta.Labels: ReadString: expects " or n, but found 1, error found in #10 byte of ...|":{"app":1.493e+92,"|..., bigger context ...|,"kind":"Deployment","metadata":{"labels":{"app":1.493e+92,"commit":1.493e+92},"name":1.493e+92},"sp|...

$ linkerd inject --manual tmp.yml | grep commit

deployment "1493e89" injected

    commit: 1493e89
        commit: 1493e89

$ ./target/cli/darwin/linkerd version
Client version: git-d6460cf0
Server version: stable-2.7.0

$ ./target/cli/darwin/linkerd inject --manual tmp.yml | k apply -f -

deployment "1493e89" injected

deployment.apps/1493e89 configured

$ ./target/cli/darwin/linkerd inject --manual tmp.yml | grep commit

deployment "1493e89" injected

    commit: "1493e89"
        commit: "1493e89"

[KIVagant]

Can confirm the issue in stable-2.7.1

[Ashish-Bansal]

@adnxn You are right. I confirm that issue has been fixed due to dependency upgrade in https://github.com/linkerd/linkerd2/pull/4221

Upstream PR - https://github.com/go-yaml/yaml/pull/171

@KIVagant Latest stable release doesn't include that patch.