Skip to content
This repository has been archived by the owner on Mar 22, 2024. It is now read-only.

Support Nested Spire with External Agent #117

Merged
merged 50 commits into from
Aug 16, 2023
Merged

Support Nested Spire with External Agent #117

merged 50 commits into from
Aug 16, 2023

Conversation

kfox1111
Copy link
Contributor

@kfox1111 kfox1111 commented Mar 13, 2023

This patch enables the nested spire use case:
https://spiffe.io/docs/latest/architecture/nested/readme/

You can have a root spire server external to Kubernetes that is the base spiffe ca. Then each of your kubernetes clusters can run an instance of this chart bound to the root server and all will be trusted.

fixes #54, #422

Copy link
Contributor

@marcofranssen marcofranssen left a comment

Choose a reason for hiding this comment

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

I find it very hard to follow this PR. Why would you want to nest spire servers on the same cluster? This PR looks like making it possible to nest on the same cluster, but I wonder why you would like to go that route with al complexity involved in this chart.

Could you elaborate on the usecases why you want to nested servers in a single cluster.

.github/tests/nested/pre-install.sh Outdated Show resolved Hide resolved
charts/spire/charts/spire-server/templates/configmap.yaml Outdated Show resolved Hide resolved
@kfox1111
Copy link
Contributor Author

Each server may be configured very differently. One is the root server which should be tightly locked down. While the second is for the k8s cluster.

The pr is not primarily about having 2 at once, but does enable that use case.

Its more about being able to test that having an upstream spire for your nested spire deployment works.

@kfox1111
Copy link
Contributor Author

Here's another use case: spiffe/spire-controller-manager#76

@kfox1111 kfox1111 marked this pull request as draft March 13, 2023 22:59
@kfox1111 kfox1111 marked this pull request as ready for review March 14, 2023 16:36
.github/tests/nested/pre-install.sh Outdated Show resolved Hide resolved
.github/tests/nested/spire-root-server-values.yaml Outdated Show resolved Hide resolved
.github/tests/nested/values.yaml Outdated Show resolved Hide resolved
@kfox1111
Copy link
Contributor Author

Rebased and flattened.

This patch enables the nested spire use case:
https://spiffe.io/docs/latest/architecture/nested/readme/

You can have a root spire server external to Kubernetes that is the base
spiffe ca. Then each of your kubernetes clusters can run an instance of
this chart bound to the root server and all will be trusted.

Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Copy link
Contributor

@edwbuck edwbuck left a comment

Choose a reason for hiding this comment

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

Many small comments. Not every one needs to be honored, but either attempts to honor them or explanations on why they can't be honored will speed the review.

charts/spire/Chart.yaml Show resolved Hide resolved
charts/spire/README.md Show resolved Hide resolved
charts/spire/charts/spire-server/README.md Outdated Show resolved Hide resolved
charts/spire/values.yaml Show resolved Hide resolved
charts/spire/values.yaml Show resolved Hide resolved
examples/nested/values.yaml Outdated Show resolved Hide resolved
examples/nested/values.yaml Show resolved Hide resolved
examples/nested/values.yaml Outdated Show resolved Hide resolved
examples/production/values.yaml Show resolved Hide resolved
examples/production/values.yaml Show resolved Hide resolved
@kfox1111 kfox1111 changed the title Support Nested Spire Support Nested Spire with External Agent Aug 3, 2023
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
@kfox1111
Copy link
Contributor Author

kfox1111 commented Aug 3, 2023

@edwbuck

I think I incorporated all the review items I could. Should be good for a re-review.

@faisal-memon
Copy link
Contributor

@edwbuck Do you have time to look at this again?

Copy link
Contributor

@edwbuck edwbuck left a comment

Choose a reason for hiding this comment

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

With the changes, this is as good as it is going to get until we un-nest our core application.

charts/spire/values.yaml Show resolved Hide resolved
examples/nested/values.yaml Show resolved Hide resolved
examples/production/values.yaml Show resolved Hide resolved
charts/spire/values.yaml Show resolved Hide resolved
examples/production/values.yaml Show resolved Hide resolved
charts/spire/values.yaml Show resolved Hide resolved
@marcofranssen marcofranssen enabled auto-merge (squash) August 16, 2023 14:35
Copy link
Contributor

