Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Spire proposal/testing results doc #445

Merged
merged 3 commits into from Sep 14, 2018

Conversation

JoshVanL
Copy link
Contributor

What this PR does / why we need it:
This adds a document detailing the feasibility of adding spire into the Tarmak cluster. It includes steps needed to achieve this and results of some testing.

fixes #436

SPIFFE/SPIRE proposal/feasibility document.

/assign @simonswine

@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 16, 2018
Copy link
Contributor

@munnerz munnerz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome write up 😄

I've not done a proper review, but just added that one comment (plus wanted to +1 the write-up!)


Use Spire within the Tarmak cluster to bolster the authentication of workload
identities, resulting in a more secure cluster. This could ultimately mean a
large or complete replacement of Vault and it's periphery tools, in favour of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to continue to use Vault (for its many other, noted, features) as an actual secret store, but use SPIFFE and Spire to provide attestation documents that can be used to authenticate with Vault? i.e. spiffe/spire replaces the init-tokens? Or is this not something that's possible/we want to explore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly, think that could be a little better written^ !
I'm am concerned however about how much code we need to touch - if any - in the security path, given spire's maturity. Need to weigh whether it's worth adding the extra layer of complexity too.


Avoiding writing our own plugins. This is better left to experts and if is
required, Spire should be dropped due to not being ready yet for our
requirements.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you find that you need plugins which aren't already available, or run into any showstoppers otherwise, please let me know.


Spire is not currently well built for HA and really only expects a single server
in cluster. PostgreSQL support for HA is quite poor from what I have found,
whilst also being the only supported "HA" backend. I was able to use a tool Patroni_ that

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SPIRE has an easy path to MySQL support... if it helps, I can prioritize this. XtraDB provides a nice galera-enabled MySQL distribution that supports multi master.

@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 24, 2018
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 24, 2018
@simonswine simonswine closed this Sep 10, 2018
@simonswine simonswine reopened this Sep 10, 2018
@jetstack-bot jetstack-bot added the dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. label Sep 10, 2018
on those instances to panic. Spire servers have to connect to the PostgreSQL
cluster master elected through Consul.

Spire uses Elliptic Curve keys which is odd and can be annoying.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too sure either provide more evidence or get rid of this sentence. There are good reasons IMHO for ECC (https://blog.cloudflare.com/ecdsa-the-digital-signature-algorithm-of-a-better-internet/)

@simonswine
Copy link
Contributor

/assign @JoshVanL
/unassign

I think james' comment is quite valid. Also the one about the ECC and last but not least DCO

@jetstack-bot jetstack-bot assigned JoshVanL and unassigned simonswine Sep 10, 2018
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 11, 2018
@jetstack-bot
Copy link
Collaborator

@JoshVanL: GitHub didn't allow me to assign the following users: sime.

Note that only jetstack members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/unasign
/assign @sime

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.

@JoshVanL
Copy link
Contributor Author

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Sep 11, 2018
@simonswine simonswine added this to the release-0.5 milestone Sep 14, 2018
@simonswine
Copy link
Contributor

Thanks @JoshVanL

/approve
/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2018
@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: simonswine

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 jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2018
@jetstack-bot jetstack-bot merged commit 852d97f into jetstack:master Sep 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discover feasibility for using SPIRE/SPIFFE in tarmak
5 participants