Skip to content

Type of jsonpath query results in workload resource mapping isn't well-defined #14

Description

@sadlerap

Something I ran into with regards to workload resource mapping: I was working with mapping a CronJob, and I was trying to determine if we should fail when we have a path query of .spec.jobTemplate.spec.template.spec.containers (rather than .spec.jobTemplate.spec.template.spec.containers[*], which is based off of our default). The difference between these two is that the first gives an array of container objects, while the second returns the same results, albeit not boxed in an array (i.e. multiple return values).

Should implementations support both of these queries as equivalent alternatives, or is one of these more correct than the other?
The default path query uses .spec.template.spec.containers[*], which inclines me toward preferring the latter type of query. That said, I don't think there's anything preventing implementations from supporting both styles (aside from maintenance burden). The specification doesn't say anything about this, however.


Filing this here because this is largely related to implementation work, and I think this should be mentioned in the implementation guide. That said, it may require spec modifications.

Activity

  1. scothis commented on Jan 22, 2022

    @scothis
    Member

    Should implementations support both of these queries as equivalent alternatives, or is one of these more correct than the other?

    I had the same thought while working on the proof of concept that lead to the current definition in the spec. The key difference is that we're looking for places where a container-like structure exists in the workload, not an array of containers. The [*] bit on the end of the JSONPath expression converts a single array of containers into a set of individual containers. For example, the RuntimeComponent resource has a container-like structure that is a single struct and not a member of an array.

    The current spec is guiding users to the most technically precise answer. That said, it may be worth relaxing this to make it more user friendly. Within the current implementation, I believe we can sniff the returned nodes to determine if they are objects or arrays and flatten the array. I'm not sure how much of a burden this would impose on other implementations.

    At a technical level:

    • the JSON Path query function signature returns []interface{}
    • the concrete type for .spec.template.spec.containers[*] is returned as []map[string]interface{}
    • the concrete type for .spec.template.spec.containers is returned as returned as [][]interface{} (where interface{} is in practice a map[string]interface{})
  2. sadlerap commented on Jan 26, 2022

    @sadlerap
    ContributorAuthor

    The current spec is guiding users to the most technically precise answer.

    I didn't quite get that understanding from the spec. AIUI, it implies via the defaults what kind of results it's looking for, but I didn't see it explicitly spelled out anywhere. I'd like to try to avoid "implied" behavior like this in the spec if possible; I'd rather not end up with implementations disagreeing on what is and is not spec compliant.

    That said, it may be worth relaxing this to make it more user friendly. Within the current implementation, I believe we can sniff the returned nodes to determine if they are objects or arrays and flatten the array. I'm not sure how much of a burden this would impose on other implementations.

    This kind of sniffing is what I'm doing in our implementation for now. It does make things more user friendly in my opinion. That said, it would be nice to specify precisely what is and is not going to be accepted; I'd like to avoid disagreeing implementations.

  3. scothis commented on Jan 26, 2022

    @scothis
    Member

    The spec says:

    A MappingContainer object MUST define a path entry which is a string containing a JSONPath that references container like locations in the target resource.

    I don't read this as talking about arrays of "containers", but I agree it's not clear enough in the other direction. For example, the JSON Paths for volumes/volumeMounts/env are all clearly defined as being for arrays of a specific type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions