trust-manager icon indicating copy to clipboard operation
trust-manager copied to clipboard

WIP: feat: add HTTPS bundle source

Open jakexks opened this issue 3 years ago • 6 comments

This PR adds a "fetch from remote server over HTTPS" bundle source. It's used like this:

spec:
  sources:
  - https:
      url: https://foo.bar/baz
      caBundle: |
        -----BEGIN CERTIFICATE-----
        ...
      pollInterval: 60s

It uses etags / if-modified-since headers to avoid redownloading bundles every pollInterval.

It's somewhat opinionated - it only allows https:// schemes and will only use the trust roots specied inline in the spec. This is to prevent users doing something unsafe, but I'm open to discussion.

jakexks avatar Jul 26 '22 20:07 jakexks

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakexks

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

jetstack-bot avatar Jul 26 '22 20:07 jetstack-bot

Hmm, I am pretty hesitant about adding something that is a dynamic/changeable source. This creates issues for trust in general, as well as observability into versioning.

From an outside perspective, I'd expect that all trust bundles defined with our CRDs are fairly immutable, so if a new version of something were to come about then a new bundle would be created and whatever standard process people have in place for migrating from the older trust store to the new one would 'kick in' (all of this can be automated away behind the scenes, but my point here being that there is one and only one way to actually update a trust store, making audit a lot easier).

Why do we want trust to deal with interacting with an external server, rather than say, an administrator doing this periodically or a controller/cron/script that periodically runs and updates/creates a TrustBundle resource in the apiserver when it deems necessary? IMO doing it that way would be closer to a 'pluggable' model too, as some orgs somewhere may have some kind of proprietary store for trust bundles that they need their own controller for reading from. Fetching from a multitude of sources seems like it risks scope creep similar to how we now have in-built issuers in cert-manager core.

Just my 2c on this really :) it'd be good to hear the use-case that has led to this, and whether there's a more general mechanism for "integrating external trust sources" that we should be discussing

munnerz avatar Jul 27 '22 09:07 munnerz

One more thing.. I don't think it makes sense to require the protocol in the URI when the block is already HTTPS

  - https:
      url: https://foo.bar/baz

JoshVanL avatar Jul 27 '22 09:07 JoshVanL

Hmm, I am pretty hesitant about adding something that is a dynamic/changeable source. This creates issues for trust in general, as well as observability into versioning.

From an outside perspective, I'd expect that all trust bundles defined with our CRDs are fairly immutable, so if a new version of something were to come about then a new bundle would be created and whatever standard process people have in place for migrating from the older trust store to the new one would 'kick in' (all of this can be automated away behind the scenes, but my point here being that there is one and only one way to actually update a trust store, making audit a lot easier).

Why do we want trust to deal with interacting with an external server, rather than say, an administrator doing this periodically or a controller/cron/script that periodically runs and updates/creates a TrustBundle resource in the apiserver when it deems necessary? IMO doing it that way would be closer to a 'pluggable' model too, as some orgs somewhere may have some kind of proprietary store for trust bundles that they need their own controller for reading from. Fetching from a multitude of sources seems like it risks scope creep similar to how we now have in-built issuers in cert-manager core.

Just my 2c on this really :) it'd be good to hear the use-case that has led to this, and whether there's a more general mechanism for "integrating external trust sources" that we should be discussing

Please also don't take my comment as binding if there is general consensus already that we want this 😊 just trying to think of different ways this problem could be approached.

munnerz avatar Jul 27 '22 10:07 munnerz

No worries, there's no consensus on anything yet. My 2 PRs were intended to spark discussion ahead of the community meeting, and to try to solutioneer to feed back into forthcoming design docs.

To be more explicit I've stuck the WIP labels on my PRs 😄

jakexks avatar Jul 27 '22 16:07 jakexks

@jakexks: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jetstack-bot avatar Aug 03 '22 07:08 jetstack-bot

As with #36 I don't think this is the approach we've agreed to take so I'll close this - if I'm wrong feel free to re-open!

SgtCoDFish avatar Nov 28 '22 14:11 SgtCoDFish