Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support opting out of various numeric base literal interpretation #276

Open
davidxia opened this issue Jun 9, 2021 · 5 comments
Open

Support opting out of various numeric base literal interpretation #276

davidxia opened this issue Jun 9, 2021 · 5 comments
Labels
yaml Issue related to YAML format backend

Comments

@davidxia
Copy link

davidxia commented Jun 9, 2021

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
Copy link
Member

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
Copy link
Author

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

@davidxia
Copy link
Author

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
Copy link
Member

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
Copy link
Author

davidxia commented Jun 14, 2021

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

@cowtowncoder cowtowncoder added the yaml Issue related to YAML format backend label Jul 9, 2021
@cowtowncoder cowtowncoder removed the 2.13 label Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
yaml Issue related to YAML format backend
Projects
None yet
Development

No branches or pull requests

2 participants