@marcofranssen marcofranssen left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@marcofranssen marcofranssen merged commit ae8941c into main Aug 16, 2023
28 checks passed
@marcofranssen marcofranssen deleted the nested-spire branch August 16, 2023 14:35
marcofranssen added a commit that referenced this pull request Aug 18, 2023
* 5e2e8a9 Adds AWS KMS KeyManager support (#435)
* 77fe43f Cron job to check for and update images (#249)
* b7e1525 Allow job hooks to be disabled (#434)
* 5e4cf6f Clarify project issues identified with nesting document (#450)
* 7289351 Update spire bits to 1.7.2 (#452)
* dc8a454 Array spacing in values is incorrect in a file. (#451)
* 94326d9 Fixup Helm docs
* ae8941c Support Nested Spire with External Agent (#117)
* f40743d Improve Tornjak documentation (#439)
* 0124f63 Bypass example-test for docs only changes (#449)
* 48a2898 Fix chainguard image references as per issue 442 (#443)
* bd393e9 Bump test chart dependencies (#445)
* a52818a Add a FAQ and switch rare issue from README to it (#437)
* e60f528 option to set KeyManager memory in spire server (#444)
* a167ce6 Bump actions/setup-go from 4.0.1 to 4.1.0
* e774584 Bump test chart dependencies (#426)
* bfec27e Fix jwtIssuer to allow for Uris including scheme (#425)
* 7a6e4f8 Change Tornjak backend default port (#436)
* 1e3039c Bump spire Helm Chart version from 0.11.0 to 0.11.1 (#419)
* d2e1606 issuer naming should respect issuer_name override (#378)
* a2e5c36 Bump test chart dependencies (#416)
* a09e054 support annotations so oidc can be annotated (#391)
* 7d94b10 Update spire to 1.7.1 (#412)
* 9f4d4ac Add aws_pca to the spire-server (#404)
* af13f1f Bump test chart dependencies (#401)
* 9a6768b Add support for disabling container selectors (#399)
* 4687e20 Merge pull request #315 from spiffe/persistence-type
* e16210c Merge branch 'main' into persistence-type
* 624ca9c Remove misadded lockfile (#400)
* 7ce67c6 Bump actions/checkout from 3.5.2 to 3.5.3 (#395)
* b85ba64 Bump helm/kind-action from 1.7.0 to 1.8.0 (#396)
* a6bdb4d Add persistence type flag

Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
faisal-memon pushed a commit that referenced this pull request Aug 21, 2023
Please review the below changelog to ensure this matches up with the
semantic version being applied.

> **Note**: **Maintainers** ensure to run following after merging this
PR to trigger the release workflow:
>
> ```shell
> git checkout main
> git pull
> git checkout release
> git pull
> git merge main
> git push
> ```

**Changes in this release**

* 5e2e8a9 Adds AWS KMS KeyManager support (#435)
* 77fe43f Cron job to check for and update images (#249)
* b7e1525 Allow job hooks to be disabled (#434)
* 5e4cf6f Clarify project issues identified with nesting document
(#450)
* 7289351 Update spire bits to 1.7.2 (#452)
* dc8a454 Array spacing in values is incorrect in a file. (#451)
* 94326d9 Fixup Helm docs
* ae8941c Support Nested Spire with External Agent (#117)
* f40743d Improve Tornjak documentation (#439)
* 0124f63 Bypass example-test for docs only changes (#449)
* 48a2898 Fix chainguard image references as per issue 442 (#443)
* bd393e9 Bump test chart dependencies (#445)
* a52818a Add a FAQ and switch rare issue from README to it (#437)
* e60f528 option to set KeyManager memory in spire server (#444)
* a167ce6 Bump actions/setup-go from 4.0.1 to 4.1.0
* e774584 Bump test chart dependencies (#426)
* bfec27e Fix jwtIssuer to allow for Uris including scheme (#425)
* 7a6e4f8 Change Tornjak backend default port (#436)
* 1e3039c Bump spire Helm Chart version from 0.11.0 to 0.11.1 (#419)
* d2e1606 issuer naming should respect issuer_name override (#378)
* a2e5c36 Bump test chart dependencies (#416)
* a09e054 support annotations so oidc can be annotated (#391)
* 7d94b10 Update spire to 1.7.1 (#412)
* 9f4d4ac Add aws_pca to the spire-server (#404)
* af13f1f Bump test chart dependencies (#401)
* 9a6768b Add support for disabling container selectors (#399)
* 4687e20 Merge pull request #315 from spiffe/persistence-type
* e16210c Merge branch 'main' into persistence-type
* 624ca9c Remove misadded lockfile (#400)
* 7ce67c6 Bump actions/checkout from 3.5.2 to 3.5.3 (#395)
* b85ba64 Bump helm/kind-action from 1.7.0 to 1.8.0 (#396)
* a6bdb4d Add persistence type flag

Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested support
5 participants