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

Openshift InvalidFormatException Cannot deserialize value for Kind Template #3460

Closed
jeromedemangel opened this issue Sep 10, 2021 · 4 comments

Comments

@jeromedemangel
Copy link

jeromedemangel commented Sep 10, 2021

Hello,

With the openshift-client library

<dependency>
    <groupId>io.fabric8</groupId>
    <artifactId>openshift-client</artifactId>
    <version>5.7.3</version>
</dependency>

I try to get a template :

openShiftClient.templates()
    .inNamespace("foo")
    .withName("bar")
    .get()

The template contains an object with :

securityContext:
  runAsUser: ${USERID}

And a parameter :

parameters:
- name: USERID
  required: true

When executing the Java code, the error is thrown

Caused by: com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `long` from String "${USERID}": not a valid `long` value
 at [Source: UNKNOWN; line: -1, column: -1] (through reference chain: io.fabric8.openshift.api.model.TemplateList["items"]->java.util.ArrayList[5]->io.fabric8.openshift.api.model.Template["objects"]->java.util.ArrayList[0]->io.fabric8.kubernetes.api.model.batch.v1.Job["spec"]->io.fabric8.kubernetes.api.model.batch.v1.JobSpec["template"]->io.fabric8.kubernetes.api.model.PodTemplateSpec["spec"]->io.fabric8.kubernetes.api.model.PodSpec["securityContext"]->io.fabric8.kubernetes.api.model.PodSecurityContext["runAsUser"])

The runAsUser is effectivly a long in the destination object, but in the template It's a string refered by the parameter

@sknot-rh
Copy link
Contributor

sknot-rh commented Sep 13, 2021

Hello, I am having a similar issue. I am trying to load an object with the Integer placeholder however, it is failing. Minimal reproducer:

Run this test in io.fabric8.openshift.client.server.mock.LoadAsTemplateTest

  @Test
  void unmarshallTemplateObjectWithIntegerPlaceholder() {
    Template template = Serialization.unmarshal(getClass().getResourceAsStream("/template-with-json-params.yml"), Template.class);
  }

It fails with Cannot deserialize value of type java.lang.Integer from String "${{CONTAINER_PORT}}": not a valid Integer value as the validator expects Integer value but the placeholder String is provided.

The same happens, when I try to deserialize a Templace containing an array imagePullSecrets: ${{IMAGE_PULL_SECRETS}}. This fails with similar error - Cannot deserialize value of type java.util.ArrayList<io.fabric8.kubernetes.api.model.LocalObjectReference> from String value (token JsonToken.VALUE_STRING)

@shawkins
Copy link
Contributor

The current logic seems to only support string value parameters. It will fail otherwise because it's using strongly typed instances when parsing the objects. @rohanKanojia @manusa the quickest change that I can think of here would be to change HasMetadata to GenericKubernetesResource:

private List<GenericKubernetesResource> objects = new ArrayList<>();

There is logic in OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl related to templates that would continue to work as far as I can tell.

However it would be a breaking change for anyone manually constructing a Template. Is there a simple way to continue to use HasMetadata, but instead direct the deserialization to use GenericKubernetesResource as the instance type? Seems like it may require a new Deserializer.

@shawkins
Copy link
Contributor

Discussed with @manusa an alternative that would somehow redirect the parser to use something like additionalProperties to track any value that doesn't parse.

There are some possible facilities in jackson for this, such as a DeserializationProblemHandler implementing handleWeirdStringValue - but that can only change the value. It cannot redirect the value to additionalProperties as the context doesn't convey which field or the hierarchy at that point you are dealing with.

There is also the issue that additionalProperties are not on all resource types, in particular CustomResoruces, so we'd just need to use a reflective check for the existence of such a field and state by convention or check to make sure that annotations are compatible with this usage.

I'll defer to Marc if there seems to be something that can be done in the near-term on this.

@manusa
Copy link
Member

manusa commented Sep 23, 2021

Thanks for documenting this @shawkins

We have a similar problem for this when trying to deserialize and serialize YAML resources intended to be used with Helm.
These YAMLs contain placeholders that will later on be replaced by Helm provided values, so it's a siutation similar to that of Templates but that can affect any Resource type.

The overall idea would be to improve the default deserializer (+ serializer) so that values that can't be deserialized due to type conflicts (e.g. a DeploymentSpec#replicas cannot hold a value ${helm.replicas} or {{ .Values.replicas | default 1 }}) can be preserved until further serialization.

As Steven suggested, one option would be to use the additionalProperties.

IMO we should probably add an additional Map for this case, since it might help cover any additional problems (such as the typed value being set between deserialization and serialization) and won't mess with the current JsonAnyGetter/JsonAnySetter functionality.

An overview of the process can be:

  • A resource is deserialized into one of the Fabric8 types.
  • Fabric8 types contain additionalProperties Map field for unrecognized properties found during deserialization.
  • Fabric8 types contain preservedProperties Map field for recognized properties that target a field with an unmatched type (deserialization errors).
  • Any property that is recognized will be deserialized into its applicable field.
    • If the type doesn't match, the value will be added to the preservedProperties Map
  • The instance of the resource type is processed in the client (maybe fields are modified or set) through its life-cycle.
  • The resource is serialized into a YAML/JSON representation
  • Properties from regular fields and additionalProperties are written into the target stream.
    • There should not be collisions of regular fields and additionalProperties.
    • There might be collisions of regular/additionalProperties with preservedProperties, if there are, preservedProperties overwrite (take precedence) the rest.

Given the previous procedure, I only see one issue, which is what happens with mistakes or errors in the original deserialized resource? With the described approach InvalidFormatException is swallowed, IMO this should be OK, but maybe someone disagrees.

shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 11, 2021
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 11, 2021
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Nov 2, 2021
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Nov 3, 2021
@manusa manusa closed this as completed in 77aadc1 Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants