Problem with inline object within additionalProperties

[vigoo]

The following example (taken from the schema of the CRD https://github.com/argoproj/argo-cd/blob/41f54aa556f3ffb3fa4cf93d784fb7d30c15041c/manifests/crds/appproject-crd.yaml):

openapi: 3.0.1
info:
  title: Whatever
  version: 1.0.0
paths: { }
components:
  schemas:
    status:
      description: AppProjectStatus contains status information for AppProject
        CRs
      properties:
        jwtTokensByRole:
          additionalProperties:
            description: JWTTokens represents a list of JWT tokens
            properties:
              items:
                items:
                  description: JWTToken holds the issuedAt and expiresAt values
                    of a token
                  properties:
                    exp:
                      format: int64
                      type: integer
                    iat:
                      format: int64
                      type: integer
                    id:
                      type: string
                  required:
                    - iat
                  type: object
                type: array
            type: object
          description: JWTTokensByRole contains a list of JWT tokens issued
            for a given role
          type: object
      type: object

generates this code:

case class Status(jwtTokensByRole: Option[Map[String, Status.JwtTokensByRole]] = None)
object Status {
  implicit val encodeStatus: _root_.io.circe.Encoder.AsObject[Status] = {
    val readOnlyKeys = _root_.scala.Predef.Set[_root_.scala.Predef.String]()
    _root_.io.circe.Encoder.AsObject.instance[Status](a => _root_.io.circe.JsonObject.fromIterable(_root_.scala.Vector(("jwtTokensByRole", a.jwtTokensByRole.asJson)))).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
  }
  implicit val decodeStatus: _root_.io.circe.Decoder[Status] = new _root_.io.circe.Decoder[Status] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Status] = for (v0 <- c.downField("jwtTokensByRole").as[Option[Map[String, Status.JwtTokensByRole]]]) yield Status(v0) }
}

in which Status.JwtTokensByRole is missing.

[blast-hardcheese]

:+1: this was a conscious decision at the time to not support arbitrary types for additionalProperties (see additionalProperties: true), but you're bringing up a very valid use case, where the schema is actually well defined.

I think this could be fairly straightforward to add, at least in the case of a $ref (but probably for literals as well) by adding a parameter additionalProperties: Map[String, A] and doing some custom parsing in the encoders and decoders.

Is this something you'd be interested and available to add? I can help with guidance in the Matrix channel if you'd like, and I can cut a release as soon as the tests pass. The nice thing is we don't have to worry about backwards compatibility here, since this is entirely new.

[blast-hardcheese]

Actually, I just saw that you're already doing some heavy lifting over there.

I'm curious about the level of customization that they needed that wasn't already available via the sbt plugin, but that's I guess out of the scope for this ticket. The module system in recent releases should give them full control to override absolutely every part of code generation with relatively little headache, so if there's some need there then I can help as well

I can look at adding support for this within the next two weeks, if that fits your timeline

[vigoo]

It's very likely that zio-k8s-crds customization can be written in a better way. I wrote it about 1.5 years ago based on guardrail 0.64 (or older) and what I basically did is I copied the circe implementation and modified that. I'm currently not actively working on zio-k8s just looked into this bug report. So whenever you have time to implement the guardrail part I will try to update it there but don't really have time to do this as well :)

Also, I think in CRDs there are no $refs ever everything is "inlined" (not sure what the proper term is in OpenAPI context). There is always a single openAPIV3Schema node in the definition which is an object and there is no place to have additional top level named definitions.

The zio-k8s-crd plugin extracts this node from the CRD yaml and uses guardrail to generate the model classes for it, and generates other K8s specific code outside of guardrail as well. The customizations in the copied circe-support files are roughly the following:

• Instead of Option it uses our own Optional type (which is now in zio-prelude)
• All K8s objects contains a metadata field which is sometimes optional sometimes not (...), and it generates an implicit K8sObject[T] and K8sObjectOps for these types which helps the K8s library work with them.
• Similarly some objects contain a top level status field as well which is special and we need to generate some other implicits to work with them
• Adds a top level implicit ResourceMetadata with information coming outside guardrails's scope (names from the CRD yaml)

(This page describes how to do it by hand: https://coralogix.github.io/zio-k8s/docs/crd/crd_custom)

[blast-hardcheese]

That makes a lot of sense, thanks for the explanation!

I wish there were a more pleasant workflow than copy paste, as I think you'll find there have been some significant changes (presence and package structure being two, the SPI stuff being a third though you may not run into that if you depend on the Scala modules directly).

Once I've got a spare moment I'll see if I can get that additional properties stuff working.

From your perspective, is my proposal of an additional parameter with Map[String, A] is sufficient? Also, the specification is correct that every extra parameter must match that schema otherwise the whole decode will fail?

[blast-hardcheese]

Sorry for the delay on getting back to this, hopefully it hasn't caused too much trouble.

I've just opened a PR that at least satisfies the example spec from this issue, though with one caveat -- properties cannot live alongside additionalProperties at this time.

If that's required I can prioritize another pass, but if not then I've at least left enough context that it should be possible to make moves in that direction when things calm down a bit more for me, or if somebody is feeling this pain particularly significantly.

[capacman]

Is there any update about this or any chance to merge?

[blast-hardcheese]

Unfortunately no, I've got a partial branch but I haven't had the time to finish it, I'm sorry to block the zio work.