FasterXML / jackson-dataformats-text

Uber-project for (some) standard Jackson textual format backends: csv, properties, yaml (xml to be added in future)
Apache License 2.0
408 stars 149 forks source link

Support opting out of various numeric base literal interpretation #276

Open davidxia opened 3 years ago

davidxia commented 3 years ago

Is there a way to disable the various numeric base literal support added by this commit? We have K8s YAML like the below.

      - name: test-service-account
        secret:
          secretName: test-secrets
          defaultMode: 0444

Version < 2.12.0 parsed defaultMode into an IntNode with value 444. Versions >= 2.12.0 parse into an IntNode with decimal value 292 (0444 is 292 in octal). We're using this library in an internal tool in a large company. A quick code search yields ~2K instances of just defaultMode: 0444.

If there isn't a way to disable this behavior, perhaps consider this a feature request or bug fix (depends on if you view this minor semantic version change as accidentally including a breaking change) to support a config knob to opt out of this behavior?

cowtowncoder commented 3 years ago

There is no way to change the behavior at this point, but a PR for adding a new option (YAMLParser.Feature.xxx) could be accepted for 2.13.0 to allow changing the behavior. If I find time I could consider adding this myself too of course.

One quick question since I do not remember exact behavior pre-2.12: were these values reported as JsonToken.VALUE_STRINGs? If so, that should be relatively easy addition. (the alternative behavior would have been that code took such numbers as decimals, and that'd be trickier approach to take).

davidxia commented 3 years ago

Thanks. Numbers in YAML were being parsed into IntNodes. I won’t have time to contribute this feature myself though.

davidxia commented 3 years ago

On closer look, it seems like our tool that deploys K8s manifests to clusters can use the decimal or octal notation. So we should be able to upgrade this library without any issues. I'm going to leave this issue open for others who may need it. But the need is less for me now.

Note that the JSON spec doesn't support octal notation, so use the value 256 for 0400 permissions. If you use YAML instead of JSON for the Pod, you can use octal notation to specify permissions in a more natural way.

— https://kubernetes.io/docs/concepts/configuration/secret/#secret-files-permissions

cowtowncoder commented 3 years ago

Correct wrt JSON/YAML -- decimal integer values must not start with 0 (with the exception of value 0) itself.

I was curious wrt IntNode just because I thought earlier Octal numbers might not have been recognized as numbers at all but passed as String tokens (JsonToken.VALUE_STRING) -- but databind level could still coerce JSON Strings into Numbers when expected target is numeric. That would NOT happen when binding as trees (JsonNode), however.

davidxia commented 3 years ago

My K8s YAML is below.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: test-service
  namespace: test-namespace
spec:
  selector:
    matchLabels:
      app: test-service
  template:
    metadata:
      labels:
        app: test-service
    spec:
      volumes:
      - name: test-service-account
        secret:
          secretName: test-secrets
          defaultMode: 0444
          items:
          - key: 'gcloud.credentials.test-namespace'
            path: test-namespace.json

      containers:
      - name: test-service
        image: $DEPLOYMENT_IMAGE

        volumeMounts:
        - name: test-service-account
          mountPath: /etc/gcloud

The code is

    YamlMapper = new YamlMapper();
    final TypeReference<ObjectNode> objNodeRef = new TypeReference<ObjectNode>() {};

  public static List<ObjectNode> read(final String content) throws IOException {

    YamlMapper
        .readValues(YAML_FACTORY.createParser(content), objNodeRef)
        .readAll()
        .stream()
        .map(rawYaml -> (ObjectNode) rawYaml)
        .collect(Collectors.toList());
  }

    final List<ObjectNode> provided = Yaml.read(fixture("path/to/yaml-above.yaml"))

Then provided.get(0).get("spec").get("template").get("spec").get("volumes").get(0).get("secret") shows IntNode in IntelliJ for versions < 2.12.0.

image