More flexible and better organized target specification in API

[erikgb]

I will start with a "max" target specification using the existing API (also including #235 - which I think will be merged before this issue is eventually addressed):

target:
  configMap:
    key: root-certs.pem
  secret:
    key: root-certs.pem
  additionalFormats:
    jks:
      key: bundle.jks
      password: jks-password
    pkcs12:
      key: bundle.p12
      password: pkcs12-password

I can identify several issues with this specification structure:

• It forces me to include a PEM encoded in target configmap/secret. There is no way to opt-out and just use JKS/PKCS12.
• The term additional appears a bit "discriminating", and does not motivate users to suggest (or contribute) new formats (e.g. DER).
• The PEM encoded bundle allows different key in configmap and secret, while for "additional formats" this is not supported.
• Encodings that require a password do not support specifying a different password for configmap and secret.
• The format does not allow me to output the same encoding in two different keys of target. I am not sure if we should support this, but it could theoretically make sense in a migration scenario. But the simple workaround is just to create a new bundle for migration.

To solve most of these issues, I suggest a more flexible (and less "discriminating") structure. It might appear more complex, but remember that most users will probably not use a "max" specification, and only specify relevant fields for their setup.

target:
  configMap:
    pem:
      key: root-certs.pem
    jks:
      key: bundle.jks
      password: jks-password
    pkcs12:
      key: bundle.p12
      password: pkcs12-password
  secret:
    pem:
      key: root-certs.pem
    jks:
      key: bundle.jks
      password: jks-password
    pkcs12:
      key: bundle.p12
      password: pkcs12-password

[hazmat345]

Maybe this could also be a chance to bring the target side more in line with the sources side? What would you think about something like this:

targets:
  - configMap:
      format: pem
      key: root-certs.pem
  - configMap:
      format: jks
      key: bundle.jks
      password: jks-password
  - configMap:
      format: pkcs12
      key: bundle.p12
      password: pkcs12-password
  - secret:
      format: pem
      key: root-certs.pem
  - secret:
      format: jks
      key: bundle.jks
      password: jks-password
  - secret:
      format: pkcs12
      key: bundle.p12
      password: pkcs12-password

[erikgb]

@hazmat345 Thanks for your input. I like your suggestion. It's probably better than what I suggested initially in this issue! 😄 We use kustomize a lot, and patching arrays could be troublesome. What if we separated configMap from secret to have two arrays, but used your suggestion with a format field? That would allow us to use key as mapKey when giving the arrays map semantics.

I agree that sources and target should be aligned in the API, but we could also adjust the sources part when breaking the API.

[erikgb]

Inspired by the suggestion from @hazmat345, I think the following might work well:

target:
  configMap:
    - format: pem
      key: root-certs.pem
    - format: jks
      key: bundle.jks
      password: jks-password
    - format: pkcs12
      key: bundle.p12
      password: pkcs12-password
  secret:
    - format: pem
      key: root-certs.pem
    - format: jks
      key: bundle.jks
      password: jks-password
    - format: pkcs12
      key: bundle.p12
      password: pkcs12-password

That will allow us to set map semantics on the arrays in configMap and secret fields, with key as the map key. With this structure, the Bundle OpenAPI spec will validate the fact that key must be unique in a single target (configMap or secret).

[SgtCoDFish]

I have a few things in mind writing this comment:

1. Most languages and libraries will want a PEM bundle, so by extension most users will want a PEM bundle.
2. I'm not sure what the value would be of outputting multiple configmaps or secrets. By definition they'll include the same data, just maybe in different formats. I'm not saying "definitely not", but I'd like to understand why because this would complicate our controller logic and working with YAML arrays is, as Erik says, much more difficult than the map we have today.

With that in mind, I'd tend towards something like this:

target:
  configMap:
    formats:
      jks:
        key: "bundle.jks"
        password: "whatever"
  secret:
    formats: {}

In this example, the configMap gets just JKS while the secret will just default to a PEM bundle with some const key that we can define (let's say bundle.pem).

This keeps it easy to write resources (since I assume people will want a PEM bundle) while allowing more flexibility for those who want it.

This could be expanded to support multiple configmaps / secrets per bundle if we wanted (but as above, I'm not feeling convinced on that).

Just throwing the idea out there!

[erikgb]

I agree that we should not support multiple configmaps/secrets. The increased controller complexity is just not worth it. Users that want/need this, can simply create additional bundle resources using the same sources.

But when it comes to specifying the target resource content, I don't think we should have a "magic" default target key for PEM bundles. I think this will bite us at some point in the future. 😁 I agree that PEM is probably the most common format, so it should be easy to configure it. Wouldn't the following be easy enough to represent your example above?

target:
  configMap:
    - format: jks
      key: bundle.jks
      password: whatever
  secret:
    # PEM is the default format
    - key: bundle.pem

[dhess]

We'd welcome multiple keys per target for the same encoding, in any case — many operators and controllers want to read self-signed CA certificates from a secret, almost always PEM-encoded, but often with different key names (cert, crt.pem, etc.).

[cert-manager-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale

[erikgb]

/remove-lifecycle stale